swiftlang / swiftlang/swift-syntax

FixItApplier crash when generating diagnostics for whole tree but applying them to a subtree

Open
#2,749 11 comments 0 reactions 0 assignees View on GitHub
Dominant language
Swift
Stars
3.7k
Forks
553
Avg merge
5d 13h
Merged PRs (30d)
16

Description

### Description

The crash:
Screenshot 2024-07-24 at 1 09 33 AM

### Steps to Reproduce

Note that `@Enumerator()` does not modify the code at all in this case.
I'm just sharing the exact code otherwise I think this should be reproducible not that hard.
Late night here, if I were to wait till tomorrow I'm pretty sure I would have ended up not reporting this bug, let me know if reproducing this doesn't end up being easy.

The exact code:
```swift
func testAppliesFixIts() {
let unterminatedString = """
let unterminated = ℹ️"This is unterminated1️⃣
"""
assertMacroExpansion(
#"""
@Enumerator("""
\#(unterminatedString)
""")
enum TestEnum {
case a(val1: String, Int)
case b
case testCase(testValue: String)
}
"""#,
expandedSource: #"""
enum TestEnum {
case a(val1: String, Int)
case b
case testCase(testValue: String)

let unterminated = "This is unterminated"
}
"""#,
macros: EnumeratorMacroEntryPoint.macros
)
}
```

How FixIts are applied in the pipeline of the macro:
```swift
var statement = statement
var diagnostics = ParseDiagnosticsGenerator.diagnostics(for: statement)

/// Returns if anything changed at all.
func tryApplyFixIts() -> Bool {
guard diagnostics.contains(where: { !$0.fixIts.isEmpty }) else {
return false
}
let fixedStatement = FixItApplier.applyFixes(
from: diagnostics,
filterByMessages: nil,
to: statement
)
var parser = Parser(fixedStatement)
let newStatement = CodeBlockItemSyntax.parse(from: &parser)
guard statement != newStatement else {
return false
}
let placeholderDetector = PlaceholderDetector()
placeholderDetector.walk(newStatement)
/// One of the FixIts added a placeholder, so the fixes are unacceptable
/// Known behavior which is fine for now: even if one FixIt is
/// misbehaving, still none of the FixIts will be applied.
if placeholderDetector.containedPlaceholder {
return false
} else {
statement = newStatement
return true
}
}

final class PlaceholderDetector: SyntaxVisitor {
var containedPlaceholder = false

override init(viewMode: SyntaxTreeViewMode = .sourceAccurate) {
super.init(viewMode: viewMode)
}

override func visit(_ node: TokenSyntax) -> SyntaxVisitorContinueKind {
if node.isEditorPlaceholder {
self.containedPlaceholder = true
return .skipChildren
}
return .visitChildren
}
}
```

Edit: Added `PlaceholderDetector` for the sake of being complete, although it doesn't matter.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.