Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GroupedDiagnostics does not handle notes on diagnostics #2166

Open
DougGregor opened this issue Sep 7, 2023 · 2 comments · May be fixed by #2214
Open

GroupedDiagnostics does not handle notes on diagnostics #2166

DougGregor opened this issue Sep 7, 2023 · 2 comments · May be fixed by #2214
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@DougGregor
Copy link
Member

DougGregor commented Sep 7, 2023

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 DougGregor added bug Something isn't working good first issue Good for newcomers labels Sep 7, 2023
@DougGregor DougGregor changed the title GroupedDiagnostics does not handle notes on its diagnostics GroupedDiagnostics does not handle notes on diagnostics Sep 7, 2023
@ahoppen
Copy link
Collaborator

ahoppen commented Sep 7, 2023

Tracked in Apple’s issue tracker as rdar://115134923

@natikgadzhi
Copy link
Contributor

I started looking! Got a bit lost in GroupedDiagnostic.annotateFiles, but getting there. I'm trying to add comments in spots that are under documented in SwiftDiagnostics as I go. I'm not yet very confident around this area, so if anyone beats me to it, go for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants