Skip to content

Commit 78d89ea

Browse files
committed
[Diagnostics] Make the node and position of Diagnostic and Note optional
Some tools that wish to make use of the diagnostics machinery don't have source locations, or even a Swift source file to point into. For example, the compiler and driver both have diagnostics that don't involve anything in source code, e.g., because they are diagnosing problems at the module level such as conflicting or incorrect compiler options. At present, the driver uses a different scheme for emitting diagnostics, and the compiler falls back to using the LLVM diagnostics machinery when there is no source location information. Making the "node" and "position" of a Diagnostic optional make it possible to express diagnostics that aren't associated with sources. Do so, and update the diagnostic formatter to support displaying diagnostics with no source location using the same `<unknown>:0:0:` scheme the compiler users.
1 parent c54a9d4 commit 78d89ea

File tree

9 files changed

+188
-48
lines changed

9 files changed

+188
-48
lines changed

Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,12 @@ extension PluginMessage.Diagnostic.Severity {
6868

6969
extension PluginMessage.Diagnostic {
7070
init(from syntaxDiag: SwiftDiagnostics.Diagnostic, in sourceManager: SourceManager) {
71-
if let position = sourceManager.position(
72-
of: syntaxDiag.node,
73-
at: .afterLeadingTrivia
74-
) {
71+
if let node = syntaxDiag.node,
72+
let position = sourceManager.position(
73+
of: node,
74+
at: .afterLeadingTrivia
75+
)
76+
{
7577
self.position = .init(fileName: position.fileName, offset: position.utf8Offset)
7678
} else {
7779
self.position = .invalid
@@ -92,7 +94,7 @@ extension PluginMessage.Diagnostic {
9294
}
9395

9496
self.notes = syntaxDiag.notes.compactMap {
95-
guard let pos = sourceManager.position(of: $0.node, at: .afterLeadingTrivia) else {
97+
guard let node = $0.node, let pos = sourceManager.position(of: node, at: .afterLeadingTrivia) else {
9698
return nil
9799
}
98100
let position = PluginMessage.Diagnostic.Position(

Sources/SwiftDiagnostics/Diagnostic.swift

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ public struct Diagnostic: CustomDebugStringConvertible, Sendable {
2121
public let diagMessage: DiagnosticMessage
2222

2323
/// The node at whose start location the message should be displayed.
24-
public let node: Syntax
24+
public let node: Syntax?
2525

2626
/// The position at which the location should be anchored.
2727
/// By default, this is the start location of `node`.
28-
public let position: AbsolutePosition
28+
public let position: AbsolutePosition?
2929

3030
/// Nodes that should be highlighted in the source code.
3131
public let highlights: [Syntax]
@@ -40,17 +40,17 @@ public struct Diagnostic: CustomDebugStringConvertible, Sendable {
4040
/// If `highlights` is `nil` then `node` will be highlighted. This is a
4141
/// reasonable default for almost all diagnostics.
4242
public init(
43-
node: some SyntaxProtocol,
43+
node: (some SyntaxProtocol)?,
4444
position: AbsolutePosition? = nil,
4545
message: DiagnosticMessage,
4646
highlights: [Syntax]? = nil,
4747
notes: [Note] = [],
4848
fixIts: [FixIt] = []
4949
) {
50-
self.node = Syntax(node)
51-
self.position = position ?? node.positionAfterSkippingLeadingTrivia
50+
self.node = node.map { Syntax($0) }
51+
self.position = position ?? node?.positionAfterSkippingLeadingTrivia
5252
self.diagMessage = message
53-
self.highlights = highlights ?? [Syntax(node)]
53+
self.highlights = highlights ?? node.map { [Syntax($0)] } ?? []
5454
self.notes = notes
5555
self.fixIts = fixIts
5656
}
@@ -67,13 +67,24 @@ public struct Diagnostic: CustomDebugStringConvertible, Sendable {
6767
}
6868

6969
/// The location at which the diagnostic should be displayed.
70-
public func location(converter: SourceLocationConverter) -> SourceLocation {
70+
public func location(converter: SourceLocationConverter) -> SourceLocation? {
71+
guard let position else {
72+
return nil
73+
}
74+
7175
return converter.location(for: position)
7276
}
7377

7478
public var debugDescription: String {
79+
guard let node else {
80+
return "\(DiagnosticsFormatter.unknownFileName):0:0: \(message)"
81+
}
82+
7583
let locationConverter = SourceLocationConverter(fileName: "", tree: node.root)
76-
let location = location(converter: locationConverter)
84+
guard let location = location(converter: locationConverter) else {
85+
return "\(DiagnosticsFormatter.unknownFileName):0:0: \(message)"
86+
}
87+
7788
return "\(location.line):\(location.column): \(message)"
7889
}
7990
}

Sources/SwiftDiagnostics/DiagnosticsFormatter.swift

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ extension Sequence where Element == Range<Int> {
5454
}
5555

5656
public struct DiagnosticsFormatter {
57+
/// The string used to identify an unknown file name.
58+
@_spi(Testing)
59+
public static let unknownFileName = "<unknown>"
5760

5861
/// A wrapper struct for a source line, its diagnostics, and any
5962
/// non-diagnostic text that follows the line.
@@ -215,14 +218,19 @@ public struct DiagnosticsFormatter {
215218
suffixTexts: [AbsolutePosition: String],
216219
sourceLocationConverter: SourceLocationConverter? = nil
217220
) -> String {
218-
let slc = sourceLocationConverter ?? SourceLocationConverter(fileName: "<unknown>", tree: tree)
221+
let slc =
222+
sourceLocationConverter ?? SourceLocationConverter(fileName: DiagnosticsFormatter.unknownFileName, tree: tree)
219223

220224
// First, we need to put each line and its diagnostics together
221225
var annotatedSourceLines = [AnnotatedSourceLine]()
222226

223227
for (sourceLineIndex, sourceLine) in slc.sourceLines.enumerated() {
224228
let diagsForLine = diags.filter { diag in
225-
return diag.location(converter: slc).line == (sourceLineIndex + 1)
229+
guard let location = diag.location(converter: slc) else {
230+
return false
231+
}
232+
233+
return location.line == (sourceLineIndex + 1)
226234
}
227235
let suffixText = suffixTexts.compactMap { (position, text) in
228236
if slc.location(for: position).line == (sourceLineIndex + 1) {
@@ -299,12 +307,14 @@ public struct DiagnosticsFormatter {
299307
}
300308

301309
let columnsWithDiagnostics = Set(
302-
annotatedLine.diagnostics.map {
303-
annotatedLine.characterColumn(ofUtf8Column: $0.location(converter: slc).column)
310+
annotatedLine.diagnostics.compactMap {
311+
$0.location(converter: slc).map {
312+
annotatedLine.characterColumn(ofUtf8Column: $0.column)
313+
}
304314
}
305315
)
306316
let diagsPerColumn = Dictionary(grouping: annotatedLine.diagnostics) { diag in
307-
annotatedLine.characterColumn(ofUtf8Column: diag.location(converter: slc).column)
317+
annotatedLine.characterColumn(ofUtf8Column: diag.location(converter: slc)!.column)
308318
}.sorted { lhs, rhs in
309319
lhs.key > rhs.key
310320
}

Sources/SwiftDiagnostics/GroupedDiagnostics.swift

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ public struct GroupedDiagnostics {
5757
/// source file IDs.
5858
var rootIndexes: [Syntax: SourceFileID] = [:]
5959

60+
/// Diagnostics that aren't associated with any source file because they have
61+
/// no location information.
62+
var floatingDiagnostics: [Diagnostic] = []
63+
6064
public init() {}
6165

6266
/// Add a new source file to the set of grouped diagnostics.
@@ -122,8 +126,16 @@ public struct GroupedDiagnostics {
122126
_ diagnostic: Diagnostic,
123127
in knownSourceFileID: SourceFileID? = nil
124128
) {
125-
guard let sourceFileID = knownSourceFileID ?? findSourceFileContaining(diagnostic.node) else {
126-
// Drop the diagnostic on the floor.
129+
guard let node = diagnostic.node else {
130+
// There is no node anchoring the diagnostic in a source file, so treat
131+
// it as floating.
132+
floatingDiagnostics.append(diagnostic)
133+
return
134+
}
135+
136+
guard let sourceFileID = knownSourceFileID ?? findSourceFileContaining(node) else {
137+
// This diagnostic is in source file, but we aren't producing diagnostics
138+
// for that file. Drop the diagnostic on the floor.
127139
return
128140
}
129141

@@ -205,7 +217,9 @@ extension GroupedDiagnostics {
205217
// If there's a primary diagnostic, print it first.
206218
if let (primaryDiagSourceFile, primaryDiag) = findPrimaryDiagnostic(in: sourceFile) {
207219
let primaryDiagSLC = primaryDiagSourceFile.sourceLocationConverter
208-
let location = primaryDiag.location(converter: primaryDiagSLC)
220+
let location =
221+
primaryDiag.location(converter: primaryDiagSLC)
222+
?? SourceLocation(line: 0, column: 0, offset: 0, file: DiagnosticsFormatter.unknownFileName)
209223

210224
// Display file/line/column and diagnostic text for the primary diagnostic.
211225
prefixString =
@@ -233,9 +247,13 @@ extension GroupedDiagnostics {
233247
prefixString += "`- \(bufferLoc.file):\(bufferLoc.line):\(bufferLoc.column): \(decoratedMessage)\n"
234248
}
235249
}
250+
} else if let firstDiag = sourceFile.diagnostics.first,
251+
let firstDiagLoc = firstDiag.location(converter: slc)
252+
{
253+
prefixString = "\(sourceFile.displayName): \(firstDiagLoc.line):"
236254
} else {
237-
let firstLine = sourceFile.diagnostics.first.map { $0.location(converter: slc).line } ?? 0
238-
prefixString = "\(sourceFile.displayName): \(firstLine):"
255+
// There are no diagnostics to print from this file.
256+
return ""
239257
}
240258

241259
suffixString = ""
@@ -275,9 +293,25 @@ extension GroupedDiagnostics {
275293
extension DiagnosticsFormatter {
276294
/// Annotate all of the source files in the given set of grouped diagnostics.
277295
public func annotateSources(in group: GroupedDiagnostics) -> String {
278-
return group.rootSourceFiles.map { rootSourceFileID in
279-
group.annotateSource(rootSourceFileID, formatter: self, indentString: "")
280-
}.joined(separator: "\n")
296+
// Render all grouped diagnostics.
297+
var renderedDiags = group.rootSourceFiles.compactMap { rootSourceFileID in
298+
let annotated = group.annotateSource(
299+
rootSourceFileID,
300+
formatter: self,
301+
indentString: ""
302+
)
303+
return annotated.isEmpty ? nil : annotated
304+
}
305+
306+
// Render any floating diagnostics, which have no anchor.
307+
renderedDiags.append(
308+
contentsOf: group.floatingDiagnostics.map { diag in
309+
let renderedMessage = diagnosticDecorator.decorateDiagnosticMessage(diag.diagMessage)
310+
return "\(DiagnosticsFormatter.unknownFileName):0:0: \(renderedMessage)"
311+
}
312+
)
313+
314+
return renderedDiags.joined(separator: "\n")
281315
}
282316

283317
public static func annotateSources(

Sources/SwiftDiagnostics/Note.swift

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,22 +37,22 @@ extension NoteMessage {
3737
/// A note that points to another node that's relevant for a Diagnostic.
3838
public struct Note: CustomDebugStringConvertible, Sendable {
3939
/// The node whose location the node is pointing.
40-
public let node: Syntax
40+
public let node: Syntax?
4141

4242
/// The position at which the location should be anchored.
4343
/// By default, this is the start location of `node`.
44-
public let position: AbsolutePosition
44+
public let position: AbsolutePosition?
4545

4646
/// A description of what this note is pointing at.
4747
public let noteMessage: NoteMessage
4848

4949
public init(
50-
node: Syntax,
50+
node: Syntax?,
5151
position: AbsolutePosition? = nil,
5252
message: NoteMessage
5353
) {
5454
self.node = node
55-
self.position = position ?? node.positionAfterSkippingLeadingTrivia
55+
self.position = position ?? node?.positionAfterSkippingLeadingTrivia
5656
self.noteMessage = message
5757
}
5858

@@ -62,17 +62,22 @@ public struct Note: CustomDebugStringConvertible, Sendable {
6262
}
6363

6464
/// The location at which the note should be displayed.
65-
public func location(converter: SourceLocationConverter) -> SourceLocation {
65+
public func location(converter: SourceLocationConverter) -> SourceLocation? {
66+
guard let position else {
67+
return nil
68+
}
69+
6670
return converter.location(for: position)
6771
}
6872

6973
public var debugDescription: String {
70-
if let root = node.root.as(SourceFileSyntax.self) {
74+
if let root = node?.root.as(SourceFileSyntax.self) {
7175
let locationConverter = SourceLocationConverter(fileName: "", tree: root)
72-
let location = location(converter: locationConverter)
73-
return "\(location): \(message)"
74-
} else {
75-
return "<unknown>: \(message)"
76+
if let location = location(converter: locationConverter) {
77+
return "\(location): \(message)"
78+
}
7679
}
80+
81+
return "\(DiagnosticsFormatter.unknownFileName):0:0: \(message)"
7782
}
7883
}

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,27 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
107107
let diagProducer = ParseDiagnosticsGenerator()
108108
diagProducer.walk(tree)
109109
diagProducer.diagnostics.sort {
110-
if $0.position != $1.position {
111-
return $0.position < $1.position
110+
guard let lhsPosition = $0.position, let rhsPosition = $1.position else {
111+
return $0.position == nil
112+
}
113+
114+
if lhsPosition != rhsPosition {
115+
return lhsPosition < rhsPosition
116+
}
117+
118+
guard let lhsNode = $0.node, let rhsNode = $1.node else {
119+
return $0.node == nil
112120
}
113121

114122
// Emit children diagnostics before parent diagnostics.
115123
// This makes sure that for missing declarations with attributes, we emit diagnostics on the attribute before we complain about the missing declaration.
116-
if $0.node.hasParent($1.node) {
124+
if lhsNode.hasParent(rhsNode) {
117125
return true
118-
} else if $1.node.hasParent($0.node) {
126+
} else if rhsNode.hasParent(lhsNode) {
119127
return false
120128
} else {
121129
// If multiple tokens are missing at the same location, emit diagnostics about nodes that occur earlier in the tree first.
122-
return $0.node.id < $1.node.id
130+
return lhsNode.id < rhsNode.id
123131
}
124132
}
125133
return diagProducer.diagnostics
@@ -157,7 +165,16 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
157165
if suppressRemainingDiagnostics {
158166
return
159167
}
160-
diagnostics.removeAll(where: { handledNodes.contains($0.node.id) })
168+
169+
diagnostics.removeAll(
170+
where: {
171+
if let nodeId = $0.node?.id {
172+
return handledNodes.contains(nodeId)
173+
}
174+
175+
return false
176+
}
177+
)
161178
diagnostics.append(diagnostic)
162179
self.handledNodes.append(contentsOf: handledNodes)
163180
}

Sources/SwiftSyntaxMacrosGenericTestSupport/Assertions.swift

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,18 @@ func assertNote(
157157
location: spec.failureLocation.underlying,
158158
failureHandler: { failureHandler(TestFailureSpec(underlying: $0)) }
159159
)
160-
let location = expansionContext.location(for: note.position, anchoredAt: note.node, fileName: "")
160+
guard let position = note.position, let node = note.node else {
161+
failureHandler(
162+
TestFailureSpec(
163+
message: "note has no position",
164+
location: spec.failureLocation
165+
)
166+
)
167+
168+
return
169+
}
170+
171+
let location = expansionContext.location(for: position, anchoredAt: node, fileName: "")
161172
if location.line != spec.line {
162173
failureHandler(
163174
TestFailureSpec(
@@ -387,7 +398,19 @@ public func assertDiagnostic(
387398
location: spec.failureLocation.underlying,
388399
failureHandler: { failureHandler(TestFailureSpec(underlying: $0)) }
389400
)
390-
let location = expansionContext.location(for: diag.position, anchoredAt: diag.node, fileName: "")
401+
guard let position = diag.position,
402+
let node = diag.node
403+
else {
404+
failureHandler(
405+
TestFailureSpec(
406+
message: "diagnostic missing location info",
407+
location: spec.failureLocation
408+
)
409+
)
410+
return
411+
}
412+
413+
let location = expansionContext.location(for: position, anchoredAt: node, fileName: "")
391414
if location.line != spec.line {
392415
failureHandler(
393416
TestFailureSpec(

0 commit comments

Comments
 (0)