diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd6ec1a0..1ad677d6 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 15960d5b..016a5519 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 a5a43c37..80664c23 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"