Honour #[mutants::skip] on block expressions by sandersaares · Pull Request #618 · sourcefrog/cargo-mutants

sandersaares · 2026-05-28T02:27:22Z

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_block override that short-circuits when attrs_excluded matches, mirroring the existing per-expression handlers. It works in:

statement position: #[mutants::skip] { ... }
expression position: let x = #[mutants::skip] { ... };
labeled blocks: #[mutants::skip] 'outer: { ... }

…because syn attaches the outer attribute to ExprBlock.attrs in all three forms.

Nightly requirement on expressions

Custom proc-macro attributes on expressions are gated behind the unstable stmt_expr_attributes and proc_macro_hygiene features, 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_skip`

Extend attr_is_mutants_skip to consume function-style cfg predicates (any(...), all(...), not(...)) and name = "value" predicates inside cfg_attr. Without this, parse_nested_meta errors out on those shapes and silently drops the mutants::skip directive, 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 the mutants::skip is 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 and name = "value" cfg-predicate handling in attr_is_mutants_skip.

Both use mutate_source_str, which parses through syn only, 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 the cfg_attr condition" 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 — clean
cargo clippy --all-targets --all-features -- -D warnings — clean
cargo test --workspace --all-features — passes on stable

🤖 Generated with the help of GitHub Copilot CLI.

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>

sandersaares · 2026-05-28T02:44:01Z

Side-finding: the #[cfg_attr(any(), mutants::skip)] form does not actually require even a dev-dependency on the mutants crate because the macro is never invoked at compile time. This might be an interesting opportunity for people who like to keep a low dependency count.

- 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>

sourcefrog · 2026-06-02T15:09:56Z

Stable Rust workaround: cfg_attr(any(), mutants::skip)

This is really clever but I would also like to keep the path open to eventually actually understanding cfg predicates so that we can understand whether to mutate OS-specific code, or feature-flag controlled code: #50.

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 mutants::skip on blocks on nightly Rust?

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

sandersaares · 2026-06-03T05:51:17Z

OK, removed the stable Rust workaround. You are right - the forward compatibility of this workaround was not so great. cfg condition parsing is indeed an obvious future direction of enhancement for cargo-mutants (every month I hear someone complain about it being missing, heh).

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>

Copilot

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] on syn::ExprBlock by short-circuiting discovery for annotated blocks.
Make attr_is_mutants_skip tolerant of function-style and name = "value" cfg predicates so cfg_attr parsing doesn’t silently drop mutants::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.

sourcefrog · 2026-06-06T16:33:35Z

+
+```bash
+cargo +nightly nextest run --all-features \
+    --config 'build.rustflags=["--cfg=mutants_nightly"]' \


Thanks for adding that.

I wonder if it would be possible to make this a feature instead, so that people don't need the verbose argument to turn it on? Or, are you relying on the buildflags to make this visible to the inner compilations somehow?

How about if we:

Only have unit tests

The unit tests are conditional of an x_nightly_tests feature.

It doesn't matter if the code inside them will only parse on nightly

It would also be good to mention this in CONTRIBUTING.md for humans. AGENTS.md already asks them to read that file.

Once we work out an approach, could you please make github CI also set this for nightly test jobs?

Indeed, the build.rustflags are used here to enable the cfg attribute for the inner compilations, so the testdata only tries to "speak nightly" when this cfg flag is set. I suppose it might also just be a Cargo feature in the testdata, true - I went with cfg flag mostly just for realism (as in a real library one would not want to pollute feature list with such stuff).

However, the unit tests do not actually depend on the nightly toolchain as they just parse the input (in the sense that syn is happy to parse nightly-only features regardless of what toolchain is used). This means if we remove the integration tests and stick to only unit tests, we should be able to just get rid of this switch.

I suppose the risk here is "is some of the syn-valid syntax (in any of its possible future evolutions) too wild even for the nightly toolchain?". Do you judge the risk worthwhile to ignore - as long as unit tests are happy, we consider it good? I am a paranoid guy when it comes to trusting software to do the right thing but I defer to your judgement.

