Merge pull request #270 from allevato/refactor-diagnostics

Refactor "diagnostics" into "findings".

Author: allevato

Package.swift

@@ -33,6 +33,7 @@ let package = Package(
         "SwiftFormatRules",
         "SwiftFormatWhitespaceLinter",
         "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
     .target(name: "SwiftFormatConfiguration"),
@@ -66,6 +67,7 @@ let package = Package(
         "SwiftFormatCore",
         "SwiftFormatRules",
         "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
     .target(
@@ -76,54 +78,61 @@ let package = Package(
         "SwiftFormatConfiguration",
         "SwiftFormatCore",
         "SwiftSyntax",
+        "TSCBasic",
       ]
     ),
     .testTarget(
       name: "SwiftFormatTests",
       dependencies: [
         "SwiftFormat",
+        "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
     .testTarget(
       name: "SwiftFormatConfigurationTests",
       dependencies: ["SwiftFormatConfiguration"]
     ),
     .testTarget(
-      name: "SwiftFormatRulesTests",
+      name: "SwiftFormatCoreTests",
       dependencies: [
         "SwiftFormatConfiguration",
         "SwiftFormatCore",
-        "SwiftFormatPrettyPrint",
-        "SwiftFormatRules",
-        "SwiftFormatTestSupport",
         "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
     .testTarget(
-      name: "SwiftFormatCoreTests",
+      name: "SwiftFormatPerformanceTests",
       dependencies: [
-        "SwiftFormatConfiguration",
-        "SwiftFormatCore",
+        "SwiftFormatTestSupport",
+        "SwiftFormatWhitespaceLinter",
         "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
     .testTarget(
-      name: "SwiftFormatPerformanceTests",
+      name: "SwiftFormatPrettyPrintTests",
       dependencies: [
+        "SwiftFormatConfiguration",
+        "SwiftFormatCore",
+        "SwiftFormatPrettyPrint",
+        "SwiftFormatRules",
         "SwiftFormatTestSupport",
-        "SwiftFormatWhitespaceLinter",
         "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
     .testTarget(
-      name: "SwiftFormatPrettyPrintTests",
+      name: "SwiftFormatRulesTests",
       dependencies: [
         "SwiftFormatConfiguration",
         "SwiftFormatCore",
         "SwiftFormatPrettyPrint",
         "SwiftFormatRules",
         "SwiftFormatTestSupport",
         "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
     .testTarget(
@@ -134,6 +143,7 @@ let package = Package(
         "SwiftFormatTestSupport",
         "SwiftFormatWhitespaceLinter",
         "SwiftSyntax",
+        "SwiftSyntaxParser",
       ]
     ),
   ]
@@ -143,12 +153,14 @@ let package = Package(
 if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
   // Building standalone.
   package.dependencies += [
-    .package(url: "https://github.com/apple/swift-syntax", .branch("main")),
     .package(url: "https://github.com/apple/swift-argument-parser.git", .branch("main")),
+    .package(url: "https://github.com/apple/swift-syntax", .branch("main")),
+    .package(url: "https://github.com/apple/swift-tools-support-core.git", .branch("main")),
   ]
 } else {
   package.dependencies += [
-    .package(path: "../swift-syntax"),
     .package(path: "../swift-argument-parser"),
+    .package(path: "../swift-syntax"),
+    .package(path: "../swift-tools-support-core"),
   ]
 }

Sources/SwiftFormat/Exports.swift

@@ -0,0 +1,20 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2021 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 SwiftFormatCore
+
+// The `SwiftFormatCore` module isn't meant for public use, but these types need to be since they
+// are also part of the public `SwiftFormat` API. Use public typealiases to "re-export" them for
+// now.
+
+public typealias Finding = SwiftFormatCore.Finding
+public typealias FindingCategorizing = SwiftFormatCore.FindingCategorizing

Sources/SwiftFormat/SwiftFormatter.swift

@@ -16,15 +16,16 @@ import SwiftFormatCore
 import SwiftFormatPrettyPrint
 import SwiftFormatRules
 import SwiftSyntax
+import SwiftSyntaxParser
 
 /// Formats Swift source code or syntax trees according to the Swift style guidelines.
 public final class SwiftFormatter {
 
   /// The configuration settings that control the formatter's behavior.
   public let configuration: Configuration
 
-  /// A diagnostic engine to which non-fatal errors will be reported.
-  public let diagnosticEngine: DiagnosticEngine?
+  /// An optional callback that will be notified with any findings encountered during formatting.
+  public let findingConsumer: ((Finding) -> Void)?
 
   /// Advanced options that are useful when debugging the formatter's behavior but are not meant for
   /// general use.
@@ -34,11 +35,12 @@ public final class SwiftFormatter {
   ///
   /// - Parameters:
   ///   - configuration: The configuration settings that control the formatter's behavior.
-  ///   - diagnosticEngine: The diagnostic engine to which non-fatal errors will be reported.
-  ///     Defaults to nil.
-  public init(configuration: Configuration, diagnosticEngine: DiagnosticEngine? = nil) {
+  ///   - findingConsumer: An optional callback that will be notified with any findings encountered
+  ///     during formatting. Unlike the `Linter` API, this defaults to nil for formatting because
+  ///     findings are typically less useful than the final formatted output.
+  public init(configuration: Configuration, findingConsumer: ((Finding) -> Void)? = nil) {
     self.configuration = configuration
-    self.diagnosticEngine = diagnosticEngine
+    self.findingConsumer = findingConsumer
   }
 
   /// Formats the Swift code at the given file URL and writes the result to an output stream.
@@ -47,9 +49,13 @@ public final class SwiftFormatter {
   ///   - url: The URL of the file containing the code to format.
   ///   - outputStream: A value conforming to `TextOutputStream` to which the formatted output will
   ///     be written.
+  ///   - parsingDiagnosticHandler: An optional callback that will be notified if there are any
+  ///     errors when parsing the source code.
   /// - Throws: If an unrecoverable error occurs when formatting the code.
   public func format<Output: TextOutputStream>(
-    contentsOf url: URL, to outputStream: inout Output
+    contentsOf url: URL,
+    to outputStream: inout Output,
+    parsingDiagnosticHandler: ((Diagnostic) -> Void)? = nil
   ) throws {
     guard FileManager.default.isReadableFile(atPath: url.path) else {
       throw SwiftFormatError.fileNotReadable
@@ -58,7 +64,7 @@ public final class SwiftFormatter {
     if FileManager.default.fileExists(atPath: url.path, isDirectory: &isDir), isDir.boolValue {
       throw SwiftFormatError.isDirectory
     }
-    let sourceFile = try SyntaxParser.parse(url)
+    let sourceFile = try SyntaxParser.parse(url, diagnosticHandler: parsingDiagnosticHandler)
     let source = try String(contentsOf: url, encoding: .utf8)
     try format(syntax: sourceFile, assumingFileURL: url, source: source, to: &outputStream)
   }
@@ -72,11 +78,17 @@ public final class SwiftFormatter {
   ///     dummy value will be used.
   ///   - outputStream: A value conforming to `TextOutputStream` to which the formatted output will
   ///     be written.
+  ///   - parsingDiagnosticHandler: An optional callback that will be notified if there are any
+  ///     errors when parsing the source code.
   /// - Throws: If an unrecoverable error occurs when formatting the code.
   public func format<Output: TextOutputStream>(
-    source: String, assumingFileURL url: URL?, to outputStream: inout Output
+    source: String,
+    assumingFileURL url: URL?,
+    to outputStream: inout Output,
+    parsingDiagnosticHandler: ((Diagnostic) -> Void)? = nil
   ) throws {
-    let sourceFile = try SyntaxParser.parse(source: source)
+    let sourceFile =
+      try SyntaxParser.parse(source: source, diagnosticHandler: parsingDiagnosticHandler)
     try format(syntax: sourceFile, assumingFileURL: url, source: source, to: &outputStream)
   }
 
@@ -108,7 +120,7 @@ public final class SwiftFormatter {
 
     let assumedURL = url ?? URL(fileURLWithPath: "source")
     let context = Context(
-      configuration: configuration, diagnosticEngine: diagnosticEngine, fileURL: assumedURL,
+      configuration: configuration, findingConsumer: findingConsumer, fileURL: assumedURL,
       sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
     let pipeline = FormatPipeline(context: context)
     let transformedSyntax = pipeline.visit(Syntax(syntax))

Sources/SwiftFormat/SwiftLinter.swift

@@ -17,6 +17,7 @@ import SwiftFormatPrettyPrint
 import SwiftFormatRules
 import SwiftFormatWhitespaceLinter
 import SwiftSyntax
+import SwiftSyntaxParser
 
 /// Diagnoses and reports problems in Swift source code or syntax trees according to the Swift style
 /// guidelines.
@@ -25,8 +26,8 @@ public final class SwiftLinter {
   /// The configuration settings that control the linter's behavior.
   public let configuration: Configuration
 
-  /// A diagnostic engine to which lint findings will be reported.
-  public let diagnosticEngine: DiagnosticEngine
+  /// A callback that will be notified with any findings encountered during linting.
+  public let findingConsumer: (Finding) -> Void
 
   /// Advanced options that are useful when debugging the linter's behavior but are not meant for
   /// general use.
@@ -36,25 +37,32 @@ public final class SwiftLinter {
   ///
   /// - Parameters:
   ///   - configuration: The configuration settings that control the linter's behavior.
-  ///   - diagnosticEngine: The diagnostic engine to which lint findings will be reported.
-  public init(configuration: Configuration, diagnosticEngine: DiagnosticEngine) {
+  ///   - findingConsumer: A callback that will be notified with any findings encountered during
+  ///     linting.
+  public init(configuration: Configuration, findingConsumer: @escaping (Finding) -> Void) {
     self.configuration = configuration
-    self.diagnosticEngine = diagnosticEngine
+    self.findingConsumer = findingConsumer
   }
 
   /// Lints the Swift code at the given file URL.
   ///
-  /// - Parameters url: The URL of the file containing the code to format.
+  /// - Parameters:
+  ///   - url: The URL of the file containing the code to format.
+  ///   - parsingDiagnosticHandler: An optional callback that will be notified if there are any
+  ///     errors when parsing the source code.
   /// - Throws: If an unrecoverable error occurs when formatting the code.
-  public func lint(contentsOf url: URL) throws {
+  public func lint(
+    contentsOf url: URL,
+    parsingDiagnosticHandler: ((Diagnostic) -> Void)? = nil
+  ) throws {
     guard FileManager.default.isReadableFile(atPath: url.path) else {
       throw SwiftFormatError.fileNotReadable
     }
     var isDir: ObjCBool = false
     if FileManager.default.fileExists(atPath: url.path, isDirectory: &isDir), isDir.boolValue {
       throw SwiftFormatError.isDirectory
     }
-    let sourceFile = try SyntaxParser.parse(url)
+    let sourceFile = try SyntaxParser.parse(url, diagnosticHandler: parsingDiagnosticHandler)
     let source = try String(contentsOf: url, encoding: .utf8)
     try lint(syntax: sourceFile, assumingFileURL: url, source: source)
   }
@@ -64,9 +72,16 @@ public final class SwiftLinter {
   /// - Parameters:
   ///   - source: The Swift source code to be linted.
   ///   - url: A file URL denoting the filename/path that should be assumed for this source code.
+  ///   - parsingDiagnosticHandler: An optional callback that will be notified if there are any
+  ///     errors when parsing the source code.
   /// - Throws: If an unrecoverable error occurs when formatting the code.
-  public func lint(source: String, assumingFileURL url: URL) throws {
-    let sourceFile = try SyntaxParser.parse(source: source)
+  public func lint(
+    source: String,
+    assumingFileURL url: URL,
+    parsingDiagnosticHandler: ((Diagnostic) -> Void)? = nil
+  ) throws {
+    let sourceFile =
+      try SyntaxParser.parse(source: source, diagnosticHandler: parsingDiagnosticHandler)
     try lint(syntax: sourceFile, assumingFileURL: url, source: source)
   }
 
@@ -88,7 +103,7 @@ public final class SwiftLinter {
     }
 
     let context = Context(
-      configuration: configuration, diagnosticEngine: diagnosticEngine, fileURL: url,
+      configuration: configuration, findingConsumer: findingConsumer, fileURL: url,
       sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
     let pipeline = LintPipeline(context: context)
     pipeline.walk(Syntax(syntax))

Sources/SwiftFormatCore/Context.swift

@@ -16,9 +16,9 @@ import SwiftSyntax
 
 /// Context contains the bits that each formatter and linter will need access to.
 ///
-/// Specifically, it is the container for the shared configuration, diagnostic engine, and URL of
+/// Specifically, it is the container for the shared configuration, diagnostic consumer, and URL of
 /// the current file.
-public class Context {
+public final class Context {
 
   /// Tracks whether `XCTest` has been imported so that certain logic can be modified for files that
   /// are known to be tests.
@@ -37,8 +37,8 @@ public class Context {
   /// The configuration for this run of the pipeline, provided by a configuration JSON file.
   public let configuration: Configuration
 
-  /// The engine in which to emit diagnostics, if running in Lint mode.
-  public let diagnosticEngine: DiagnosticEngine?
+  /// Emits findings to the finding consumer.
+  public let findingEmitter: FindingEmitter
 
   /// The URL of the file being linted or formatted.
   public let fileURL: URL
@@ -58,14 +58,14 @@ public class Context {
   /// Creates a new Context with the provided configuration, diagnostic engine, and file URL.
   public init(
     configuration: Configuration,
-    diagnosticEngine: DiagnosticEngine?,
+    findingConsumer: ((Finding) -> Void)?,
     fileURL: URL,
     sourceFileSyntax: SourceFileSyntax,
     source: String? = nil,
     ruleNameCache: [ObjectIdentifier: String]
   ) {
     self.configuration = configuration
-    self.diagnosticEngine = diagnosticEngine
+    self.findingEmitter = FindingEmitter(consumer: findingConsumer)
     self.fileURL = fileURL
     self.importsXCTest = .notDetermined
     self.sourceLocationConverter =