Merge remote-tracking branch 'sourcefrog/main' into 115-mod-not-found

NEWS.md

@@ -4,6 +4,8 @@
 
 - Fixed: Correctly traverse `mod` statements within package top source files that are not named `lib.rs` or `main.rs`, by following the `path` setting of each target within the manifest.
 
+- 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/console.rs

@@ -9,7 +9,8 @@ use std::io;
 use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant};
 
-use camino::{Utf8Path, Utf8PathBuf};
+use anyhow::Context;
+use camino::Utf8Path;
 use console::{style, StyledObject};
 
 use nutmeg::Destination;
@@ -19,7 +20,8 @@ use tracing_subscriber::prelude::*;
 
 use crate::outcome::{LabOutcome, SummaryOutcome};
 use crate::scenario::Scenario;
-use crate::{last_line, Mutant, Options, Phase, Result, ScenarioOutcome};
+use crate::tail_file::TailFile;
+use crate::{Mutant, Options, Phase, Result, ScenarioOutcome};
 
 static COPY_MESSAGE: &str = "Copy source to scratch directory";
 
@@ -61,12 +63,13 @@ impl Console {
     }
 
     /// Update that a cargo task is starting.
-    pub fn scenario_started(&self, scenario: &Scenario, log_file: &Utf8Path) {
+    pub fn scenario_started(&self, scenario: &Scenario, log_file: &Utf8Path) -> Result<()> {
         let start = Instant::now();
-        let scenario_model = ScenarioModel::new(scenario, start, log_file.to_owned());
+        let scenario_model = ScenarioModel::new(scenario, start, log_file)?;
         self.view.update(|model| {
             model.scenario_models.push(scenario_model);
         });
+        Ok(())
     }
 
     /// Update that cargo finished.
@@ -486,19 +489,20 @@ struct ScenarioModel {
     phase: Option<Phase>,
     /// Previously-executed phases and durations.
     previous_phase_durations: Vec<(Phase, Duration)>,
-    log_file: Utf8PathBuf,
+    log_tail: TailFile,
 }
 
 impl ScenarioModel {
-    fn new(scenario: &Scenario, start: Instant, log_file: Utf8PathBuf) -> ScenarioModel {
-        ScenarioModel {
+    fn new(scenario: &Scenario, start: Instant, log_file: &Utf8Path) -> Result<ScenarioModel> {
+        let log_tail = TailFile::new(log_file).context("Failed to open log file")?;
+        Ok(ScenarioModel {
             scenario: scenario.clone(),
             name: style_scenario(scenario, true),
             phase: None,
             phase_start: start,
-            log_file,
+            log_tail,
             previous_phase_durations: Vec::new(),
-        }
+        })
     }
 
     fn phase_started(&mut self, phase: Phase) {
@@ -531,7 +535,7 @@ impl nutmeg::Model for ScenarioModel {
             ));
         }
         write!(s, "{}", prs.join(" + ")).unwrap();
-        if let Ok(last_line) = last_line(&self.log_file) {
+        if let Ok(last_line) = self.log_tail.last_line() {
             write!(s, "\n    {}", style(last_line).dim()).unwrap();
         }
         s

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/lab.rs

@@ -185,7 +185,7 @@ fn test_scenario(
         log_file.message(&format!("mutation diff:\n{}", mutant.diff()));
         mutant.apply(build_dir)?;
     }
-    console.scenario_started(scenario, log_file.path());
+    console.scenario_started(scenario, log_file.path())?;
 
     let mut outcome = ScenarioOutcome::new(&log_file, scenario.clone());
     let phases: &[Phase] = if options.check_only {

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/log_file.rs

@@ -1,9 +1,9 @@
-// Copyright 2021, 2022 Martin Pool
+// Copyright 2021-2023 Martin Pool
 
 //! Manage per-scenario log files, which contain the output from cargo
 //! and test cases, mixed with commentary from cargo-mutants.
 
-use std::fs::{self, File, OpenOptions};
+use std::fs::{File, OpenOptions};
 use std::io::{self, Write};
 
 use anyhow::Context;
@@ -67,21 +67,6 @@ impl LogFile {
     }
 }
 
-/// Return the last non-empty line from a file, if it has any content.
-pub fn last_line(path: &Utf8Path) -> Result<String> {
-    // This is somewhat inefficient: we could potentially remember how long
-    // the file was last time, seek to that point, and deal with incomplete
-    // lines. However, probably these files will never get so colossal that
-    // reading them is a big problem; they are almost certainly in cache;
-    // and this should only be called a few times per second...
-    Ok(fs::read_to_string(path)?
-        .lines()
-        .filter(|s| !s.trim().is_empty())
-        .last()
-        .unwrap_or_default()
-        .to_owned())
-}
-
 fn clean_filename(s: &str) -> String {
     let s = s.replace('/', "__");
     s.chars()
@@ -103,37 +88,4 @@ mod test {
             "1__2_3_4_5_6_7_8_9_0"
         );
     }
-
-    #[test]
-    fn last_line_of_file() {
-        let mut tempfile = tempfile::NamedTempFile::new().unwrap();
-        let path: Utf8PathBuf = tempfile.path().to_owned().try_into().unwrap();
-
-        assert_eq!(
-            last_line(&path).unwrap(),
-            "",
-            "empty file has an empty last line"
-        );
-
-        tempfile.write_all(b"hello").unwrap();
-        assert_eq!(
-            last_line(&path).unwrap(),
-            "hello",
-            "single line file with no terminator has that line as last line"
-        );
-
-        tempfile.write_all(b"\n\n\n").unwrap();
-        assert_eq!(
-            last_line(&path).unwrap(),
-            "hello",
-            "trailing blank lines are ignored"
-        );
-
-        tempfile.write_all(b"that's all folks!\n").unwrap();
-        assert_eq!(
-            last_line(&path).unwrap(),
-            "that's all folks!",
-            "newline terminated last line is returned"
-        );
-    }
 }

src/main.rs

@@ -26,6 +26,7 @@ mod process;
 mod scenario;
 mod source;
 mod span;
+mod tail_file;
 mod visit;
 mod workspace;
 
@@ -51,7 +52,7 @@ use crate::in_diff::diff_filter;
 use crate::interrupt::check_interrupted;
 use crate::lab::test_mutants;
 use crate::list::{list_files, list_mutants, FmtToIoWrite};
-use crate::log_file::{last_line, LogFile};
+use crate::log_file::LogFile;
 use crate::manifest::fix_manifest;
 use crate::mutate::{Genre, Mutant};
 use crate::options::Options;

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

@@ -30,7 +30,8 @@ src/config.rs: replace Config::read_tree_config -> Result<Config> with Err(::any
 src/console.rs: replace Console::walk_tree_start with ()
 src/console.rs: replace Console::walk_tree_update with ()
 src/console.rs: replace Console::walk_tree_done with ()
-src/console.rs: replace Console::scenario_started with ()
+src/console.rs: replace Console::scenario_started -> Result<()> with Ok(())
+src/console.rs: replace Console::scenario_started -> Result<()> with Err(::anyhow::anyhow!("mutated!"))
 src/console.rs: replace Console::scenario_finished with ()
 src/console.rs: replace || with && in Console::scenario_finished
 src/console.rs: replace || with == in Console::scenario_finished
@@ -68,7 +69,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 +140,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
@@ -235,9 +235,6 @@ src/log_file.rs: replace LogFile::open_append -> Result<File> with Ok(Default::d
 src/log_file.rs: replace LogFile::open_append -> Result<File> with Err(::anyhow::anyhow!("mutated!"))
 src/log_file.rs: replace LogFile::message with ()
 src/log_file.rs: replace LogFile::path -> &Utf8Path with &Default::default()
-src/log_file.rs: replace last_line -> Result<String> with Ok(String::new())
-src/log_file.rs: replace last_line -> Result<String> with Ok("xyzzy".into())
-src/log_file.rs: replace last_line -> Result<String> with Err(::anyhow::anyhow!("mutated!"))
 src/log_file.rs: replace clean_filename -> String with String::new()
 src/log_file.rs: replace clean_filename -> String with "xyzzy".into()
 src/manifest.rs: replace fix_manifest -> Result<()> with Ok(())
@@ -466,6 +463,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!"))
@@ -548,6 +547,11 @@ src/span.rs: replace <impl From for Span>::from -> Self with Default::default()
 src/span.rs: replace <impl From for Span>::from -> Self with Default::default()
 src/span.rs: replace <impl Debug for Span>::fmt -> fmt::Result with Ok(Default::default())
 src/span.rs: replace <impl Debug for Span>::fmt -> fmt::Result with Err(::anyhow::anyhow!("mutated!"))
+src/tail_file.rs: replace TailFile::last_line -> Result<&str> with Ok("")
+src/tail_file.rs: replace TailFile::last_line -> Result<&str> with Ok("xyzzy")
+src/tail_file.rs: replace TailFile::last_line -> Result<&str> with Err(::anyhow::anyhow!("mutated!"))
+src/tail_file.rs: replace > with == in TailFile::last_line
+src/tail_file.rs: replace > with < in TailFile::last_line
 src/visit.rs: replace walk_tree -> Result<Discovered> with Ok(Default::default())
 src/visit.rs: replace walk_tree -> Result<Discovered> with Err(::anyhow::anyhow!("mutated!"))
 src/visit.rs: replace && with || in walk_tree
@@ -568,7 +572,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
@@ -642,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![])