Skip to content

Commit 94f6ec8

Browse files
committed
FixItApplier: Add parameter for skipping duplicate insertions
1 parent f6d3ff1 commit 94f6ec8

File tree

2 files changed

+134
-37
lines changed

2 files changed

+134
-37
lines changed

Sources/SwiftIDEUtils/FixItApplier.swift

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,15 @@ public enum FixItApplier {
2727
/// - filterByMessages: An optional array of message strings to filter which Fix-Its to apply.
2828
/// If `nil`, the first Fix-It from each diagnostic is applied.
2929
/// - tree: The syntax tree to which the Fix-Its will be applied.
30+
/// - allowDuplicateInsertions: Whether to apply duplicate insertions.
31+
/// Defaults to `true`.
3032
///
3133
/// - Returns: A `String` representation of the modified syntax tree after applying the Fix-Its.
3234
public static func applyFixes(
3335
from diagnostics: [Diagnostic],
3436
filterByMessages messages: [String]?,
35-
to tree: some SyntaxProtocol
37+
to tree: some SyntaxProtocol,
38+
allowDuplicateInsertions: Bool = true
3639
) -> String {
3740
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
3841

@@ -43,18 +46,22 @@ public enum FixItApplier {
4346
.filter { messages.contains($0.message.message) }
4447
.flatMap(\.edits)
4548

46-
return self.apply(edits: edits, to: tree)
49+
return self.apply(edits: edits, to: tree, allowDuplicateInsertions: allowDuplicateInsertions)
4750
}
4851

4952
/// Applies the given edits to the given syntax tree.
5053
///
5154
/// - Parameters:
5255
/// - edits: The edits to apply.
5356
/// - tree: The syntax tree to which the edits should be applied.
57+
/// - allowDuplicateInsertions: Whether to apply duplicate insertions.
58+
/// Defaults to `true`.
59+
///
5460
/// - Returns: A `String` representation of the modified syntax tree.
5561
public static func apply(
5662
edits: [SourceEdit],
57-
to tree: some SyntaxProtocol
63+
to tree: some SyntaxProtocol,
64+
allowDuplicateInsertions: Bool = true
5865
) -> String {
5966
var edits = edits
6067
var source = tree.description
@@ -89,10 +96,22 @@ public enum FixItApplier {
8996
continue
9097
}
9198

92-
guard !remainingEdit.range.overlaps(edit.range) else {
93-
// The edit overlaps with the previous edit. We can't apply both
94-
// without conflicts. Drop this one by swapping it for a no-op
95-
// edit.
99+
func shouldDropRemainingEdit() -> Bool {
100+
// Insertions never conflict between themselves, unless we were asked
101+
// to drop duplicate insertions.
102+
if edit.range.isEmpty && remainingEdit.range.isEmpty {
103+
if allowDuplicateInsertions {
104+
return false
105+
}
106+
107+
return edit == remainingEdit
108+
}
109+
110+
return remainingEdit.range.overlaps(edit.range)
111+
}
112+
113+
guard !shouldDropRemainingEdit() else {
114+
// Drop the edit by swapping it for an empty one.
96115
edits[editIndex] = SourceEdit()
97116
continue
98117
}

Tests/SwiftIDEUtilsTest/FixItApplierTests.swift

Lines changed: 108 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ class FixItApplierApplyEditsTests: XCTestCase {
166166
.init(range: 3..<7, replacement: "cd"),
167167
],
168168
// The second edit is skipped.
169-
possibleOutputs: ["aboo = 1", "varcd = 1"]
169+
outputs: [
170+
.init(oneOf: "aboo = 1", "varcd = 1")
171+
]
170172
)
171173
}
172174

@@ -180,19 +182,36 @@ class FixItApplierApplyEditsTests: XCTestCase {
180182
.init(range: 0..<5, replacement: "_"),
181183
.init(range: 0..<3, replacement: "let"),
182184
],
183-
possibleOutputs: ["_ = 11", "let x = 11"]
185+
outputs: [
186+
.init(oneOf: "_ = 11", "let x = 11")
187+
]
184188
)
185189
}
186190

187191
func testOverlappingInsertions() {
192+
assertAppliedEdits(
193+
to: "x = 1",
194+
edits: [
195+
.init(range: 1..<1, replacement: "y"),
196+
.init(range: 1..<1, replacement: "z"),
197+
],
198+
outputs: [
199+
.init(oneOf: "xyz = 1", "xzy = 1")
200+
]
201+
)
202+
188203
assertAppliedEdits(
189204
to: "x = 1",
190205
edits: [
191206
.init(range: 0..<0, replacement: "var "),
192207
.init(range: 0..<0, replacement: "var "),
208+
.init(range: 4..<5, replacement: "2"),
193209
.init(range: 0..<0, replacement: "var "),
194210
],
195-
output: "var var var x = 1"
211+
outputs: [
212+
.init(deterministic: "var var var x = 2", allowDuplicateInsertions: true),
213+
.init(deterministic: "var x = 2", allowDuplicateInsertions: false),
214+
]
196215
)
197216
}
198217

