Skip to content

Commit 387a79c

Browse files
committed
[Macro test] Handle source locations in macro diagnostics better.
"Relink" the folded syntax node back into the primary syntax node. When we do this, we get consistent source locations that do not require any adjustment. Test this by adding a Fix-It to the silly AddBlocker macro, replacing the `+` with a `-`.
1 parent 3be6344 commit 387a79c

File tree

4 files changed

+35
-26
lines changed

4 files changed

+35
-26
lines changed

lib/ASTGen/Sources/ASTGen/Diagnostics.swift

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import SwiftSyntax
55
fileprivate func emitDiagnosticParts(
66
diagEnginePtr: UnsafeMutablePointer<UInt8>,
77
sourceFileBuffer: UnsafeMutableBufferPointer<UInt8>,
8-
nodeStartOffset: Int?,
98
message: String,
109
severity: DiagnosticSeverity,
1110
position: AbsolutePosition,
@@ -22,18 +21,8 @@ fileprivate func emitDiagnosticParts(
2221

2322
// Form a source location for the given absolute position
2423
func sourceLoc(
25-
at origPosition: AbsolutePosition
24+
at position: AbsolutePosition
2625
) -> UnsafeMutablePointer<UInt8>? {
27-
// FIXME: Our tree is very confused about absolute offsets. Work around
28-
// the issue in a very hacky way.
29-
let position: AbsolutePosition
30-
if let nodeStartOffset = nodeStartOffset,
31-
origPosition.utf8Offset < nodeStartOffset {
32-
position = origPosition + SourceLength(utf8Length: nodeStartOffset)
33-
} else {
34-
position = origPosition
35-
}
36-
3726
if let sourceFileBase = sourceFileBuffer.baseAddress,
3827
position.utf8Offset >= 0 &&
3928
position.utf8Offset < sourceFileBuffer.count {
@@ -99,7 +88,6 @@ fileprivate func emitDiagnosticParts(
9988
func emitDiagnostic(
10089
diagEnginePtr: UnsafeMutablePointer<UInt8>,
10190
sourceFileBuffer: UnsafeMutableBufferPointer<UInt8>,
102-
nodeStartOffset: Int? = nil,
10391
diagnostic: Diagnostic
10492
) {
10593
// Collect all of the Fix-It changes based on their Fix-It ID.
@@ -113,7 +101,6 @@ func emitDiagnostic(
113101
emitDiagnosticParts(
114102
diagEnginePtr: diagEnginePtr,
115103
sourceFileBuffer: sourceFileBuffer,
116-
nodeStartOffset: nodeStartOffset,
117104
message: diagnostic.diagMessage.message,
118105
severity: diagnostic.diagMessage.severity,
119106
position: diagnostic.position,
@@ -126,7 +113,6 @@ func emitDiagnostic(
126113
emitDiagnosticParts(
127114
diagEnginePtr: diagEnginePtr,
128115
sourceFileBuffer: sourceFileBuffer,
129-
nodeStartOffset: nodeStartOffset,
130116
message: note.message,
131117
severity: .note, position: note.position,
132118
fixItChanges: fixItChangesByID[note.noteMessage.fixItID] ?? []

lib/ASTGen/Sources/ASTGen/Macros.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ func evaluateMacro(
182182
emitDiagnostic(
183183
diagEnginePtr: diagEnginePtr,
184184
sourceFileBuffer: .init(mutating: sourceFile.pointee.buffer),
185-
nodeStartOffset: parentSyntax.position.utf8Offset,
186185
diagnostic: diag
187186
)
188187
}

test/Macros/Inputs/syntax_macro_definitions.swift

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ struct SimpleDiagnosticMessage: DiagnosticMessage {
6363
let severity: DiagnosticSeverity
6464
}
6565

66+
extension SimpleDiagnosticMessage: FixItMessage {
67+
var fixItID: MessageID { diagnosticID }
68+
}
69+
6670
public struct AddBlocker: ExpressionMacro {
6771
public static func expansion(
6872
of node: MacroExpansionExprSyntax, in context: inout MacroExpansionContext
@@ -77,12 +81,13 @@ public struct AddBlocker: ExpressionMacro {
7781
context.diagnose(error.asDiagnostic)
7882
}
7983

84+
// Link the folded argument back into the tree.
85+
var node = node.withArgumentList(node.argumentList.replacing(childAt: 0, with: node.argumentList.first!.withExpression(foldedArgument.as(ExprSyntax.self)!)))
86+
8087
class AddVisitor: SyntaxVisitor {
8188
var diagnostics: [Diagnostic] = []
82-
let startPosition: AbsolutePosition
8389

84-
init(startPosition: AbsolutePosition) {
85-
self.startPosition = startPosition
90+
init() {
8691
super.init(viewMode: .sourceAccurate)
8792
}
8893

@@ -91,18 +96,38 @@ public struct AddBlocker: ExpressionMacro {
9196
) -> SyntaxVisitorContinueKind {
9297
if let binOp = node.operatorOperand.as(BinaryOperatorExprSyntax.self) {
9398
if binOp.operatorToken.text == "+" {
99+
let messageID = MessageID(domain: "silly", id: "addblock")
94100
diagnostics.append(
95101
Diagnostic(
96102
node: Syntax(node.operatorOperand),
97-
position: startPosition + SourceLength(utf8Length: binOp.operatorToken.position.utf8Offset),
98103
message: SimpleDiagnosticMessage(
99-
message: "blocked an add",
100-
diagnosticID: MessageID(domain: "silly", id: "addblock"),
104+
message: "blocked an add; did you mean to subtract?",
105+
diagnosticID: messageID,
101106
severity: .error
102107
),
103108
highlights: [
104109
Syntax(node.leftOperand.withoutTrivia()),
105110
Syntax(node.rightOperand.withoutTrivia())
111+
],
112+
fixIts: [
113+
FixIt(
114+
message: SimpleDiagnosticMessage(
115+
message: "use '-'",
116+
diagnosticID: messageID,
117+
severity: .error
118+
),
119+
changes: [
120+
FixIt.Change.replace(
121+
oldNode: Syntax(binOp.operatorToken.withoutTrivia()),
122+
newNode: Syntax(
123+
TokenSyntax(
124+
.spacedBinaryOperator("-"),
125+
presence: .present
126+
)
127+
)
128+
)
129+
]
130+
),
106131
]
107132
)
108133
)
@@ -113,8 +138,8 @@ public struct AddBlocker: ExpressionMacro {
113138
}
114139
}
115140

116-
let visitor = AddVisitor(startPosition: argument.position)
117-
visitor.walk(Syntax(foldedArgument))
141+
let visitor = AddVisitor()
142+
visitor.walk(Syntax(node))
118143

119144
for diag in visitor.diagnostics {
120145
context.diagnose(diag)

test/Macros/macro_expand.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ macro addBlocker<T>(_ value: T) -> T = MacroDefinition.AddBlocker
5353
func testAddBlocker(a: Int, b: Int, c: Int) {
5454
_ = #addBlocker(a * b * c)
5555
#if TEST_DIAGNOSTICS
56-
// FIXME: Highlight locations are wrong.
57-
_ = #addBlocker(a + b * c) // expected-error{{blocked an add}}
56+
_ = #addBlocker(a + b * c) // expected-error{{blocked an add; did you mean to subtract?}}{{21-22=-}}
5857
#endif
5958
}

0 commit comments

Comments
 (0)