Conversation
The culprit in this case was the legacy syntax
```
infix operator *** { associativity left precedence 140 }
```
which is not accepted by the current Swift parser. Because of this, the
offending tokens were placed inside the emitted AST as an unexpected
collection of syntax nodes. This then got serialised into an array
inside of an array in the JSON produced. The deserialiser did not expect
arrays to nest in this way, which caused the deserialisation failure
(which then in turn brought down the entire extractor).
To fix this, we now recursively flatten such arrays, rather than just
the top level.
Previously, if desugaring failed, then it would panic and take down the whole extractor. Now it just writes an error message to the output (and continues with the rest of the files).
Some files in `swiftlang/swift` ran into this limit which caused desugaring to fail. The current bump should give us ample headroom.
We were running into this on valid files from `swiftlang/swift` (admittedly ones explicitly testing the limits of the Swift compiler). Unfortunately, there's no way to just bump the limit -- it's 128 or infinity.
These are apparently valid in Swift (there's a test for it in `swiftlang/swift` that is parsed -- with a warning -- by the Swift compiler). To allow these, the SwiftSyntaxFFI now passes a string-with-length rather than a NUL-terminated string.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unbounded recursive JSON handling can terminate the extractor through stack overflow.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
What changed in this PR
Improves Unified Swift extraction resilience for problematic source files and deeply nested syntax.
Changes:
- Supports interior NUL bytes and nested Swift syntax.
- Continues extraction after per-file failures.
- Raises desugaring depth limits.
| File | Description |
|---|---|
SwiftSyntaxFFI.swift |
Accepts length-delimited UTF-8 buffers. |
swift-syntax-rs/src/lib.rs |
Updates the Rust FFI wrapper. |
adapter.rs |
Handles nested collections and deep JSON. |
unified/extractor/Cargo.toml |
Enables unbounded JSON depth. |
shared/yeast/src/lib.rs |
Raises rewrite-depth limit. |
simple.rs |
Adopts fallible extraction API. |
extractor/mod.rs |
Returns parsing and desugaring errors. |
driver.rs |
Isolates failures to individual files. |
desugaring.rs |
Propagates extraction errors. |
Cargo.lock |
Records the direct Serde dependency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
To avoid malicious JSON from taking down the extractor, we calculate the nesting depth before attempting the deserialisation. A limit of 2048 seems like it should cover our needs for the time being.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses all of the extraction failures I encountered on
swiftlang/swift. Every other failure comes from files that are actually malformed in some way (and thus rejected by the official compiler).