Will document in CONTRIBUTING.md (if it even makes sense to keep it - please let me know your thoughts on the risk-reward tradeoff). [I will just go ahead and remove them if I open my IDE before I see a reply, assuming you judge the risk trivial as seems likely]

PR updated to remove the integration tests, which also removed the need for mutants_nightly as the unit tests also work with stable toolchain.

sourcefrog · 2026-06-06T16:46:32Z

@@ -0,0 +1,76 @@
+//! Verify that `#[mutants::skip]` placed directly on a block expression


These look like basically the same tests as in the unit tests, but run as integration tests? I don't think we need the clutter (and test runtime cost). It seems probably unlikely that there would be bugs related to this that only show up when run through the CLI?

When I started this codebase I was originally writing mostly integration tests, so you see a lot of them, but I've later felt that integration tests are faster to run and sometimes easier to debug or to write precise matchers. See https://github.com/sourcefrog/cargo-mutants/blob/main/DESIGN.md#integration-and-unit-tests.

If there's nothing specifically unique to the testdata tree and integration test, let's drop it?

I see. OK, sure - I will drop the duplicative integration tests. I saw there were lots of integration tests in the repo and just tried to follow the pattern but I did not have any deeper meaning to adding these. Indeed, they are somewhat redundant, if perhaps a bit more realistic. But as you say, merely for something like #[mutants::skip] it is unlikely the integration tests can meaningfully deviate from the unit tests.

PR updated to remove the integration tests.

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>

sandersaares and others added 2 commits May 28, 2026 04:54

sandersaares marked this pull request as draft May 28, 2026 02:36

sandersaares marked this pull request as ready for review May 28, 2026 02:43

sourcefrog self-assigned this Jun 2, 2026

sandersaares and others added 2 commits June 3, 2026 08:43

Merge remote-tracking branch 'upstream/main' into skip-attr-block

fdf71d6

# Conflicts: # NEWS.md

sandersaares and others added 2 commits June 3, 2026 09:26

NEWS: refer to the mutants::skip attribute directly

e6777b6

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

sourcefrog requested review from Copilot and sourcefrog and removed request for sourcefrog June 6, 2026 16:39

Copilot started reviewing on behalf of sourcefrog June 6, 2026 16:39 View session

Copilot AI reviewed Jun 6, 2026

View reviewed changes

Comment thread testdata/skip_attr_block/README.md Outdated

sourcefrog reviewed Jun 6, 2026

View reviewed changes

sourcefrog mentioned this pull request Jun 6, 2026

Add #[mutants::exclude_re("pattern")] attribute #607

Open

sandersaares and others added 2 commits June 6, 2026 23:27

Potential fix for pull request finding

4a1394d

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Honour #[mutants::skip] on block expressions#618

Honour #[mutants::skip] on block expressions#618
sandersaares wants to merge 10 commits into
sourcefrog:mainfrom
sandersaares:skip-attr-block

sandersaares commented May 28, 2026 •

edited

Loading

Uh oh!

sandersaares commented May 28, 2026 •

edited

Loading

Uh oh!

sourcefrog commented Jun 2, 2026

Uh oh!

sandersaares commented Jun 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

sourcefrog Jun 6, 2026

Uh oh!

sandersaares Jun 6, 2026 •

edited

Loading

Uh oh!

sandersaares Jun 7, 2026

Uh oh!

sourcefrog Jun 6, 2026

Uh oh!

sandersaares Jun 6, 2026

Uh oh!

sandersaares Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,76 @@
		//! Verify that `#[mutants::skip]` placed directly on a block expression

Uh oh!

Conversation

sandersaares commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Nightly requirement on expressions

Robustness fix in attr_is_mutants_skip

Tests

Docs

Verification

Uh oh!

sandersaares commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sourcefrog commented Jun 2, 2026

Uh oh!

sandersaares commented Jun 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

sourcefrog Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

sandersaares Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandersaares Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

sourcefrog Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

sandersaares Jun 6, 2026

Choose a reason for hiding this comment

Uh oh!

sandersaares Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sandersaares commented May 28, 2026 •

edited

Loading

Robustness fix in `attr_is_mutants_skip`

sandersaares commented May 28, 2026 •

edited

Loading

sandersaares Jun 6, 2026 •

edited

Loading