swiftlang / swiftlang/swift-syntax
GroupedDiagnostics does not handle notes on diagnostics
- Dominant language
- Swift
- Stars
- 3.7k
- Forks
- 553
- Avg merge
- 5d 13h
- Merged PRs (30d)
- 16
Description
### Description
The `GroupedDiagnostics` formatter takes an array of `Diagnostic`s that it formats into a buffer. However, it only handles the top-level diagnostic messages, not any [attached notes](https://github.com/apple/swift-syntax/blob/main/Sources/SwiftDiagnostics/Diagnostic.swift#L30C3-L31C1)---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.
```
Contributor guide
Assessment
This issue has not been assessed yet.