Adds @_spi to RuleGenerator, and improves broken rule warnings.

Author: natikgadzhi

Sources/swift-format/Frontend/Frontend.swift

@@ -11,10 +11,11 @@
 //===----------------------------------------------------------------------===//
 
 import Foundation
-@_spi(Internal) import SwiftFormat
 import SwiftSyntax
 import SwiftParser
 
+@_spi(Internal) import SwiftFormat
+
 class Frontend {
   /// Represents a file to be processed by the frontend and any file-specific options associated
   /// with it.
@@ -176,50 +177,80 @@ class Frontend {
     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.)
+    // The actual config file that this frontend is going to use.
+    let configURL: URL?
+
+    // If the config path is provided, use that.
+    // Otherwise, try inferring config path based on the input .swift file path.
     if let configurationFileURL = configurationFileURL {
-      do {
-        let configuration = try configurationLoader.configuration(at: configurationFileURL)
-        self.verifyConfigurationRules(for: configuration)
-        return configuration
-      } catch {
-        diagnosticsEngine.emitError("Unable to read configuration: \(error.localizedDescription)")
-        return nil
-      }
+      configURL = configurationFileURL
+    } else if let swiftFileURL = swiftFileURL {
+      configURL = Configuration.url(forConfigurationFileApplyingTo: swiftFileURL)
+    } else {
+
+      // If neither path was given (for example, formatting standard input with no assumed filename)
+      // or if there was no configuration found by inferring it from the source file path, return the
+      // default configuration.
+      return Configuration()
     }
+    
+    // Now that we have the configURL, we're going to load it and validate the rules in it.
+    do {
+      // Ok to force unwrap because the only branch where configURL would not be set returned early.
+      let configuration = try configurationLoader.configuration(at: configURL!)
+      try self.verifyConfigurationRules(for: configuration, at: configURL!)
 
-    // 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 swiftFileURL = swiftFileURL {
-      do {
-        if let configuration = try configurationLoader.configuration(forSwiftFileAt: swiftFileURL) {
-          self.verifyConfigurationRules(for: configuration)
-          return configuration
-        }
-        // Fall through to the default return at the end of the function.
-      } catch {
+      return configuration
+    } catch {
+      if let swiftFileURL {
         diagnosticsEngine.emitError(
           "Unable to read configuration for \(swiftFileURL.path): \(error.localizedDescription)")
-        return nil
+      } else {
+        diagnosticsEngine.emitError("Unable to read configuration: \(error.localizedDescription)")
       }
+      return nil
     }
-
-    // If neither path was given (for example, formatting standard input with no assumed filename)
-    // or if there was no configuration found by inferring it from the source file path, return the
-    // default configuration.
-    return Configuration()
   }
 
   /// Checks if all the rules in the given configuration are supported by the registry.
-  /// If there are any rules that are not supported, they are emitted as a warning.
-  private func verifyConfigurationRules(for configuration: Configuration) {
+  /// If there are any rules that are not recognized by this version of swift-format, they are emitted as a warning.
+  ///
+  /// - parameters:
+  ///   - configuration: `Configuration` to verify
+  ///   - configURL: Configuration file `URL` to use in `Diagnostic.Location`
+  private func verifyConfigurationRules(for configuration: Configuration, at configURL: URL) throws {
     // If any rules in the decoded configuration are not supported by the registry,
     // emit them into the diagnosticsEngine as warnings.
-    // That way they will be printed out, but we'll continue execution on the valid rules.
     let invalidRules = configuration.rules.filter { !RuleRegistry.rules.keys.contains($0.key) }
-    if !invalidRules.isEmpty {
-      diagnosticsEngine.emitWarning("Configuration contains an unrecognized rule: \(invalidRules.keys.joined(separator: ", "))", location: nil)
+
+    let configData = try Data(contentsOf: configURL)
+    guard let configString = String(data: configData, encoding: .utf8) else {
+      diagnosticsEngine.emitError("Unable to read the configuration file at \(configURL.description)")
+      return
+    }
+
+    for rule  in invalidRules {
+      let location = diagnosticLocationFor(substring: rule.key, in: configString, filePath: configURL.relativePath)
+      diagnosticsEngine.emitWarning(
+        "Configuration contains an unrecognized rule \(rule.key)",
+        location: location
+      )
     }
   }
+
+  /// Returns a `Diagnostic.Location` of the substring in an array of Strings.
+  /// It's intented to find the location of a configuration Rule in an array of Strings representing the encoded JSON swift-format configuration file.
+  private func diagnosticLocationFor(substring: String, in configuration: String, filePath: String) -> Diagnostic.Location? {
+    if let offset = configuration.range(of: substring)?.lowerBound {
+      let splits = configuration.prefix(through: offset).split(separator: "\n")
+      let line = splits.count
+
+      // In theory, that should never happen, but if we can't fetch the column, just use 1
+      let column = splits.last?.count ?? 1
+      return Diagnostic.Location(file: filePath, line: line, column: column)
+    }
+
+    // If the config string did not contain the substring at all, return nil.
+    return nil
+  }
 }

Sources/swift-format/Utilities/Diagnostic.swift

@@ -47,6 +47,12 @@ struct Diagnostic {
       self.line = findingLocation.line
       self.column = findingLocation.column
     }
+
+    init(file: String, line: Int, column: Int) {
+      self.file = file
+      self.line = line
+      self.column = column
+    }
   }
 
   /// The severity of the diagnostic.

Sources/swift-format/Utilities/DiagnosticsEngine.swift

@@ -69,13 +69,13 @@ final class DiagnosticsEngine {
   ///
   /// - Parameters:
   ///   - message: The message associated with the error.
-  ///   - location: The location in the source code associated with the error, or nil if there is no
+  ///   - location: The `Diagnostic.Location` in the source code associated with the error, or nil if there is no
   ///     location associated with the error.
-  func emitWarning(_ message: String, location: SourceLocation? = nil) {
+  func emitWarning(_ message: String, location: Diagnostic.Location? = nil) {
     emit(
       Diagnostic(
         severity: .warning,
-        location: location.map(Diagnostic.Location.init),
+        location: location,
         message: message))
   }