Skip to content

[ASTGen/MacroEvaluation] Fix and adjust FixIt application #78670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 8 additions & 34 deletions lib/ASTGen/Sources/ASTGen/DiagnosticsBridge.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fileprivate func emitDiagnosticParts(
position: AbsolutePosition,
offset: Int,
highlights: [Syntax] = [],
fixItChanges: [FixIt.Change] = []
edits: [SourceEdit] = []
) {
// Map severity
let bridgedSeverity = severity.bridged
Expand Down Expand Up @@ -52,40 +52,12 @@ fileprivate func emitDiagnosticParts(
}

// Emit changes for a Fix-It.
for change in fixItChanges {
let replaceStartLoc: BridgedSourceLoc
let replaceEndLoc: BridgedSourceLoc
var newText: String

switch change {
case .replace(let oldNode, let newNode):
replaceStartLoc = bridgedSourceLoc(at: oldNode.position)
replaceEndLoc = bridgedSourceLoc(at: oldNode.endPosition)
newText = newNode.description

case .replaceLeadingTrivia(let oldToken, let newTrivia):
replaceStartLoc = bridgedSourceLoc(at: oldToken.position)
replaceEndLoc = bridgedSourceLoc(
at: oldToken.positionAfterSkippingLeadingTrivia
)
newText = newTrivia.description

case .replaceTrailingTrivia(let oldToken, let newTrivia):
replaceStartLoc = bridgedSourceLoc(at: oldToken.endPositionBeforeTrailingTrivia)
replaceEndLoc = bridgedSourceLoc(at: oldToken.endPosition)
newText = newTrivia.description

case .replaceChild(let replacingChildData):
let replacementRange = replacingChildData.replacementRange
replaceStartLoc = bridgedSourceLoc(at: replacementRange.lowerBound)
replaceEndLoc = bridgedSourceLoc(at: replacementRange.upperBound)
newText = replacingChildData.newChild.description
}

for edit in edits {
var newText: String = edit.replacement
newText.withBridgedString { bridgedMessage in
diag.fixItReplace(
start: replaceStartLoc,
end: replaceEndLoc,
start: bridgedSourceLoc(at: edit.range.lowerBound),
end: bridgedSourceLoc(at: edit.range.upperBound),
replacement: bridgedMessage
)
}
Expand Down Expand Up @@ -115,6 +87,7 @@ public func emitDiagnostic(
)

// Emit Fix-Its.
// FIXME: Ths assumes the fixIt is on the same tree/buffer, which is not guaranteed.
for fixIt in diagnostic.fixIts {
emitDiagnosticParts(
diagnosticEngine: diagnosticEngine,
Expand All @@ -123,11 +96,12 @@ public func emitDiagnostic(
severity: .note,
position: diagnostic.position,
offset: sourceFileBufferOffset,
fixItChanges: fixIt.changes
edits: fixIt.edits
)
}

// Emit any notes as follow-ons.
// FIXME: Ths assumes the node is on the same tree/buffer, which is not guaranteed.
for note in diagnostic.notes {
emitDiagnosticParts(
diagnosticEngine: diagnosticEngine,
Expand Down
21 changes: 19 additions & 2 deletions lib/ASTGen/Sources/MacroEvaluation/SourceManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,31 @@ extension SourceManager {

switch change {
case .replace(let oldNode, let newNode):
// Replace the whole node including leading/trailing trivia, but if
// the trivia are the same, don't include them in the replacing range.
let leadingMatch = oldNode.leadingTrivia == newNode.leadingTrivia
let trailingMatch = oldNode.trailingTrivia == newNode.trailingTrivia
replaceStartLoc = bridgedSourceLoc(
for: oldNode,
at: oldNode.positionAfterSkippingLeadingTrivia
at: leadingMatch ? oldNode.positionAfterSkippingLeadingTrivia : oldNode.position
)
replaceEndLoc = bridgedSourceLoc(
for: oldNode,
at: oldNode.endPositionBeforeTrailingTrivia
at: trailingMatch ? oldNode.endPositionBeforeTrailingTrivia : oldNode.endPosition
)
var newNode = newNode.detached
if leadingMatch {
newNode.leadingTrivia = []
}
if trailingMatch {
newNode.trailingTrivia = []
}
newText = newNode.description

case .replaceLeadingTrivia(let oldToken, let newTrivia):
guard oldToken.leadingTrivia != newTrivia else {
continue
}
replaceStartLoc = bridgedSourceLoc(for: oldToken)
replaceEndLoc = bridgedSourceLoc(
for: oldToken,
Expand All @@ -178,6 +192,9 @@ extension SourceManager {
newText = newTrivia.description

case .replaceTrailingTrivia(let oldToken, let newTrivia):
guard oldToken.trailingTrivia != newTrivia else {
continue
}
replaceStartLoc = bridgedSourceLoc(
for: oldToken,
at: oldToken.endPositionBeforeTrailingTrivia
Expand Down
2 changes: 1 addition & 1 deletion test/ASTGen/diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func testEditorPlaceholder() -> Int {

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

@freestanding // expected-error {{expected arguments for 'freestanding' attribute}}
func dummy() {}
Expand Down
7 changes: 1 addition & 6 deletions test/Macros/Inputs/syntax_macro_definitions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,7 @@ public enum AddBlocker: ExpressionMacro {
changes: [
FixIt.Change.replace(
oldNode: Syntax(binOp.operator),
newNode: Syntax(
TokenSyntax(
.binaryOperator("-"),
presence: .present
)
)
newNode: Syntax(binOp.operator.detached.with(\.tokenKind, .binaryOperator("-"))),
)
]
),
Expand Down