From ac3f8d7bad70d1e3366b3d9ce40e274d6afee352 Mon Sep 17 00:00:00 2001 From: Elijah Zupancic Date: Sun, 13 Sep 2026 15:40:32 -0700 Subject: [PATCH] test(features): build tests/api under single-language feature sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sweeping all 22 single-language subsets of `-p big-code-analysis --all-targets` under `clippy -D warnings` found five failing legs, not the two the issue reported, from three distinct defects — every one of them a hand-maintained `cfg` union in `tests/api/ast_seam_test.rs` that had drifted from the items it gates: c `PathBuf` / `MetricsOptions` gated on `rust`/`python`/`cpp`, used by a `c`-gated test: the imports vanish and the test cannot resolve them (E0433). cpp, python `ops_tree_contains_operator` gated on three languages with one `rust`-gated caller: it builds unused (`dead_code`). tcl, irules the two-row Tcl-family table in `count_string_agrees_with_find_on_tcl_family_bodies` has cfg-gated rows, so enabling one of the pair leaves a singleton (`clippy::single_element_loop`). `LANG` / `Source` carried the same `c`-shaped drift as `PathBuf`, and the sweep reported that leg green anyway: its union ended in `not(feature = "javascript")`, which is true in every single-language build, so the union was satisfied without any language arm and none of them was load-bearing. The arm was load-bearing for a real reason — the two `LanguageDisabled` tests need these names with no language enabled at all — so it is not simply dropped: those two tests import the names themselves, and the shared union becomes the bare language list. That is what makes each language's own leg able to see a dropped arm. The unions are corrected; the helper narrows to its sole caller's gate; the table becomes two `#[cfg]` blocks calling a shared `check`, matching the sibling test above it and correct at one row or two. The `#[cfg(any(tcl, irules))]` gate on the `fn` and the `ran > 0` non-vacuity assertion both stay, per `.claude/rules/testing.md`. CI could not see any of it: `feature-matrix` built only `default`, `no-default-features` and `minimal-langs`, and `rust` is in the last of those, so nearly every union was satisfied by accident. The job now runs `cargo clippy --all-targets -- -D warnings` instead of `cargo check`, which reaches the `single_element_loop` class `check` does not run at all (the other two are already hard errors under this workflow's top-level `RUSTFLAGS: -D warnings`). It gains single-language legs `c`, `go`, `python` and `tcl` for the library and `go` for the parse layer. All ten pre-existing legs were verified against the stricter command first. The library itself always built, so no published configuration was affected. Tests that compile under a partial feature set and panic at run time are a separate class, still open as #1285 / #1413; the `c` leg has 24 of them in `suppression_test.rs` and `derive_eq_hash_ord.rs`, untouched here. Verified by re-running the same 22-leg sweep after the fix: 22 pass, 0 fail, against 17/5 before. `big-code-analysis-ast` was clean on all 22 both times. Each corrected union was confirmed load-bearing by perturbation, and the `LANG` / `Source` restructure by the pair that motivates it: with the old union, dropping `feature = "c"` still compiles under `--features c` (exit 0 — the leg is blind); with the bare language list the same drift is an `E0433`. Fixes #1426 --- .github/workflows/ci.yml | 56 +++++++++++++++++++++++++++++++++++--- CHANGELOG.md | 32 ++++++++++++++++++++++ tests/api/ast_seam_test.rs | 54 +++++++++++++++++++++++++++--------- 3 files changed, 125 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd6ec1a04..1ad677d65 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -437,6 +437,43 @@ jobs: flags: -p big-code-analysis-web - name: no-default-features (web) flags: --no-default-features -p big-code-analysis-web + # Single-language legs (issue #1426). `minimal-langs` above is + # the only subset CI used to build, and `rust` is in it, so a + # test item gated on a union of languages was measured against + # a configuration that satisfied nearly every union by + # accident. Three of these four failed on `main` when the leg + # was written, each for a different reason, and each reason is + # invisible to every other leg in this matrix: + # + # c a union too *narrow* — `PathBuf` / `MetricsOptions` + # were gated on `rust`/`python`/`cpp` while a `c`-gated + # test used them, so the imports vanished (E0433). + # python a union too *wide* — a helper gated on three + # languages had one `rust`-gated caller, so it built + # unused here (`dead_code`). + # tcl a two-row fixture table whose rows are cfg-gated + # collapses to one row (`clippy::single_element_loop`). + # + # `go` is the fourth because `.claude/rules/testing.md` names + # `--features go` as the subset contributors run to verify a + # feature-gated fixture table: it is the nearest thing to a + # language outside every union, enabling none of the five + # (`rust`, `python`, `c`, `cpp`, `tcl`/`irules`) that + # `tests/api/ast_seam_test.rs` gates its shared items on. + - name: lang-c (lib) + flags: --no-default-features --features c -p big-code-analysis + - name: lang-go (lib) + flags: --no-default-features --features go -p big-code-analysis + - name: lang-python (lib) + flags: --no-default-features --features python -p big-code-analysis + - name: lang-tcl (lib) + flags: --no-default-features --features tcl -p big-code-analysis + # The ast twin, mirroring the lib legs above. The parse layer + # is clean on all 22 single-language subsets today; this keeps + # it that way, in the crate where the per-language wiring is + # densest. + - name: lang-go (ast) + flags: --no-default-features --features go -p big-code-analysis-ast steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -445,11 +482,22 @@ jobs: - uses: dtolnay/rust-toolchain@6c977a6ca4077a0ceb28ffbe03f59d46e9ac8772 # stable tip with: toolchain: stable + components: clippy - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - # `cargo check --all-targets` exercises bins, tests, and examples, - # so a cfg-gated test file that only compiles under default features - # does not slip through the no-default-features matrix leg. - - run: cargo check --all-targets ${{ matrix.flags }} --locked + # `--all-targets` exercises bins, tests, and examples, so a + # cfg-gated test file that only compiles under default features does + # not slip through a partial leg. + # + # `clippy … -- -D warnings` rather than `cargo check` (#1426): of + # the three ways a partial feature set broke the test crate, the + # third is out of `check`'s reach entirely. E0433 is a hard error + # either way and `dead_code` is a rustc lint this workflow's + # top-level `RUSTFLAGS: -D warnings` already promotes, but + # `single_element_loop` is a clippy lint `check` never runs at all. + # The `-- -D warnings` is belt-and-braces against that env var + # being narrowed later. All ten pre-existing legs were verified + # against the stricter command before it landed. + - run: cargo clippy --all-targets ${{ matrix.flags }} --locked -- -D warnings deny: name: cargo-deny diff --git a/CHANGELOG.md b/CHANGELOG.md index 15960d5b3..016a55190 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,6 +135,38 @@ for historical reference. ### Fixed +- **Single-language feature subsets build their tests again, and CI + gates them** (#1426). Five of the twenty-two single-language subsets + of `-p big-code-analysis --all-targets` failed, all in + `tests/api/ast_seam_test.rs` and all from a hand-maintained `cfg` + union that had drifted from the items it gates: `PathBuf` / + `MetricsOptions` were gated too *narrowly* to cover a `c`-gated test + that uses them (`--features c`, E0433), as were `LANG` / `Source`, + whose union reached that same test only through its + `not(feature = "javascript")` arm (`--features c,javascript`); an + `Ops` helper was gated too *widely* for its one `rust`-gated caller, + so it built unused (`--features cpp`, `--features python`, + `dead_code`); and a two-row Tcl-family fixture table collapsed to one + row when only one of the pair was enabled (`--features tcl`, + `--features irules`, `clippy::single_element_loop`). The table is now + two `#[cfg]` blocks calling a shared helper, matching its sibling test + and correct at one row or two. The library itself always built, so no + released configuration was affected — this was a contributor-facing + break, and a costly one, because `--features go` is the subset + `.claude/rules/testing.md` tells contributors to run and the compile + error landed in a file unrelated to their change. The `feature-matrix` + CI job now runs `cargo clippy --all-targets -- -D warnings` instead of + `cargo check` — `check` cannot see the `clippy::single_element_loop` + class at all — and adds single-language legs (`c`, `go`, `python`, + `tcl` for the library, `go` for the parse layer). For those legs to + discriminate, the `LANG` / `Source` union also had to drop its + trailing `not(feature = "javascript")` arm — true in every + single-language build, so it satisfied the union by itself and left + the language arms unexercised, which is how `c` went missing from + them. The two `LanguageDisabled` tests that needed the names with no + language enabled now import them directly. Tests that *compile* + under a partial feature set and panic at run time remain out of scope + and tracked in #1285 / #1413. - **A Bash heredoc no longer bills its command-line prefix as part of the literal** (#1443). `heredoc_redirect` spans the prefix as well as the heredoc, and the grammar lets that prefix cross rows — a diff --git a/tests/api/ast_seam_test.rs b/tests/api/ast_seam_test.rs index a5a43c371..80664c23f 100644 --- a/tests/api/ast_seam_test.rs +++ b/tests/api/ast_seam_test.rs @@ -22,7 +22,7 @@ #![allow(clippy::float_cmp)] -#[cfg(any(feature = "rust", feature = "cpp"))] +#[cfg(any(feature = "rust", feature = "c", feature = "cpp"))] use std::path::PathBuf; use big_code_analysis::Ast; @@ -30,17 +30,26 @@ use big_code_analysis::Ast; use big_code_analysis::Metric; #[cfg(not(feature = "javascript"))] use big_code_analysis::MetricsError; -#[cfg(any(feature = "rust", feature = "python", feature = "cpp"))] +#[cfg(any(feature = "rust", feature = "python", feature = "c", feature = "cpp"))] use big_code_analysis::MetricsOptions; #[cfg(feature = "rust")] use big_code_analysis::SpaceKind; +// Deliberately the bare language list, with no `not(feature = +// "javascript")` arm (#1426). The two `LanguageDisabled` tests below also +// need these names, and with no language enabled at all, so the union used +// to carry that arm too — but it is true in every *single*-language build, +// which satisfied the union on its own and left the language arms below +// never load-bearing. `feature = "c"` had silently gone missing from them +// for exactly that reason. Those two tests now import the names +// themselves, so a dropped arm here is a hard error on that language's own +// feature-matrix leg. #[cfg(any( feature = "rust", feature = "python", + feature = "c", feature = "cpp", feature = "tcl", - feature = "irules", - not(feature = "javascript") + feature = "irules" ))] use big_code_analysis::{LANG, Source}; @@ -335,6 +344,11 @@ fn as_tree_sitter_walks_held_source() { #[cfg(not(feature = "javascript"))] #[test] fn ast_parse_returns_language_disabled_for_off_feature() { + // Imported here rather than taken from the shared `use` above: this + // test runs with no language feature enabled at all, which that + // import's union deliberately no longer covers. + use big_code_analysis::{LANG, Source}; + let err = Ast::parse(Source::new(LANG::Javascript, b"")).unwrap_err(); assert!(matches!( err, @@ -345,6 +359,8 @@ fn ast_parse_returns_language_disabled_for_off_feature() { #[cfg(all(feature = "rust", not(feature = "javascript")))] #[test] fn ast_from_tree_sitter_returns_language_disabled_for_off_feature() { + use big_code_analysis::LANG; + // The dispatch arm rejects the disabled language *before* touching // the tree, so it is fine to hand it a tree built from an enabled // grammar (Rust here). This exercises the `Err(LanguageDisabled)` @@ -443,7 +459,7 @@ fn language_and_name_accessors_match_constructors() { // Does `op` appear as an operator anywhere in the `Ops` tree? Operators // live on whichever space owns them, so a fixture's operator may sit in a // nested function space rather than the file-level `Ops`. -#[cfg(any(feature = "rust", feature = "python", feature = "cpp"))] +#[cfg(feature = "rust")] fn ops_tree_contains_operator(ops: &big_code_analysis::Ops, op: &str) -> bool { ops.operators.iter().any(|o| o == op) || ops.spaces.iter().any(|s| ops_tree_contains_operator(s, op)) @@ -831,17 +847,17 @@ fn find_string_reports_tcl_family_literals_and_not_script_bodies() { /// /// The two walks are separate functions over one predicate list, and only /// one of them is exercised above. +/// +/// The per-language calls are `#[cfg]` blocks rather than rows of a table +/// the test loops over, matching the sibling above. A cfg-gated row list +/// reduces to a single element whenever exactly one of the two features +/// is enabled, which `clippy::single_element_loop` denies under the +/// workspace's `-D warnings` — so the loop shape compiled only in builds +/// that happened to enable both (#1426). #[cfg(any(feature = "tcl", feature = "irules"))] #[test] fn count_string_agrees_with_find_on_tcl_family_bodies() { - let mut ran = 0; - for (lang, code) in [ - #[cfg(feature = "tcl")] - (LANG::Tcl, TCL_SCRIPT_AND_LITERALS), - #[cfg(feature = "irules")] - (LANG::Irules, IRULES_SCRIPT_AND_LITERALS), - ] { - ran += 1; + fn check(lang: LANG, code: &str) { let found = strings_found(lang, code).len(); assert_eq!(found, 2, "{lang:?}: fixture must report both literals"); let (matching, total) = Ast::parse(Source::new(lang, code.as_bytes())) @@ -858,6 +874,18 @@ fn count_string_agrees_with_find_on_tcl_family_bodies() { contain a script body" ); } + + let mut ran = 0; + #[cfg(feature = "tcl")] + { + ran += 1; + check(LANG::Tcl, TCL_SCRIPT_AND_LITERALS); + } + #[cfg(feature = "irules")] + { + ran += 1; + check(LANG::Irules, IRULES_SCRIPT_AND_LITERALS); + } assert!( ran > 0, "neither tcl nor irules is enabled; this test asserted nothing"