From a171af0fb961e0ebe56b27f9d7bd9b2cbb6a1088 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 27 Mar 2022 19:08:45 -0700 Subject: [PATCH] Skip `new` and `default` Helps with #25 --- .gitignore | 1 + NEWS.md | 6 +++++ src/visit.rs | 25 ++++++++++++++++--- testdata/tree/well_tested/src/methods.rs | 15 +++++++++-- .../cli__list_mutants_json_well_tested.snap | 9 +------ .../cli__list_mutants_well_tested.snap | 3 +-- .../cli__well_tested_tree_check_only.snap | 4 +-- ...i__well_tested_tree_finds_no_problems.snap | 4 +-- .../cli__well_tested_tree_quiet.snap | 3 ++- 9 files changed, 50 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index 38d5ab98..c826157d 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ mutants.out mutants.out.old .cargo/config.toml +wiki diff --git a/NEWS.md b/NEWS.md index 00ecac59..9b3deef3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # cargo-mutants changelog +## Unreleased + +- Improved: Don't attempt to mutate functions called `new` or implementations of + `Default`. cargo-mutants can not yet generate good mutations for these so they + are generally false positives. + ## 0.2.4 Released 2022-03-26 diff --git a/src/visit.rs b/src/visit.rs index dbddc500..50c2d13b 100644 --- a/src/visit.rs +++ b/src/visit.rs @@ -43,7 +43,7 @@ impl DiscoveryVisitor { ) { self.in_namespace(&ident.to_string(), |v| { let function_name = Arc::new(v.namespace_stack.join("::")); - let return_type_str = Arc::new(format!("{}", return_type.to_token_stream())); + let return_type_str = Arc::new(return_type_to_string(return_type)); for op in ops_for_return_type(return_type) { v.mutants.push(Mutant::new( v.source_file.clone(), @@ -87,15 +87,28 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor { if attrs_excluded(&i.attrs) { return; } + let type_name = type_name_string(&i.self_ty); + // The trait being implemented. + if let Some((_, trait_path, _)) = &i.trait_ { + let trait_name = &trait_path.segments.last().unwrap().ident; + if trait_name == "Default" { + // We don't know (yet) how to generate an interestingly-broken + // Default::default. + return; + } + } // Make an approximately-right namespace. - let name = type_name_string(&i.self_ty); - self.in_namespace(&name, |v| syn::visit::visit_item_impl(v, i)); + // TODO: For `impl X for Y` get both X and Y onto the namespace + // stack so that we can show a more descriptive name. + self.in_namespace(&type_name, |v| syn::visit::visit_item_impl(v, i)); } /// Visit `fn foo()` within an `impl`. fn visit_impl_item_method(&mut self, i: &'ast syn::ImplItemMethod) { if attrs_excluded(&i.attrs) { return; + } else if i.sig.ident == "new" { + return; // don't look inside constructors because there's often no good alternative } self.collect_fn_mutants(&i.sig.ident, &i.sig.output, &i.block.brace_token.span); self.in_namespace(&i.sig.ident.to_string(), |v| { @@ -103,6 +116,7 @@ impl<'ast> Visit<'ast> for DiscoveryVisitor { }); } + /// Visit `mod foo { ... }`. fn visit_item_mod(&mut self, node: &'ast syn::ItemMod) { if !attrs_excluded(&node.attrs) { self.in_namespace(&node.ident.to_string(), |v| { @@ -154,6 +168,11 @@ fn type_name_string(ty: &syn::Type) -> String { } } +fn return_type_to_string(return_type: &syn::ReturnType) -> String { + // TODO: Remove unnecessary spaces. + format!("{}", return_type.to_token_stream()) +} + fn path_is_result(path: &syn::Path) -> bool { path.segments .last() diff --git a/testdata/tree/well_tested/src/methods.rs b/testdata/tree/well_tested/src/methods.rs index f4fbd451..3fad1ca3 100644 --- a/testdata/tree/well_tested/src/methods.rs +++ b/testdata/tree/well_tested/src/methods.rs @@ -12,10 +12,9 @@ impl Foo { } } -#[mutants::skip] impl Default for Foo { fn default() -> Self { - Foo { i: 1 } + Foo::new() } } @@ -28,3 +27,15 @@ fn double() { foo.double(); assert_eq!(foo.i, 128); } + +#[test] +fn default() { + let foo = Foo::default(); + assert_eq!(foo.i, 32); +} + +#[test] +fn new_foo() { + let foo = Foo::new(); + assert_eq!(foo.i, 32); +} diff --git a/tests/snapshots/cli__list_mutants_json_well_tested.snap b/tests/snapshots/cli__list_mutants_json_well_tested.snap index 68f89a04..186601e1 100644 --- a/tests/snapshots/cli__list_mutants_json_well_tested.snap +++ b/tests/snapshots/cli__list_mutants_json_well_tested.snap @@ -1,7 +1,7 @@ --- source: tests/cli.rs +assertion_line: 41 expression: "String::from_utf8_lossy(&output.stdout)" - --- [ { @@ -11,13 +11,6 @@ expression: "String::from_utf8_lossy(&output.stdout)" "return_type": "-> & 'static str", "replacement": "Default::default()" }, - { - "file": "src/methods.rs", - "line": 6, - "function": "Foo::new", - "return_type": "-> Foo", - "replacement": "Default::default()" - }, { "file": "src/methods.rs", "line": 10, diff --git a/tests/snapshots/cli__list_mutants_well_tested.snap b/tests/snapshots/cli__list_mutants_well_tested.snap index 9b3a5f9b..c38a08de 100644 --- a/tests/snapshots/cli__list_mutants_well_tested.snap +++ b/tests/snapshots/cli__list_mutants_well_tested.snap @@ -1,10 +1,9 @@ --- source: tests/cli.rs +assertion_line: 41 expression: "String::from_utf8_lossy(&output.stdout)" - --- src/inside_mod.rs:3: replace outer::inner::name -> & 'static str with Default::default() -src/methods.rs:6: replace Foo::new -> Foo with Default::default() src/methods.rs:10: replace Foo::double with () src/nested_function.rs:1: replace has_nested -> u32 with Default::default() src/nested_function.rs:2: replace has_nested::inner -> u32 with Default::default() diff --git a/tests/snapshots/cli__well_tested_tree_check_only.snap b/tests/snapshots/cli__well_tested_tree_check_only.snap index 994ed977..eb3c6a14 100644 --- a/tests/snapshots/cli__well_tested_tree_check_only.snap +++ b/tests/snapshots/cli__well_tested_tree_check_only.snap @@ -1,13 +1,13 @@ --- source: tests/cli.rs +assertion_line: 240 expression: stdout --- Freshen source tree ... ok Copy source and build products to scratch directory ... done Unmutated baseline ... ok -Found 13 mutants to test +Found 12 mutants to test src/inside_mod.rs:3: replace outer::inner::name -> & 'static str with Default::default() ... check ok -src/methods.rs:6: replace Foo::new -> Foo with Default::default() ... check ok src/methods.rs:10: replace Foo::double with () ... check ok src/nested_function.rs:1: replace has_nested -> u32 with Default::default() ... check ok src/nested_function.rs:2: replace has_nested::inner -> u32 with Default::default() ... check ok diff --git a/tests/snapshots/cli__well_tested_tree_finds_no_problems.snap b/tests/snapshots/cli__well_tested_tree_finds_no_problems.snap index 4c628f94..76aa2dae 100644 --- a/tests/snapshots/cli__well_tested_tree_finds_no_problems.snap +++ b/tests/snapshots/cli__well_tested_tree_finds_no_problems.snap @@ -1,13 +1,13 @@ --- source: tests/cli.rs +assertion_line: 221 expression: stdout --- Freshen source tree ... ok Copy source and build products to scratch directory ... done Unmutated baseline ... ok -Found 13 mutants to test +Found 12 mutants to test src/inside_mod.rs:3: replace outer::inner::name -> & 'static str with Default::default() ... caught -src/methods.rs:6: replace Foo::new -> Foo with Default::default() ... caught src/methods.rs:10: replace Foo::double with () ... caught src/nested_function.rs:1: replace has_nested -> u32 with Default::default() ... caught src/nested_function.rs:2: replace has_nested::inner -> u32 with Default::default() ... caught diff --git a/tests/snapshots/cli__well_tested_tree_quiet.snap b/tests/snapshots/cli__well_tested_tree_quiet.snap index 4da2e139..8498f0a1 100644 --- a/tests/snapshots/cli__well_tested_tree_quiet.snap +++ b/tests/snapshots/cli__well_tested_tree_quiet.snap @@ -1,9 +1,10 @@ --- source: tests/cli.rs +assertion_line: 199 expression: stdout --- Freshen source tree ... ok Copy source and build products to scratch directory ... done Unmutated baseline ... ok -Found 13 mutants to test +Found 12 mutants to test