Skip to content

Commit 2d93270

Browse files
tausbnCopilot
andcommitted
unified: Parse Swift in-process instead of spawning a parser
The Swift front-end shelled out to a separate `swift-syntax-parse` executable and read a JSON syntax tree back over a pipe. That kept the Swift toolchain off the extractor's build path, so working on the other (tree-sitter based) languages needed no Swift -- but it meant the extractor had to *find* that executable at run time, and everything downstream of that was a workaround: - the binary is a shell wrapper that sets `LD_LIBRARY_PATH` and execs `swift-syntax-parse.real`, because the Swift runtime libraries have to sit beside it; - that only works in a flattened layout, so packaging went through `codeql_pkg_runfiles` and the runtime libraries reached the pack only as a side effect of shipping the parser; - and the corpus tests skipped themselves when they could not find the binary. A skip still prints `test result: ok`, so the suite silently tested nothing. Supporting Swift is the current priority, so the extractor may now depend on building the Swift half. Link `swift-syntax-rs` in and call `parse_to_json` directly: `parse.rs` loses ~100 lines of process plumbing, and the tree is necessarily produced by the swift-syntax build this extractor was built against, so it cannot silently run a stale parser. The parser survives as a debugging aid for looking at raw swift-syntax JSON: echo 'let x = 1' | bazel run //unified/swift-syntax-rs:swift-syntax-parse It is no longer shipped, so the wrapper, the `.real` split and the runfiles packaging all go; the Swift runtime libraries are now packaged explicitly. Linking Swift means the dynamic loader must resolve those libraries before `main` runs. Bazel links against them through the toolchain's own directory, which does not exist in an installed pack, so `swift_syntax_rs` now carries an `$ORIGIN` runpath and the executable finds them beside itself. It arrives through `CcInfo` rather than `rustc_flags` because `experimental_use_cc_common_link` makes the link a `cc_common.link` action that `rustc_flags` never reaches. `cargo test` can no longer build the tests without a local Swift toolchain, so they run through Bazel, whose toolchain is hermetic on Linux. That also fixes two things the old arrangement hid: `//unified/extractor:all_tests` covers all three test files rather than only the corpus, and `test_corpus` now fails instead of passing vacuously when it finds no cases at all. `scripts/update-corpus.sh` drops the sandbox so the test can write the regenerated `.output` files back through the runfiles symlinks. Passing that on the command line rather than tagging the target keeps the ordinary test run hermetic. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 297ff885-7c13-4202-a9d0-bc0b17f68d8e
1 parent 478878c commit 2d93270

12 files changed

Lines changed: 196 additions & 208 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

unified/AGENTS.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
# Agent instructions
22

3-
This is a CodeQL extractor based on tree-sitter.
3+
This is a CodeQL extractor based on tree-sitter, with a Swift front-end built on
4+
`swift-syntax`.
5+
6+
Build and test with Bazel, whose Swift toolchain is hermetic on Linux, so
7+
nothing needs to be installed locally. The extractor links `swift-syntax`, so a
8+
`cargo` build additionally needs a local Swift toolchain.
49

510
## Building
6-
- To build the extractor, run `scripts/create-extractor-pack.sh`
11+
- To build the extractor pack, run `scripts/create-extractor-pack.sh`.
712

813
## Swift Parser
914
- The Swift parser is defined by `extractor/tree-sitter-swift/grammar.js` and can be edited if needed.
@@ -17,7 +22,7 @@ This is a CodeQL extractor based on tree-sitter.
1722

1823
- The mapping from the parse tree to the target AST is found in `extractor/src/languages/swift/swift.rs`
1924

20-
- To run tests for the parser and mapping, run `cargo test` in the `extractor` directory.
25+
- To run tests for the parser and mapping, run `bazel test //unified/extractor:all_tests`.
2126

2227
- Extractor test cases are located at `extractor/tests/corpus/swift/*/*.swift`.
2328

unified/BUILD.bazel

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,13 @@ codeql_pkg_files(
4747
prefix = "tools/{CODEQL_PLATFORM}",
4848
)
4949

