diff --git a/NEWS.md b/NEWS.md index c2f956e2..5b4c53e0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Unreleased +- New: `#[mutants::skip]` is now honoured on block expressions (`{ ... }`), in both statement and expression position. All mutants generated inside the annotated block are suppressed. Note that the `#[mutants::skip]` attribute on expressions requires a nightly Rust toolchain (`stmt_expr_attributes` and `proc_macro_hygiene` feature gates). + - Fixed: Support the new TOML 1.1 syntax that's supported by Cargo in Rust 1.94. Thanks to @Coruscant11. - New: Mutate `NonZero` into `1`, and also `-1` when `T` is or may be signed. diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index f281855b..6d18a58d 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -9,7 +9,7 @@ - [Exit codes](exit-codes.md) - [The `mutants.out` directory](mutants-out.md) - [Skipping untestable code](skip.md) - - [Skipping functions with an attribute](attrs.md) + - [Skipping mutations with an attribute](attrs.md) - [Skipping function calls](skip_calls.md) - [Filtering files](skip_files.md) - [Filtering functions and mutants](filter_mutants.md) diff --git a/book/src/attrs.md b/book/src/attrs.md index 63a4fe1f..f75f604f 100644 --- a/book/src/attrs.md +++ b/book/src/attrs.md @@ -14,7 +14,7 @@ code. It only flags the item for cargo-mutants. **Note:** `cargo-mutants` does not evaluate the `cfg_attr` condition; the inner `mutants::skip` is always honoured regardless of whether the condition -would hold during compilation. +would hold during compilation. This may change in future versions. You may want to also add a comment explaining why the item is skipped. @@ -60,6 +60,10 @@ mod test { - **`mod` blocks** — applies to all items within the module. - **Files** (as an inner attribute `#![mutants::skip]`) — applies to the entire file. - **Expressions** that can syntactically carry an outer attribute, including - `match`, struct literal (`Foo { ... }`), call (`foo(...)`), method-call - (`x.foo(...)`), and unary expressions (`!x`, `-x`) — applies to the - expression and everything nested inside it. + block (`{ ... }`), `match`, struct literal (`Foo { ... }`), call + (`foo(...)`), method-call (`x.foo(...)`), and unary expressions (`!x`, + `-x`) — applies to the expression and everything nested inside it. + Note that the `#[mutants::skip]` macro on expressions requires the + unstable `stmt_expr_attributes` and `proc_macro_hygiene` features, so + expression-level `#[mutants::skip]` is currently only usable on a + nightly Rust toolchain. diff --git a/src/visit.rs b/src/visit.rs index 2dfdaec4..f0635311 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -751,6 +751,23 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor<'_> { syn::visit::visit_expr_struct(self, i); } + + /// Visit a block expression, e.g. `{ ... }` used as a statement or as the + /// right-hand side of `let x = { ... };`. + /// + /// An outer `#[mutants::skip]` attribute attached to the block (in either + /// position) suppresses mutants generated for every expression inside the + /// block. `Block` itself carries no attributes — they live on the enclosing + /// `ExprBlock`, which is what this handler inspects. + fn visit_expr_block(&mut self, i: &'ast syn::ExprBlock) { + let _span = trace_span!("expr_block", line = i.span().start().line).entered(); + trace!("visit block expression"); + if attrs_excluded(&i.attrs) { + trace!("block excluded by attrs"); + return; + } + syn::visit::visit_expr_block(self, i); + } } // Get the span of the block excluding the braces, or None if it is empty. @@ -917,7 +934,13 @@ fn path_ends_with(path: &syn::Path, ident: &str) -> bool { /// True if the attribute contains `mutants::skip`. /// -/// This for example returns true for `#[mutants::skip]` or `#[cfg_attr(test, mutants::skip)]`. +/// This for example returns true for `#[mutants::skip]` or for +/// `#[cfg_attr(, mutants::skip)]` regardless of what `` is — +/// cargo-mutants does not evaluate the cfg condition, so any predicate +/// shape (a plain ident like `test`, a function-style predicate like +/// `any()`/`not(...)`, or a `name = "value"` form) is treated the same. +/// This is an implementation detail, not a public guarantee; see +/// `book/src/attrs.md` for what we actually promise to users. fn attr_is_mutants_skip(attr: &Attribute) -> bool { if path_is(attr.path(), &["mutants", "skip"]) { return true; @@ -929,6 +952,16 @@ fn attr_is_mutants_skip(attr: &Attribute) -> bool { if let Err(err) = attr.parse_nested_meta(|meta| { if path_is(&meta.path, &["mutants", "skip"]) { skip = true; + } else if meta.input.peek(syn::token::Paren) { + // Function-style cfg predicate like `any(...)`, `all(...)`, `not(...)`. + // We don't evaluate the predicate; just consume and discard its + // contents so parse_nested_meta can advance to the next item. + let content; + let _ = syn::parenthesized!(content in meta.input); + let _: proc_macro2::TokenStream = content.parse()?; + } else if meta.input.peek(syn::Token![=]) { + // `name = "value"` form (e.g. `target_os = "linux"`); consume the value. + let _: syn::Expr = meta.value()?.parse()?; } Ok(()) }) { @@ -984,6 +1017,7 @@ mod test { use super::*; mod skip_attr_cfg_attr; + mod skip_attr_expr_block; mod skip_attr_expr_call; mod skip_attr_expr_match; mod skip_attr_expr_method_call; diff --git a/src/visit/test/skip_attr_cfg_attr.rs b/src/visit/test/skip_attr_cfg_attr.rs index 439deda6..bc930282 100644 --- a/src/visit/test/skip_attr_cfg_attr.rs +++ b/src/visit/test/skip_attr_cfg_attr.rs @@ -71,3 +71,79 @@ fn cfg_attr_mutants_skip_on_mod_suppresses_inner_items() { "sibling function outside the mod should still produce mutants: {names:?}" ); } + +// The tests below pin the implementation detail that `attr_is_mutants_skip` +// ignores the cfg condition for *every* shape of `cfg_attr` predicate, not +// just plain identifiers like `test`. This keeps the behavior consistent +// regardless of how the user spells the condition. It is not a public +// guarantee — `book/src/attrs.md` deliberately does not promise that the +// condition is ignored — but the consistency matters internally because a +// silently-dropped `mutants::skip` would be very surprising. + +#[test] +fn cfg_attr_with_function_style_predicate_still_treats_mutants_skip_as_skip() { + let mutants = mutate_source_str( + indoc! {r#" + #[cfg_attr(any(), mutants::skip)] + fn add(a: i32, b: i32) -> i32 { + a + b + } + + fn outside(a: i32, b: i32) -> i32 { + a * b + } + "#}, + &Options::default(), + ) + .unwrap(); + let names: Vec = mutants.iter().map(|m| m.name(false)).collect(); + + assert!( + !names.iter().any(|n| n.contains("add")), + "cfg_attr with a function-style predicate must still be recognised as carrying mutants::skip: {names:?}" + ); + assert!( + names.iter().any(|n| n.contains("outside")), + "sibling function should still produce mutants: {names:?}" + ); +} + +#[test] +fn cfg_attr_with_nested_function_style_predicate_still_treats_mutants_skip_as_skip() { + let mutants = mutate_source_str( + indoc! {r#" + #[cfg_attr(not(all()), mutants::skip)] + fn add(a: i32, b: i32) -> i32 { + a + b + } + "#}, + &Options::default(), + ) + .unwrap(); + let names: Vec = mutants.iter().map(|m| m.name(false)).collect(); + + assert!( + !names.iter().any(|n| n.contains("add")), + "cfg_attr with a nested function-style predicate must still be recognised as carrying mutants::skip: {names:?}" + ); +} + +#[test] +fn cfg_attr_with_name_value_predicate_still_treats_mutants_skip_as_skip() { + let mutants = mutate_source_str( + indoc! {r#" + #[cfg_attr(target_os = "linux", mutants::skip)] + fn add(a: i32, b: i32) -> i32 { + a + b + } + "#}, + &Options::default(), + ) + .unwrap(); + let names: Vec = mutants.iter().map(|m| m.name(false)).collect(); + + assert!( + !names.iter().any(|n| n.contains("add")), + "cfg_attr with a name = value predicate must still be recognised as carrying mutants::skip: {names:?}" + ); +} diff --git a/src/visit/test/skip_attr_expr_block.rs b/src/visit/test/skip_attr_expr_block.rs new file mode 100644 index 00000000..de96177a --- /dev/null +++ b/src/visit/test/skip_attr_expr_block.rs @@ -0,0 +1,141 @@ +//! Tests that `#[mutants::skip]` on a block expression `{ ... }` suppresses +//! mutants generated inside that block, while sibling code in the same +//! function remains mutated. + +use indoc::indoc; +use test_log::test; + +use crate::Options; +use crate::visit::mutate_source_str; + +#[test] +fn skip_attr_on_statement_position_block_suppresses_nested_mutants() { + let mutants = mutate_source_str( + indoc! {r#" + fn driver(a: i32, b: i32, c: i32, d: i32) { + #[mutants::skip] + { + let _ = a + b; + } + let _ = c - d; + } + "#}, + &Options::default(), + ) + .unwrap(); + let names: Vec = mutants.iter().map(|m| m.name(false)).collect(); + + assert!( + !names.iter().any(|n| n.contains("replace + with")), + "`+` inside skipped block should not produce mutants: {names:?}" + ); + assert!( + names.iter().any(|n| n.contains("replace - with")), + "`-` in the unannotated sibling code should still produce mutants: {names:?}" + ); +} + +#[test] +fn skip_attr_on_expression_position_block_suppresses_nested_mutants() { + let mutants = mutate_source_str( + indoc! {r#" + fn driver(a: i32, b: i32, c: i32, d: i32) -> i32 { + let x = #[mutants::skip] { + a + b + }; + let y = { + c - d + }; + x | y + } + "#}, + &Options::default(), + ) + .unwrap(); + let names: Vec = mutants.iter().map(|m| m.name(false)).collect(); + + assert!( + !names.iter().any(|n| n.contains("replace + with")), + "`+` inside skipped block should not produce mutants: {names:?}" + ); + assert!( + names.iter().any(|n| n.contains("replace - with")), + "`-` in the unannotated sibling block should still produce mutants: {names:?}" + ); + assert!( + names.iter().any(|n| n.contains("replace | with")), + "`|` in the unannotated tail expression should still produce mutants: {names:?}" + ); +} + +#[test] +fn skip_attr_on_block_suppresses_all_genres_within() { + let mutants = mutate_source_str( + indoc! {r#" + fn pick(x: i32, y: i32) -> &'static str { + #[mutants::skip] + { + let _ = !true; + match x { + 0 => "zero", + n if n > y => "gt", + _ => "other", + } + } + } + "#}, + &Options::default(), + ) + .unwrap(); + let names: Vec = mutants.iter().map(|m| m.name(false)).collect(); + + assert!( + !names.iter().any(|n| n.contains("delete !")), + "unary mutants inside skipped block should be suppressed: {names:?}" + ); + assert!( + !names.iter().any(|n| n.contains("delete match arm")), + "match arm deletion mutants inside skipped block should be suppressed: {names:?}" + ); + assert!( + !names.iter().any(|n| n.contains("replace match guard")), + "match guard mutants inside skipped block should be suppressed: {names:?}" + ); + assert!( + !names.iter().any(|n| n.contains("replace > with")), + "binary mutants inside skipped block should be suppressed: {names:?}" + ); +} + +#[test] +fn skip_attr_on_labeled_block_suppresses_nested_mutants() { + let mutants = mutate_source_str( + indoc! {r#" + fn driver(a: i32, b: i32) -> i32 { + #[mutants::skip] + 'block: { + if a > b { + break 'block a + b; + } + a - b + } + } + "#}, + &Options::default(), + ) + .unwrap(); + let names: Vec = mutants.iter().map(|m| m.name(false)).collect(); + + assert!( + !names.iter().any(|n| n.contains("replace + with")), + "`+` inside skipped labeled block should not produce mutants: {names:?}" + ); + assert!( + !names.iter().any(|n| n.contains("replace - with")), + "`-` inside skipped labeled block should not produce mutants: {names:?}" + ); + assert!( + !names.iter().any(|n| n.contains("replace > with")), + "`>` inside skipped labeled block should not produce mutants: {names:?}" + ); +}