Skip to content

Commit 84ebafb

Browse files
committed
Tweak grouped-diagnostics formatting to put the primary diagnostics up front
One of the usability issues with the new grouped diagnostics code is that the actual error/warning/remark buried within the source code, with only a header like === t.swift:1 === to show where the issue occurs. Instead, display the diagnostic file:line:column and text right up front, the same way as the existing compiler's style, e.g., t.swift:5:3: error: no exact matches in call to global function 'f' The is better for humans, because it puts the primary issue right up front, and also better for tools that scrape logs to find the diagnostics using this form.
1 parent fe6ce4e commit 84ebafb

File tree

3 files changed

+48
-24
lines changed

3 files changed

+48
-24
lines changed

Sources/SwiftDiagnostics/DiagnosticsFormatter.swift

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -176,20 +176,24 @@ public struct DiagnosticsFormatter {
176176
return resultSourceString
177177
}
178178

179+
struct PrimaryDiagnostic {
180+
var location: SourceLocation
181+
var diagnostic: Diagnostic
182+
}
183+
179184
/// Print given diagnostics for a given syntax tree on the command line
180185
///
181186
/// - Parameters:
182187
/// - suffixTexts: suffix text to be printed at the given absolute
183188
/// locations within the source file.
184189
func annotatedSource(
185-
fileName: String?,
186190
tree: some SyntaxProtocol,
187191
diags: [Diagnostic],
188192
indentString: String,
189193
suffixTexts: [AbsolutePosition: String],
190194
sourceLocationConverter: SourceLocationConverter? = nil
191195
) -> String {
192-
let slc = sourceLocationConverter ?? SourceLocationConverter(fileName: fileName ?? "", tree: tree)
196+
let slc = sourceLocationConverter ?? SourceLocationConverter(fileName: "<unknown>", tree: tree)
193197

194198
// First, we need to put each line and its diagnostics together
195199
var annotatedSourceLines = [AnnotatedSourceLine]()
@@ -218,20 +222,9 @@ public struct DiagnosticsFormatter {
218222
return nil
219223
}
220224

225+
// Accumulate the fully annotated source files here.
221226
var annotatedSource = ""
222227

223-
// If there was a filename, add it first.
224-
if let fileName {
225-
let header = colorizeBufferOutline("===")
226-
let firstLine =
227-
1
228-
+ (annotatedSourceLines.enumerated().first { (lineIndex, sourceLine) in
229-
!sourceLine.isFreeOfAnnotations
230-
}?.offset ?? 0)
231-
232-
annotatedSource.append("\(indentString)\(header) \(fileName):\(firstLine) \(header)\n")
233-
}
234-
235228
/// Keep track if a line missing char should be printed
236229
var hasLineBeenSkipped = false
237230

@@ -318,7 +311,6 @@ public struct DiagnosticsFormatter {
318311
diags: [Diagnostic]
319312
) -> String {
320313
return annotatedSource(
321-
fileName: nil,
322314
tree: tree,
323315
diags: diags,
324316
indentString: "",
@@ -328,7 +320,7 @@ public struct DiagnosticsFormatter {
328320

329321
/// Annotates the given ``DiagnosticMessage`` with an appropriate ANSI color code (if the value of the `colorize`
330322
/// property is `true`) and returns the result as a printable string.
331-
private func colorizeIfRequested(_ message: DiagnosticMessage) -> String {
323+
func colorizeIfRequested(_ message: DiagnosticMessage) -> String {
332324
switch message.severity {
333325
case .error:
334326
let annotation = ANSIAnnotation(color: .red, trait: .bold)

Sources/SwiftDiagnostics/GroupedDiagnostics.swift

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,31 @@ extension GroupedDiagnostics {
133133
}
134134
}
135135

136+
// Find the "primary" diagnostic that will be shown at the top of the diagnostic
137+
// message. This is typically the error, warning, or remark.
138+
private func findPrimaryDiagnostic(in sourceFile: SourceFile) -> (SourceFile, Diagnostic)? {
139+
// If there is a non-note diagnostic, it's the primary diagnostic.
140+
if let primaryDiag = sourceFile.diagnostics.first(where: { $0.diagMessage.severity != .note }) {
141+
return (sourceFile, primaryDiag)
142+
}
143+
144+
// If one of our child source files has a primary diagnostic, return that.
145+
for childID in sourceFile.children {
146+
if let foundInChild = findPrimaryDiagnostic(in: sourceFiles[childID.id]) {
147+
return foundInChild
148+
}
149+
}
150+
151+
// If this is a root note, take the first note.
152+
if sourceFile.parent == nil,
153+
let note = sourceFile.diagnostics.first {
154+
return (sourceFile, note)
155+
}
156+
157+
// There is no primary diagnostic.
158+
return nil
159+
}
160+
136161
/// Annotate the source for a given source file ID, embedding its child
137162
/// source files.
138163
func annotateSource(
@@ -168,7 +193,17 @@ extension GroupedDiagnostics {
168193
let suffixString: String
169194

170195
if isRoot {
171-
prefixString = ""
196+
// If there's a primary diagnostic,
197+
if let (primaryDiagSourceFile, primaryDiag) = findPrimaryDiagnostic(in: sourceFile) {
198+
let primaryDiagSLC = SourceLocationConverter(fileName: primaryDiagSourceFile.displayName, tree: primaryDiagSourceFile.tree)
199+
let location = primaryDiag.location(converter: primaryDiagSLC)
200+
201+
prefixString = "\(location.file):\(location.line):\(location.column): \(formatter.colorizeIfRequested(primaryDiag.diagMessage))\n"
202+
} else {
203+
let firstLine = sourceFile.diagnostics.first.map { $0.location(converter: slc).line } ?? 0
204+
prefixString = "\(sourceFile.displayName): \(firstLine):"
205+
}
206+
172207
suffixString = ""
173208
} else {
174209
let padding = indentString.dropLast(1)
@@ -190,7 +225,6 @@ extension GroupedDiagnostics {
190225
// Render the buffer.
191226
return prefixString
192227
+ formatter.annotatedSource(
193-
fileName: isRoot ? sourceFile.displayName : nil,
194228
tree: sourceFile.tree,
195229
diags: sourceFile.diagnostics,
196230
indentString: colorizeBufferOutline(indentString),

Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,7 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
112112
assertStringsEqualWithDiff(
113113
annotated,
114114
"""
115-
=== main.swift:5 ===
116-
115+
main.swift:6:14: error: expected ')' to end function call
117116
3 │ // test
118117
4 │ let pi = 3.14159
119118
5 │ #myAssert(pi == 3)
@@ -141,7 +140,7 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
141140
"""
142141
let pi = 3.14159
143142
1️⃣#myAssert(pi == 3)
144-
print("hello"
143+
print("hello")
145144
""",
146145
displayName: "main.swift",
147146
extraDiagnostics: ["1️⃣": ("in expansion of macro 'myAssert' here", .note)]
@@ -181,7 +180,7 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
181180
assertStringsEqualWithDiff(
182181
annotated,
183182
"""
184-
=== main.swift:2 ===
183+
#invertedEqualityCheck:1:7: error: no matching operator '==' for types 'Double' and 'Int'
185184
1 │ let pi = 3.14159
186185
2 │ #myAssert(pi == 3)
187186
│ ╰─ note: in expansion of macro 'myAssert' here
@@ -197,8 +196,7 @@ final class GroupedDiagnosticsFormatterTests: XCTestCase {
197196
│4 │ fatalError("assertion failed: pi != 3")
198197
│5 │ }
199198
╰─────────────────────────────────────────────────────────────────────
200-
3 │ print("hello"
201-
│ ╰─ error: expected ')' to end function call
199+
3 │ print("hello")
202200
203201
"""
204202
)

0 commit comments

Comments
 (0)