From 17cbb4eb6b66115eefadd7134b2ca64b31d3d3c2 Mon Sep 17 00:00:00 2001 From: Taus Date: Fri, 24 Jul 2026 13:18:13 +0000 Subject: [PATCH] unified: Harden the external Swift parser integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes prompted by review of the swift-syntax switch-over. Parser resolution (`parse.rs`): `parse_bin` now resolves the `swift-syntax-parse` executable in priority order — the `CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE` override, then a copy next to the extractor executable (as a shipped extractor pack lays it out: `tools//{extractor,swift-syntax-parse}`), then a bare `PATH` lookup. This lets a packaged extractor find its parser with no environment setup. (Bundling the binary into the pack, together with its Swift runtime, is a separate follow-up.) Corpus test guard (`corpus_tests.rs`): `parser_available` previously treated *any* parser error as "unavailable" and skipped the entire corpus suite, so a parser that was present but crashed or emitted invalid JSON would silently skip the exact regressions the suite exists to catch. It now uses the new `binary_available`, which reports whether the *executable* can be launched (false only when it cannot be found, e.g. no Swift toolchain); a launchable-but-failing parser makes the suite run and fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../extractor/src/languages/swift/parse.rs | 54 +++++++++++++++++-- unified/extractor/tests/corpus_tests.rs | 16 ++++-- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/unified/extractor/src/languages/swift/parse.rs b/unified/extractor/src/languages/swift/parse.rs index 21ef1b9ad21..c8633f1a27a 100644 --- a/unified/extractor/src/languages/swift/parse.rs +++ b/unified/extractor/src/languages/swift/parse.rs @@ -16,9 +16,12 @@ use codeql_extractor::extractor::ParsedTree; use super::swift_adapter; /// Environment variable naming the `swift-syntax-parse` executable. When unset, -/// `swift-syntax-parse` is looked up on `PATH`. +/// the parser is resolved next to the extractor executable, then on `PATH`. const PARSE_BIN_ENV: &str = "CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE"; +/// Base name of the `swift-syntax-parse` executable as shipped / looked up. +const PARSE_BIN_NAME: &str = "swift-syntax-parse"; + /// Parse Swift `source` into a [`ParsedTree`] (a raw `yeast::Ast` plus /// side-channel `extra` tokens), ready to be desugared via `run_from_ast`. pub fn parse(source: &[u8]) -> Result { @@ -33,9 +36,54 @@ pub fn parse(source: &[u8]) -> Result { }) } -/// The `swift-syntax-parse` executable to invoke. +/// The `swift-syntax-parse` executable to invoke, resolved in priority order: +/// +/// 1. the `CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE` override, if set; +/// 2. a copy shipped next to the extractor executable — this is how the CodeQL +/// 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`. fn parse_bin() -> String { - std::env::var(PARSE_BIN_ENV).unwrap_or_else(|_| "swift-syntax-parse".to_string()) + if let Ok(bin) = std::env::var(PARSE_BIN_ENV) { + if !bin.is_empty() { + return bin; + } + } + 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(); + } + } + } + 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, + } } /// Run the external parser, feeding `source` on stdin and returning its JSON diff --git a/unified/extractor/tests/corpus_tests.rs b/unified/extractor/tests/corpus_tests.rs index 66374e56526..b2700b999b7 100644 --- a/unified/extractor/tests/corpus_tests.rs +++ b/unified/extractor/tests/corpus_tests.rs @@ -20,12 +20,18 @@ fn update_mode_enabled() -> bool { .unwrap_or(false) } -/// Whether the external swift-syntax parser is available. When it is not (e.g. -/// no Swift toolchain / the `swift-syntax-parse` binary is not on `PATH` and -/// `CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE` is unset), the corpus test is -/// skipped rather than failed — it cannot run without the Swift-backed parser. +/// 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::parse(b"").is_ok() + languages::swift_parse::binary_available() } /// Parse a corpus `.output` file. The file holds a single test case made of