From 0e5674ec8a6cb3e70d032af39d45bba1946dfe0c Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 29 Jul 2026 11:33:57 +0000 Subject: [PATCH 1/2] unified: Always run the corpus tests The corpus tests skipped themselves when the `swift-syntax-parse` binary could not be launched. That was meant to keep the suite usable without a Swift toolchain, but it hid far more than it helped: a skip still reports `test result: ok`, and the message explaining why only appears under `cargo test -- --nocapture`. `scripts/update-corpus.sh` never set the variable that locates the parser, so the documented way to regenerate the corpus silently exercised nothing at all. Drop the guard, so a missing parser fails loudly, and make the parser easy to find so that failing is rare: - Resolve the parser one directory above the running executable as well as beside it. Test binaries live in `target//deps/`, so the existing sibling lookup could never find `target//swift-syntax-parse` and `cargo test` always fell through to `PATH`. - Report a missing binary with the command that builds it, rather than a bare `No such file or directory`. - Build the parser in `scripts/update-corpus.sh`, so regenerating the corpus works from a clean checkout. `cargo test` in `extractor` still needs the parser built first, since the extractor deliberately does not depend on the crate that provides it -- that is what keeps the Swift toolchain off the build path for the other languages. `AGENTS.md` now says so. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- unified/AGENTS.md | 2 +- .../extractor/src/languages/swift/parse.rs | 53 +++++++++---------- unified/extractor/tests/corpus_tests.rs | 22 -------- unified/scripts/update-corpus.sh | 7 +++ 4 files changed, 33 insertions(+), 51 deletions(-) diff --git a/unified/AGENTS.md b/unified/AGENTS.md index a50a49868a24..595838c9f147 100644 --- a/unified/AGENTS.md +++ b/unified/AGENTS.md @@ -17,7 +17,7 @@ This is a CodeQL extractor based on tree-sitter. - The mapping from the parse tree to the target AST is found in `extractor/src/languages/swift/swift.rs` -- To run tests for the parser and mapping, run `cargo test` in the `extractor` directory. +- To run tests for the parser and mapping, run `cargo test` in the `extractor` directory. The corpus tests shell out to the `swift-syntax-parse` binary, which lives in a separate crate that `cargo test` does not build, so build it first with `cargo build -p swift-syntax-rs --bin swift-syntax-parse` (this needs a Swift toolchain; `scripts/update-corpus.sh` does it for you). - Extractor test cases are located at `extractor/tests/corpus/swift/*/*.swift`. diff --git a/unified/extractor/src/languages/swift/parse.rs b/unified/extractor/src/languages/swift/parse.rs index c8633f1a27a5..1e3ef610c446 100644 --- a/unified/extractor/src/languages/swift/parse.rs +++ b/unified/extractor/src/languages/swift/parse.rs @@ -43,7 +43,10 @@ pub fn parse(source: &[u8]) -> Result { /// extractor pack lays it out (`tools//{extractor, /// swift-syntax-parse}`), so a packaged extractor is self-contained with no /// environment setup; -/// 3. a bare `swift-syntax-parse`, looked up on `PATH`. +/// 3. a copy one directory further up, which is where `cargo` leaves it when +/// the running executable is a test binary: those live in +/// `target//deps/`, one level below `target//`; +/// 4. a bare `swift-syntax-parse`, looked up on `PATH`. fn parse_bin() -> String { if let Ok(bin) = std::env::var(PARSE_BIN_ENV) { if !bin.is_empty() { @@ -51,38 +54,32 @@ fn parse_bin() -> String { } } if let Ok(exe) = std::env::current_exe() { - if let Some(sibling) = exe.parent().map(|dir| dir.join(PARSE_BIN_NAME)) { - if sibling.is_file() { - return sibling.to_string_lossy().into_owned(); + let exe_dir = exe.parent(); + let candidates = [exe_dir, exe_dir.and_then(|dir| dir.parent())]; + for dir in candidates.into_iter().flatten() { + let candidate = dir.join(PARSE_BIN_NAME); + if candidate.is_file() { + return candidate.to_string_lossy().into_owned(); } } } PARSE_BIN_NAME.to_string() } -/// Whether the `swift-syntax-parse` executable can be launched at all. -/// -/// This reports availability of the *executable*, deliberately not whether -/// parsing succeeds: a binary that launches but then crashes or emits invalid -/// JSON is still "available", so callers run and surface the failure rather -/// than silently skipping. Only a genuinely missing/unlaunchable binary (e.g. -/// no Swift toolchain is installed) reports `false`. -pub fn binary_available() -> bool { - match Command::new(parse_bin()) - .stdin(Stdio::null()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - { - Ok(mut child) => { - let _ = child.wait(); - true - } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => false, - // Any other spawn failure (e.g. a permissions problem) is a genuine - // issue worth surfacing, so treat the parser as available and let the - // caller fail rather than masking it as "unavailable". - Err(_) => true, +/// Explain a failure to launch the parser. A missing binary is by far the most +/// common way this fails — the Swift half of the build is separate, so it is +/// easy to have never built it — so that case carries the remedy rather than a +/// bare OS error. +fn spawn_error(bin: &str, error: std::io::Error) -> String { + if error.kind() == std::io::ErrorKind::NotFound { + format!( + "could not find the Swift parser `{bin}`. Build it with \ + `cargo build -p swift-syntax-rs --bin swift-syntax-parse` (this needs a Swift \ + toolchain — see `unified/swift-syntax-rs/.swift-version` for the pinned version), \ + or point `{PARSE_BIN_ENV}` at an existing copy." + ) + } else { + format!("failed to spawn Swift parser `{bin}`: {error}") } } @@ -95,7 +92,7 @@ fn run_parser(source: &str) -> Result { .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .map_err(|e| format!("failed to spawn Swift parser `{bin}`: {e}"))?; + .map_err(|e| spawn_error(&bin, e))?; // The parser reads all of stdin before writing any stdout, so writing the // whole source and then closing stdin (by dropping it) cannot deadlock. diff --git a/unified/extractor/tests/corpus_tests.rs b/unified/extractor/tests/corpus_tests.rs index b2700b999b7c..39cbab3194c7 100644 --- a/unified/extractor/tests/corpus_tests.rs +++ b/unified/extractor/tests/corpus_tests.rs @@ -20,20 +20,6 @@ fn update_mode_enabled() -> bool { .unwrap_or(false) } -/// Whether the external swift-syntax parser is available. When the parser -/// binary genuinely cannot be found/launched (e.g. no Swift toolchain, and -/// neither `CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE` nor a `swift-syntax-parse` -/// on `PATH`), the corpus test is skipped rather than failed — it cannot run -/// without the Swift-backed parser. -/// -/// Crucially this checks only that the executable *launches*: a parser that is -/// present but crashes, emits invalid JSON, or otherwise regresses is -/// considered available, so the suite runs and fails (rather than silently -/// skipping the very failures CI needs to catch). -fn parser_available() -> bool { - languages::swift_parse::binary_available() -} - /// Parse a corpus `.output` file. The file holds a single test case made of /// three sections separated by `---` delimiter lines: /// @@ -112,14 +98,6 @@ fn collect_corpus_stems(dir: &Path, out: &mut Vec) { #[test] fn test_corpus() { - if !parser_available() { - eprintln!( - "skipping test_corpus: the swift-syntax parser is unavailable \ - (set CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE or put \ - `swift-syntax-parse` on PATH)" - ); - return; - } let update_mode = update_mode_enabled(); let all_languages = languages::all_language_specs(); let corpus_dir = Path::new("tests/corpus"); diff --git a/unified/scripts/update-corpus.sh b/unified/scripts/update-corpus.sh index 2f3ebade8cb3..a4dd27095dc1 100755 --- a/unified/scripts/update-corpus.sh +++ b/unified/scripts/update-corpus.sh @@ -4,5 +4,12 @@ IFS=$'\n\t' cd "$(dirname "$0")/.." +# The corpus is produced by the external swift-syntax parser. That parser is a +# separate crate which `cargo test` does not build (the extractor deliberately +# does not depend on it, so working on other languages needs no Swift +# toolchain), so build it up front — otherwise the tests below fail on a +# missing binary. +cargo build -p swift-syntax-rs --bin swift-syntax-parse + cd extractor UNIFIED_UPDATE_CORPUS=1 cargo test From 8fa46a56225a0744875357d659f8ee5d179bae84 Mon Sep 17 00:00:00 2001 From: Taus Date: Wed, 29 Jul 2026 11:47:29 +0000 Subject: [PATCH 2/2] unified: Build the Swift parser without a local toolchain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the corpus tests no longer skip themselves, running them requires a `swift-syntax-parse` binary — and so far the only way to get one was `cargo build`, which needs Swift installed locally. Bazel already builds the same binary against a hermetic swift.org toolchain, so offer that instead and make it the default. The wrapper script locates its Swift runtime libraries beside itself, so it only works in the flattened layout the extractor pack uses; under `bazel-bin`, and in the runfiles tree, the libraries are in a different directory and it fails to start. Add a `pkg_install` target that stages that layout into a directory, and `scripts/build-parser.sh` to run it and print the resulting path: export CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE=$(scripts/build-parser.sh) `--cargo` selects the previous behaviour, which is quicker to iterate on when a local toolchain is available. Both pin swift-syntax 603.0.2, so either parser produces the same trees. `scripts/update-corpus.sh` now goes through the script and forwards its arguments, so regenerating the corpus needs only Bazel. The staging directory sits under `unified/target`, which is already ignored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- unified/AGENTS.md | 2 +- .../extractor/src/languages/swift/parse.rs | 6 +-- unified/scripts/build-parser.sh | 44 +++++++++++++++++++ unified/scripts/update-corpus.sh | 6 ++- unified/swift-syntax-rs/BUILD.bazel | 16 +++++++ 5 files changed, 68 insertions(+), 6 deletions(-) create mode 100755 unified/scripts/build-parser.sh diff --git a/unified/AGENTS.md b/unified/AGENTS.md index 595838c9f147..dc54f351bee0 100644 --- a/unified/AGENTS.md +++ b/unified/AGENTS.md @@ -17,7 +17,7 @@ This is a CodeQL extractor based on tree-sitter. - The mapping from the parse tree to the target AST is found in `extractor/src/languages/swift/swift.rs` -- To run tests for the parser and mapping, run `cargo test` in the `extractor` directory. The corpus tests shell out to the `swift-syntax-parse` binary, which lives in a separate crate that `cargo test` does not build, so build it first with `cargo build -p swift-syntax-rs --bin swift-syntax-parse` (this needs a Swift toolchain; `scripts/update-corpus.sh` does it for you). +- To run tests for the parser and mapping, run `cargo test` in the `extractor` directory. The corpus tests shell out to the `swift-syntax-parse` binary, which lives in a separate crate that `cargo test` does not build; build it and point the tests at it with `export CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE=$(scripts/build-parser.sh)`. That builds with Bazel, whose Swift toolchain is hermetic, so no local Swift installation is needed; pass `--cargo` to build with a local toolchain instead. `scripts/update-corpus.sh` does this for you. - Extractor test cases are located at `extractor/tests/corpus/swift/*/*.swift`. diff --git a/unified/extractor/src/languages/swift/parse.rs b/unified/extractor/src/languages/swift/parse.rs index 1e3ef610c446..616ab00baef9 100644 --- a/unified/extractor/src/languages/swift/parse.rs +++ b/unified/extractor/src/languages/swift/parse.rs @@ -74,9 +74,9 @@ fn spawn_error(bin: &str, error: std::io::Error) -> String { if error.kind() == std::io::ErrorKind::NotFound { format!( "could not find the Swift parser `{bin}`. Build it with \ - `cargo build -p swift-syntax-rs --bin swift-syntax-parse` (this needs a Swift \ - toolchain — see `unified/swift-syntax-rs/.swift-version` for the pinned version), \ - or point `{PARSE_BIN_ENV}` at an existing copy." + `unified/scripts/build-parser.sh`, which prints the path to set \ + `{PARSE_BIN_ENV}` to (it uses Bazel's Swift toolchain, so no local \ + Swift installation is needed)." ) } else { format!("failed to spawn Swift parser `{bin}`: {error}") diff --git a/unified/scripts/build-parser.sh b/unified/scripts/build-parser.sh new file mode 100755 index 000000000000..3be0d7a919a4 --- /dev/null +++ b/unified/scripts/build-parser.sh @@ -0,0 +1,44 @@ +#!/bin/bash +# Build `swift-syntax-parse`, the binary the Swift front-end shells out to, and +# print the path to it. +# +# By default this builds with Bazel, whose Swift toolchain is hermetic: nothing +# has to be installed locally. Pass `--cargo` to build through cargo instead, +# which is quicker to iterate on but needs a local Swift toolchain matching +# `swift-syntax-rs/.swift-version`. Both pin the same swift-syntax release, so +# the two produce equivalent parsers. +# +# Typical use: +# +# export CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE=$(scripts/build-parser.sh) +# +# Progress output goes to stderr so that only the path lands on stdout. +set -euo pipefail +IFS=$'\n\t' + +cd "$(dirname "$0")/.." + +mode=bazel +case "${1:-}" in + "" | --bazel) ;; + --cargo) mode=cargo ;; + *) + echo "usage: $(basename "$0") [--bazel | --cargo]" >&2 + exit 2 + ;; +esac + +if [[ $mode == cargo ]]; then + cargo build -p swift-syntax-rs --bin swift-syntax-parse >&2 + # Cargo builds workspace members into the target directory at the + # repository root, one level above this directory. + echo "$(cd .. && pwd)/target/debug/swift-syntax-parse" + exit 0 +fi + +# The wrapper script finds its Swift runtime libraries beside itself, so it only +# works in the flattened layout the installer produces; under `bazel-bin` and in +# the runfiles tree the libraries sit in a different directory. +dest=$PWD/target/swift-syntax-parse +bazel run //unified/swift-syntax-rs:install-parser -- --destdir "$dest" >&2 +echo "$dest/swift-syntax-parse" diff --git a/unified/scripts/update-corpus.sh b/unified/scripts/update-corpus.sh index a4dd27095dc1..91b12b53a7e3 100755 --- a/unified/scripts/update-corpus.sh +++ b/unified/scripts/update-corpus.sh @@ -8,8 +8,10 @@ cd "$(dirname "$0")/.." # separate crate which `cargo test` does not build (the extractor deliberately # does not depend on it, so working on other languages needs no Swift # toolchain), so build it up front — otherwise the tests below fail on a -# missing binary. -cargo build -p swift-syntax-rs --bin swift-syntax-parse +# missing binary. This defaults to Bazel's hermetic Swift toolchain; pass +# `--cargo` to use a local one instead. +CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE=$(scripts/build-parser.sh "$@") +export CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE cd extractor UNIFIED_UPDATE_CORPUS=1 cargo test diff --git a/unified/swift-syntax-rs/BUILD.bazel b/unified/swift-syntax-rs/BUILD.bazel index db451e5b7b9a..e0978bf7d527 100644 --- a/unified/swift-syntax-rs/BUILD.bazel +++ b/unified/swift-syntax-rs/BUILD.bazel @@ -1,3 +1,4 @@ +load("@rules_pkg//pkg:install.bzl", "pkg_install") load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library", "rust_test") load("@rules_shell//shell:sh_binary.bzl", "sh_binary") load("//misc/bazel:pkg.bzl", "codeql_pkg_runfiles") @@ -105,3 +106,18 @@ rust_test( edition = "2024", target_compatible_with = _SWIFT_SUPPORTED_PLATFORMS, ) + +# Stage the packaged parser into a directory, so it can be run outside Bazel: +# +# bazel run //unified/swift-syntax-rs:install-parser -- --destdir +# +# The wrapper only works in this flattened layout, where it sits next to the +# real binary and the runtime libraries; in `bazel-bin` and in the runfiles tree +# those live in separate directories. Bazel's Swift toolchain is hermetic, so +# this is also how the extractor's tests are run without installing Swift — see +# `unified/scripts/build-parser.sh`. +pkg_install( + name = "install-parser", + srcs = [":swift-syntax-parse-pkg"], + target_compatible_with = _SWIFT_SUPPORTED_PLATFORMS, +)