diff --git a/src/diff_filter.rs b/src/diff_filter.rs index 0ebe24c1..bf5768b1 100644 --- a/src/diff_filter.rs +++ b/src/diff_filter.rs @@ -9,7 +9,8 @@ use std::collections::HashMap; use anyhow::{anyhow, bail, Context}; use camino::Utf8Path; -use patch::{Patch, Range}; +use itertools::Itertools; +use patch::{Line, Patch, Range}; use tracing::{trace, warn}; use crate::mutate::Mutant; @@ -20,26 +21,35 @@ pub fn diff_filter(mutants: Vec, diff_text: &str) -> Result> // 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}"))?; - let mut patch_by_path: HashMap<&Utf8Path, &Patch> = HashMap::new(); + let mut lines_changed_by_path: HashMap<&Utf8Path, Vec> = HashMap::new(); for patch in &patches { let path = strip_patch_path(&patch.new.path); - if patch_by_path.insert(path, patch).is_some() { + if lines_changed_by_path + .insert(path, remove_context(patch)) + .is_some() + { bail!("Patch input contains repeated filename: {path:?}"); } } - /* The line numbers in the diff include context lines, which don't count as really changed. - */ let mut matched: Vec = Vec::with_capacity(mutants.len()); 'mutant: for mutant in mutants.into_iter() { - if let Some(patch) = patch_by_path.get(mutant.source_file.path()) { - for hunk in &patch.hunks { - if range_overlaps(&hunk.new_range, &mutant) { + let path = mutant.source_file.path(); + if let Some(lines_changed) = lines_changed_by_path.get(path) { + // We could do be smarter about searching for an intersection of ranges, rather + // than probing one line at a time... But, the numbers are likely to be small + // enough that this is tolerable... + // + // We could also search for each unique span in each file, and then include + // every mutant that intersects any of those spans, since commonly there will + // be multiple mutants in the same function. + for line in mutant.span.start.line..=mutant.span.end.line { + if lines_changed.binary_search(&line).is_ok() { trace!( - path = ?patch.new.path, - diff_range = ?hunk.new_range, + ?path, + line, + %mutant, mutant_span = ?&mutant.span, - mutant = %mutant, - "diff hunk matched mutant" + "diff matched mutant" ); matched.push(mutant); continue 'mutant; @@ -56,10 +66,47 @@ fn strip_patch_path(path: &str) -> &Utf8Path { path.strip_prefix("b").unwrap_or(path) } -fn range_overlaps(diff_range: &Range, mutant: &Mutant) -> bool { - let diff_end = diff_range.start + diff_range.count; - diff_end >= mutant.span.start.line.try_into().unwrap() - && diff_range.start <= mutant.span.end.line.try_into().unwrap() +/// Given a diff, return the ranges of actually-changed lines, ignoring context lines. +/// +/// Code that's only included as context doesn't need to be tested. +/// +/// This returns a list of line numbers that are either added to the new file, or +/// adjacent to deletions. +/// +/// (A list of ranges would be more concise but this is easier for a first version.) +/// +/// If a line is deleted then the range will span from the line before to the line after. +fn remove_context(patch: &Patch) -> Vec { + let mut r = Vec::new(); + for hunk in &patch.hunks { + let mut lineno: usize = hunk.new_range.start.try_into().unwrap(); + for line in &hunk.lines { + match line { + Line::Context(_) => { + lineno += 1; + } + Line::Add(_) => { + if r.last().map_or(true, |last| *last < lineno) { + r.push(lineno); + } + lineno += 1; + } + Line::Remove(_) => { + if lineno > 1 && r.last().map_or(true, |last| *last < (lineno - 1)) { + r.push(lineno - 1); + } + if lineno > 0 && r.last().map_or(true, |last| *last < lineno) { + r.push(lineno); + } + } + } + } + } + debug_assert!( + r.iter().tuple_windows().all(|(a, b)| a < b), + "remove_context: line numbers not sorted and unique: {r:?}" + ); + r } #[cfg(test)] diff --git a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap index 157a2743..18474718 100644 --- a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap +++ b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap @@ -108,8 +108,9 @@ src/diff_filter.rs: replace diff_filter -> Result> with Ok(vec![]) src/diff_filter.rs: replace diff_filter -> Result> with Ok(vec![Default::default()]) src/diff_filter.rs: replace diff_filter -> Result> with Err(::anyhow::anyhow!("mutated!")) src/diff_filter.rs: replace strip_patch_path -> &Utf8Path with &Default::default() -src/diff_filter.rs: replace range_overlaps -> bool with true -src/diff_filter.rs: replace range_overlaps -> bool with false +src/diff_filter.rs: replace remove_context -> Vec with vec![] +src/diff_filter.rs: replace remove_context -> Vec with vec![0] +src/diff_filter.rs: replace remove_context -> Vec with vec![1] src/fnvalue.rs: replace return_type_replacements -> impl Iterator with ::std::iter::empty() src/fnvalue.rs: replace return_type_replacements -> impl Iterator with ::std::iter::once(Default::default()) src/fnvalue.rs: replace type_replacements -> impl Iterator with ::std::iter::empty()