swift-syntax icon indicating copy to clipboard operation
swift-syntax copied to clipboard

GroupedDiagnostics does not handle notes on diagnostics

Open DougGregor opened this issue 1 year ago • 2 comments

Description

The GroupedDiagnostics formatter takes an array of Diagnostics that it formats into a buffer. However, it only handles the top-level diagnostic messages, not any attached notes---which also have locations and messages and should be treated the same way as top-level diagnostics.

Note: a similar issue exists with Fix-Its, where we'd need a way to render what they actually do.

Steps to Reproduce

To reproduce, pass a Diagnostic with a non-empty notes member into the grouped diagnostics formatter, and note that the note doesn't show up at all.

The following patch updates the test infrastructure for GroupedDiagnostics to make it easier to create notes associated with the diagnostics, demonstrating this issue.

diff --git a/Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift b/T
ests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift
index eccd50dc1..461bc2333 100644
--- a/Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift
+++ b/Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift
@@ -27,6 +27,32 @@ extension SimpleDiagnosticMessage: FixItMessage {
   var fixItID: MessageID { diagnosticID }
 }
 
+struct SimpleNoteMessage: NoteMessage {
+  let message: String
+  let fixItID: MessageID
+}
+
+/// The severity of a particular marked diagnostic. This differs from
+/// DiagnosticSeverity in that notes are associated within another marked
+/// diagnostic.
+enum MarkedSeverity {
+  case error
+  case warning
+  case remark
+
+  /// Note attached to the diagnostic at a particular location
+  case note(String)
+
+  var asSeverity: DiagnosticSeverity {
+    switch self {
+      case .error: return .error
+      case .warning: return .warning
+      case .remark: return .remark
+      case .note: fatalError("Notes should never be mapped this way")
+    }
+  }
+}
+
 extension GroupedDiagnostics {
   /// Add a new test file to the group, starting with marked source and using
   /// the markers to add any suggested extra diagnostics at the marker
@@ -35,29 +61,58 @@ extension GroupedDiagnostics {
     _ markedSource: String,
     displayName: String,
     parent: (SourceFileID, AbsolutePosition)? = nil,
-    extraDiagnostics: [String: (String, DiagnosticSeverity)] = [:]
+    extraDiagnostics: [String: (String, MarkedSeverity)] = [:]
   ) -> (SourceFileID, [String: AbsolutePosition]) {
     // Parse the source file and produce parser diagnostics.
     let (markers, source) = extractMarkers(markedSource)
     let tree = Parser.parse(source: source)
     var diagnostics = ParseDiagnosticsGenerator.diagnostics(for: tree)
 
+    // Queue up notes for diagnostics.
+    var allNotes: [String: [Note]] = [:]
+    for (marker, (message, severity)) in extraDiagnostics {
+      guard case .note(let owner) = severity else {
+        continue
+      }
+
+      let pos = AbsolutePosition(utf8Offset: markers[marker]!)
+      let node = tree.token(at: pos)!.parent!
+
+      allNotes[owner, default: []].append(.init(
+        node: node,
+        message: SimpleNoteMessage(
+          message: message, fixItID: MessageID(domain: "test", id: "conjured")
+        )
+      ))
+    }
+
     // Add on any extra diagnostics provided, at their marker locations.
     for (marker, (message, severity)) in extraDiagnostics {
+      // Skip notes; they've been handled above.

DougGregor avatar Sep 07 '23 20:09 DougGregor