From e14ee65365b9e04a0c16695684caf76b7486ae35 Mon Sep 17 00:00:00 2001 From: Josh Wisenbaker Date: Thu, 5 Dec 2024 08:04:54 -0800 Subject: [PATCH] Fix support for `FileIterator` when working directory is `/` Added an additional check that tests to see if the working directory is root. https://github.com/swiftlang/swift-format/issues/862 --- Sources/SwiftFormat/API/Configuration.swift | 14 ---- Sources/SwiftFormat/CMakeLists.txt | 3 +- .../SwiftFormat/Utilities/FileIterator.swift | 23 ++++-- .../SwiftFormat/Utilities/URL+isRoot.swift | 27 +++++++ .../Utilities/FileIteratorTests.swift | 74 +++++++++++++++---- 5 files changed, 107 insertions(+), 34 deletions(-) create mode 100644 Sources/SwiftFormat/Utilities/URL+isRoot.swift diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index 20b491a0..70ac916a 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -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 - } -} diff --git a/Sources/SwiftFormat/CMakeLists.txt b/Sources/SwiftFormat/CMakeLists.txt index 62c2d4df..46937f71 100644 --- a/Sources/SwiftFormat/CMakeLists.txt +++ b/Sources/SwiftFormat/CMakeLists.txt @@ -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 diff --git a/Sources/SwiftFormat/Utilities/FileIterator.swift b/Sources/SwiftFormat/Utilities/FileIterator.swift index 9d1a1d2c..ceb764c7 100644 --- a/Sources/SwiftFormat/Utilities/FileIterator.swift +++ b/Sources/SwiftFormat/Utilities/FileIterator.swift @@ -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) @@ -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: "") @@ -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 @@ -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) diff --git a/Sources/SwiftFormat/Utilities/URL+isRoot.swift b/Sources/SwiftFormat/Utilities/URL+isRoot.swift new file mode 100644 index 00000000..43cfda85 --- /dev/null +++ b/Sources/SwiftFormat/Utilities/URL+isRoot.swift @@ -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 + } +} diff --git a/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift b/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift index ba482063..26609a0c 100644 --- a/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift +++ b/Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift @@ -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! @@ -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") @@ -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 { @@ -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 { @@ -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() { @@ -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 { @@ -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 }