Honour #[mutants::skip] on block expressions#618
Conversation
The visitor checks for `#[mutants::skip]` at most scopes — fn, impl,
trait, mod, file, and several expression kinds (call, method-call,
match, struct literal, unary). Plain block expressions `{ ... }` were
omitted, so writing `#[mutants::skip] { ... }` inside a function body
silently mutated the contents anyway.
Add a `visit_expr_block` override that short-circuits when
`attrs_excluded` matches, mirroring the existing per-expression
handlers. This works in both statement position
(`#[mutants::skip] { ... }`) and expression position
(`let x = #[mutants::skip] { ... };`), as well as on labeled blocks,
because syn attaches the outer attribute to `ExprBlock.attrs` in all
three forms.
New unit tests under `src/visit/test/skip_attr_expr_block.rs` cover
each of those forms and verify that nested mutants of every supported
genre (binary, unary, match arms, match guards) are suppressed inside
the annotated block, while sibling code outside the block is still
mutated.
Documentation updated:
- `book/src/attrs.md` lists block expressions in the Scope section.
- `NEWS.md` adds an Unreleased entry under the existing section.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds testdata/skip_attr_block tree exercising the new visit_expr_block override against real cargo. Asserts the expected 22 mutants are produced and all pass cargo check. Stable Rust gates custom proc-macro attributes on expression positions behind the unstable stmt_expr_attributes and proc_macro_hygiene features, so the tree uses the cfg_attr(mutants, mutants::skip) wrapping form (same trick as testdata/cfg_attr_mutants_skip). Cargo never sets cfg(mutants) during normal builds, so the inner mutants::skip is parsed by cargo-mutants but never expanded by rustc. Updates book/src/attrs.md to document the stable Rust caveat and the cfg_attr workaround. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cargo-mutants's attr_is_mutants_skip walked the cfg_attr contents via
syn's parse_nested_meta. The callback only consumed plain idents, so
function-style cfg predicates like any(), all(...), and not(...) caused
parse_nested_meta to error out with `expected `,``, silently dropping
the mutants::skip suppression. `name = "value"` cfg forms had the same
problem.
Extend the callback to consume:
* function-style predicates (any(...), all(...), not(...)) by parsing
and discarding the parenthesised contents
* name = value predicates by parsing and discarding the value
This unblocks `#[cfg_attr(any(), mutants::skip)]` as the recommended
stable-Rust workaround for skipping expression-position mutants:
* `any()` is built into rustc and evaluates to false at compile time,
so the inner attribute is never expanded
* unlike a made-up cfg name (`mutants`, `never`, etc.) it does not
trigger the `unexpected_cfgs` lint
Switches the testdata/skip_attr_block tree and the book docs from the
made-up `cfg_attr(mutants, ...)` form to `cfg_attr(any(), ...)`.
Adds unit tests for any(), not(all()), and `name = "value"` predicates
in src/visit/test/skip_attr_cfg_attr.rs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Side-finding: the |
- Promote the workaround note in the attrs book page from a "caveat"
callout to a top-level "Hiding the attribute from rustc with
cfg_attr(any(), ...)" section, explaining:
* Two use cases: stable-Rust expression-position skips, and avoiding
a dependency on the `mutants` crate altogether.
* Why `any()` works (built-in, false with no operands) and avoids
the `unexpected_cfgs` lint.
* That the `mutants` crate dependency is not needed when only the
`cfg_attr(any(), ...)` form is used, because rustc never expands
the inner attribute.
- Align the SUMMARY (TOC) entry to the actual page title:
"Skipping mutations with an attribute" rather than "Skipping
functions with an attribute".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This is really clever but I would also like to keep the path open to eventually actually understanding I have some qualms about encouraging people to rely on them always evaluating to true. If we took that out of this PR, could we still support |
Custom proc-macro attributes on expressions (including block expressions)
are gated behind unstable rustc features (stmt_expr_attributes and
proc_macro_hygiene), so the direct `#[mutants::skip] { ... }` form does
not compile on stable Rust. Be honest about this in the docs and add a
real integration test that exercises the feature on nightly.
Introduce a new `mutants_nightly` custom cfg as the opt-in for
integration tests that depend on nightly-only Rust syntax:
* Register the cfg in the top-level Cargo.toml's [lints.rust] so the
`unexpected_cfgs` lint stays clean whether or not it is set.
* Add testdata/skip_attr_block/, which uses the direct
`#[mutants::skip]` form and gates the required feature attributes
on `#![cfg_attr(mutants_nightly, feature(...))]`.
* Add the integration test
`check_tree_with_skip_attr_on_block_expressions` in tests/main.rs.
It is `#[ignore]`d unless `mutants_nightly` is set, and forwards
the cfg to the cargo-mutants subprocess (and therefore to the
testdata's `cargo check --tests`) via RUSTFLAGS.
* Document the convention in AGENTS.md so future nightly-only tests
follow the same pattern.
Documentation:
* book/src/attrs.md: state honestly that expression-level
`#[mutants::skip]` is currently nightly-only. Soften the
pre-existing note about cargo-mutants ignoring the cfg_attr condition
with a brief "This may change in future versions."
* book/src/SUMMARY.md: align the TOC entry "Skipping mutations with an
attribute" with the actual page title.
* NEWS.md: mention the nightly requirement next to the block-expression
entry.
Implementation:
* src/visit.rs: extend `attr_is_mutants_skip` to consume function-style
cfg predicates (`any(...)`, `all(...)`, `not(...)`) and
`name = "value"` predicates inside `cfg_attr`. Previously these
caused parse_nested_meta to error out and silently drop the
`mutants::skip` directive. The doc comment notes that this is an
implementation detail and not a public guarantee.
* src/visit/test/skip_attr_cfg_attr.rs: add unit tests pinning the
behavior for all three predicate shapes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # NEWS.md
|
OK, removed the stable Rust workaround. You are right - the forward compatibility of this workaround was not so great. For nightly this workaround had no relevance - as long as the required features are enabled, the code will work. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ssions The block-expression skip is end-to-end covered by the skip_attr_block testdata tree and check_tree_with_skip_attr_on_block_expressions; the remaining expression genres (call, method call, match, struct literal, unary) were only covered by parser-only unit tests in src/visit/test/skip_attr_expr_*.rs. Add a skip_attr_expressions testdata tree that pairs an annotated expression of each remaining genre with an un-annotated sibling of the same genre in the same function, plus the matching cargo-mutants --check integration test check_tree_with_skip_attr_on_expressions. Like the block test, both the tree and the test are nightly-only via the mutants_nightly cfg (stmt_expr_attributes + proc_macro_hygiene are required to attach a custom attribute to an expression on rustc). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds missing #[mutants::skip] support for block expressions ({ ... }) during mutant discovery, improves robustness of cfg_attr(..., mutants::skip) parsing, and extends both unit and end-to-end integration coverage (including a documented mutants_nightly opt-in for nightly-only syntax).
Changes:
- Honor
#[mutants::skip]onsyn::ExprBlockby short-circuiting discovery for annotated blocks. - Make
attr_is_mutants_skiptolerant of function-style andname = "value"cfg predicates socfg_attrparsing doesn’t silently dropmutants::skip. - Add unit tests + nightly-gated integration testdata and documentation updates describing the nightly requirement and test convention.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/visit.rs |
Adds visit_expr_block exclusion handling; improves cfg_attr predicate parsing in attr_is_mutants_skip. |
src/visit/test/skip_attr_expr_block.rs |
New unit tests ensuring block-expression skip suppresses nested mutants across genres. |
src/visit/test/skip_attr_cfg_attr.rs |
Adds tests for robust cfg_attr predicate-shape handling. |
tests/main.rs |
Adds nightly-gated end-to-end integration tests validating skip behavior via --check and expected mutant listings. |
testdata/skip_attr_block/src/lib.rs |
New integration test tree exercising #[mutants::skip] on blocks in multiple positions. |
testdata/skip_attr_block/README.md |
Documents the block-skip integration tree and nightly requirement. |
testdata/skip_attr_block/Cargo_test.toml |
Registers mutants_nightly as expected cfg in testdata and depends on mutants. |
testdata/skip_attr_expressions/src/lib.rs |
New integration tree covering already-supported expression-level skip cases end-to-end. |
testdata/skip_attr_expressions/README.md |
Documents the expression-skip integration tree and nightly requirement. |
testdata/skip_attr_expressions/Cargo_test.toml |
Registers mutants_nightly as expected cfg in testdata and depends on mutants. |
Cargo.toml |
Registers mutants_nightly under unexpected_cfgs to keep linting clean when absent. |
book/src/attrs.md |
Updates scope list to include block expressions; documents nightly requirement for expression-level attributes. |
book/src/SUMMARY.md |
Aligns TOC entry name with the page title. |
AGENTS.md |
Documents the mutants_nightly convention for nightly-only integration tests. |
NEWS.md |
Notes the new block-expression skip support and the nightly requirement for expression-level usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The two integration tests added in this branch (check_tree_with_skip_attr_on_block_expressions and check_tree_with_skip_attr_on_expressions) test the same mutation-generator behavior that's already covered by the unit tests in src/visit/test/skip_attr_expr_block.rs and skip_attr_cfg_attr.rs. The orthogonal concerns they appeared to add value for (mutated source compiles, exact stdout/JSON output, line+column spans) are already exercised by many other integration tests in the suite. The one genuinely novel concern (inner #![cfg_attr(..., feature(...))] preservation through mutation rewriting) is not skip-attribute-specific and is better left as a manual smoke test on testdata/nightly_only, matching the existing project convention documented there. Additionally, CI never sets --cfg=mutants_nightly, so both tests were always ignored in CI runs and protected against nothing in practice. Removes: - Both integration tests from tests/main.rs - testdata/skip_attr_block/ and testdata/skip_attr_expressions/ trees - The mutants_nightly cfg registration in Cargo.toml - The Nightly-only integration tests section in AGENTS.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The skip-attr work taught attr_is_mutants_skip to consume function-style cfg predicates (any(), not(...)) and name = value forms so a mutants::skip after them is still recognised. The parallel exclude_re cfg_attr parser lacked the same handling, so an exclude_re following such a predicate was silently dropped, contradicting its own doc claim of parity with attr_is_mutants_skip. Mirror the predicate-consuming logic in collect_exclude_re_from_cfg_attr and cover it with tests. Also document that exclude_re is honoured on block expressions, which it now is via visit_expr_block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
I see a mutant failure in the GitHub checks but I am unable to reproduce that locally. Maybe a spurious timeout? |
Honor
#[mutants::skip]on plain block expressions{ ... }inside function bodies.The visitor already checks
#[mutants::skip]at most scopes — fn, impl, trait, mod, file, and several expression kinds (call, method-call, match, struct literal, unary). Block expressions were omitted, so writing#[mutants::skip] { ... }inside a function body silently mutated the contents anyway.This PR adds a
visit_expr_blockoverride that short-circuits whenattrs_excludedmatches, mirroring the existing per-expression handlers. It works in:#[mutants::skip] { ... }let x = #[mutants::skip] { ... };#[mutants::skip] 'outer: { ... }…because
synattaches the outer attribute toExprBlock.attrsin all three forms.Nightly requirement on expressions
Custom proc-macro attributes on expressions are gated behind the unstable
stmt_expr_attributesandproc_macro_hygienefeatures, so the direct form#[mutants::skip] { ... }only compiles on nightly. This is the case for the other expression-level scopes already supported (call, method-call, match, struct literal, unary) too; the PR makes this explicit in the book.Robustness fix in
attr_is_mutants_skipExtend
attr_is_mutants_skipto consume function-style cfg predicates (any(...),all(...),not(...)) andname = "value"predicates insidecfg_attr. Without this,parse_nested_metaerrors out on those shapes and silently drops themutants::skipdirective, so e.g.#[cfg_attr(any(target_os = "linux", target_os = "macos"), mutants::skip)]would not be recognised as a skip. The condition is still ignored but now themutants::skipis respected.Tests
src/visit/test/skip_attr_expr_block.rs— covers all four block forms (statement-position, expression-position, all-genres-within, labeled) and verifies that nested mutants of every supported genre (binary, unary, match arms, match guards) are suppressed inside an annotated block while sibling code outside it is still mutated.src/visit/test/skip_attr_cfg_attr.rs— three new tests pinning the function-style, nested andname = "value"cfg-predicate handling inattr_is_mutants_skip.Both use
mutate_source_str, which parses throughsynonly, so they work on stable and run in normal CI.Docs
book/src/attrs.md: block expressions added to the Scope list, with a note that expression-level#[mutants::skip]requires nightly. The existing "cargo-mutants does not evaluate thecfg_attrcondition" note gets a "This may change in future versions." sentence.book/src/SUMMARY.md: TOC entry aligned to the actual page title.NEWS.md: Unreleased entry mentions block-expression support and the nightly requirement.Verification
cargo fmt --check— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo test --workspace --all-features— passes on stable🤖 Generated with the help of GitHub Copilot CLI.