Skip to content

Commit 7b3c872

Browse files
jketemajketema
authored
Merge pull request #22290 from github/jketema/unified-build
Unified: Drop the separate swift-syntax binary and make tests runnable both with Bazel and cargo
2 parents 731feb9 + c2d9383 commit 7b3c872

12 files changed

Lines changed: 143 additions & 228 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/BUILD.bazel‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,9 @@ codeql_pkg_files(
4343
name = "extractor-arch",
4444
exes = [
4545
"//unified/extractor",
46-
],
47-
prefix = "tools/{CODEQL_PLATFORM}",
48-
)
49-
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(
46+
] + select_os(
47+
linux = ["//unified/swift-syntax-rs:swift_runtime_libs"],
5748
otherwise = [],
58-
posix = ["//unified/swift-syntax-rs:swift-syntax-parse-pkg"],
5949
),
6050
prefix = "tools/{CODEQL_PLATFORM}",
6151
)
@@ -66,7 +56,6 @@ codeql_pack(
6656
":codeql-extractor-yml",
6757
":dbscheme-group",
6858
":extractor-arch",
69-
":swift-syntax-parse-arch",
7059
"//unified/tools",
7160
],
7261
)

‎unified/extractor/BUILD.bazel‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
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")
4+
load("//unified:platforms.bzl", "UNIFIED_SUPPORTED_PLATFORMS")
35

46
exports_files(["Cargo.toml"])
57

@@ -11,14 +13,71 @@ codeql_rust_binary(
1113
"ast_types.yml",
1214
"swift_node_types.yml",
1315
],
16+
data = select({
17+
"@platforms//os:linux": ["//unified/swift-syntax-rs:swift_runtime_libs"],
18+
"//conditions:default": [],
19+
}),
1420
proc_macro_deps = all_crate_deps(
1521
proc_macro = True,
1622
),
23+
target_compatible_with = UNIFIED_SUPPORTED_PLATFORMS,
1724
visibility = ["//visibility:public"],
1825
deps = all_crate_deps(
1926
normal = True,
2027
) + [
2128
"//shared/tree-sitter-extractor",
2229
"//shared/yeast",
30+
"//unified/swift-syntax-rs:swift_syntax_rs",
2331
],
2432
)
33+
34+
_TESTS = {
35+
"corpus_tests": {
36+
"data": glob(["tests/corpus/**"]),
37+
"compile_data": [],
38+
"size": "medium",
39+
},
40+
# `include_str!`s a checked-in `parse_to_json` dump.
41+
"swift_syntax_pipeline": {
42+
"data": [],
43+
"compile_data": glob(["tests/fixtures/**"]),
44+
"size": "small",
45+
},
46+
}
47+
48+
[
49+
rust_test(
50+
name = test_name,
51+
size = spec["size"],
52+
srcs = ["tests/%s.rs" % test_name] + glob(["src/**/*.rs"]),
53+
aliases = aliases(),
54+
compile_data = [
55+
"ast_types.yml",
56+
"swift_node_types.yml",
57+
] + spec["compile_data"],
58+
crate_root = "tests/%s.rs" % test_name,
59+
data = spec["data"] + select({
60+
"@platforms//os:linux": ["//unified/swift-syntax-rs:swift_runtime_libs"],
61+
"//conditions:default": [],
62+
}),
63+
edition = "2024",
64+
proc_macro_deps = all_crate_deps(
65+
proc_macro = True,
66+
),
67+
rustc_flags = ["--cfg=bazel"],
68+
target_compatible_with = UNIFIED_SUPPORTED_PLATFORMS,
69+
deps = all_crate_deps(
70+
normal = True,
71+
) + [
72+
"//shared/tree-sitter-extractor",
73+
"//shared/yeast",
74+
"//unified/swift-syntax-rs:swift_syntax_rs",
75+
],
76+
)
77+
for test_name, spec in _TESTS.items()
78+
]
79+
80+
test_suite(
81+
name = "all_tests",
82+
tests = [":%s" % test_name for test_name in _TESTS],
83+
)

‎unified/extractor/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,4 @@ serde_json = "1.0.145"
1818

1919
codeql-extractor = { path = "../../shared/tree-sitter-extractor" }
2020
yeast = { path = "../../shared/yeast" }
21+
swift-syntax-rs = { path = "../swift-syntax-rs" }

‎unified/extractor/build.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
fn main() {
2+
println!("cargo:rustc-check-cfg=cfg(bazel)");
3+
4+
if let Ok(dir) = std::env::var("DEP_SWIFTSYNTAXFFI_LIBDIR") {
5+
println!("cargo:rustc-link-search=native={dir}");
6+
println!("cargo:rustc-link-lib=dylib=SwiftSyntaxFFI");
7+
println!("cargo:rustc-link-arg=-Wl,-rpath,{dir}");
8+
}
9+
if let Ok(dir) = std::env::var("DEP_SWIFTSYNTAXFFI_RUNTIMEDIR") {
10+
println!("cargo:rustc-link-arg=-Wl,-rpath,{dir}");
11+
}
12+
}

‎unified/extractor/src/languages/swift/parse.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: 19 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,29 @@ fn collect_corpus_stems(dir: &Path, out: &mut Vec<std::path::PathBuf>) {
11096
}
11197
}
11298

99+
#[cfg(bazel)]
100+
fn corpus_dir() -> std::path::PathBuf {
101+
let base = std::path::PathBuf::from(
102+
std::env::var("RUNFILES_DIR").expect("RUNFILES_DIR not set"),
103+
);
104+
std::fs::read_dir(&base)
105+
.expect("failed to read RUNFILES_DIR")
106+
.filter_map(Result::ok)
107+
.map(|entry| entry.path().join("unified/extractor/tests/corpus"))
108+
.find(|path| path.exists())
109+
.expect("corpus not found under any runfiles repo root")
110+
}
111+
112+
#[cfg(not(bazel))]
113+
fn corpus_dir() -> std::path::PathBuf {
114+
std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/corpus")
115+
}
116+
113117
#[test]
114118
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-
}
123119
let update_mode = update_mode_enabled();
124120
let all_languages = languages::all_language_specs();
125-
let corpus_dir = Path::new("tests/corpus");
121+
let corpus_dir = corpus_dir();
126122

127123
for lang in all_languages {
128124
let output_schema = yeast::node_types_yaml::schema_from_yaml(languages::OUTPUT_AST_SCHEMA)

‎unified/platforms.bzl‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
"""Shared platform constraint for the unified extractor."""
2+
3+
# swift-syntax requires a Swift toolchain, which is only available
4+
# through rules_swift.
5+
UNIFIED_SUPPORTED_PLATFORMS = select({
6+
"@platforms//os:linux": [],
7+
"@platforms//os:macos": [],
8+
"//conditions:default": ["@platforms//:incompatible"],
9+
})

0 commit comments

Comments
 (0)