Avoid generating no-op mutants (#188)

For FnValue mutants: if the replacement has the same tokens as the
original source, don't omit it, it can't find anything useful.

Fixes #134

NEWS.md

@@ -1,5 +1,9 @@
 # cargo-mutants changelog
 
+## Unreleased
+
+- Improved: Don't generate function mutants that have the same AST as the code they're replacing.
+
 ## 23.12.0
 
 An exciting step forward: cargo-mutants can now generate mutations smaller than a whole function. To start with, several binary operators are mutated.

src/fnvalue.rs

@@ -17,12 +17,11 @@ use tracing::trace;
 pub(crate) fn return_type_replacements(
     return_type: &ReturnType,
     error_exprs: &[Expr],
-) -> impl Iterator<Item = TokenStream> {
+) -> Vec<TokenStream> {
     match return_type {
         ReturnType::Default => vec![quote! { () }],
         ReturnType::Type(_rarrow, type_) => type_replacements(type_, error_exprs).collect_vec(),
     }
-    .into_iter()
 }
 
 /// Generate some values that we hope are reasonable replacements for a type.
@@ -696,6 +695,7 @@ mod test {
     fn check_replacements(return_type: ReturnType, error_exprs: &[Expr], expected: &[&str]) {
         assert_eq!(
             return_type_replacements(&return_type, error_exprs)
+                .into_iter()
                 .map(|t| t.to_pretty_string())
                 .collect_vec(),
             expected

src/in_diff.rs

@@ -4,7 +4,6 @@
 //! for example from uncommitted or unmerged changes.
 
 use std::collections::HashMap;
-use std::sync::Arc;
 
 use anyhow::{anyhow, bail};
 use camino::Utf8Path;
@@ -63,17 +62,17 @@ pub fn diff_filter(mutants: Vec<Mutant>, diff_text: &str) -> Result<Vec<Mutant>>
 
 /// Error if the new text from the diffs doesn't match the source files.
 fn check_diff_new_text_matches(patches: &[Patch], mutants: &[Mutant]) -> Result<()> {
-    let mut source_by_name: HashMap<&Utf8Path, Arc<SourceFile>> = HashMap::new();
+    let mut source_by_name: HashMap<&Utf8Path, &SourceFile> = HashMap::new();
     for mutant in mutants {
         source_by_name
             .entry(mutant.source_file.path())
-            .or_insert_with(|| Arc::clone(&mutant.source_file));
+            .or_insert_with(|| &mutant.source_file);
     }
     for patch in patches {
         let path = strip_patch_path(&patch.new.path);
         if let Some(source_file) = source_by_name.get(&path) {
             let reconstructed = partial_new_file(patch);
-            let lines = source_file.code.lines().collect_vec();
+            let lines = source_file.code().lines().collect_vec();
             for (lineno, diff_content) in reconstructed {
                 let source_content = lines.get(lineno - 1).unwrap_or(&"");
                 if diff_content != *source_content {

src/list.rs

@@ -4,7 +4,6 @@
 
 use std::fmt;
 use std::io;
-use std::sync::Arc;
 
 use serde_json::{json, Value};
 
@@ -62,7 +61,7 @@ pub(crate) fn list_mutants<W: fmt::Write>(
 
 pub(crate) fn list_files<W: fmt::Write>(
     mut out: W,
-    source_files: &[Arc<SourceFile>],
+    source_files: &[SourceFile],
     options: &Options,
 ) -> Result<()> {
     if options.emit_json {

src/mutate.rs

@@ -31,7 +31,7 @@ pub enum Genre {
 #[derive(Clone, Eq, PartialEq)]
 pub struct Mutant {
     /// Which file is being mutated.
-    pub source_file: Arc<SourceFile>,
+    pub source_file: SourceFile,
 
     /// The function that's being mutated: the nearest enclosing function, if they are nested.
     ///
@@ -71,7 +71,7 @@ impl Mutant {
     /// Return text of the whole file with the mutation applied.
     pub fn mutated_code(&self) -> String {
         self.span.replace(
-            &self.source_file.code,
+            self.source_file.code(),
             &format!("{} {}", &self.replacement, MUTATION_MARKER_COMMENT),
         )
     }
@@ -143,7 +143,7 @@ impl Mutant {
     }
 
     pub fn original_text(&self) -> String {
-        self.span.extract(&self.source_file.code)
+        self.span.extract(self.source_file.code())
     }
 
     /// Return the text inserted for this mutation.
@@ -165,7 +165,7 @@ impl Mutant {
         let old_label = self.source_file.tree_relative_slashes();
         // There shouldn't be any newlines, but just in case...
         let new_label = self.describe_change().replace('\n', " ");
-        TextDiff::from_lines(&self.source_file.code, &self.mutated_code())
+        TextDiff::from_lines(self.source_file.code(), &self.mutated_code())
             .unified_diff()
             .context_radius(8)
             .header(&old_label, &new_label)
@@ -178,7 +178,7 @@ impl Mutant {
     }
 
     pub fn unapply(&self, build_dir: &mut BuildDir) -> Result<()> {
-        self.write_in_dir(build_dir, &self.source_file.code)
+        self.write_in_dir(build_dir, self.source_file.code())
     }
 
     #[allow(unknown_lints, clippy::needless_pass_by_ref_mut)]

src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap

@@ -68,7 +68,6 @@ src/console.rs: replace <impl MakeWriter for TerminalWriter>::make_writer -> Sel
 src/console.rs: replace <impl Write for TerminalWriter>::write -> std::io::Result<usize> with Ok(0)
 src/console.rs: replace <impl Write for TerminalWriter>::write -> std::io::Result<usize> with Ok(1)
 src/console.rs: replace <impl Write for TerminalWriter>::write -> std::io::Result<usize> with Err(::anyhow::anyhow!("mutated!"))
-src/console.rs: replace <impl Write for TerminalWriter>::flush -> std::io::Result<()> with Ok(())
 src/console.rs: replace <impl Write for TerminalWriter>::flush -> std::io::Result<()> with Err(::anyhow::anyhow!("mutated!"))
 src/console.rs: replace <impl MakeWriter for DebugLogWriter>::make_writer -> Self::Writer with Default::default()
 src/console.rs: replace <impl Write for DebugLogWriter>::write -> io::Result<usize> with Ok(0)
@@ -140,8 +139,8 @@ src/copy_tree.rs: replace copy_tree -> Result<TempDir> with Ok(Default::default(
 src/copy_tree.rs: replace copy_tree -> Result<TempDir> with Err(::anyhow::anyhow!("mutated!"))
 src/copy_tree.rs: replace copy_symlink -> Result<()> with Ok(())
 src/copy_tree.rs: replace copy_symlink -> Result<()> with Err(::anyhow::anyhow!("mutated!"))
-src/fnvalue.rs: replace return_type_replacements -> impl Iterator<Item = TokenStream> with ::std::iter::empty()
-src/fnvalue.rs: replace return_type_replacements -> impl Iterator<Item = TokenStream> with ::std::iter::once(Default::default())
+src/fnvalue.rs: replace return_type_replacements -> Vec<TokenStream> with vec![]
+src/fnvalue.rs: replace return_type_replacements -> Vec<TokenStream> with vec![Default::default()]
 src/fnvalue.rs: replace type_replacements -> impl Iterator<Item = TokenStream> with ::std::iter::empty()
 src/fnvalue.rs: replace type_replacements -> impl Iterator<Item = TokenStream> with ::std::iter::once(Default::default())
 src/fnvalue.rs: replace path_ends_with -> bool with true
@@ -466,6 +465,8 @@ src/scenario.rs: replace Scenario::log_file_name_base -> String with "xyzzy".int
 src/source.rs: replace SourceFile::tree_relative_slashes -> String with String::new()
 src/source.rs: replace SourceFile::tree_relative_slashes -> String with "xyzzy".into()
 src/source.rs: replace SourceFile::path -> &Utf8Path with &Default::default()
+src/source.rs: replace SourceFile::code -> &str with ""
+src/source.rs: replace SourceFile::code -> &str with "xyzzy"
 src/span.rs: replace <impl From for LineColumn>::from -> Self with Default::default()
 src/span.rs: replace <impl Debug for LineColumn>::fmt -> fmt::Result with Ok(Default::default())
 src/span.rs: replace <impl Debug for LineColumn>::fmt -> fmt::Result with Err(::anyhow::anyhow!("mutated!"))
@@ -568,7 +569,9 @@ src/visit.rs: replace walk_file -> Result<(Vec<Mutant>, Vec<String>)> with Ok((v
 src/visit.rs: replace walk_file -> Result<(Vec<Mutant>, Vec<String>)> with Err(::anyhow::anyhow!("mutated!"))
 src/visit.rs: replace DiscoveryVisitor<'o>::enter_function -> Arc<Function> with Arc::new(Default::default())
 src/visit.rs: replace DiscoveryVisitor<'o>::leave_function with ()
+src/visit.rs: replace DiscoveryVisitor<'o>::collect_mutant with ()
 src/visit.rs: replace DiscoveryVisitor<'o>::collect_fn_mutants with ()
+src/visit.rs: replace == with != in DiscoveryVisitor<'o>::collect_fn_mutants
 src/visit.rs: replace DiscoveryVisitor<'o>::in_namespace -> T with Default::default()
 src/visit.rs: replace <impl Visit for DiscoveryVisitor<'_>>::visit_item_fn with ()
 src/visit.rs: replace || with && in <impl Visit for DiscoveryVisitor<'_>>::visit_item_fn
@@ -645,9 +648,9 @@ src/workspace.rs: replace Workspace::package_tops -> Result<Vec<PackageTop>> wit
 src/workspace.rs: replace Workspace::package_tops -> Result<Vec<PackageTop>> with Ok(vec![Default::default()])
 src/workspace.rs: replace Workspace::package_tops -> Result<Vec<PackageTop>> with Err(::anyhow::anyhow!("mutated!"))
 src/workspace.rs: replace == with != in Workspace::package_tops
-src/workspace.rs: replace Workspace::top_sources -> Result<Vec<Arc<SourceFile>>> with Ok(vec![])
-src/workspace.rs: replace Workspace::top_sources -> Result<Vec<Arc<SourceFile>>> with Ok(vec![Arc::new(Default::default())])
-src/workspace.rs: replace Workspace::top_sources -> Result<Vec<Arc<SourceFile>>> with Err(::anyhow::anyhow!("mutated!"))
+src/workspace.rs: replace Workspace::top_sources -> Result<Vec<SourceFile>> with Ok(vec![])
+src/workspace.rs: replace Workspace::top_sources -> Result<Vec<SourceFile>> with Ok(vec![Default::default()])
+src/workspace.rs: replace Workspace::top_sources -> Result<Vec<SourceFile>> with Err(::anyhow::anyhow!("mutated!"))
 src/workspace.rs: replace Workspace::discover -> Result<Discovered> with Ok(Default::default())
 src/workspace.rs: replace Workspace::discover -> Result<Discovered> with Err(::anyhow::anyhow!("mutated!"))
 src/workspace.rs: replace Workspace::mutants -> Result<Vec<Mutant>> with Ok(vec![])

src/source.rs

@@ -19,16 +19,19 @@ use crate::path::Utf8PathSlashes;
 ///
 /// Code is normalized to Unix line endings as it's read in, and modified
 /// files are written with Unix line endings.
-#[derive(Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 pub struct SourceFile {
     /// Package within the workspace.
     pub package: Arc<Package>,
 
     /// Path of this source file relative to workspace.
     pub tree_relative_path: Utf8PathBuf,
 
-    /// Full copy of the source.
-    pub code: String,
+    /// Full copy of the unmodified source.
+    ///
+    /// This is held in an [Arc] so that SourceFiles can be cloned without using excessive
+    /// amounts of memory.
+    pub code: Arc<String>,
 }
 
 impl SourceFile {
@@ -41,9 +44,11 @@ impl SourceFile {
         package: &Arc<Package>,
     ) -> Result<SourceFile> {
         let full_path = tree_path.join(&tree_relative_path);
-        let code = std::fs::read_to_string(&full_path)
-            .with_context(|| format!("failed to read source of {full_path:?}"))?
-            .replace("\r\n", "\n");
+        let code = Arc::new(
+            std::fs::read_to_string(&full_path)
+                .with_context(|| format!("failed to read source of {full_path:?}"))?
+                .replace("\r\n", "\n"),
+        );
         Ok(SourceFile {
             tree_relative_path,
             code,
@@ -59,6 +64,10 @@ impl SourceFile {
     pub fn path(&self) -> &Utf8Path {
         self.tree_relative_path.as_path()
     }
+
+    pub fn code(&self) -> &str {
+        self.code.as_str()
+    }
 }
 
 #[cfg(test)]
@@ -89,6 +98,6 @@ mod test {
             }),
         )
         .unwrap();
-        assert_eq!(source_file.code, "fn main() {\n    640 << 10;\n}\n");
+        assert_eq!(source_file.code(), "fn main() {\n    640 << 10;\n}\n");
     }
 }