@@ -214,47 +233,105 @@ class FixItApplierApplyEditsTests: XCTestCase {
214233
.init(range: 2..<2, replacement: "a"), // Insertion
215234
],
216235
// FIXME: This behavior where these edits are not considered overlapping doesn't feel desirable
217-
possibleOutputs: ["_x = 1", "_ a= 1"]
236+
outputs: [
237+
.init(oneOf: "_x = 1", "_ a= 1")
238+
]
218239
)
219240
}
220241
}
221242

222-
/// Asserts that at least one element in `possibleOutputs` matches the result
223-
/// of applying an array of edits to `input`, for all permutations of `edits`.
243+
/// Represents an output expectation.
244+
private struct OutputExpectation {
245+
var possibleOutputs: [String]
246+
var allowDuplicateInsertions: Bool?
247+
var line: UInt
248+
249+
/// Create a deterministic output expectation for the given value of
250+
/// `allowDuplicateInsertions`. If `allowDuplicateInsertions` is `nil`,
251+
/// the expectation holds for both `true` and `false`.
252+
init(
253+
deterministic output: String,
254+
allowDuplicateInsertions: Bool? = nil,
255+
line: UInt = #line
256+
) {
257+
self.possibleOutputs = [output]
258+
self.allowDuplicateInsertions = allowDuplicateInsertions
259+
self.line = line
260+
}
261+
262+
/// Create a "one of the given possible outputs" expectation for the given
263+
/// value of `allowDuplicateInsertions`. If `allowDuplicateInsertions` is
264+
/// `nil`, the expectation holds for both `true` and `false`.
265+
init(
266+
oneOf possibleOutputs: String...,
267+
allowDuplicateInsertions: Bool? = nil,
268+
line: UInt = #line
269+
) {
270+
self.possibleOutputs = possibleOutputs
271+
self.allowDuplicateInsertions = allowDuplicateInsertions
272+
self.line = line
273+
}
274+
}
275+
276+
/// Asserts that the given outputs match the result of applying an array of
277+
/// edits to `input`, for all permutations of `edits`.
224278
private func assertAppliedEdits(
225279
to tree: SourceFileSyntax,
226280
edits: [SourceEdit],
227-
possibleOutputs: [String],
228-
line: UInt = #line
281+
outputs: [OutputExpectation]
229282
) {
230-
precondition(!possibleOutputs.isEmpty)
283+
precondition(!outputs.isEmpty)
231284

232-
var indices = Array(edits.indices)
233-
while true {
234-
let editsPermutation = indices.map { edits[$0] }
285+
NEXT_OUTPUT: for output in outputs {
286+
let allowDuplicateInsertionsValues =
287+
if let value = output.allowDuplicateInsertions {
288+
[value]
289+
} else {
290+
[true, false]
291+
}
235292

236-
let actualOutput = FixItApplier.apply(edits: editsPermutation, to: tree)
237-
guard possibleOutputs.contains(actualOutput) else {
238-
XCTFail(
239-
"""
240-
Actual output \"\(actualOutput)\" does not match one of \(possibleOutputs)
241-
Edits:
242-
\(editsPermutation)
243-
""",
244-
line: line
245-
)
246-
return
247-
}
293+
let possibleOutputs = output.possibleOutputs
294+
295+
// Check this output against all permutations of edits.
296+
var indices = Array(edits.indices)
297+
while true {
298+
let editsPermutation = indices.map { edits[$0] }
248299

249-
let keepGoing = indices.nextPermutation()
250-
guard keepGoing else {
251-
break
300+
for allowDuplicateInsertionsValue in allowDuplicateInsertionsValues {
301+
let actualOutput = FixItApplier.apply(
302+
edits: editsPermutation,
303+
to: tree,
304+
allowDuplicateInsertions: allowDuplicateInsertionsValue
305+
)
306+
307+
guard possibleOutputs.contains(actualOutput) else {
308+
XCTFail(
309+
"""
310+
Actual output \"\(actualOutput)\" does not match one of \(possibleOutputs)
311+
Edits:
312+
\(editsPermutation)
313+
allowDuplicateInsertions:
314+
\(allowDuplicateInsertionsValue)
315+
""",
316+
line: output.line
317+
)
318+
319+
// Fail once for all permutations to avoid excessive logging.
320+
continue NEXT_OUTPUT
321+
}
322+
}
323+
324+
let keepGoing = indices.nextPermutation()
325+
guard keepGoing else {
326+
break
327+
}
252328
}
253329
}
254330
}
255331

256332
/// Asserts that `output` matches the result of applying an array of edits to
257-
/// `input`, for all permutations of `edits`.
333+
/// `input`, for all permutations of `edits` and for `allowDuplicateInsertions`
334+
/// both `true` and `false`.
258335
private func assertAppliedEdits(
259336
to tree: SourceFileSyntax,
260337
edits: [SourceEdit],
@@ -264,8 +341,9 @@ private func assertAppliedEdits(
264341
assertAppliedEdits(
265342
to: tree,
266343
edits: edits,
267-
possibleOutputs: [output],
268-
line: line
344+
outputs: [
345+
.init(deterministic: output, allowDuplicateInsertions: nil, line: line)
346+
]
269347
)
270348
}
271349

0 commit comments

Comments
 (0)