50-
# The Swift front-end parser (wrapper + real binary + bundled Swift runtime),
51-
# shipped next to the extractor. Only on platforms where swift-syntax builds
52-
# (Linux/macOS); elsewhere the group is empty so the pack still builds (Swift
53-
# extraction is simply unavailable there).
54-
pkg_filegroup(
55-
name = "swift-syntax-parse-arch",
56-
srcs = select_os(
50+
# The Swift runtime, which the extractor loads at startup. Linux only: macOS
51+
# provides it with the OS.
52+
codeql_pkg_files(
53+
name = "swift-runtime-arch",
54+
exes = select_os(
55+
linux = ["//unified/swift-syntax-rs:swift_runtime_libs"],
5756
otherwise = [],
58-
posix = ["//unified/swift-syntax-rs:swift-syntax-parse-pkg"],
5957
),
6058
prefix = "tools/{CODEQL_PLATFORM}",
6159
)
@@ -66,7 +64,7 @@ codeql_pack(
6664
":codeql-extractor-yml",
6765
":dbscheme-group",
6866
":extractor-arch",
69-
":swift-syntax-parse-arch",
67+
":swift-runtime-arch",
7068
"//unified/tools",
7169
],
7270
)

unified/extractor/BUILD.bazel

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
1+
load("@rules_rust//rust:defs.bzl", "rust_test")
12
load("//misc/bazel:rust.bzl", "codeql_rust_binary")
23
load("//misc/bazel/3rdparty/tree_sitter_extractors_deps:defs.bzl", "aliases", "all_crate_deps")
34

45
exports_files(["Cargo.toml"])
56

7+
# swift-syntax builds on Linux and macOS only.
8+
_SWIFT_SUPPORTED_PLATFORMS = select({
9+
"@platforms//os:linux": [],
10+
"@platforms//os:macos": [],
11+
"//conditions:default": ["@platforms//:incompatible"],
12+
})
13+
614
codeql_rust_binary(
715
name = "extractor",
816
srcs = glob(["src/**/*.rs"]),
@@ -11,15 +19,84 @@ codeql_rust_binary(
1119
"ast_types.yml",
1220
"swift_node_types.yml",
1321
],
22+
# Only for running from the build tree: there the Swift runtime is resolved
23+
# through the toolchain-relative part of the runpath, which needs the
24+
# libraries in the runfiles tree. An installed pack uses `$ORIGIN` instead.
25+
data = select({
26+
"@platforms//os:linux": ["//unified/swift-syntax-rs:swift_runtime_libs"],
27+
"//conditions:default": [],
28+
}),
1429
proc_macro_deps = all_crate_deps(
1530
proc_macro = True,
1631
),
32+
target_compatible_with = _SWIFT_SUPPORTED_PLATFORMS,
1733
visibility = ["//visibility:public"],
1834
deps = all_crate_deps(
1935
normal = True,
2036
) + [
2137
"//shared/tree-sitter-extractor",
2238
"//shared/yeast",
2339
"//unified/extractor/tree-sitter-swift",
40+
"//unified/swift-syntax-rs:swift_syntax_rs",
2441
],
2542
)
43+
44+
# One target per file in `tests/`. Each pulls in `src/**` too, because the tests
45+
# reach into the crate's modules with `#[path]`.
46+
_TESTS = {
47+
"corpus_tests": {
48+
"data": glob(["tests/corpus/**"]),
49+
"compile_data": [],
50+
"size": "medium",
51+
},
52+
# Type-checks rules against the schemas at compile time, so they are
53+
# compile data rather than runtime data.
54+
"rules_macro_smoke": {
55+
"data": [],
56+
"compile_data": ["//unified/extractor/tree-sitter-swift:node-types.yml"],
57+
"size": "small",
58+
},
59+
# `include_str!`s a checked-in `parse_to_json` dump.
60+
"swift_syntax_pipeline": {
61+
"data": [],
62+
"compile_data": glob(["tests/fixtures/**"]),
63+
"size": "small",
64+
},
65+
}
66+
67+
[
68+
rust_test(
69+
name = test_name,
70+
size = spec["size"],
71+
srcs = ["tests/%s.rs" % test_name] + glob(["src/**/*.rs"]),
72+
aliases = aliases(),
73+
compile_data = [
74+
"ast_types.yml",
75+
"swift_node_types.yml",
76+
] + spec["compile_data"],
77+
crate_root = "tests/%s.rs" % test_name,
78+
data = spec["data"] + select({
79+
"@platforms//os:linux": ["//unified/swift-syntax-rs:swift_runtime_libs"],
80+
"//conditions:default": [],
81+
}),
82+
edition = "2024",
83+
proc_macro_deps = all_crate_deps(
84+
proc_macro = True,
85+
),
86+
target_compatible_with = _SWIFT_SUPPORTED_PLATFORMS,
87+
deps = all_crate_deps(
88+
normal = True,
89+
) + [
90+
"//shared/tree-sitter-extractor",
91+
"//shared/yeast",
92+
"//unified/extractor/tree-sitter-swift",
93+
"//unified/swift-syntax-rs:swift_syntax_rs",
94+
],
95+
)
96+
for test_name, spec in _TESTS.items()
97+
]
98+
99+
test_suite(
100+
name = "all_tests",
101+
tests = [":%s" % test_name for test_name in _TESTS],
102+
)

unified/extractor/Cargo.toml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,8 @@ serde_json = "1.0.145"
2121

2222
codeql-extractor = { path = "../../shared/tree-sitter-extractor" }
2323
yeast = { path = "../../shared/yeast" }
24+
# The Swift front-end links swift-syntax through this crate's FFI shim. Its
25+
# Swift half is built by Bazel, so `cargo build`/`test` cannot link the
26+
# extractor: use `bazel build //unified/extractor` and
27+
# `bazel test //unified/extractor:all_tests`.
28+
swift-syntax-rs = { path = "../swift-syntax-rs" }
Lines changed: 5 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,121 +1,22 @@
1-
//! Swift front-end parser: shells out to the separate `swift-syntax-parse`
2-
//! binary (which links swift-syntax) to obtain a JSON syntax tree, then adapts
3-
//! that JSON into a `yeast::Ast` via the pure-Rust [`swift_adapter`] module.
4-
//!
5-
//! Running the parser in a separate process keeps the Swift toolchain out of
6-
//! the extractor's own build: the extractor never links Swift, so working on
7-
//! other (e.g. tree-sitter based) languages needs no Swift toolchain. Each call
8-
//! spawns the parser afresh; a longer-lived parser process could be swapped in
9-
//! behind this same seam later without touching the extraction pipeline.
10-
11-
use std::io::Write;
12-
use std::process::{Command, Stdio};
1+
//! Swift front-end parser: calls into the `swift-syntax-rs` crate (which links
2+
//! swift-syntax) to obtain a JSON syntax tree, then adapts that JSON into a
3+
//! `yeast::Ast` via the pure-Rust [`swift_adapter`] module.
134
145
use codeql_extractor::extractor::ParsedTree;
156

167
use super::swift_adapter;
178

18-
/// Environment variable naming the `swift-syntax-parse` executable. When unset,
19-
/// the parser is resolved next to the extractor executable, then on `PATH`.
20-
const PARSE_BIN_ENV: &str = "CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE";
21-
22-
/// Base name of the `swift-syntax-parse` executable as shipped / looked up.
23-
const PARSE_BIN_NAME: &str = "swift-syntax-parse";
24-
259
/// Parse Swift `source` into a [`ParsedTree`] (a raw `yeast::Ast` plus
2610
/// side-channel `extra` tokens), ready to be desugared via `run_from_ast`.
2711
pub fn parse(source: &[u8]) -> Result<ParsedTree, String> {
2812
let source =
2913
std::str::from_utf8(source).map_err(|e| format!("Swift source is not valid UTF-8: {e}"))?;
30-
let json = run_parser(source)?;
14+
let json =
15+
swift_syntax_rs::parse_to_json(source).map_err(|e| format!("Swift parser failed: {e}"))?;
3116
let mut adapted = swift_adapter::json_to_ast(&json)?;
3217
adapted.ast.set_source(source.as_bytes().to_vec());
3318
Ok(ParsedTree {
3419
ast: adapted.ast,
3520
extras: adapted.extras,
3621
})
3722
}
38-
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`.
47-
fn parse_bin() -> 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-
}
87-
}
88-
89-
/// Run the external parser, feeding `source` on stdin and returning its JSON
90-
/// stdout.
91-
fn run_parser(source: &str) -> Result<String, String> {
92-
let bin = parse_bin();
93-
let mut child = Command::new(&bin)
94-
.stdin(Stdio::piped())
95-
.stdout(Stdio::piped())
96-
.stderr(Stdio::piped())
97-
.spawn()
98-
.map_err(|e| format!("failed to spawn Swift parser `{bin}`: {e}"))?;
99-
100-
// The parser reads all of stdin before writing any stdout, so writing the
101-
// whole source and then closing stdin (by dropping it) cannot deadlock.
102-
child
103-
.stdin
104-
.take()
105-
.expect("child stdin was piped")
106-
.write_all(source.as_bytes())
107-
.map_err(|e| format!("failed to write source to Swift parser `{bin}`: {e}"))?;
108-
109-
let output = child
110-
.wait_with_output()
111-
.map_err(|e| format!("failed to run Swift parser `{bin}`: {e}"))?;
112-
if !output.status.success() {
113-
return Err(format!(
114-
"Swift parser `{bin}` failed ({}): {}",
115-
output.status,
116-
String::from_utf8_lossy(&output.stderr).trim()
117-
));
118-
}
119-
String::from_utf8(output.stdout)
120-
.map_err(|e| format!("Swift parser produced non-UTF-8 output: {e}"))
121-
}

unified/extractor/tests/corpus_tests.rs

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

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).
33-
fn parser_available() -> bool {
34-
languages::swift_parse::binary_available()
35-
}
36-
3723
/// Parse a corpus `.output` file. The file holds a single test case made of
3824
/// three sections separated by `---` delimiter lines:
3925
///
@@ -110,19 +96,32 @@ fn collect_corpus_stems(dir: &Path, out: &mut Vec<std::path::PathBuf>) {
11096
}
11197
}
11298

99+
/// The corpus root, in the runfiles tree.
100+
///
101+
/// Bazel runs tests from the runfiles root, under a directory named after the
102+
/// repository the test came from — `_main` when `github/codeql` is built
103+
/// standalone, `ql+` when it is consumed as a dependency — so look for it
104+
/// rather than hard-coding either name.
105+
fn corpus_dir() -> std::path::PathBuf {
106+
let srcdir = std::env::var_os("TEST_SRCDIR")
107+
.expect("TEST_SRCDIR is unset; these tests are run with `bazel test`");
108+
let entries = fs::read_dir(&srcdir)
109+
.unwrap_or_else(|e| panic!("failed to read TEST_SRCDIR {srcdir:?}: {e}"));
110+
for entry in entries.flatten() {
111+
let candidate = entry.path().join("unified/extractor/tests/corpus");
112+
if candidate.is_dir() {
113+
return candidate;
114+
}
115+
}
116+
panic!("no `unified/extractor/tests/corpus` under TEST_SRCDIR {srcdir:?}");
117+
}
118+
113119
#[test]
114120
fn test_corpus() {
115-
if !parser_available() {
116-
eprintln!(
117-
"skipping test_corpus: the swift-syntax parser is unavailable \
118-
(set CODEQL_EXTRACTOR_UNIFIED_SWIFT_SYNTAX_PARSE or put \
119-
`swift-syntax-parse` on PATH)"
120-
);
121-
return;
122-
}
123121
let update_mode = update_mode_enabled();
124122
let all_languages = languages::all_language_specs();
125-
let corpus_dir = Path::new("tests/corpus");
123+
let corpus_dir = corpus_dir();
124+
let mut tested = 0usize;
126125

127126
for lang in all_languages {
128127
let output_schema = yeast::node_types_yaml::schema_from_yaml(languages::OUTPUT_AST_SCHEMA)
@@ -139,6 +138,7 @@ fn test_corpus() {
139138
stems.dedup();
140139

141140
for stem in stems {
141+
tested += 1;
142142
let swift_path = stem.with_extension("swift");
143143
let output_path = stem.with_extension("output");
144144
let mut failures = Vec::new();
@@ -265,4 +265,13 @@ fn test_corpus() {
265265
}
266266
}
267267
}
268+
269+
// Every language whose corpus directory is missing is skipped silently
270+
// above, which is right when a language simply has no corpus — but if that
271+
// leaves nothing at all to check, the run is vacuous and must not pass.
272+
assert!(
273+
tested > 0,
274+
"no corpus cases found under {}; the suite would have passed vacuously",
275+
corpus_dir.display()
276+
);
268277
}

unified/extractor/tree-sitter-swift/BUILD.bazel

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,8 @@ rust_library(
3939
),
4040
)
4141

42-
exports_files(["Cargo.toml"])
42+
exports_files([
43+
"Cargo.toml",
44+
# The schema `//unified/extractor:rules_macro_smoke` type-checks rules against.
45+
"node-types.yml",
46+
])

0 commit comments

Comments
 (0)