Merge pull request #2143 from DougGregor/diagnostic-format-improvements

Author: DougGregor

Release Notes/510.md

@@ -17,6 +17,10 @@
   - Issue: https://github.com/apple/swift-syntax/issues/1984
   - Pull Request: https://github.com/apple/swift-syntax/pull/2112
 
+- `DiagnosticSeverity` and `PluginMessage.Diagnostic.Severity` now have new case named `remark`
+  - Description: Remarks are used by the Swift compiler and other tools to describe some aspect of translation that doesn't reflect correctness, but may be useful for the user. Remarks have been added to the diagnostic severity enums to align with the Swift compiler.
+  - Pull Request: https://github.com/apple/swift-syntax/pull/2143
+
 ## API Behavior Changes
 
 ## Deprecations

Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift

@@ -53,6 +53,7 @@ extension PluginMessage.Diagnostic.Severity {
     case .error: self = .error
     case .warning: self = .warning
     case .note: self = .note
+    case .remark: self = .remark
     }
   }
 }

Sources/SwiftCompilerPluginMessageHandling/PluginMessages.swift

@@ -155,6 +155,7 @@ public enum PluginMessage {
       case error
       case warning
       case note
+      case remark
     }
     public struct Position: Codable {
       public var fileName: String

Sources/SwiftDiagnostics/DiagnosticsFormatter.swift

@@ -182,14 +182,13 @@ public struct DiagnosticsFormatter {
   ///   - suffixTexts: suffix text to be printed at the given absolute
   ///                  locations within the source file.
   func annotatedSource(
-    fileName: String?,
     tree: some SyntaxProtocol,
     diags: [Diagnostic],
     indentString: String,
     suffixTexts: [AbsolutePosition: String],
     sourceLocationConverter: SourceLocationConverter? = nil
   ) -> String {
-    let slc = sourceLocationConverter ?? SourceLocationConverter(fileName: fileName ?? "", tree: tree)
+    let slc = sourceLocationConverter ?? SourceLocationConverter(fileName: "<unknown>", tree: tree)
 
     // First, we need to put each line and its diagnostics together
     var annotatedSourceLines = [AnnotatedSourceLine]()
@@ -218,20 +217,9 @@ public struct DiagnosticsFormatter {
       return nil
     }
 
+    // Accumulate the fully annotated source files here.
     var annotatedSource = ""
 
-    // If there was a filename, add it first.
-    if let fileName {
-      let header = colorizeBufferOutline("===")
-      let firstLine =
-        1
-        + (annotatedSourceLines.enumerated().first { (lineIndex, sourceLine) in
-          !sourceLine.isFreeOfAnnotations
-        }?.offset ?? 0)
-
-      annotatedSource.append("\(indentString)\(header) \(fileName):\(firstLine) \(header)\n")
-    }
-
     /// Keep track if a line missing char should be printed
     var hasLineBeenSkipped = false
 
@@ -318,7 +306,6 @@ public struct DiagnosticsFormatter {
     diags: [Diagnostic]
   ) -> String {
     return annotatedSource(
-      fileName: nil,
       tree: tree,
       diags: diags,
       indentString: "",
@@ -328,22 +315,36 @@ public struct DiagnosticsFormatter {
 
   /// Annotates the given ``DiagnosticMessage`` with an appropriate ANSI color code (if the value of the `colorize`
   /// property is `true`) and returns the result as a printable string.
-  private func colorizeIfRequested(_ message: DiagnosticMessage) -> String {
-    switch message.severity {
+  func colorizeIfRequested(_ message: DiagnosticMessage) -> String {
+    colorizeIfRequested(severity: message.severity, message: message.message)
+  }
+
+  /// Annotates a diagnostic message with the given severity and text with an appropriate ANSI color code.
+  func colorizeIfRequested(severity: DiagnosticSeverity, message: String) -> String {
+    let severityText: String
+    let severityAnnotation: ANSIAnnotation
+
+    switch severity {
     case .error:
-      let annotation = ANSIAnnotation(color: .red, trait: .bold)
-      return colorizeIfRequested("error: \(message.message)", annotation: annotation)
+      severityText = "error"
+      severityAnnotation = .errorText
 
     case .warning:
-      let color = ANSIAnnotation(color: .yellow)
-      let prefix = colorizeIfRequested("warning: ", annotation: color.withTrait(.bold))
+      severityText = "warning"
+      severityAnnotation = .warningText
 
-      return prefix + colorizeIfRequested(message.message, annotation: color);
     case .note:
-      let color = ANSIAnnotation(color: .default, trait: .bold)
-      let prefix = colorizeIfRequested("note: ", annotation: color)
-      return prefix + message.message
+      severityText = "note"
+      severityAnnotation = .noteText
+
+    case .remark:
+      severityText = "remark"
+      severityAnnotation = .remarkText
     }
+
+    let prefix = colorizeIfRequested("\(severityText): ", annotation: severityAnnotation)
+
+    return prefix + colorizeIfRequested(message, annotation: .diagnosticText);
   }
 
   /// Apply the given color and trait to the specified text, when we are
@@ -421,4 +422,24 @@ struct ANSIAnnotation {
   static var sourceHighlight: ANSIAnnotation {
     ANSIAnnotation(color: .default, trait: .underline)
   }
+
+  static var diagnosticText: ANSIAnnotation {
+    ANSIAnnotation(color: .default, trait: .bold)
+  }
+
+  static var errorText: ANSIAnnotation {
+    ANSIAnnotation(color: .red, trait: .bold)
+  }
+
+  static var warningText: ANSIAnnotation {
+    ANSIAnnotation(color: .yellow, trait: .bold)
+  }
+
+  static var noteText: ANSIAnnotation {
+    ANSIAnnotation(color: .default, trait: .bold)
+  }
+
+  static var remarkText: ANSIAnnotation {
+    ANSIAnnotation(color: .blue, trait: .bold)
+  }
 }

Sources/SwiftDiagnostics/GroupedDiagnostics.swift

@@ -133,6 +133,32 @@ extension GroupedDiagnostics {
     }
   }
 
+  // Find the "primary" diagnostic that will be shown at the top of the diagnostic
+  // message. This is typically the error, warning, or remark.
+  private func findPrimaryDiagnostic(in sourceFile: SourceFile) -> (SourceFile, Diagnostic)? {
+    // If there is a non-note diagnostic, it's the primary diagnostic.
+    if let primaryDiag = sourceFile.diagnostics.first(where: { $0.diagMessage.severity != .note }) {
+      return (sourceFile, primaryDiag)
+    }
+
+    // If one of our child source files has a primary diagnostic, return that.
+    for childID in sourceFile.children {
+      if let foundInChild = findPrimaryDiagnostic(in: sourceFiles[childID.id]) {
+        return foundInChild
+      }
+    }
+
+    // If this is a root note, take the first note.
+    if sourceFile.parent == nil,
+      let note = sourceFile.diagnostics.first
+    {
+      return (sourceFile, note)
+    }
+
+    // There is no primary diagnostic.
+    return nil
+  }
+
   /// Annotate the source for a given source file ID, embedding its child
   /// source files.
   func annotateSource(
@@ -164,11 +190,41 @@ extension GroupedDiagnostics {
 
     // If this is a nested source file, draw a box around it.
     let isRoot = sourceFile.parent == nil
-    let prefixString: String
+    var prefixString: String
     let suffixString: String
 
     if isRoot {
-      prefixString = ""
+      // If there's a primary diagnostic, print it first.
+      if let (primaryDiagSourceFile, primaryDiag) = findPrimaryDiagnostic(in: sourceFile) {
+        let primaryDiagSLC = SourceLocationConverter(fileName: primaryDiagSourceFile.displayName, tree: primaryDiagSourceFile.tree)
+        let location = primaryDiag.location(converter: primaryDiagSLC)
+
+        // Display file/line/column and diagnostic text for the primary diagnostic.
+        prefixString = "\(location.file):\(location.line):\(location.column): \(formatter.colorizeIfRequested(primaryDiag.diagMessage))\n"
+
+        // If the primary diagnostic source file is not the same as the root source file, we're pointing into a generated buffer.
+        // Provide a link back to the original source file where this generated buffer occurred, so it's easy to find if
+        // (for example) the generated buffer is no longer available.
+        if sourceFile.id != primaryDiagSourceFile.id,
+          var (rootSourceID, rootPosition) = primaryDiagSourceFile.parent
+        {
+          // Go all the way up to the root to find the absolute position of the outermost generated buffer within the
+          // root source file.
+          while let parent = sourceFiles[rootSourceID.id].parent {
+            (rootSourceID, rootPosition) = parent
+          }
+
+          if rootSourceID == sourceFileID {
+            let bufferLoc = slc.location(for: rootPosition)
+            let coloredMessage = formatter.colorizeIfRequested(severity: .note, message: "expanded code originates here")
+            prefixString += "╰─ \(bufferLoc.file):\(bufferLoc.line):\(bufferLoc.column): \(coloredMessage)\n"
+          }
+        }
+      } else {
+        let firstLine = sourceFile.diagnostics.first.map { $0.location(converter: slc).line } ?? 0
+        prefixString = "\(sourceFile.displayName): \(firstLine):"
+      }
+
       suffixString = ""
     } else {
       let padding = indentString.dropLast(1)
@@ -190,7 +246,6 @@ extension GroupedDiagnostics {
     // Render the buffer.
     return prefixString
       + formatter.annotatedSource(
-        fileName: isRoot ? sourceFile.displayName : nil,
         tree: sourceFile.tree,
         diags: sourceFile.diagnostics,
         indentString: colorizeBufferOutline(indentString),

Sources/SwiftDiagnostics/Message.swift

@@ -30,6 +30,7 @@ public enum DiagnosticSeverity {
   case error
   case warning
   case note
+  case remark
 }
 
 /// Types conforming to this protocol represent diagnostic messages that can be

Tests/SwiftDiagnosticsTest/DiagnosticsFormatterTests.swift

@@ -101,7 +101,7 @@ final class DiagnosticsFormatterTests: XCTestCase {
 
     let expectedOutput = """
       \u{001B}[0;36m1 │\u{001B}[0;0m var foo = bar +
-        \u{001B}[0;36m│\u{001B}[0;0m                ╰─ \u{001B}[1;31merror: expected expression after operator\u{001B}[0;0m
+        \u{001B}[0;36m│\u{001B}[0;0m                ╰─ \u{001B}[1;31merror: \u{001B}[0;0m\u{001B}[1;39mexpected expression after operator\u{001B}[0;0m
 
       """
     assertStringsEqualWithDiff(annotate(source: source, colorize: true), expectedOutput)
@@ -113,9 +113,9 @@ final class DiagnosticsFormatterTests: XCTestCase {
       """
     let expectedOutput = """
       \u{001B}[0;36m1 │\u{001B}[0;0m foo.[].[].[]
-        \u{001B}[0;36m│\u{001B}[0;0m     │  │  ╰─ \u{001B}[1;31merror: expected name in member access\u{001B}[0;0m
-        \u{001B}[0;36m│\u{001B}[0;0m     │  ╰─ \u{001B}[1;31merror: expected name in member access\u{001B}[0;0m
-        \u{001B}[0;36m│\u{001B}[0;0m     ╰─ \u{001B}[1;31merror: expected name in member access\u{001B}[0;0m
+        \u{001B}[0;36m│\u{001B}[0;0m     │  │  ╰─ \u{001B}[1;31merror: \u{001B}[0;0m\u{001B}[1;39mexpected name in member access\u{001B}[0;0m
+        \u{001B}[0;36m│\u{001B}[0;0m     │  ╰─ \u{001B}[1;31merror: \u{001B}[0;0m\u{001B}[1;39mexpected name in member access\u{001B}[0;0m
+        \u{001B}[0;36m│\u{001B}[0;0m     ╰─ \u{001B}[1;31merror: \u{001B}[0;0m\u{001B}[1;39mexpected name in member access\u{001B}[0;0m
 
       """
     assertStringsEqualWithDiff(annotate(source: source, colorize: true), expectedOutput)
@@ -128,8 +128,8 @@ final class DiagnosticsFormatterTests: XCTestCase {
 
     let expectedOutput = """
       \u{001B}[0;36m1 │\u{001B}[0;0m for \u{001B}[4;39m(i\u{001B}[0;0m \u{001B}[4;39m= 🐮; i != 👩‍👩‍👦‍👦; i += 1)\u{001B}[0;0m { }
-        \u{001B}[0;36m│\u{001B}[0;0m │      ╰─ \u{001B}[1;31merror: expected ')' to end tuple pattern\u{001B}[0;0m
-        \u{001B}[0;36m│\u{001B}[0;0m ╰─ \u{001B}[1;31merror: C-style for statement has been removed in Swift 3\u{001B}[0;0m
+        \u{001B}[0;36m│\u{001B}[0;0m │      ╰─ \u{001B}[1;31merror: \u{001B}[0;0m\u{001B}[1;39mexpected ')' to end tuple pattern\u{001B}[0;0m
+        \u{001B}[0;36m│\u{001B}[0;0m ╰─ \u{001B}[1;31merror: \u{001B}[0;0m\u{001B}[1;39mC-style for statement has been removed in Swift 3\u{001B}[0;0m
 
       """
 

Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift

@@ -112,8 +112,7 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
     assertStringsEqualWithDiff(
       annotated,
       """
-      === main.swift:5 ===
-        ┆
+      main.swift:6:14: error: expected ')' to end function call
       3 │ // test
       4 │ let pi = 3.14159
       5 │ #myAssert(pi == 3)
@@ -141,7 +140,7 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
       """
       let pi = 3.14159
       1️⃣#myAssert(pi == 3)
-      print("hello"
+      print("hello")
       """,
       displayName: "main.swift",
       extraDiagnostics: ["1️⃣": ("in expansion of macro 'myAssert' here", .note)]
@@ -181,7 +180,8 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
     assertStringsEqualWithDiff(
       annotated,
       """
-      === main.swift:2 ===
+      #invertedEqualityCheck:1:7: error: no matching operator '==' for types 'Double' and 'Int'
+      ╰─ main.swift:2:1: note: expanded code originates here
       1 │ let pi = 3.14159
       2 │ #myAssert(pi == 3)
         │ ╰─ note: in expansion of macro 'myAssert' here
@@ -197,8 +197,7 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
         │4 │   fatalError("assertion failed: pi != 3")
         │5 │ }
         ╰─────────────────────────────────────────────────────────────────────
-      3 │ print("hello"
-        │              ╰─ error: expected ')' to end function call
+      3 │ print("hello")
 
       """
     )