Skip to content

Commit 5910e1b

Browse files
tausbnCopilot
andcommitted
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>
1 parent 4aa661a commit 5910e1b

2 files changed

Lines changed: 62 additions & 8 deletions

File tree

‎unified/extractor/src/languages/swift/parse.rs‎

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,12 @@ use codeql_extractor::extractor::ParsedTree;
1616
use super::swift_adapter;
1717

1818
/// Environment variable naming the `swift-syntax-parse` executable. When unset,
19-
/// `swift-syntax-parse` is looked up on `PATH`.
19+
/// the parser is resolved next to the extractor executable, then on `PATH`.
2020
const PARSE_BIN_ENV: &str = "CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE";
2121

22+
/// Base name of the `swift-syntax-parse` executable as shipped / looked up.
23+
const PARSE_BIN_NAME: &str = "swift-syntax-parse";
24+
2225
/// Parse Swift `source` into a [`ParsedTree`] (a raw `yeast::Ast` plus
2326
/// side-channel `extra` tokens), ready to be desugared via `run_from_ast`.
2427
pub fn parse(source: &[u8]) -> Result<ParsedTree, String> {
@@ -33,9 +36,54 @@ pub fn parse(source: &[u8]) -> Result<ParsedTree, String> {
3336
})
3437
}
3538

36-
/// The `swift-syntax-parse` executable to invoke.
39+
/// The `swift-syntax-parse` executable to invoke, resolved in priority order:
40+
///
41+
/// 1. the `CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE` override, if set;
42+
/// 2. a copy shipped next to the extractor executable — this is how the CodeQL
43+
/// extractor pack lays it out (`tools/<platform>/{extractor,
44+
/// swift-syntax-parse}`), so a packaged extractor is self-contained with no
45+
/// environment setup;
46+
/// 3. a bare `swift-syntax-parse`, looked up on `PATH`.
3747
fn parse_bin() -> String {
38-
std::env::var(PARSE_BIN_ENV).unwrap_or_else(|_| "swift-syntax-parse".to_string())
48+
if let Ok(bin) = std::env::var(PARSE_BIN_ENV) {
49+
if !bin.is_empty() {
50+
return bin;
51+
}
52+
}
53+
if let Ok(exe) = std::env::current_exe() {
54+
if let Some(sibling) = exe.parent().map(|dir| dir.join(PARSE_BIN_NAME)) {
55+
if sibling.is_file() {
56+
return sibling.to_string_lossy().into_owned();
57+
}
58+
}
59+
}
60+
PARSE_BIN_NAME.to_string()
61+
}
62+
63+
/// Whether the `swift-syntax-parse` executable can be launched at all.
64+
///
65+
/// This reports availability of the *executable*, deliberately not whether
66+
/// parsing succeeds: a binary that launches but then crashes or emits invalid
67+
/// JSON is still "available", so callers run and surface the failure rather
68+
/// than silently skipping. Only a genuinely missing/unlaunchable binary (e.g.
69+
/// no Swift toolchain is installed) reports `false`.
70+
pub fn binary_available() -> bool {
71+
match Command::new(parse_bin())
72+
.stdin(Stdio::null())
73+
.stdout(Stdio::null())
74+
.stderr(Stdio::null())
75+
.spawn()
76+
{
77+
Ok(mut child) => {
78+
let _ = child.wait();
79+
true
80+
}
81+
Err(e) if e.kind() == std::io::ErrorKind::NotFound => false,
82+
// Any other spawn failure (e.g. a permissions problem) is a genuine
83+
// issue worth surfacing, so treat the parser as available and let the
84+
// caller fail rather than masking it as "unavailable".
85+
Err(_) => true,
86+
}
3987
}
4088

4189
/// Run the external parser, feeding `source` on stdin and returning its JSON

‎unified/extractor/tests/corpus_tests.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,18 @@ fn update_mode_enabled() -> bool {
2020
.unwrap_or(false)
2121
}
2222

23-
/// Whether the external swift-syntax parser is available. When it is not (e.g.
24-
/// no Swift toolchain / the `swift-syntax-parse` binary is not on `PATH` and
25-
/// `CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE` is unset), the corpus test is
26-
/// skipped rather than failed — it cannot run without the Swift-backed parser.
23+
/// Whether the external swift-syntax parser is available. When the parser
24+
/// binary genuinely cannot be found/launched (e.g. no Swift toolchain, and
25+
/// neither `CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE` nor a `swift-syntax-parse`
26+
/// on `PATH`), the corpus test is skipped rather than failed — it cannot run
27+
/// without the Swift-backed parser.
28+
///
29+
/// Crucially this checks only that the executable *launches*: a parser that is
30+
/// present but crashes, emits invalid JSON, or otherwise regresses is
31+
/// considered available, so the suite runs and fails (rather than silently
32+
/// skipping the very failures CI needs to catch).
2733
fn parser_available() -> bool {
28-
languages::swift_parse::parse(b"").is_ok()
34+
languages::swift_parse::binary_available()
2935
}
3036

3137
/// Parse a corpus `.output` file. The file holds a single test case made of

0 commit comments

Comments
 (0)