Skip to content

Commit

Permalink
Check diff text matches the tree
Browse files Browse the repository at this point in the history
  • Loading branch information
sourcefrog committed Nov 13, 2023
1 parent dec840d commit 6d375d2
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 6 deletions.
48 changes: 42 additions & 6 deletions src/in_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,26 @@
//! Filter mutants to those intersecting a diff on the file tree,
//! for example from uncommitted or unmerged changes.
#![allow(unused_imports)]

use std::collections::HashMap;
use std::sync::Arc;

use anyhow::{anyhow, bail, Context};
use anyhow::{anyhow, bail};
use camino::Utf8Path;
use indoc::formatdoc;
use itertools::Itertools;
use patch::{Line, Patch, Range};
use patch::{Line, Patch};
use tracing::{trace, warn};
use tracing_subscriber::field::debug;

use crate::mutate::Mutant;
use crate::source::SourceFile;
use crate::Result;

/// Return only mutants to functions whose source was touched by this diff.
pub fn diff_filter(mutants: Vec<Mutant>, diff_text: &str) -> Result<Vec<Mutant>> {
// Flatten the error to a string because otherwise it references the diff, and can't be returned.
let patches =
Patch::from_multiple(diff_text).map_err(|err| anyhow!("Failed to parse diff: {err}"))?;
check_diff_new_text_matches(&patches, &mutants)?;
let mut lines_changed_by_path: HashMap<&Utf8Path, Vec<usize>> = HashMap::new();
for patch in &patches {
let path = strip_patch_path(&patch.new.path);
Expand Down Expand Up @@ -60,6 +61,42 @@ pub fn diff_filter(mutants: Vec<Mutant>, diff_text: &str) -> Result<Vec<Mutant>>
Ok(matched)
}

/// 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();
for mutant in mutants {
source_by_name
.entry(mutant.source_file.path())
.or_insert_with(|| Arc::clone(&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();
for (lineno, diff_content) in reconstructed {
let source_content = lines.get(lineno - 1).unwrap_or(&"");
if diff_content != *source_content {
warn!(
?path,
lineno,
?diff_content,
?source_content,
"Diff content doesn't match source file"
);
bail!(formatdoc! { "\
Diff content doesn't match source file: {path} line {lineno}
diff has: {diff_content:?}
source has: {source_content:?}
The diff might be out of date with this source tree.
"});
}
}
}
}
Ok(())
}

/// Remove the `b/` prefix commonly found in paths within diffs.
fn strip_patch_path(path: &str) -> &Utf8Path {
let path = Utf8Path::new(path);
Expand Down Expand Up @@ -160,7 +197,6 @@ fn partial_new_file<'d>(patch: &Patch<'d>) -> Vec<(usize, &'d str)> {
mod test_super {
use std::fs::read_to_string;

use assert_cmd::assert;
use pretty_assertions::assert_eq;
use similar::TextDiff;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ src/fnvalue.rs: replace match_first_type_arg -> Option<&'p Type> with Some(&Defa
src/in_diff.rs: replace diff_filter -> Result<Vec<Mutant>> with Ok(vec![])
src/in_diff.rs: replace diff_filter -> Result<Vec<Mutant>> with Ok(vec![Default::default()])
src/in_diff.rs: replace diff_filter -> Result<Vec<Mutant>> with Err(::anyhow::anyhow!("mutated!"))
src/in_diff.rs: replace check_diff_new_text_matches -> Result<()> with Ok(())
src/in_diff.rs: replace check_diff_new_text_matches -> Result<()> with Err(::anyhow::anyhow!("mutated!"))
src/in_diff.rs: replace strip_patch_path -> &Utf8Path with &Default::default()
src/in_diff.rs: replace affected_lines -> Vec<usize> with vec![]
src/in_diff.rs: replace affected_lines -> Vec<usize> with vec![0]
Expand Down
30 changes: 30 additions & 0 deletions tests/cli/in_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,33 @@ fn list_mutants_changed_in_diff1() {
2
);
}

/// If the text in the diff doesn't look like the tree then error out.
#[test]
fn mismatched_diff_causes_error() {
let src0 = read_to_string("testdata/tree/diff0/src/lib.rs").unwrap();
let src1 = read_to_string("testdata/tree/diff1/src/lib.rs").unwrap();
let diff = TextDiff::from_lines(&src0, &src1)
.unified_diff()
.context_radius(2)
.header("a/src/lib.rs", "b/src/lib.rs")
.to_string();
let diff = diff.replace("fn", "FUNCTION");
println!("{diff}");

let mut diff_file = NamedTempFile::new().unwrap();
diff_file.write_all(diff.as_bytes()).unwrap();

let tmp = copy_of_testdata("diff1");

run()
.args(["mutants", "--no-shuffle", "-d"])
.arg(tmp.path())
.arg("--in-diff")
.arg(diff_file.path())
.assert()
.failure()
.stderr(predicates::str::contains(
"Diff content doesn't match source file: src/lib.rs",
));
}

0 comments on commit 6d375d2

Please sign in to comment.