Skip to content

Commit

Permalink
Fix support for FileIterator when working directory is /
Browse files Browse the repository at this point in the history
Added an additional check that tests to see if the working directory is root.
#862
  • Loading branch information
macshome authored and ahoppen committed Dec 5, 2024
1 parent 36d00f0 commit e14ee65
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 34 deletions.
14 changes: 0 additions & 14 deletions Sources/SwiftFormat/API/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -486,17 +486,3 @@ public struct NoAssignmentInExpressionsConfiguration: Codable, Equatable {

public init() {}
}

fileprivate extension URL {
var isRoot: Bool {
#if os(Windows)
// FIXME: We should call into Windows' native check to check if this path is a root once https://github.com/swiftlang/swift-foundation/issues/976 is fixed.
// https://github.com/swiftlang/swift-format/issues/844
return self.pathComponents.count <= 1
#else
// On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980
// TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed.
return self.path == "/" || self.path == ""
#endif
}
}
3 changes: 2 additions & 1 deletion Sources/SwiftFormat/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ add_library(SwiftFormat
Rules/UseTripleSlashForDocumentationComments.swift
Rules/UseWhereClausesInForLoops.swift
Rules/ValidateDocumentationComments.swift
Utilities/FileIterator.swift)
Utilities/FileIterator.swift
Utilities/URL+isRoot.swift)
target_link_libraries(SwiftFormat PUBLIC
SwiftMarkdown::Markdown
SwiftSyntax::SwiftSyntax
Expand Down
23 changes: 17 additions & 6 deletions Sources/SwiftFormat/Utilities/FileIterator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

import Foundation

#if os(Windows)
import WinSDK
#endif

/// Iterator for looping over lists of files and directories. Directories are automatically
/// traversed recursively, and we check for files with a ".swift" extension.
@_spi(Internal)
Expand All @@ -32,7 +36,7 @@ public struct FileIterator: Sequence, IteratorProtocol {

/// The current working directory of the process, which is used to relativize URLs of files found
/// during iteration.
private let workingDirectory = URL(fileURLWithPath: ".")
private let workingDirectory: URL

/// Keep track of the current directory we're recursing through.
private var currentDirectory = URL(fileURLWithPath: "")
Expand All @@ -46,8 +50,13 @@ public struct FileIterator: Sequence, IteratorProtocol {
/// Create a new file iterator over the given list of file URLs.
///
/// The given URLs may be files or directories. If they are directories, the iterator will recurse
/// into them.
public init(urls: [URL], followSymlinks: Bool) {
/// into them. Symlinks are never followed on Windows platforms as Foundation doesn't support it.
/// - Parameters:
/// - urls: `Array` of files or directories to iterate.
/// - followSymlinks: `Bool` to indicate if symbolic links should be followed when iterating.
/// - workingDirectory: `URL` that indicates the current working directory. Used for testing.
public init(urls: [URL], followSymlinks: Bool, workingDirectory: URL = URL(fileURLWithPath: ".")) {
self.workingDirectory = workingDirectory
self.urls = urls
self.urlIterator = self.urls.makeIterator()
self.followSymlinks = followSymlinks
Expand Down Expand Up @@ -145,9 +154,11 @@ public struct FileIterator: Sequence, IteratorProtocol {
// be displayed as relative paths. Otherwise, they will still be displayed as absolute
// paths.
let relativePath =
path.hasPrefix(workingDirectory.path)
? String(path.dropFirst(workingDirectory.path.count + 1))
: path
if !workingDirectory.isRoot, path.hasPrefix(workingDirectory.path) {
String(path.dropFirst(workingDirectory.path.count).drop(while: { $0 == "/" || $0 == #"\"# }))
} else {
path
}
output =
URL(fileURLWithPath: relativePath, isDirectory: false, relativeTo: workingDirectory)

Expand Down
27 changes: 27 additions & 0 deletions Sources/SwiftFormat/Utilities/URL+isRoot.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import Foundation

extension URL {
@_spi(Testing) public var isRoot: Bool {
#if os(Windows)
// FIXME: We should call into Windows' native check to check if this path is a root once https://github.com/swiftlang/swift-foundation/issues/976 is fixed.
// https://github.com/swiftlang/swift-format/issues/844
return self.pathComponents.count <= 1
#else
// On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980
// TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed.
return self.path == "/" || self.path == ""
#endif
}
}
74 changes: 61 additions & 13 deletions Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
@_spi(Internal) import SwiftFormat
@_spi(Internal) @_spi(Testing) import SwiftFormat
import XCTest

extension URL {
/// Assuming this is a file URL, resolves all symlinks in the path.
///
/// - Note: We need this because `URL.resolvingSymlinksInPath()` not only resolves symlinks but also standardizes the
/// path by stripping away `private` prefixes. Since sourcekitd is not performing this standardization, using
/// `resolvingSymlinksInPath` can lead to slightly mismatched URLs between the sourcekit-lsp response and the test
/// assertion.
fileprivate var realpath: URL {
#if canImport(Darwin)
return self.path.withCString { path in
guard let realpath = Darwin.realpath(path, nil) else {
return self
}
let result = URL(fileURLWithPath: String(cString: realpath))
free(realpath)
return result
}
#else
// Non-Darwin platforms don't have the `/private` stripping issue, so we can just use `self.resolvingSymlinksInPath`
// here.
return self.resolvingSymlinksInPath()
#endif
}
}

final class FileIteratorTests: XCTestCase {
private var tmpdir: URL!

Expand All @@ -10,7 +35,7 @@ final class FileIteratorTests: XCTestCase {
in: .userDomainMask,
appropriateFor: FileManager.default.temporaryDirectory,
create: true
)
).realpath

// Create a simple file tree used by the tests below.
try touch("project/real1.swift")
Expand All @@ -31,8 +56,8 @@ final class FileIteratorTests: XCTestCase {
#endif
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false)
XCTAssertEqual(seen.count, 2)
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real1.swift") })
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real2.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real1.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real2.swift") })
}

func testFollowSymlinks() throws {
Expand All @@ -41,10 +66,10 @@ final class FileIteratorTests: XCTestCase {
#endif
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: true)
XCTAssertEqual(seen.count, 3)
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real1.swift") })
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real2.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real1.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real2.swift") })
// Hidden but found through the visible symlink project/link.swift
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.hidden.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") })
}

func testTraversesHiddenFilesIfExplicitlySpecified() throws {
Expand All @@ -56,8 +81,8 @@ final class FileIteratorTests: XCTestCase {
followSymlinks: false
)
XCTAssertEqual(seen.count, 2)
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.build/generated.swift") })
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.hidden.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.build/generated.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") })
}

func testDoesNotFollowSymlinksIfFollowSymlinksIsFalseEvenIfExplicitlySpecified() {
Expand All @@ -71,6 +96,25 @@ final class FileIteratorTests: XCTestCase {
)
XCTAssertTrue(seen.isEmpty)
}

func testDoesNotTrimFirstCharacterOfPathIfRunningInRoot() throws {
// Find the root of tmpdir. On Unix systems, this is always `/`. On Windows it is the drive.
var root = tmpdir!
while !root.isRoot {
root.deleteLastPathComponent()
}
// Make sure that we don't drop the beginning of the path if we are running in root.
// https://github.com/swiftlang/swift-format/issues/862
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false, workingDirectory: root)
XCTAssertTrue(seen.allSatisfy { $0.relativePath.hasPrefix(root.path) }, "\(seen) does not contain root directory")
}

func testShowsRelativePaths() throws {
// Make sure that we still show the relative path if using them.
// https://github.com/swiftlang/swift-format/issues/862
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false, workingDirectory: tmpdir)
XCTAssertEqual(Set(seen.map(\.relativePath)), ["project/real1.swift", "project/real2.swift"])
}
}

extension FileIteratorTests {
Expand Down Expand Up @@ -111,11 +155,15 @@ extension FileIteratorTests {
}

/// Computes the list of all files seen by using `FileIterator` to iterate over the given URLs.
private func allFilesSeen(iteratingOver urls: [URL], followSymlinks: Bool) -> [String] {
let iterator = FileIterator(urls: urls, followSymlinks: followSymlinks)
var seen: [String] = []
private func allFilesSeen(
iteratingOver urls: [URL],
followSymlinks: Bool,
workingDirectory: URL = URL(fileURLWithPath: ".")
) -> [URL] {
let iterator = FileIterator(urls: urls, followSymlinks: followSymlinks, workingDirectory: workingDirectory)
var seen: [URL] = []
for next in iterator {
seen.append(next.path)
seen.append(next)
}
return seen
}
Expand Down

0 comments on commit e14ee65

Please sign in to comment.