Skip to content

Commit 70f0720

Browse files
committed
Rewrite FixItApplier to be string based
1 parent f6b2617 commit 70f0720

File tree

4 files changed

+188
-115
lines changed

4 files changed

+188
-115
lines changed

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
175175
let correctTokens = correctTokens.compactMap({ $0 })
176176

177177
// Ignore `correctTokens` that are already present.
178-
let correctAndMissingTokens = correctTokens.filter({ $0.isMissing })
178+
let correctAndMissingTokens = correctTokens.filter({ $0.isMissing }).reversed()
179179
var changes: [FixIt.MultiNodeChange] = []
180180
if let misplacedToken = misplacedTokens.only, let correctToken = correctTokens.only,
181181
misplacedToken.nextToken(viewMode: .all) == correctToken || misplacedToken.previousToken(viewMode: .all) == correctToken,
@@ -580,7 +580,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
580580
replacements: getTokens(between: negatedAvailability.availabilityKeyword, and: negatedAvailability.rightParen)
581581
),
582582
changes: [
583-
.replace(oldNode: Syntax(node), newNode: Syntax(negatedAvailability))
583+
.replace(oldNode: Syntax(node.with(\.trailingTrivia, [])), newNode: Syntax(negatedAvailability.with(\.trailingTrivia, [])))
584584
]
585585
)
586586
],
@@ -760,8 +760,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
760760
if let unexpected = node.unexpectedBetweenRequirementAndTrailingComma,
761761
let token = unexpected.presentTokens(satisfying: { $0.tokenKind == .binaryOperator("&&") }).first,
762762
let trailingComma = node.trailingComma,
763-
trailingComma.isMissing,
764-
let previous = node.unexpectedBetweenRequirementAndTrailingComma?.previousToken(viewMode: .sourceAccurate)
763+
trailingComma.isMissing
765764
{
766765

767766
addDiagnostic(
@@ -771,9 +770,8 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
771770
FixIt(
772771
message: ReplaceTokensFixIt(replaceTokens: [token], replacements: [.commaToken()]),
773772
changes: [
774-
.makeMissing(token),
773+
.makeMissing(token, transferTrivia: false),
775774
.makePresent(trailingComma),
776-
FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previous, newTrivia: [])),
777775
]
778776
)
779777
],
@@ -795,7 +793,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
795793
name,
796794
.deinitCannotHaveName,
797795
fixIts: [
798-
FixIt(message: RemoveNodesFixIt(name), changes: .makeMissing(name))
796+
FixIt(message: RemoveNodesFixIt(name), changes: .makeMissing(name, transferTrivia: false))
799797
],
800798
handledNodes: [name.id]
801799
)
@@ -807,7 +805,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
807805
params,
808806
.deinitCannotHaveParameters,
809807
fixIts: [
810-
FixIt(message: RemoveNodesFixIt(params), changes: .makeMissing(params))
808+
FixIt(message: RemoveNodesFixIt(params), changes: .makeMissing(params, transferTrivia: false))
811809
],
812810
handledNodes: [params.id]
813811
)
@@ -819,7 +817,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
819817
returnType,
820818
.deinitCannotHaveReturnType,
821819
fixIts: [
822-
FixIt(message: RemoveNodesFixIt(returnType), changes: .makeMissing(returnType))
820+
FixIt(message: RemoveNodesFixIt(returnType), changes: .makeMissing(returnType, transferTrivia: false))
823821
],
824822
handledNodes: [returnType.id]
825823
)
@@ -1862,7 +1860,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
18621860
replacements: [node.colon]
18631861
),
18641862
changes: [
1865-
FixIt.MultiNodeChange.makeMissing(equalToken),
1863+
FixIt.MultiNodeChange.makeMissing(equalToken, transferTrivia: false),
18661864
FixIt.MultiNodeChange.makePresent(node.colon),
18671865
]
18681866
)
@@ -1972,7 +1970,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
19721970
message: fixItMessage,
19731971
changes: [
19741972
FixIt.MultiNodeChange.makePresent(detail.detail)
1975-
] + unexpectedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
1973+
] + unexpectedTokens.map { FixIt.MultiNodeChange.makeMissing($0, transferTrivia: false) }
19761974
)
19771975
],
19781976
handledNodes: [detail.id] + unexpectedTokens.map(\.id)

Sources/_SwiftSyntaxTestSupport/FixItApplier.swift

Lines changed: 58 additions & 37 deletions
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

Lines changed: 1 addition & 1 deletion
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)