Skip to content

Commit 0dbe575

Browse files
kimdvkimdv
committed
Rewrite FixItApplier to be string based
1 parent e695b37 commit 0dbe575

File tree

7 files changed

+332
-164
lines changed

7 files changed

+332
-164
lines changed

‎Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,28 +48,21 @@ extension FixIt {
4848

4949
extension FixIt.MultiNodeChange {
5050
/// Replaced a present token with a missing node.
51+
///
5152
/// If `transferTrivia` is `true`, the leading and trailing trivia of the
5253
/// removed node will be transferred to the trailing trivia of the previous token.
5354
static func makeMissing(_ token: TokenSyntax, transferTrivia: Bool = true) -> Self {
5455
return makeMissing([token], transferTrivia: transferTrivia)
5556
}
5657

5758
/// Replace present tokens with missing tokens.
58-
/// If `transferTrivia` is `true`, the leading and trailing trivia of the
59-
/// removed node will be transferred to the trailing trivia of the previous token.
59+
///
60+
/// If `transferTrivia` is `true`, the leading trivia of the first token and
61+
/// the trailing trivia of the last token will be transferred to their adjecent
62+
/// tokens.
6063
static func makeMissing(_ tokens: [TokenSyntax], transferTrivia: Bool = true) -> Self {
61-
precondition(!tokens.isEmpty)
62-
precondition(tokens.allSatisfy({ $0.isPresent }))
63-
var changes = tokens.map {
64-
FixIt.Change.replace(
65-
oldNode: Syntax($0),
66-
newNode: Syntax($0.with(\.presence, .missing))
67-
)
68-
}
69-
if transferTrivia {
70-
changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: tokens).primitiveChanges
71-
}
72-
return FixIt.MultiNodeChange(primitiveChanges: changes)
64+
precondition(tokens.allSatisfy(\.isPresent))
65+
return .makeMissing(tokens.map(Syntax.init), transferTrivia: transferTrivia)
7366
}
7467

7568
/// If `transferTrivia` is `true`, the leading and trailing trivia of the
@@ -104,6 +97,25 @@ extension FixIt.MultiNodeChange {
10497
return FixIt.MultiNodeChange()
10598
}
10699
}
100+
101+
/// Replace present nodes with their missing equivalents.
102+
///
103+
/// If `transferTrivia` is `true`, the leading trivia of the first node and
104+
/// the trailing trivia of the last node will be transferred to their adjecent
105+
/// tokens.
106+
static func makeMissing(_ nodes: [Syntax], transferTrivia: Bool = true) -> Self {
107+
precondition(!nodes.isEmpty)
108+
var changes = nodes.map {
109+
FixIt.Change.replace(
110+
oldNode: $0,
111+
newNode: MissingMaker().rewrite($0, detach: true)
112+
)
113+
}
114+
if transferTrivia {
115+
changes += FixIt.MultiNodeChange.transferTriviaAtSides(from: nodes).primitiveChanges
116+
}
117+
return FixIt.MultiNodeChange(primitiveChanges: changes)
118+
}
107119
}
108120

