Skip to content

Commit dd6721d

Browse files
committed
Rewrite FixItApplier to be string based
1 parent 404fce6 commit dd6721d

File tree

4 files changed

+188
-115
lines changed

4 files changed

+188
-115
lines changed

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

+9-11
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
236236
exchangeTokens(
237237
unexpected: misplacedSpecifiers,
238238
unexpectedTokenCondition: { EffectSpecifier(token: $0) != nil },
239-
correctTokens: [effectSpecifiers?.throwsSpecifier, effectSpecifiers?.asyncSpecifier],
239+
correctTokens: [effectSpecifiers?.asyncSpecifier, effectSpecifiers?.throwsSpecifier],
240240
message: { EffectsSpecifierAfterArrow(effectsSpecifiersAfterArrow: $0) },
241241
moveFixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) },
242242
removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) }
@@ -571,6 +571,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
571571
node
572572
.with(\.availabilityKeyword, negatedAvailabilityKeyword)
573573
.with(\.unexpectedAfterRightParen, nil)
574+
.with(\.trailingTrivia, [])
574575
addDiagnostic(
575576
unexpectedAfterRightParen,
576577
AvailabilityConditionAsExpression(availabilityToken: node.availabilityKeyword, negatedAvailabilityToken: negatedAvailabilityKeyword),
@@ -581,7 +582,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
581582
replacements: getTokens(between: negatedAvailability.availabilityKeyword, and: negatedAvailability.rightParen)
582583
),
583584
changes: [
584-
.replace(oldNode: Syntax(node), newNode: Syntax(negatedAvailability))
585+
.replace(oldNode: Syntax(node.with(\.trailingTrivia, [])), newNode: Syntax(negatedAvailability))
585586
]
586587
)
587588
],
@@ -761,20 +762,17 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
761762
if let unexpected = node.unexpectedBetweenRequirementAndTrailingComma,
762763
let token = unexpected.presentTokens(satisfying: { $0.tokenKind == .binaryOperator("&&") }).first,
763764
let trailingComma = node.trailingComma,
764-
trailingComma.isMissing,
765-
let previous = node.unexpectedBetweenRequirementAndTrailingComma?.previousToken(viewMode: .sourceAccurate)
765+
trailingComma.isMissing
766766
{
767-
768767
addDiagnostic(
769768
unexpected,
770769
.expectedCommaInWhereClause,
771770
fixIts: [
772771
FixIt(
773772
message: ReplaceTokensFixIt(replaceTokens: [token], replacements: [.commaToken()]),
774773
changes: [
775-
.makeMissing(token),
774+
.makeMissing(token, transferTrivia: false),
776775
.makePresent(trailingComma),
777-
FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previous, newTrivia: [])),
778776
]
779777
)
780778
],
@@ -815,7 +813,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
815813
fixIts: [
816814
FixIt(
817815
message: RemoveNodesFixIt(nodes),
818-
changes: nodes.map { .makeMissing($0) }
816+
changes: nodes.map { .makeMissing($0, transferTrivia: !node.deinitKeyword.trailingTrivia.contains(where: { $0.isWhitespace })) }
819817
)
820818
],
821819
handledNodes: nodes.map { $0.id }
@@ -1859,8 +1857,8 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
18591857
replacements: [node.colon]
18601858
),
18611859
changes: [
1862-
FixIt.MultiNodeChange.makeMissing(equalToken),
1863-
FixIt.MultiNodeChange.makePresent(node.colon),
1860+
.makeMissing(equalToken, transferTrivia: false),
1861+
.makePresent(node.colon),
18641862
]
18651863
)
18661864
],
@@ -1969,7 +1967,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
19691967
message: fixItMessage,
19701968
changes: [
19711969
FixIt.MultiNodeChange.makePresent(detail.detail)
1972-
] + unexpectedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
1970+
] + unexpectedTokens.map { .makeMissing($0, transferTrivia: false) }
19731971
)
19741972
],
19751973
handledNodes: [detail.id] + unexpectedTokens.map(\.id)

Sources/_SwiftSyntaxTestSupport/FixItApplier.swift

+58-37
Original file line numberDiff line numberDiff line change
@@ -10,22 +10,24 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
import SwiftSyntax
1413
import SwiftDiagnostics
14+
import SwiftSyntax
1515

