unified: Always run corpus tests - #22248
Open
tausbn wants to merge 3 commits into
Open
Conversation
tausbn
force-pushed
the
tausbn/unified-always-run-corpus-tests
branch
2 times, most recently
from
July 30, 2026 15:09
3f04e54 to
5ec7a27
Compare
The Swift front-end shelled out to a separate `swift-syntax-parse` executable
and read a JSON syntax tree back over a pipe. That kept the Swift toolchain off
the extractor's build path, so working on the other (tree-sitter based)
languages needed no Swift -- but it meant the extractor had to *find* that
executable at run time, and everything downstream of that was a workaround:
- the binary is a shell wrapper that sets `LD_LIBRARY_PATH` and execs
`swift-syntax-parse.real`, because the Swift runtime libraries have to sit
beside it;
- that only works in a flattened layout, so packaging went through
`codeql_pkg_runfiles` and the runtime libraries reached the pack only as a
side effect of shipping the parser;
- and the corpus tests skipped themselves when they could not find the binary.
A skip still prints `test result: ok`, so the suite silently tested nothing.
Supporting Swift is the current priority, so the extractor may now depend on
building the Swift half. Link `swift-syntax-rs` in and call `parse_to_json`
directly: `parse.rs` loses ~100 lines of process plumbing, and the tree is
necessarily produced by the swift-syntax build this extractor was built
against, so it cannot silently run a stale parser.
The parser survives as a debugging aid for looking at raw swift-syntax JSON:
echo 'let x = 1' | bazel run //unified/swift-syntax-rs:swift-syntax-parse
It is no longer shipped, so the wrapper, the `.real` split and the runfiles
packaging all go; the Swift runtime libraries are now packaged explicitly.
Linking Swift means the dynamic loader must resolve those libraries before
`main` runs. Bazel links against them through the toolchain's own directory,
which does not exist in an installed pack, so `swift_syntax_rs` now carries an
`$ORIGIN` runpath and the executable finds them beside itself. It arrives
through `CcInfo` rather than `rustc_flags` because `experimental_use_cc_common_link`
makes the link a `cc_common.link` action that `rustc_flags` never reaches.
`cargo test` can no longer build the tests without a local Swift toolchain, so
they run through Bazel, whose toolchain is hermetic on Linux. That also fixes
two things the old arrangement hid: `//unified/extractor:all_tests` covers all
three test files rather than only the corpus, and `test_corpus` now fails
instead of passing vacuously when it finds no cases at all.
`scripts/update-corpus.sh` drops the sandbox so the test can write the
regenerated `.output` files back through the runfiles symlinks. Passing that on
the command line rather than tagging the target keeps the ordinary test run
hermetic.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 297ff885-7c13-4202-a9d0-bc0b17f68d8e
Nothing builds this extractor with `cargo` any more: the tests, the extractor pack and the corpus regeneration all go through Bazel. The `cargo` half of the build is now dead weight, and it is not free -- it is a second, independent way of compiling the same Swift code, kept in step by hand. `build.rs` shelled out to `swift build` on the SwiftPM package in `swift/`, scraped `swiftc -print-target-info` for the runtime directory, and emitted the resulting link and rpath flags. Bazel does none of that: it compiles `SwiftSyntaxFFI.swift` directly against `@swift-syntax`, resolved from the Bazel Central Registry. `Package.swift` and `Package.resolved` existed only for that `swift build`. Deleting them collapses three version pins into one. `MODULE.bazel` pinned the toolchain, `.swift-version` pinned it again for `swiftly` and `build.rs`, and `Package.swift` pinned the `swift-syntax` release a second time -- with a comment asking that they be kept in sync and nothing enforcing it. Two Swift builds that could silently diverge become one that cannot. `cargo check` still works, since checking does not link, so rust-analyzer keeps working across the crate -- completion, go-to-definition and inline diagnostics included. What no longer works is `cargo build`/`cargo test`, and hence the editor's "run test" lens; use `bazel test //unified/extractor:all_tests`. The Cargo manifests stay because `misc/bazel/3rdparty` resolves third-party crate versions from them. `scripts/create-extractor-pack.sh` becomes a wrapper around the `codeql_pack` installer that already existed, rather than a parallel cargo-based assembly of the same pack. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 297ff885-7c13-4202-a9d0-bc0b17f68d8e
tausbn
force-pushed
the
tausbn/unified-always-run-corpus-tests
branch
from
July 30, 2026 15:19
5ec7a27 to
2aba5f1
Compare
`swift-syntax-rs` no longer has a build script, so `crate_universe` stops emitting the vendored aliases for it. Generated by `misc/bazel/3rdparty/update_cargo_deps.sh`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 297ff885-7c13-4202-a9d0-bc0b17f68d8e
tausbn
added
no-change-note-required
tausbn
marked this pull request as ready for review
July 30, 2026 19:01
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the unified extractor to a Bazel-only, in-process Swift parser and enables Bazel corpus tests.
Changes:
- Links
swift-syntax-rsdirectly into the extractor. - Adds Bazel corpus and pipeline test targets.
- Removes Cargo/SwiftPM build and external-parser packaging paths.
Show a summary per file
| File | Description |
|---|---|
MODULE.bazel |
Updates Swift pin documentation. |
Cargo.lock |
Adds extractor dependency on swift-syntax-rs. |
unified/AGENTS.md |
Documents Bazel-only workflows. |
unified/BUILD.bazel |
Packages Swift runtime libraries. |
unified/extractor/BUILD.bazel |
Adds Swift linkage and Bazel tests. |
unified/extractor/Cargo.toml |
Adds the Swift parser crate. |
unified/extractor/src/languages/swift/parse.rs |
Calls Swift parsing in-process. |
unified/extractor/tests/corpus_tests.rs |
Makes corpus execution mandatory under Bazel. |
unified/scripts/create-extractor-pack.sh |
Builds packs through Bazel. |
unified/scripts/update-corpus.sh |
Updates corpus through Bazel. |
unified/swift-syntax-rs/.gitignore |
Removes obsolete SwiftPM output exclusion. |
unified/swift-syntax-rs/.swift-version |
Removes the local Swift pin. |
unified/swift-syntax-rs/BUILD.bazel |
Adds runtime linkage and simplifies the CLI. |
unified/swift-syntax-rs/Cargo.toml |
Documents Cargo limitations. |
unified/swift-syntax-rs/README.md |
Documents the Bazel-only build. |
unified/swift-syntax-rs/build.rs |
Removes Cargo’s Swift build. |
unified/swift-syntax-rs/swift-syntax-parse.sh |
Removes the runtime wrapper. |
unified/swift-syntax-rs/swift/Package.resolved |
Removes SwiftPM resolution data. |
unified/swift-syntax-rs/swift/Package.swift |
Removes the SwiftPM package. |
misc/bazel/3rdparty/tree_sitter_extractors_deps/BUILD.bazel |
Removes obsolete aliases. |
misc/bazel/3rdparty/tree_sitter_extractors_deps/defs.bzl |
Removes obsolete dependency mappings. |
Review details
- Files reviewed: 20/21 changed files
- Comments generated: 2
- Review effort level: Medium
Comment on lines
+90
to
+93
| test_suite( | ||
| name = "all_tests", | ||
| tests = [":%s" % test_name for test_name in _TESTS], | ||
| ) |
Comment on lines
+130
to
+133
| Everything is built by Bazel, which downloads a Swift toolchain from swift.org | ||
| via the official `rules_swift` standalone toolchain extension (wired up in the | ||
| repo-root `MODULE.bazel`) and pulls `swift-syntax` from the Bazel Central | ||
| Registry. Nothing has to be installed locally on Linux: |
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.
Changes our test setup so that the corpus tests are always run. In addition to this, it makes it so that Bazel is the only supported way of building the unified extractor (which greatly simplifies the setup).