109121
// MARK: - Make present

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
182182
correctToken.isMissing
183183
{
184184
// We are exchanging two adjacent tokens, transfer the trivia from the incorrect token to the corrected token.
185-
changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0, transferTrivia: false) }
186185
changes.append(
187186
FixIt.MultiNodeChange.makePresent(
188187
correctToken,
@@ -191,6 +190,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
191190
trailingTrivia: misplacedToken.trailingTrivia.isEmpty ? nil : misplacedToken.trailingTrivia
192191
)
193192
)
193+
changes.append(FixIt.MultiNodeChange.makeMissing(misplacedTokens, transferTrivia: false))
194194
} else {
195195
changes += misplacedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
196196
changes += correctAndMissingTokens.map { FixIt.MultiNodeChange.makePresent($0) }
@@ -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) }
@@ -764,20 +764,17 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
764764
if let unexpected = node.unexpectedBetweenRequirementAndTrailingComma,
765765
let token = unexpected.presentTokens(satisfying: { $0.tokenKind == .binaryOperator("&&") }).first,
766766
let trailingComma = node.trailingComma,
767-
trailingComma.isMissing,
768-
let previous = node.unexpectedBetweenRequirementAndTrailingComma?.previousToken(viewMode: .sourceAccurate)
767+
trailingComma.isMissing
769768
{
770-
771769
addDiagnostic(
772770
unexpected,
773771
.expectedCommaInWhereClause,
774772
fixIts: [
775773
FixIt(
776774
message: ReplaceTokensFixIt(replaceTokens: [token], replacements: [.commaToken()]),
777775
changes: [
778-
.makeMissing(token),
779-
.makePresent(trailingComma),
780-
FixIt.MultiNodeChange(.replaceTrailingTrivia(token: previous, newTrivia: [])),
776+
.makeMissing(token, transferTrivia: false),
777+
.makePresent(trailingComma, leadingTrivia: token.leadingTrivia, trailingTrivia: token.trailingTrivia),
781778
]
782779
)
783780
],
@@ -818,7 +815,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
818815
fixIts: [
819816
FixIt(
820817
message: RemoveNodesFixIt(nodes),
821-
changes: nodes.map { .makeMissing($0) }
818+
changes: .makeMissing(nodes)
822819
)
823820
],
824821
handledNodes: nodes.map { $0.id }
@@ -1542,7 +1539,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
15421539
fixIts: [
15431540
FixIt(
15441541
message: RemoveNodesFixIt(rawDelimiters),
1545-
changes: rawDelimiters.map { .makeMissing($0) }
1542+
changes: .makeMissing(rawDelimiters)
15461543
)
15471544
],
15481545
handledNodes: rawDelimiters.map { $0.id }
@@ -1862,8 +1859,8 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
18621859
replacements: [node.colon]
18631860
),
18641861
changes: [
1865-
FixIt.MultiNodeChange.makeMissing(equalToken),
1866-
FixIt.MultiNodeChange.makePresent(node.colon),
1862+
.makeMissing(equalToken, transferTrivia: false),
1863+
.makePresent(node.colon, leadingTrivia: equalToken.leadingTrivia, trailingTrivia: equalToken.trailingTrivia),
18671864
]
18681865
)
18691866
],
@@ -1971,8 +1968,9 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
19711968
FixIt(
19721969
message: fixItMessage,
19731970
changes: [
1974-
FixIt.MultiNodeChange.makePresent(detail.detail)
1975-
] + unexpectedTokens.map { FixIt.MultiNodeChange.makeMissing($0) }
1971+
.makePresent(detail.detail),
1972+
.makeMissing(unexpectedTokens),
1973+
]
19761974
)
19771975
],
19781976
handledNodes: [detail.id] + unexpectedTokens.map(\.id)

