Merge pull request #273 from allevato/diagnostic-paths

Fix up file URL handling.

Author: allevato

Package.swift

@@ -16,6 +16,9 @@ import Foundation
 
 let package = Package(
   name: "swift-format",
+  platforms: [
+    .macOS(.v10_11)
+  ],
   products: [
     .executable(name: "swift-format", targets: ["swift-format"]),
     .library(name: "SwiftFormat", targets: ["SwiftFormat", "SwiftFormatConfiguration"]),

Sources/SwiftFormatConfiguration/Configuration.swift

@@ -233,21 +233,25 @@ public struct Configuration: Codable, Equatable {
 
   /// Returns the URL of the configuration file that applies to the given file or directory.
   public static func url(forConfigurationFileApplyingTo url: URL) -> URL? {
-    var path = url.absoluteURL
-    let configFilename = ".swift-format"
+    // Despite the variable's name, this value might start out first as a file path (the path to a
+    // source file being formatted). However, it will immediately have its basename removed in the
+    // loop below, and from then on serve as a directory path only.
+    var candidateDirectory = url.absoluteURL.standardized
     var isDirectory: ObjCBool = false
-    if FileManager.default.fileExists(atPath: path.path, isDirectory: &isDirectory), 
-      isDirectory.boolValue {
-      // will be deleted in a loop
-      path.appendPathComponent("placeholder")
+    if FileManager.default.fileExists(atPath: candidateDirectory.path, isDirectory: &isDirectory),
+      isDirectory.boolValue
+    {
+      // If the path actually was a directory, append a fake basename so that the trimming code
+      // below doesn't have to deal with the first-time special case.
+      candidateDirectory.appendPathComponent("placeholder")
     }
     repeat {
-      path.deleteLastPathComponent()
-      let candidateFile = path.appendingPathComponent(configFilename)
+      candidateDirectory.deleteLastPathComponent()
+      let candidateFile = candidateDirectory.appendingPathComponent(".swift-format")
       if FileManager.default.isReadableFile(atPath: candidateFile.path) {
         return candidateFile
       }
-    } while path.path != "/"
+    } while candidateDirectory.path != "/"
 
     return nil
   }

Sources/SwiftFormatCore/Context.swift

@@ -69,8 +69,8 @@ public final class Context {
     self.fileURL = fileURL
     self.importsXCTest = .notDetermined
     self.sourceLocationConverter =
-      source.map { SourceLocationConverter(file: fileURL.path, source: $0) }
-      ?? SourceLocationConverter(file: fileURL.path, tree: sourceFileSyntax)
+      source.map { SourceLocationConverter(file: fileURL.relativePath, source: $0) }
+      ?? SourceLocationConverter(file: fileURL.relativePath, tree: sourceFileSyntax)
     self.ruleMask = RuleMask(
       syntaxNode: Syntax(sourceFileSyntax),
       sourceLocationConverter: sourceLocationConverter

Sources/swift-format/Frontend/ConfigurationLoader.swift

@@ -16,25 +16,17 @@ import SwiftFormatConfiguration
 /// Loads formatter configurations, caching them in memory so that multiple operations in the same
 /// directory do not repeatedly hit the file system.
 struct ConfigurationLoader {
-  /// A mapping from configuration file URLs to the loaded configuration data.
-  private var cache = [URL: Configuration]()
-
-  /// Returns the configuration associated with the configuration file at the given path.
-  ///
-  /// - Throws: If an error occurred loading the configuration.
-  mutating func configuration(atPath path: String) throws -> Configuration {
-    return try configuration(at: URL(fileURLWithPath: path))
-  }
+  /// The cache of previously loaded configurations.
+  private var cache = [String: Configuration]()
 
   /// Returns the configuration found by searching in the directory (and ancestor directories)
   /// containing the given `.swift` source file.
   ///
   /// If no configuration file was found during the search, this method returns nil.
   ///
   /// - Throws: If a configuration file was found but an error occurred loading it.
-  mutating func configuration(forSwiftFileAtPath path: String) throws -> Configuration? {
-    let swiftFileURL = URL(fileURLWithPath: path)
-    guard let configurationFileURL = Configuration.url(forConfigurationFileApplyingTo: swiftFileURL)
+  mutating func configuration(forSwiftFileAt url: URL) throws -> Configuration? {
+    guard let configurationFileURL = Configuration.url(forConfigurationFileApplyingTo: url)
     else {
       return nil
     }
@@ -44,13 +36,14 @@ struct ConfigurationLoader {
   /// Returns the configuration associated with the configuration file at the given URL.
   ///
   /// - Throws: If an error occurred loading the configuration.
-  private mutating func configuration(at url: URL) throws -> Configuration {
-    if let cachedConfiguration = cache[url] {
+  mutating func configuration(at url: URL) throws -> Configuration {
+    let cacheKey = url.absoluteURL.standardized.path
+    if let cachedConfiguration = cache[cacheKey] {
       return cachedConfiguration
     }
 
     let configuration = try Configuration(contentsOf: url)
-    cache[url] = configuration
+    cache[cacheKey] = configuration
     return configuration
   }
 }

Sources/swift-format/Frontend/FormatFrontend.swift

@@ -32,35 +32,35 @@ class FormatFrontend: Frontend {
     let formatter = SwiftFormatter(configuration: fileToProcess.configuration, findingConsumer: nil)
     formatter.debugOptions = debugOptions
 
-    let path = fileToProcess.path
+    let url = fileToProcess.url
     guard let source = fileToProcess.sourceText else {
-      diagnosticsEngine.emitError("Unable to read source for formatting from \(path).")
+      diagnosticsEngine.emitError(
+        "Unable to format \(url.relativePath): file is not readable or does not exist.")
       return
     }
 
     var stdoutStream = FileHandle.standardOutput
     do {
-      let assumingFileURL = URL(fileURLWithPath: path)
       if inPlace {
         var buffer = ""
         try formatter.format(
           source: source,
-          assumingFileURL: assumingFileURL,
+          assumingFileURL: url,
           to: &buffer,
           parsingDiagnosticHandler: diagnosticsEngine.consumeParserDiagnostic)
 
         let bufferData = buffer.data(using: .utf8)!  // Conversion to UTF-8 cannot fail
-        try bufferData.write(to: assumingFileURL, options: .atomic)
+        try bufferData.write(to: url, options: .atomic)
       } else {
         try formatter.format(
           source: source,
-          assumingFileURL: assumingFileURL,
+          assumingFileURL: url,
           to: &stdoutStream,
           parsingDiagnosticHandler: diagnosticsEngine.consumeParserDiagnostic)
       }
     } catch SwiftFormatError.fileNotReadable {
       diagnosticsEngine.emitError(
-        "Unable to format \(path): file is not readable or does not exist.")
+        "Unable to format \(url.relativePath): file is not readable or does not exist.")
       return
     } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
       guard !lintFormatOptions.ignoreUnparsableFiles else {
@@ -71,12 +71,12 @@ class FormatFrontend: Frontend {
         stdoutStream.write(source)
         return
       }
-      let location = SourceLocationConverter(file: path, source: source).location(for: position)
+      let location = SourceLocationConverter(file: url.path, source: source).location(for: position)
       diagnosticsEngine.emitError(
         "file contains invalid or unrecognized Swift syntax.", location: location)
       return
     } catch {
-      diagnosticsEngine.emitError("Unable to format \(path): \(error)")
+      diagnosticsEngine.emitError("Unable to format \(url.relativePath): \(error)")
     }
   }
 }

Sources/swift-format/Frontend/Frontend.swift

@@ -23,15 +23,15 @@ class Frontend {
     /// An open file handle to the source code of the file.
     private let fileHandle: FileHandle
 
-    /// The path to the source file being processed.
+    /// A file URL representing the path to the source file being processed.
     ///
     /// It is the responsibility of the specific frontend to make guarantees about the validity of
     /// this path. For example, the formatting frontend ensures that it is a path to an existing
     /// file only when doing in-place formatting (so that the file can be replaced). In other
     /// situations, it may correspond to a different file than the underlying file handle (if
     /// standard input is used with the `--assume-filename` flag), or it may not be a valid path at
     /// all (the string `"<stdin>"`).
-    let path: String
+    let url: URL
 
     /// The configuration that should applied for this file.
     let configuration: Configuration
@@ -46,9 +46,9 @@ class Frontend {
       return String(data: sourceData, encoding: .utf8)
     }()
 
-    init(fileHandle: FileHandle, path: String, configuration: Configuration) {
+    init(fileHandle: FileHandle, url: URL, configuration: Configuration) {
       self.fileHandle = fileHandle
-      self.path = path
+      self.url = url
       self.configuration = configuration
     }
   }
@@ -81,12 +81,12 @@ class Frontend {
 
   /// Runs the linter or formatter over the inputs.
   final func run() {
-    let paths = lintFormatOptions.paths
-
-    if paths.isEmpty {
+    if lintFormatOptions.paths.isEmpty {
       processStandardInput()
     } else {
-      processPaths(paths, parallel: lintFormatOptions.parallel)
+      processURLs(
+        lintFormatOptions.paths.map(URL.init(fileURLWithPath:)),
+        parallel: lintFormatOptions.parallel)
     }
   }
 
@@ -103,52 +103,55 @@ class Frontend {
   /// Processes source content from standard input.
   private func processStandardInput() {
     guard let configuration = configuration(
-      atPath: lintFormatOptions.configurationPath,
-      orInferredFromSwiftFileAtPath: nil)
+      at: lintFormatOptions.configurationPath.map(URL.init(fileURLWithPath:)),
+      orInferredFromSwiftFileAt: nil)
     else {
       // Already diagnosed in the called method.
       return
     }
 
     let fileToProcess = FileToProcess(
       fileHandle: FileHandle.standardInput,
-      path: lintFormatOptions.assumeFilename ?? "<stdin>",
+      url: URL(fileURLWithPath: lintFormatOptions.assumeFilename ?? "<stdin>"),
       configuration: configuration)
     processFile(fileToProcess)
   }
 
-  /// Processes source content from a list of files and/or directories provided as paths.
-  private func processPaths(_ paths: [String], parallel: Bool) {
+  /// Processes source content from a list of files and/or directories provided as file URLs.
+  private func processURLs(_ urls: [URL], parallel: Bool) {
     precondition(
-      !paths.isEmpty,
-      "processPaths(_:) should only be called when paths is non-empty.")
+      !urls.isEmpty,
+      "processURLs(_:) should only be called when 'urls' is non-empty.")
 
     if parallel {
-      let filesToProcess = FileIterator(paths: paths).compactMap(openAndPrepareFile)
+      let filesToProcess = FileIterator(urls: urls).compactMap(openAndPrepareFile)
       DispatchQueue.concurrentPerform(iterations: filesToProcess.count) { index in
         processFile(filesToProcess[index])
       }
     } else {
-      FileIterator(paths: paths).lazy.compactMap(openAndPrepareFile).forEach(processFile)
+      FileIterator(urls: urls).lazy.compactMap(openAndPrepareFile).forEach(processFile)
     }
   }
 
   /// Read and prepare the file at the given path for processing, optionally synchronizing
   /// diagnostic output.
-  private func openAndPrepareFile(atPath path: String) -> FileToProcess? {
-    guard let sourceFile = FileHandle(forReadingAtPath: path) else {
-      diagnosticsEngine.emitError("Unable to open \(path)")
+  private func openAndPrepareFile(at url: URL) -> FileToProcess? {
+    guard let sourceFile = try? FileHandle(forReadingFrom: url) else {
+      diagnosticsEngine.emitError(
+        "Unable to open \(url.relativePath): file is not readable or does not exist")
       return nil
     }
 
-    guard let configuration = configuration(
-      atPath: lintFormatOptions.configurationPath, orInferredFromSwiftFileAtPath: path)
+    guard
+      let configuration = configuration(
+        at: lintFormatOptions.configurationPath.map(URL.init(fileURLWithPath:)),
+        orInferredFromSwiftFileAt: url)
     else {
       // Already diagnosed in the called method.
       return nil
     }
 
-    return FileToProcess(fileHandle: sourceFile, path: path, configuration: configuration)
+    return FileToProcess(fileHandle: sourceFile, url: url, configuration: configuration)
   }
 
   /// Returns the configuration that applies to the given `.swift` source file, when an explicit
@@ -164,14 +167,14 @@ class Frontend {
   ///   `swiftFilePath` if one exists, or the default configuration otherwise. If an error occurred
   ///   when reading the configuration, a diagnostic is emitted and `nil` is returned.
   private func configuration(
-    atPath configurationFilePath: String?,
-    orInferredFromSwiftFileAtPath swiftFilePath: String?
+    at configurationFileURL: URL?,
+    orInferredFromSwiftFileAt swiftFileURL: URL?
   ) -> Configuration? {
     // If an explicit configuration file path was given, try to load it and fail if it cannot be
     // loaded. (Do not try to fall back to a path inferred from the source file path.)
-    if let configurationFilePath = configurationFilePath {
+    if let configurationFileURL = configurationFileURL {
       do {
-        return try configurationLoader.configuration(atPath: configurationFilePath)
+        return try configurationLoader.configuration(at: configurationFileURL)
       } catch {
         diagnosticsEngine.emitError("Unable to read configuration: \(error.localizedDescription)")
         return nil
@@ -180,17 +183,15 @@ class Frontend {
 
     // If no explicit configuration file path was given but a `.swift` source file path was given,
     // then try to load the configuration by inferring it based on the source file path.
-    if let swiftFilePath = swiftFilePath {
+    if let swiftFileURL = swiftFileURL {
       do {
-        if let configuration =
-          try configurationLoader.configuration(forSwiftFileAtPath: swiftFilePath)
-        {
+        if let configuration = try configurationLoader.configuration(forSwiftFileAt: swiftFileURL) {
           return configuration
         }
         // Fall through to the default return at the end of the function.
       } catch {
         diagnosticsEngine.emitError(
-          "Unable to read configuration for \(swiftFilePath): \(error.localizedDescription)")
+          "Unable to read configuration for \(swiftFileURL.path): \(error.localizedDescription)")
         return nil
       }
     }

Sources/swift-format/Frontend/LintFrontend.swift

@@ -22,33 +22,33 @@ class LintFrontend: Frontend {
       configuration: fileToProcess.configuration, findingConsumer: diagnosticsEngine.consumeFinding)
     linter.debugOptions = debugOptions
 
-    let path = fileToProcess.path
+    let url = fileToProcess.url
     guard let source = fileToProcess.sourceText else {
-      diagnosticsEngine.emitError("Unable to read source for linting from \(path).")
+      diagnosticsEngine.emitError(
+        "Unable to lint \(url.relativePath): file is not readable or does not exist.")
       return
     }
 
     do {
-      let assumingFileURL = URL(fileURLWithPath: path)
       try linter.lint(
         source: source,
-        assumingFileURL: assumingFileURL,
+        assumingFileURL: url,
         parsingDiagnosticHandler: diagnosticsEngine.consumeParserDiagnostic)
     } catch SwiftFormatError.fileNotReadable {
       diagnosticsEngine.emitError(
-        "Unable to lint \(path): file is not readable or does not exist.")
+        "Unable to lint \(url.relativePath): file is not readable or does not exist.")
       return
     } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
       guard !lintFormatOptions.ignoreUnparsableFiles else {
         // The caller wants to silently ignore this error.
         return
       }
-      let location = SourceLocationConverter(file: path, source: source).location(for: position)
+      let location = SourceLocationConverter(file: url.path, source: source).location(for: position)
       diagnosticsEngine.emitError(
         "file contains invalid or unrecognized Swift syntax.", location: location)
       return
     } catch {
-      diagnosticsEngine.emitError("Unable to lint \(path): \(error)")
+      diagnosticsEngine.emitError("Unable to lint \(url.relativePath): \(error)")
       return
     }
   }