From 8a2d5ee7b8a947cba8089227635d1bd3cf611b36 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Fri, 29 May 2026 00:45:02 -0700 Subject: [PATCH 1/2] Guarantee config-file search loop terminates on non-progressing paths The parent-directory walk in Configuration.url(forConfigurationFileApplyingTo:) relied solely on reaching the filesystem root to terminate. On toolchains where URL.deleteLastPathComponent() stops making progress for paths containing redundant separators (e.g. /path/to///main.swift), the loop could spin forever. Add a guard that breaks the loop when removing the last path component yields no change, so termination no longer depends on Foundation collapsing redundant separators. Fixes #1035 --- Sources/SwiftFormat/API/Configuration.swift | 9 +++++ .../API/ConfigurationTests.swift | 39 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index 49283687e..c200b34d7 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -515,11 +515,20 @@ public struct Configuration: Codable, Equatable { candidateDirectory.appendPathComponent("placeholder") } repeat { + let previousDirectory = candidateDirectory candidateDirectory.deleteLastPathComponent() let candidateFile = candidateDirectory.appendingPathComponent(Self.swiftFormatFilename) if FileManager.default.isReadableFile(atPath: candidateFile.path) { return candidateFile } + + // Stop if removing the last path component made no progress. This can happen when the path + // contains redundant separators (e.g. `/path/to///main.swift`), which `standardized` does not + // collapse; without this guard the loop would never reach the root and would spin forever. + // See https://github.com/swiftlang/swift-format/issues/1035. + if candidateDirectory == previousDirectory { + break + } } while !candidateDirectory.isRoot return nil diff --git a/Tests/SwiftFormatTests/API/ConfigurationTests.swift b/Tests/SwiftFormatTests/API/ConfigurationTests.swift index d983e9cb9..b3b3becf6 100644 --- a/Tests/SwiftFormatTests/API/ConfigurationTests.swift +++ b/Tests/SwiftFormatTests/API/ConfigurationTests.swift @@ -9,6 +9,7 @@ // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors // //===----------------------------------------------------------------------===// +import Foundation import SwiftFormat import XCTest @@ -135,4 +136,42 @@ final class ConfigurationTests: XCTestCase { XCTAssertEqual(config, expected) #endif } + + func testMissingConfigurationFilePathWithRedundantSlashesTerminates() throws { + // A path that contains a run of redundant separators (here `///`) is not collapsed by `URL`'s + // standardization, so walking up its parent directories must not loop forever when no + // configuration file exists. See https://github.com/swiftlang/swift-format/issues/1035. + #if os(Windows) + let path = #"C:\test\path\no\configuration\\\exists\anywhere\main.swift"# + #else + let path = "/test/path/no/configuration///exists/anywhere/main.swift" + #endif + XCTAssertNil(Configuration.url(forConfigurationFileApplyingTo: URL(fileURLWithPath: path))) + } + + func testFindsConfigurationFileWhenPathContainsRedundantSlashes() throws { + // The parent-directory walk must terminate *and* still locate the configuration file when the + // source file path contains redundant separators (e.g. `///`), which can happen when paths are + // composed by other tools. See https://github.com/swiftlang/swift-format/issues/1035. + #if os(Windows) + try XCTSkipIf(true, "Redundant POSIX-style `///` separators are not meaningful on Windows.") + #else + let projectDir = FileManager.default.temporaryDirectory + .appendingPathComponent("swift-format-1035-\(UUID().uuidString)", isDirectory: true) + let sourceDir = projectDir.appendingPathComponent("Sources").appendingPathComponent("MyModule") + try FileManager.default.createDirectory(at: sourceDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: projectDir) } + + let configURL = projectDir.appendingPathComponent(".swift-format") + try Data("{}".utf8).write(to: configURL) + + // Build a source file path that contains redundant `///` separators below `projectDir`. + let redundantSlashPath = projectDir.path + "/Sources/MyModule///main.swift" + let sourceFile = URL(fileURLWithPath: redundantSlashPath) + XCTAssertTrue(sourceFile.path.contains("///"), "Test setup must preserve the redundant slashes.") + + let foundConfigURL = Configuration.url(forConfigurationFileApplyingTo: sourceFile) + XCTAssertEqual(foundConfigURL?.standardizedFileURL, configURL.standardizedFileURL) + #endif + } } From 59162033395dabe1aa52ceb7c1d817aca699e383 Mon Sep 17 00:00:00 2001 From: Aditya Singh Date: Mon, 1 Jun 2026 17:35:46 -0700 Subject: [PATCH 2/2] Clarify the termination guard is defensive on current Foundation deleteLastPathComponent() collapses redundant separators as it ascends on the current toolchain, so the loop already reaches root; the guard backstops the Foundation that exhibited #1035, where a trailing redundant separator stalled the ascent. Note that breaking on a non-progressing path can never skip a searchable parent, since the operation is a pure function of the path. --- Sources/SwiftFormat/API/Configuration.swift | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index c200b34d7..ef72ab552 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -522,9 +522,13 @@ public struct Configuration: Codable, Equatable { return candidateFile } - // Stop if removing the last path component made no progress. This can happen when the path - // contains redundant separators (e.g. `/path/to///main.swift`), which `standardized` does not - // collapse; without this guard the loop would never reach the root and would spin forever. + // Stop if removing the last path component made no progress. `deleteLastPathComponent()` is a + // pure function of the path, so once it is a no-op it stays a no-op: there is no reachable + // parent above this point, and continuing would only spin until `isRoot` (which it never + // reaches). On the Foundation that originally exhibited #1035, a trailing redundant separator + // (e.g. `/path/to//main.swift`) stalled here; current Foundation collapses such separators as + // it ascends, so this guard is a defensive backstop rather than the load-bearing fix on that + // toolchain. Breaking here can never skip a directory the loop could otherwise have searched. // See https://github.com/swiftlang/swift-format/issues/1035. if candidateDirectory == previousDirectory { break