Diagnostics: teach the parser API to listen to diagnostics emitted from the compiler(parser).

This patch adds a parameter of DiagnosticConsumer to the parser entry point
to capture any diagnostics emitted from the compiler(parser). We also convert the
c structures collected from the compiler side to an existing Diagnostic type
in SwiftSyntax to unify the API.

To avoid unnecessarily calculating line/column from a utf8 offset, a property
is added to DiagnosticConsumer to indicate whether the users want line/column;
by default they are calculated.

rdar://48439271

Author: nkcsgexi

Sources/SwiftSyntax/Diagnostic.swift

@@ -14,7 +14,8 @@
 
 /// A FixIt represents a change to source code in order to "correct" a
 /// diagnostic.
-public enum FixIt: Codable {
+public enum FixIt: Codable, CustomDebugStringConvertible {
+
   /// Remove the characters from the source file over the provided source range.
   case remove(SourceRange)
 
@@ -32,6 +33,10 @@ public enum FixIt: Codable {
     case string
   }
 
+  public var debugDescription: String {
+    return "Fixit: \(range.debugDescription) Text: \"\(text)\""
+  }
+
   public init(from decoder: Decoder) throws {
     let container = try decoder.container(keyedBy: CodingKeys.self)
     let type = try container.decode(String.self, forKey: .type)
@@ -119,8 +124,14 @@ public struct Note: Codable {
 }
 
 /// A Diagnostic message that can be emitted regarding some piece of code.
-public struct Diagnostic: Codable {
-  public struct Message: Codable {
+public struct Diagnostic: Codable, CustomDebugStringConvertible {
+
+  public struct Message: Codable, CustomDebugStringConvertible {
+
+    public var debugDescription: String {
+      return "\(severity): \(text)"
+    }
+
     /// The severity of diagnostic. This can be note, error, or warning.
     public let severity: Severity
 
@@ -157,6 +168,18 @@ public struct Diagnostic: Codable {
   /// An array of possible FixIts to apply to this diagnostic.
   public let fixIts: [FixIt]
 
+  public var debugDescription: String {
+    var lines: [String] = []
+    if let location = location {
+      lines.append("\(location) \(message.debugDescription)")
+    } else {
+      lines.append("\(message.debugDescription)")
+    }
+    fixIts.forEach { lines.append("\($0.debugDescription)") }
+    highlights.forEach { lines.append("Hightlight: \($0.debugDescription)") }
+    return lines.joined(separator: "\n")
+  }
+
   /// A diagnostic builder that exposes mutating operations for notes,
   /// highlights, and FixIts. When a Diagnostic is created, a builder
   /// will be provided in a closure where the user can conditionally

Sources/SwiftSyntax/DiagnosticConsumer.swift

@@ -15,10 +15,19 @@
 /// An object that intends to receive notifications when diagnostics are
 /// emitted.
 public protocol DiagnosticConsumer {
+
+  /// Whether the collected diagnostics should calculate line:column pair; true
+  /// by default.
+  var calculateLineColumn: Bool { get }
+
   /// Handle the provided diagnostic which has just been registered with the
   /// DiagnosticEngine.
   func handle(_ diagnostic: Diagnostic)
 
   /// Finalize the consumption of diagnostics, flushing to disk if necessary.
   func finalize()
 }
+
+public extension DiagnosticConsumer  {
+  var calculateLineColumn: Bool { return true }
+}

Sources/SwiftSyntax/SourceLocation.swift

@@ -12,7 +12,7 @@
 
 /// Represent the user-facing part of SourceLocation that can be calculated
 /// on demand.
-struct ComputedLocation: Codable {
+struct ComputedLocation: Codable, CustomDebugStringConvertible {
   /// The line in the file where this location resides. 1-based.
   let line: Int
 
@@ -23,6 +23,11 @@ struct ComputedLocation: Codable {
   /// The file in which this location resides.
   let file: String
 
+  var debugDescription: String {
+    // Print file name?
+    return "\(line):\(column)"
+  }
+
   init(line: Int, column: Int, file: String) {
     self.line = line
     self.column = column
@@ -38,7 +43,7 @@ struct ComputedLocation: Codable {
 }
 
 /// Represents a source location in a Swift file.
-public struct SourceLocation: Codable {
+public struct SourceLocation: Codable, CustomDebugStringConvertible {
 
   /// Line and column that can be computed on demand.
   private var compLoc: ComputedLocation?
@@ -62,6 +67,13 @@ public struct SourceLocation: Codable {
     return compLoc?.file
   }
 
+  public var debugDescription: String {
+    guard let compLoc = compLoc else {
+      return "\(offset)"
+    }
+    return compLoc.debugDescription
+  }
+
   public init(line: Int, column: Int, offset: Int, file: String) {
     self.offset = offset
     self.compLoc = ComputedLocation(line: line, column: column, file: file)
@@ -79,13 +91,18 @@ public struct SourceLocation: Codable {
 }
 
 /// Represents a start and end location in a Swift file.
-public struct SourceRange: Codable {
+public struct SourceRange: Codable, CustomDebugStringConvertible {
+
   /// The beginning location in the source range.
   public let start: SourceLocation
 
   /// The beginning location in the source range.
   public let end: SourceLocation
 
+  public var debugDescription: String {
+    return "(\(start.debugDescription),\(end.debugDescription))"
+  }
+
   public init(start: SourceLocation, end: SourceLocation) {
     self.start = start
     self.end = end
@@ -111,6 +128,15 @@ public final class SourceLocationConverter {
     assert(tree.byteSize == endOfFile.utf8Offset)
   }
 
+  /// - Parameters:
+  ///   - file: The file path associated with the syntax tree.
+  ///   - source: The source code to convert positions to line/columns for.
+  public init(file: String, source: String) {
+    self.file = file
+    (self.lines, endOfFile) = computeLines(source)
+    assert(source.lengthOfBytes(using: .utf8) == endOfFile.utf8Offset)
+  }
+
   /// Convert a `AbsolutePosition` to a `SourceLocation`. If the position is
   /// exceeding the file length then the `SourceLocation` for the end of file
   /// is returned. If position is negative the location for start of file is
@@ -305,6 +331,22 @@ fileprivate func computeLines(
   return (lines, position)
 }
 
+fileprivate func computeLines(_ source: String) ->
+    ([AbsolutePosition], AbsolutePosition) {
+  var lines: [AbsolutePosition] = []
+  // First line starts from the beginning.
+  lines.append(.startOfFile)
+  var position: AbsolutePosition = .startOfFile
+  let addLine = { (lineLength: SourceLength) in
+    position += lineLength
+    lines.append(position)
+  }
+  var curPrefix: SourceLength = .zero
+  curPrefix = source.forEachLineLength(prefix: curPrefix, body: addLine)
+  position += curPrefix
+  return (lines, position)
+}
+
 fileprivate extension String {
   /// Walks and passes to `body` the `SourceLength` for every detected line,
   /// with the newline character included.

Sources/SwiftSyntax/SyntaxParser.swift

@@ -28,6 +28,9 @@ typealias CTokenData = swiftparse_token_data_t
 typealias CLayoutData = swiftparse_layout_data_t
 typealias CParseLookupResult = swiftparse_lookup_result_t
 typealias CClientNode = swiftparse_client_node_t
+typealias CDiagnostic = swiftparser_diagnostic_t
+typealias CFixit = swiftparse_diagnostic_fixit_t
+typealias CRange = swiftparse_range_t
 
 /// A list of possible errors that could be encountered while parsing a
 /// Syntax tree.
@@ -66,7 +69,9 @@ public enum SyntaxParser {
   /// - Throws: `ParserError`
   public static func parse(
     source: String,
-    parseTransition: IncrementalParseTransition? = nil
+    file: String = "",
+    parseTransition: IncrementalParseTransition? = nil,
+    diagConsumer: DiagnosticConsumer? = nil
   ) throws -> SourceFileSyntax {
     guard nodeHashVerifyResult else {
       throw ParserError.parserCompatibilityCheckFailed
@@ -77,7 +82,7 @@ public enum SyntaxParser {
     var utf8Source = source
     utf8Source.makeNativeUTF8IfNeeded()
 
-    let rawSyntax = parseRaw(utf8Source, parseTransition)
+    let rawSyntax = parseRaw(file, utf8Source, parseTransition, diagConsumer)
 
     guard let file = makeSyntax(.forRoot(rawSyntax)) as? SourceFileSyntax else {
       throw ParserError.invalidSyntaxData
@@ -92,19 +97,23 @@ public enum SyntaxParser {
   /// - Returns: A top-level Syntax node representing the contents of the tree,
   ///            if the parse was successful.
   /// - Throws: `ParserError`
-  public static func parse(_ url: URL) throws -> SourceFileSyntax {
+  public static func parse(_ url: URL,
+      diagConsumer: DiagnosticConsumer? = nil) throws -> SourceFileSyntax {
     // Avoid using `String(contentsOf:)` because it creates a wrapped NSString.
     var fileData = try Data(contentsOf: url)
     fileData.append(0) // null terminate.
     let source = fileData.withUnsafeBytes { (ptr: UnsafePointer<CChar>) in
       return String(cString: ptr)
     }
-    return try parse(source: source)
+    return try parse(source: source, file: url.absoluteString,
+                     diagConsumer: diagConsumer)
   }
 
   private static func parseRaw(
+    _ file: String,
     _ source: String,
-    _ parseTransition: IncrementalParseTransition?
+    _ parseTransition: IncrementalParseTransition?,
+    _ diagConsumer: DiagnosticConsumer?
   ) -> RawSyntax {
     assert(source.isNativeUTF8)
     let c_parser = swiftparse_parser_create()
@@ -135,9 +144,81 @@ public enum SyntaxParser {
       swiftparse_parser_set_node_lookup(c_parser, nodeLookup);
     }
 
+    // Set up diagnostics consumer if requested by the caller.
+    if let diagConsumer = diagConsumer {
+      // If requested, we should set up a source location converter to calculate
+      // line and column.
+      let converter = diagConsumer.calculateLineColumn ?
+        SourceLocationConverter(file: file, source: source) : nil
+      let diagHandler = { (diag: CDiagnostic!) in
+        diagConsumer.handle(Diagnostic(diag: diag, using: converter))
+      }
+      swiftparse_parser_set_diagnostic_handler(c_parser, diagHandler)
+    }
+
     let c_top = swiftparse_parse_string(c_parser, source)
 
   // Get ownership back from the C parser.
     return RawSyntax.moveFromOpaque(c_top)!
   }
 }
+
+extension SourceRange {
+  init(_ range: CRange, _ converter: SourceLocationConverter?) {
+    let start = SourceLocation(offset: Int(range.offset), converter: converter)
+    let end = SourceLocation(offset: Int(range.offset + range.length),
+                             converter: converter)
+    self.init(start: start, end: end)
+  }
+}
+
+extension Diagnostic.Message {
+  init(_ diag: CDiagnostic) {
+    let message = String(cString: swiftparse_diagnostic_get_message(diag))
+    switch swiftparse_diagnostic_get_severity(diag) {
+    case SWIFTPARSER_DIAGNOSTIC_SEVERITY_ERROR:
+      self.init(.error, message)
+    case SWIFTPARSER_DIAGNOSTIC_SEVERITY_WARNING:
+      self.init(.warning, message)
+    case SWIFTPARSER_DIAGNOSTIC_SEVERITY_NOTE:
+      self.init(.note, message)
+    default:
+      fatalError("unrecognized diagnostic level")
+    }
+  }
+}
+
+extension FixIt {
+  init(_ cfixit: CFixit, _ converter: SourceLocationConverter?) {
+    let replacement = String(cString: cfixit.text)
+    let range = SourceRange(cfixit.range, converter)
+    if cfixit.range.length == 0 {
+      // Insert
+      self = .insert(SourceLocation(offset: Int(cfixit.range.offset),
+                                    converter: converter), replacement)
+    } else if replacement.isEmpty {
+      // Remove
+      self = .remove(range)
+    } else {
+      // Replace
+      self = .replace(range, replacement)
+    }
+  }
+}
+
+extension Diagnostic {
+  init(diag: CDiagnostic, using converter: SourceLocationConverter?) {
+    // Collect highlighted ranges
+    let hightlights = (0..<swiftparse_diagnostic_get_range_count(diag)).map {
+      return SourceRange(swiftparse_diagnostic_get_range(diag, $0), converter)
+    }
+    // Collect fixits
+    let fixits = (0..<swiftparse_diagnostic_get_fixit_count(diag)).map {
+      return FixIt(swiftparse_diagnostic_get_fixit(diag, $0), converter)
+    }
+    self.init(message: Diagnostic.Message(diag),
+      location: SourceLocation(offset: Int(swiftparse_diagnostic_get_source_loc(diag)),
+                               converter: converter),
+      notes: [], highlights: hightlights, fixIts: fixits)
+  }
+}

Sources/lit-test-helper/main.swift

@@ -392,6 +392,27 @@ func printSyntaxTree(args: CommandLineArguments) throws {
   tree.walk(&printer)
 }
 
+func printParserDiags(args: CommandLineArguments) throws {
+  let treeURL = URL(fileURLWithPath: try args.getRequired("-source-file"))
+  class ParserDiagPrinter: DiagnosticConsumer {
+    var counter = [0, 0, 0]
+    var calculateLineColumn: Bool { return true }
+    func finalize() {}
+    func handle(_ diag: Diagnostic) {
+      switch diag.message.severity {
+      case .error: counter[0] += 1
+      case .warning: counter[1] += 1
+      case .note: counter[2] += 1
+      }
+      print(diag.debugDescription)
+    }
+    deinit {
+      print("\(counter[0]) error(s) \(counter[1]) warnings(s) \(counter[2]) note(s)")
+    }
+  }
+  _ = try SyntaxParser.parse(treeURL, diagConsumer: ParserDiagPrinter())
+}
+
 do {
   let args = try CommandLineArguments.parse(CommandLine.arguments.dropFirst())
 
@@ -403,6 +424,8 @@ do {
     try performRoundtrip(args: args)
   } else if args.has("-print-tree") {
     try printSyntaxTree(args: args)
+  } else if args.has("-dump-diags") {
+    try printParserDiags(args: args)
   } else if args.has("-help") {
     printHelp()
   } else {

lit_tests/parser-diags.swift

@@ -0,0 +1,27 @@
+// RUN: %lit-test-helper -source-file %s -dump-diags 2>&1 | %FileCheck %s
+
+// CHECK: [[@LINE+2]]:11 error: consecutive statements on a line must be separated by ';'
+// CHECK-NEXT: Fixit: ([[@LINE+1]]:11,[[@LINE+1]]:11) Text: ";"
+let number⁚ Int
+// CHECK-NEXT: [[@LINE-1]]:11 error: operator with postfix spacing cannot start a subexpression
+
+// CHECK-NEXT: [[@LINE+2]]:3 error: invalid character in source file
+// CHECK-NEXT: Fixit: ([[@LINE+1]]:3,[[@LINE+1]]:6) Text: " "
+5 ‒ 5
+// CHECK-NEXT: [[@LINE-1]]:3 note: unicode character '‒' looks similar to '-'; did you mean to use '-'?
+// CHECK-NEXT: Fixit: ([[@LINE-2]]:3,[[@LINE-2]]:6) Text: "-"
+// CHECK-NEXT: [[@LINE-3]]:2 error: consecutive statements on a line must be separated by ';'
+// CHECK-NEXT: Fixit: ([[@LINE-4]]:2,[[@LINE-4]]:2) Text: ";"
+
+// CHECK-NEXT: [[@LINE+2]]:10 error: expected ',' separator
+// CHECK-NEXT: Fixit: ([[@LINE+1]]:9,[[@LINE+1]]:9) Text: ","
+if (true ꝸꝸꝸ false) {}
+
+if (5 ‒ 5) == 0 {}
+// CHECK-NEXT: [[@LINE-1]]:7 error: invalid character in source file
+// CHECK-NEXT: Fixit: ([[@LINE-2]]:7,[[@LINE-2]]:10) Text: " "
+// CHECK-NEXT: [[@LINE-3]]:7 note: unicode character '‒' looks similar to '-'; did you mean to use '-'?
+// CHECK-NEXT: Fixit: ([[@LINE-4]]:7,[[@LINE-4]]:10) Text: "-"
+// CHECK-NEXT: [[@LINE-5]]:11 error: expected ',' separator
+// CHECK-NEXT: Fixit: ([[@LINE-6]]:6,[[@LINE-6]]:6) Text: ","
+// CHECK-NEXT: 7 error(s) 0 warnings(s) 2 note(s)