‎Sources/_SwiftSyntaxTestSupport/FixItApplier.swift

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import SwiftDiagnostics
14+
import SwiftSyntax
15+
16+
public enum FixItApplier {
17+
fileprivate struct Edit: Equatable {
18+
var startUtf8Offset: Int
19+
var endUtf8Offset: Int
20+
let replacement: String
21+
22+
var originalLength: Int {
23+
return endUtf8Offset - startUtf8Offset
24+
}
25+
26+
var replacementLength: Int {
27+
return replacement.utf8.count
28+
}
29+
30+
var replacementRange: Range<Int> {
31+
return startUtf8Offset..<endUtf8Offset
32+
}
33+
}
34+
35+
/// Applies selected or all Fix-Its from the provided diagnostics to a given syntax tree.
36+
///
37+
/// - Parameters:
38+
/// - diagnostics: An array of `Diagnostic` objects, each containing one or more Fix-Its.
39+
/// - filterByMessages: An optional array of message strings to filter which Fix-Its to apply.
40+
/// If `nil`, all Fix-Its in `diagnostics` are applied.
41+
/// - tree: The syntax tree to which the Fix-Its will be applied.
42+
///
43+
/// - Returns: A ``String`` representation of the modified syntax tree after applying the Fix-Its.
44+
public static func applyFixes(
45+
from diagnostics: [Diagnostic],
46+
filterByMessages messages: [String]?,
47+
to tree: any SyntaxProtocol
48+
) -> String {
49+
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
50+
51+
let changes =
52+
diagnostics
53+
.flatMap(\.fixIts)
54+
.filter { messages.contains($0.message.message) }
55+
.flatMap(\.changes)
56+
57+
var edits: [Edit] = []
58+
59+
for change in changes {
60+
switch change {
61+
case .replace(let oldNode, let newNode):
62+
edits.append(
63+
Edit(
64+
startUtf8Offset: oldNode.position.utf8Offset,
65+
endUtf8Offset: oldNode.endPosition.utf8Offset,
66+
replacement: newNode.description
67+
)
68+
)
69+
70+
case .replaceLeadingTrivia(let token, let newTrivia):
71+
edits.append(
72+
Edit(
73+
startUtf8Offset: token.position.utf8Offset,
74+
endUtf8Offset: token.positionAfterSkippingLeadingTrivia.utf8Offset,
75+
replacement: newTrivia.description
76+
)
77+
)
78+
79+
case .replaceTrailingTrivia(let token, let newTrivia):
80+
edits.append(
81+
Edit(
82+
startUtf8Offset: token.endPositionBeforeTrailingTrivia.utf8Offset,
83+
endUtf8Offset: token.endPosition.utf8Offset,
84+
replacement: newTrivia.description
85+
)
86+
)
87+
}
88+
}
89+
90+
var source = tree.description
91+
92+
// As we need to start apply the edits at the end of a source, start by reversing edit
93+
// and then sort edits by decrementing start offset. If they are equal then descrementing end offset.
94+
// edits = edits.reversed().sorted(by: { edit1, edit2 in
95+
// if edit1.startUtf8Offset == edit2.startUtf8Offset {
96+
// return edit1.endUtf8Offset > edit2.endUtf8Offset
97+
// } else {
98+
// return edit1.startUtf8Offset > edit2.startUtf8Offset
99+
// }
100+
// })
101+
102+
while let edit = edits.first {
103+
edits = Array(edits.dropFirst())
104+
105+
let startIndex = source.utf8.index(source.utf8.startIndex, offsetBy: edit.startUtf8Offset)
106+
let endIndex = source.utf8.index(source.utf8.startIndex, offsetBy: edit.endUtf8Offset)
107+
108+
source.replaceSubrange(startIndex..<endIndex, with: edit.replacement)
109+
110+
edits = edits.compactMap { remainingEdit -> FixItApplier.Edit? in
111+
var remainingEdit = remainingEdit
112+
113+
if remainingEdit.replacementRange.overlaps(edit.replacementRange) {
114+
// The edit overlaps with the previous edit. We can't apply both
115+
// without conflicts. Apply the one that's listed first and drop the
116+
// later edit.
117+
return nil
118+
}
119+
120+
// Keep the origianl length as we start to edit the offset.
121+
let originalLength = remainingEdit.originalLength
122+
123+
// If the current is after the edit we want to update, don't change the offset.
124+
// If the remaining edit have same start and end position, don't change the start offset.
125+
if edit.endUtf8Offset <= remainingEdit.startUtf8Offset {
126+
// Take the largest value as we in some cases can end up with a negative offset.
127+
remainingEdit.startUtf8Offset = Swift.max(remainingEdit.startUtf8Offset - edit.originalLength + edit.replacementLength, 0)
128+
}
129+
130+
// If we need to replace a string at the end, we need to ensure we don't get an out-of-bounds error.
131+
remainingEdit.endUtf8Offset = Swift.min(remainingEdit.startUtf8Offset + originalLength, source.utf8.count)
132+
133+
return remainingEdit
134+
}
135+
}
136+
137+
return source
138+
}
139+
}
140+
141+
//extension Array where Element == FixItApplier.Edit {
142+
// fileprivate func canInsert(editToApply: Element) -> Bool {
143+
// return self.contains { edit in
144+
// return
145+
// !(editToApply.startUtf8Offset >= edit.startUtf8Offset
146+
// && editToApply.endUtf8Offset < edit.endUtf8Offset)
147+
// }
148+
// }
149+
//}

