Skip to content

Commit 08bc2f0

Browse files
committed
[ASTGen/MacroEvaluation] Fix and adjust FixIt application
* Fix mismatch on 'FixIt.Change.replace' between several application implementations. Specifically '.replace' should includes the trivia. * Ignore no-op 'FixIt.Change.replace{Leading|Trailing}Triviia'.
1 parent e2235e2 commit 08bc2f0

File tree

3 files changed

+28
-37
lines changed

3 files changed

+28
-37
lines changed

lib/ASTGen/Sources/ASTGen/DiagnosticsBridge.swift

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ fileprivate func emitDiagnosticParts(
2323
position: AbsolutePosition,
2424
offset: Int,
2525
highlights: [Syntax] = [],
26-
fixItChanges: [FixIt.Change] = []
26+
edits: [SourceEdit] = []
2727
) {
2828
// Map severity
2929
let bridgedSeverity = severity.bridged
@@ -52,40 +52,12 @@ fileprivate func emitDiagnosticParts(
5252
}
5353

5454
// Emit changes for a Fix-It.
55-
for change in fixItChanges {
56-
let replaceStartLoc: BridgedSourceLoc
57-
let replaceEndLoc: BridgedSourceLoc
58-
var newText: String
59-
60-
switch change {
61-
case .replace(let oldNode, let newNode):
62-
replaceStartLoc = bridgedSourceLoc(at: oldNode.position)
63-
replaceEndLoc = bridgedSourceLoc(at: oldNode.endPosition)
64-
newText = newNode.description
65-
66-
case .replaceLeadingTrivia(let oldToken, let newTrivia):
67-
replaceStartLoc = bridgedSourceLoc(at: oldToken.position)
68-
replaceEndLoc = bridgedSourceLoc(
69-
at: oldToken.positionAfterSkippingLeadingTrivia
70-
)
71-
newText = newTrivia.description
72-
73-
case .replaceTrailingTrivia(let oldToken, let newTrivia):
74-
replaceStartLoc = bridgedSourceLoc(at: oldToken.endPositionBeforeTrailingTrivia)
75-
replaceEndLoc = bridgedSourceLoc(at: oldToken.endPosition)
76-
newText = newTrivia.description
77-
78-
case .replaceChild(let replacingChildData):
79-
let replacementRange = replacingChildData.replacementRange
80-
replaceStartLoc = bridgedSourceLoc(at: replacementRange.lowerBound)
81-
replaceEndLoc = bridgedSourceLoc(at: replacementRange.upperBound)
82-
newText = replacingChildData.newChild.description
83-
}
84-
55+
for edit in edits {
56+
var newText: String = edit.replacement
8557
newText.withBridgedString { bridgedMessage in
8658
diag.fixItReplace(
87-
start: replaceStartLoc,
88-
end: replaceEndLoc,
59+
start: bridgedSourceLoc(at: edit.range.lowerBound),
60+
end: bridgedSourceLoc(at: edit.range.upperBound),
8961
replacement: bridgedMessage
9062
)
9163
}
@@ -115,6 +87,7 @@ public func emitDiagnostic(
11587
)
11688

11789
// Emit Fix-Its.
90+
// FIXME: Ths assumes the fixIt is on the same tree/buffer, which is not guaranteed.
11891
for fixIt in diagnostic.fixIts {
11992
emitDiagnosticParts(
12093
diagnosticEngine: diagnosticEngine,
@@ -123,11 +96,12 @@ public func emitDiagnostic(
12396
severity: .note,
12497
position: diagnostic.position,
12598
offset: sourceFileBufferOffset,
126-
fixItChanges: fixIt.changes
99+
edits: fixIt.edits
127100
)
128101
}
129102

130103
// Emit any notes as follow-ons.
104+
// FIXME: Ths assumes the node is on the same tree/buffer, which is not guaranteed.
131105
for note in diagnostic.notes {
132106
emitDiagnosticParts(
133107
diagnosticEngine: diagnosticEngine,

lib/ASTGen/Sources/MacroEvaluation/SourceManager.swift

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,31 @@ extension SourceManager {
159159

160160
switch change {
161161
case .replace(let oldNode, let newNode):
162+
// Replace the whole node including leading/trailing trivia, but if
163+
// the trivia are the same, don't include them in the replacing range.
164+
let leadingMatch = oldNode.leadingTrivia == newNode.leadingTrivia
165+
let trailingMatch = oldNode.trailingTrivia == newNode.trailingTrivia
162166
replaceStartLoc = bridgedSourceLoc(
163167
for: oldNode,
164-
at: oldNode.positionAfterSkippingLeadingTrivia
168+
at: leadingMatch ? oldNode.positionAfterSkippingLeadingTrivia : oldNode.position
165169
)
166170
replaceEndLoc = bridgedSourceLoc(
167171
for: oldNode,
168-
at: oldNode.endPositionBeforeTrailingTrivia
172+
at: trailingMatch ? oldNode.endPositionBeforeTrailingTrivia : oldNode.endPosition
169173
)
174+
var newNode = newNode.detached
175+
if leadingMatch {
176+
newNode.leadingTrivia = []
177+
}
178+
if trailingMatch {
179+
newNode.trailingTrivia = []
180+
}
170181
newText = newNode.description
171182

172183
case .replaceLeadingTrivia(let oldToken, let newTrivia):
184+
guard oldToken.leadingTrivia != newTrivia else {
185+
continue
186+
}
173187
replaceStartLoc = bridgedSourceLoc(for: oldToken)
174188
replaceEndLoc = bridgedSourceLoc(
175189
for: oldToken,
@@ -178,6 +192,9 @@ extension SourceManager {
178192
newText = newTrivia.description
179193

180194
case .replaceTrailingTrivia(let oldToken, let newTrivia):
195+
guard oldToken.trailingTrivia != newTrivia else {
196+
continue
197+
}
181198
replaceStartLoc = bridgedSourceLoc(
182199
for: oldToken,
183200
at: oldToken.endPositionBeforeTrailingTrivia

test/ASTGen/diagnostics.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func testEditorPlaceholder() -> Int {
2121

2222
_ = [(Int) -> async throws Int]()
2323
// expected-error@-1{{'async throws' must precede '->'}}
24-
// expected-note@-2{{move 'async throws' in front of '->'}}{{15-21=}} {{21-28=}} {{20-21= }} {{12-12=async }} {{12-12=throws }}
24+
// expected-note@-2{{move 'async throws' in front of '->'}}{{15-21=}} {{21-28=}} {{12-12=async }} {{12-12=throws }}
2525

2626
@freestanding // expected-error {{expected arguments for 'freestanding' attribute}}
2727
func dummy() {}

0 commit comments

Comments
 (0)