diff --git a/unified/AGENTS.md b/unified/AGENTS.md index a50a49868a24..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. +- 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 c8633f1a27a5..616ab00baef9 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 \ + `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}") } } @@ -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/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 2f3ebade8cb3..91b12b53a7e3 100755 --- a/unified/scripts/update-corpus.sh +++ b/unified/scripts/update-corpus.sh @@ -4,5 +4,14 @@ 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. 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, +)