‎Tests/SwiftParserTest/Assertions.swift

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -276,58 +276,6 @@ struct DiagnosticSpec {
276276
}
277277
}
278278

279-
class FixItApplier: SyntaxRewriter {
280-
var changes: [FixIt.Change]
281-
282-
init(diagnostics: [Diagnostic], withMessages messages: [String]?) {
283-
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
284-
285-
self.changes =
286-
diagnostics
287-
.flatMap { $0.fixIts }
288-
.filter {
289-
return messages.contains($0.message.message)
290-
}
291-
.flatMap { $0.changes }
292-
293-
super.init(viewMode: .all)
294-
}
295-
296-
public override func visitAny(_ node: Syntax) -> Syntax? {
297-
for change in changes {
298-
switch change {
299-
case .replace(oldNode: let oldNode, newNode: let newNode) where oldNode.id == node.id:
300-
return newNode
301-
default:
302-
break
303-
}
304-
}
305-
return nil
306-
}
307-
308-
override func visit(_ node: TokenSyntax) -> TokenSyntax {
309-
var modifiedNode = node
310-
for change in changes {
311-
switch change {
312-
case .replaceLeadingTrivia(token: let changedNode, newTrivia: let newTrivia) where changedNode.id == node.id:
313-
modifiedNode = node.with(\.leadingTrivia, newTrivia)
314-
case .replaceTrailingTrivia(token: let changedNode, newTrivia: let newTrivia) where changedNode.id == node.id:
315-
modifiedNode = node.with(\.trailingTrivia, newTrivia)
316-
default:
317-
break
318-
}
319-
}
320-
return modifiedNode
321-
}
322-
323-
/// If `messages` is `nil`, applies all Fix-Its in `diagnostics` to `tree` and returns the fixed syntax tree.
324-
/// If `messages` is not `nil`, applies only Fix-Its whose message is in `messages`.
325-
public static func applyFixes<T: SyntaxProtocol>(in diagnostics: [Diagnostic], withMessages messages: [String]?, to tree: T) -> Syntax {
326-
let applier = FixItApplier(diagnostics: diagnostics, withMessages: messages)
327-
return applier.rewrite(tree)
328-
}
329-
}
330-
331279
/// Assert that `location` is the same as that of `locationMarker` in `tree`.
332280
func assertLocation<T: SyntaxProtocol>(
333281
_ location: SourceLocation,
@@ -679,7 +627,7 @@ extension ParserTestCase {
679627
if expectedDiagnostics.contains(where: { !$0.fixIts.isEmpty }) && expectedFixedSource == nil {
680628
XCTFail("Expected a fixed source if the test case produces diagnostics with Fix-Its", file: file, line: line)
681629
} else if let expectedFixedSource = expectedFixedSource {
682-
let fixedTree = FixItApplier.applyFixes(in: diags, withMessages: applyFixIts, to: tree)
630+
let fixedTree = FixItApplier.applyFixes(from: diags, filterByMessages: applyFixIts, to: tree)
683631
var fixedTreeDescription = fixedTree.description
684632
if options.contains(.normalizeNewlinesInFixedSource) {
685633
fixedTreeDescription =

0 commit comments

Comments
 (0)