mirror of
https://github.com/github/codeql.git
synced 2026-07-29 06:46:46 +02:00
unified: Harden the external Swift parser integration
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/<platform>/{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>
This commit is contained in:
@@ -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<ParsedTree, String> {
|
||||
@@ -33,9 +36,54 @@ pub fn parse(source: &[u8]) -> Result<ParsedTree, String> {
|
||||
})
|
||||
}
|
||||
|
||||
/// 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/<platform>/{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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user