Merge pull request swiftlang#147 from dylansturg/better_bad_code_errors

Use a location specific diagnostic message when the input is invalid.

Author: allevato

Sources/SwiftFormat/SwiftFormatError.swift

@@ -10,6 +10,8 @@
 //
 //===----------------------------------------------------------------------===//
 
+import SwiftSyntax
+
 /// Errors that can be thrown by the `SwiftFormatter` and `SwiftLinter` APIs.
 public enum SwiftFormatError: Error {
 
@@ -20,5 +22,5 @@ public enum SwiftFormatError: Error {
   case isDirectory
 
   /// The file contains invalid or unrecognized Swift syntax and cannot be handled safely.
-  case fileContainsInvalidSyntax
+  case fileContainsInvalidSyntax(position: AbsolutePosition)
 }

Sources/SwiftFormat/SwiftFormatter.swift

@@ -91,8 +91,8 @@ public final class SwiftFormatter {
   public func format<Output: TextOutputStream>(
     syntax: SourceFileSyntax, assumingFileURL url: URL?, to outputStream: inout Output
   ) throws {
-    guard isSyntaxValidForProcessing(Syntax(syntax)) else {
-      throw SwiftFormatError.fileContainsInvalidSyntax
+    if let position = firstInvalidSyntaxPosition(in: Syntax(syntax)) {
+      throw SwiftFormatError.fileContainsInvalidSyntax(position: position)
     }
 
     let assumedURL = url ?? URL(fileURLWithPath: "source")

Sources/SwiftFormat/SwiftLinter.swift

@@ -75,8 +75,8 @@ public final class SwiftLinter {
   ///   - url: A file URL denoting the filename/path that should be assumed for this syntax tree.
   /// - Throws: If an unrecoverable error occurs when formatting the code.
   public func lint(syntax: SourceFileSyntax, assumingFileURL url: URL) throws {
-    guard isSyntaxValidForProcessing(Syntax(syntax)) else {
-      throw SwiftFormatError.fileContainsInvalidSyntax
+    if let position = firstInvalidSyntaxPosition(in: Syntax(syntax)) {
+      throw SwiftFormatError.fileContainsInvalidSyntax(position: position)
     }
 
     let context = Context(

Sources/SwiftFormat/SyntaxValidatingVisitor.swift

@@ -14,61 +14,62 @@ import SwiftSyntax
 
 /// A SyntaxVisitor that searches for nodes that cannot be handled safely.
 fileprivate class SyntaxValidatingVisitor: SyntaxVisitor {
-  /// Tracks whether an invalid node has been encountered.
-  var isValidSyntax = true
+  /// Stores the start position of the first node that contains invalid syntax.
+  var invalidSyntaxStartPosition: AbsolutePosition?
 
   override func visit(_ node: UnknownSyntax) -> SyntaxVisitorContinueKind {
-    isValidSyntax = false
+    invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
     return .skipChildren
   }
 
   override func visit(_ node: UnknownDeclSyntax) -> SyntaxVisitorContinueKind {
-    isValidSyntax = false
+    invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
     return .skipChildren
   }
 
   override func visit(_ node: UnknownExprSyntax) -> SyntaxVisitorContinueKind {
-    isValidSyntax = false
+    invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
     return .skipChildren
   }
 
   override func visit(_ node: UnknownStmtSyntax) -> SyntaxVisitorContinueKind {
-    isValidSyntax = false
+    invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
     return .skipChildren
   }
 
   override func visit(_ node: UnknownTypeSyntax) -> SyntaxVisitorContinueKind {
-    isValidSyntax = false
+    invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
     return .skipChildren
   }
 
   override func visit(_ node: UnknownPatternSyntax) -> SyntaxVisitorContinueKind {
-    isValidSyntax = false
+    invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
     return .skipChildren
   }
 
   override func visit(_ node: NonEmptyTokenListSyntax) -> SyntaxVisitorContinueKind {
-    isValidSyntax = false
+    invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
     return .skipChildren
   }
 
   override func visit(_ node: AttributeSyntax) -> SyntaxVisitorContinueKind {
     // The token list is used to collect any unexpected tokens. When it's missing or empty, then
     // there were no unexpected tokens. Otherwise, the attribute is invalid.
     guard node.tokenList?.isEmpty ?? true else {
-      isValidSyntax = false
+      invalidSyntaxStartPosition = node.positionAfterSkippingLeadingTrivia
       return .skipChildren
     }
     return .visitChildren
   }
 }
 
-/// Returns whether the given syntax contains any nodes which are invalid or unrecognized and
-/// cannot be handled safely.
+/// Determines whether the given syntax has any nodes which are invalid or unrecognized, and, if
+/// so, returns the starting position of the first such node. Otherwise, returns nil indicating the
+/// syntax is valid.
 ///
 /// - Parameter syntax: The root of a tree of syntax nodes to check for compatibility.
-func isSyntaxValidForProcessing(_ syntax: Syntax) -> Bool {
+func firstInvalidSyntaxPosition(in syntax: Syntax) -> AbsolutePosition? {
   let visitor = SyntaxValidatingVisitor()
   visitor.walk(syntax)
-  return visitor.isValidSyntax
+  return visitor.invalidSyntaxStartPosition
 }

Sources/swift-format/Run.swift

@@ -48,13 +48,14 @@ func lintMain(
     diagnosticEngine.diagnose(
       Diagnostic.Message(.error, "Unable to lint \(path): file is not readable or does not exist."))
     return 1
-  } catch SwiftFormatError.fileContainsInvalidSyntax {
-     let path = assumingFileURL.path
-     diagnosticEngine.diagnose(
-       Diagnostic.Message(
-         .error, "Unable to line \(path): file contains invalid or unrecognized Swift syntax."))
-     return 1
-   } catch {
+  } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
+    let path = assumingFileURL.path
+    let location = SourceLocationConverter(file: path, source: source).location(for: position)
+    diagnosticEngine.diagnose(
+      Diagnostic.Message(.error, "file contains invalid or unrecognized Swift syntax."),
+      location: location)
+    return 1
+  } catch {
     let path = assumingFileURL.path
     diagnosticEngine.diagnose(Diagnostic.Message(.error, "Unable to lint \(path): \(error)"))
     exit(1)
@@ -107,11 +108,12 @@ func formatMain(
       Diagnostic.Message(
         .error, "Unable to format \(path): file is not readable or does not exist."))
     return 1
-  } catch SwiftFormatError.fileContainsInvalidSyntax {
+  } catch SwiftFormatError.fileContainsInvalidSyntax(let position) {
     let path = assumingFileURL.path
+    let location = SourceLocationConverter(file: path, source: source).location(for: position)
     diagnosticEngine.diagnose(
-      Diagnostic.Message(
-        .error, "Unable to format \(path): file contains invalid or unrecognized Swift syntax."))
+      Diagnostic.Message(.error, "file contains invalid or unrecognized Swift syntax."),
+      location: location)
     return 1
   } catch {
     let path = assumingFileURL.path

Tests/SwiftFormatTests/SyntaxValidatingVisitorTests.swift

@@ -18,7 +18,7 @@ final class SyntaxValidatingVisitorTests: XCTestCase {
       @unknown default: break
       }
       """
-    XCTAssertTrue(isSyntaxValidForProcessing(createSyntax(from: input)))
+    XCTAssertNil(firstInvalidSyntaxPosition(in: createSyntax(from: input)))
   }
 
   func testInvalidSyntax() {
@@ -28,7 +28,7 @@ final class SyntaxValidatingVisitorTests: XCTestCase {
         var bar = 0
       }
       """
-    XCTAssertFalse(isSyntaxValidForProcessing(createSyntax(from: input)))
+    assertInvalidSyntax(in: input, atLine: 1, column: 1)
 
     input =
       """
@@ -37,17 +37,31 @@ final class SyntaxValidatingVisitorTests: XCTestCase {
       @unknown what_is_this default: break
       }
       """
-    XCTAssertFalse(isSyntaxValidForProcessing(createSyntax(from: input)))
+    assertInvalidSyntax(in: input, atLine: 3, column: 1)
 
     input =
       """
       @unknown c class Foo {}
       """
-    XCTAssertFalse(isSyntaxValidForProcessing(createSyntax(from: input)))
+    assertInvalidSyntax(in: input, atLine: 1, column: 1)
   }
 
   /// Parses the given source into a syntax tree.
   private func createSyntax(from source: String) -> Syntax {
     return Syntax(try! SyntaxParser.parse(source: source))
   }
+
+  /// Asserts that `SyntaxValidatingVisitor` finds invalid syntax in the given source code at the
+  /// given line and column.
+  private func assertInvalidSyntax(
+    in source: String, atLine: Int, column: Int, file: StaticString = #file, line: UInt = #line
+  ) {
+    guard let position = firstInvalidSyntaxPosition(in: createSyntax(from: source)) else {
+      XCTFail("No invalid syntax was found", file: file, line: line)
+      return
+    }
+    let location = SourceLocationConverter(file: "", source: source).location(for: position)
+    XCTAssertEqual(location.line, atLine, file: file, line: line)
+    XCTAssertEqual(location.column, column, file: file, line: line)
+  }
 }