16-
public class FixItApplier: SyntaxRewriter {
16+
public class FixItApplier {
1717
fileprivate struct Edit {
1818
let startUtf8Offset: Int
1919
let endUtf8Offset: Int
2020
let replacement: String
2121
}
2222

23-
var changes: [FixIt.Change]
23+
private let changes: [FixIt.Change]
24+
private let tree: any SyntaxProtocol
2425

25-
private var locationConverter: SourceLocationConverter?
26-
private var edits: [Edit] = []
26+
private lazy var locationConverter: SourceLocationConverter = {
27+
return SourceLocationConverter(fileName: "", tree: tree)
28+
}()
2729

28-
init(diagnostics: [Diagnostic], withMessages messages: [String]?) {
30+
init(diagnostics: [Diagnostic], withMessages messages: [String]?, tree: any SyntaxProtocol) {
2931
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
3032

3133
self.changes =
@@ -36,51 +38,70 @@ public class FixItApplier: SyntaxRewriter {
3638
}
3739
.flatMap { $0.changes }
3840

39-
super.init(viewMode: .all)
41+
self.tree = tree
4042
}
4143

42-
public override func visitPre(_ node: Syntax) {
43-
if locationConverter == nil && !node.hasParent {
44-
locationConverter = SourceLocationConverter(fileName: "", tree: node)
44+
func rewrite() -> String {
45+
var edits: [Edit] = []
4546

46-
for change in self.changes {
47-
switch change {
48-
case .replace(oldNode: let oldNode, newNode: let newNode):
49-
let oldStartLocation = oldNode.startLocation(converter: locationConverter!)
50-
let oldEndLocation = oldNode.startLocation(converter: locationConverter!)
47+
for change in changes {
48+
switch change {
49+
case .replace(let oldNode, let newNode):
50+
edits.append(
51+
Edit(
52+
startUtf8Offset: oldNode.position.utf8Offset,
53+
endUtf8Offset: oldNode.endPosition.utf8Offset,
54+
replacement: newNode.description
55+
)
56+
)
5157

52-
edits.append(Edit(startUtf8Offset: oldStartLocation.offset, endUtf8Offset: oldEndLocation.offset, replacement: newNode.description))
58+
case .replaceLeadingTrivia(let token, let newTrivia):
59+
edits.append(
60+
Edit(
61+
startUtf8Offset: token.position.utf8Offset,
62+
endUtf8Offset: token.endPosition.utf8Offset,
63+
replacement: token.with(\.leadingTrivia, newTrivia).description
64+
)
65+
)
5366

54-
default:
55-
break
56-
}
67+
case .replaceTrailingTrivia(let token, let newTrivia):
68+
edits.append(
69+
Edit(
70+
startUtf8Offset: token.position.utf8Offset,
71+
endUtf8Offset: token.endPosition.utf8Offset,
72+
replacement: token.with(\.trailingTrivia, newTrivia).description
73+
)
74+
)
5775
}
5876
}
59-
}
6077

61-
public override func visitAny(_ node: Syntax) -> Syntax? {
62-
return nil
63-
}
78+
var source = tree.description
6479

65-
public override func visit(_ node: TokenSyntax) -> TokenSyntax {
66-
var modifiedNode = node
67-
for change in changes {
68-
switch change {
69-
case .replaceLeadingTrivia(token: let changedNode, newTrivia: let newTrivia) where changedNode.id == node.id:
70-
modifiedNode = node.with(\.leadingTrivia, newTrivia)
71-
case .replaceTrailingTrivia(token: let changedNode, newTrivia: let newTrivia) where changedNode.id == node.id:
72-
modifiedNode = node.with(\.trailingTrivia, newTrivia)
73-
default:
74-
break
80+
// Ensure edits are not overlapping:
81+
// If we only need to apply at the end of a source, start by reversing edit
82+
// and then sort edits by decrementing start offset. If they are equal then descrementing end offset.
83+
edits = edits.reversed().sorted(by: { edit1, edit2 in
84+
if edit1.startUtf8Offset == edit2.startUtf8Offset {
85+
return edit1.endUtf8Offset > edit2.endUtf8Offset
86+
} else {
87+
return edit1.startUtf8Offset > edit2.startUtf8Offset
7588
}
89+
})
90+
91+
for edit in edits {
92+
let startIndex = source.utf8.index(source.utf8.startIndex, offsetBy: edit.startUtf8Offset)
93+
let endIndex = source.utf8.index(source.utf8.startIndex, offsetBy: edit.endUtf8Offset)
94+
95+
source.replaceSubrange(startIndex..<endIndex, with: edit.replacement)
7696
}
77-
return modifiedNode
97+
98+
return source
7899
}
79100

80101
/// If `messages` is `nil`, applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
81102
/// If `messages` is not `nil`, applies only Fix-Its whose message is in `messages`.
82-
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], withMessages messages: [String]?, to tree: T) -> Syntax {
83-
let applier = FixItApplier(diagnostics: diagnostics, withMessages: messages)
84-
return applier.rewrite(tree)
103+
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], withMessages messages: [String]?, to tree: T) -> String {
104+
let applier = FixItApplier(diagnostics: diagnostics, withMessages: messages, tree: tree)
105+
return applier.rewrite()
85106
}
86107
}

Tests/SwiftParserTest/translated/AvailabilityQueryUnavailabilityTests.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ final class AvailabilityQueryUnavailabilityTests: ParserTestCase {
572572
),
573573
],
574574
fixedSource: """
575-
if #unavailable(*) , true {
575+
if #unavailable(*), true {
576576
}
577577
"""
578578
)

0 commit comments

Comments
 (0)