unified: Add Swift node type schema generator - #4
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: github#22459 Source head: 51a5929
✅ Shipwright · ApproveRecommendation: approve PR #4 · Tier
Findings (6)
Fireworks usage: 15,229 input · 683 output · 15,912 total tokens · $0.0038 · 12s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
|
|
||
| - The raw parse tree's shape is described by `extractor/swift_node_types.yml`, | ||
| which is maintained by hand. | ||
| which is generated from swift-syntax by `swift-syntax-rs/schemagen`. Do not |
There was a problem hiding this comment.
Shipwright · HIGH
The generated schema is committed to the repository, but there is no CI check that verifies it is up to date with the pinned swift-syntax version.
Impact: The generated schema is committed to the repository, but there is no CI check that verifies it is up to date with the pinned swift-syntax version. A future contributor can change the pin and forget to run the regeneration script, leaving the committed schema stale and the extractor silently out of sync.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| ) | ||
| checkout="$schemagen_dir/.build/checkouts/swift-syntax" | ||
| syntax_support="$checkout/CodeGeneration/Sources/SyntaxSupport" | ||
| if [[ ! -d $syntax_support ]]; then |
There was a problem hiding this comment.
Shipwright · HIGH
The regeneration script copies SyntaxSupport sources from a resolved checkout into a git-ignored directory, but does not verify that the resolved swift-syntax revision matches the
Impact: The regeneration script copies SyntaxSupport sources from a resolved checkout into a git-ignored directory, but does not verify that the resolved swift-syntax revision matches the exact pin in Package.swift. If Package.resolved drifts or SwiftPM resolves a different revision, the generated schema can silently diverge from the pinned version, producing a schema that does not match the parser actually used by the extr…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| # Generate to a temporary file first: redirecting straight into `$output` would | ||
| # truncate the existing schema before the build has even run, leaving nothing | ||
| # behind if it fails. | ||
| tmp=$(mktemp) |
There was a problem hiding this comment.
Shipwright · HIGH
The script uses 'mktemp' without a template and then redirects 'swift run schemagen' output into it.
Impact: The script uses 'mktemp' without a template and then redirects 'swift run schemagen' output into it. If 'swift run' emits build progress or warnings to stdout, those lines will be written into the YAML schema and corrupt it. The script only checks that the file is non-empty, not that it is valid YAML or that it starts with the expected generated header.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| run_swift() { | ||
| if [[ ${GIT_CONFIG_KEY_0:-} == "safe.bareRepository" ]]; then | ||
| GIT_CONFIG_VALUE_0=all swift "$@" | ||
| else |
There was a problem hiding this comment.
Shipwright · HIGH
The script modifies Git configuration behavior by setting 'GIT_CONFIG_VALUE_0=all' when 'GIT_CONFIG_KEY_0' is 'safe.bareRepository'.
Impact: The script modifies Git configuration behavior by setting 'GIT_CONFIG_VALUE_0=all' when 'GIT_CONFIG_KEY_0' is 'safe.bareRepository'. This overrides a security-relevant Git setting for all Swift subprocesses, potentially allowing SwiftPM to operate on bare repositories that the user's environment intentionally restricted. The override is broader than necessary and is not scoped to the specific repository path.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| if [[ ! -d $syntax_support ]]; then | ||
| echo "error: $syntax_support not found after resolving swift-syntax." >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Shipwright · HIGH
The script copies source files from a resolved dependency checkout into the package's own 'Sources' directory and builds them as first-party code.
Impact: The script copies source files from a resolved dependency checkout into the package's own 'Sources' directory and builds them as first-party code. If the swift-syntax repository or the resolved checkout is compromised or tampered with, the copied sources are compiled and executed without any integrity verification against the pinned revision hash.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| return [kind.rawValue] | ||
| case .nodeChoices(let choices, _): | ||
| return choices.flatMap { typeRefs($0) } | ||
| case .collection(let kind, _, _, _, _): |
There was a problem hiding this comment.
Shipwright · LOW
The 'typeRefs' function has a fallback branch that returns '[kind.rawValue]' when a collection node is not found in 'SYNTAX_NODES'.
Impact: The 'typeRefs' function has a fallback branch that returns '[kind.rawValue]' when a collection node is not found in 'SYNTAX_NODES'. This silently masks a schema inconsistency and makes it hard for a newcomer to know whether the fallback is expected or a bug.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
Adds tooling for automatically updating
swift_node_types.ymlwhen new versions ofswift-syntaxare released.NB: I have not tested this process on macOS, but I have confirmed that it works well on Codespaces.
Source merge-base:
9567f1a22fceaf9f9124e5b4adef879a8aef058eSource head:
51a59291347c91826822487054ebbe66d355454f