From c8c1b637f4b600adddfe3d0dc857c44f4ec90c3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20R=C3=BC=C3=9Fler?= Date: Wed, 8 Jan 2025 15:24:59 +0100 Subject: [PATCH 1/2] Rework how blame is passed to parents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this commit, blame would have been passed to the first parent in cases where there was more than one. This commit changes that by only removing a particular suspect from further consideration once it has been compared to all of its parents. For hunks where blame cannot be passed to any parent, we can safely assume that they were introduced by a particular suspect, so we remove those hunks from `hunks_to_blame` and create a `BlameEntry` out of them. We can illustrate the change using the following small example history: ```text ---1----2 \ \ 3----4--- ``` Let’s now say that we’re blaming a file that has the following content: ```text line 1 # introduced by (1), then changed by (3) line 2 # introduced by (1) line 3 # introduced by (1), then changed by (2) ``` The resulting blame should look like this: ```text (3) line 1 (1) line 2 (2) line 3 ``` The previous version of the algorithm would have passed blame to just (2) or (3), depending on which one came first in the list of parents. --- gix-blame/src/file/function.rs | 60 +- gix-blame/src/file/mod.rs | 119 +-- gix-blame/src/file/tests.rs | 819 ++++++++++---------- gix-blame/tests/blame.rs | 1 + gix-blame/tests/fixtures/make_blame_repo.sh | 18 + 5 files changed, 500 insertions(+), 517 deletions(-) diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 51bd2e7e1a0..1365b23bad7 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -172,9 +172,10 @@ where if more_than_one_parent { // None of the changes affected the file we’re currently blaming. // Copy blame to parent. - for unblamed_hunk in &mut hunks_to_blame { - unblamed_hunk.clone_blame(suspect, parent_id); - } + hunks_to_blame = hunks_to_blame + .into_iter() + .map(|unblamed_hunk| unblamed_hunk.clone_blame(suspect, parent_id)) + .collect(); } else { pass_blame_from_to(suspect, parent_id, &mut hunks_to_blame); } @@ -196,14 +197,59 @@ where } gix_diff::tree::recorder::Change::Modification { previous_oid, oid, .. } => { let changes = blob_changes(&odb, resource_cache, oid, previous_oid, file_path, &mut stats)?; - hunks_to_blame = process_changes(&mut out, hunks_to_blame, changes, suspect); - pass_blame_from_to(suspect, parent_id, &mut hunks_to_blame); + hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent_id); } } } - if more_than_one_parent { - for unblamed_hunk in &mut hunks_to_blame { + + hunks_to_blame = hunks_to_blame + .into_iter() + .filter_map(|mut unblamed_hunk| { + if unblamed_hunk.suspects.len() == 1 { + if let Some(entry) = BlameEntry::from_unblamed_hunk(&unblamed_hunk, suspect) { + // At this point, we have copied blame for every hunk to a parent. Hunks + // that have only `suspect` left in `suspects` have not passed blame to any + // parent and so they can be converted to a `BlameEntry` and moved to + // `out`. + out.push(entry); + + return None; + } + } + unblamed_hunk.remove_blame(suspect); + + Some(unblamed_hunk) + }) + .collect(); + + // This block asserts that line ranges for each suspect never overlap. If they did overlap + // this would mean that the same line in a *Source File* would map to more than one line in + // the *Blamed File* and this is not possible. + #[cfg(debug_assertions)] + { + let ranges = hunks_to_blame.iter().fold( + std::collections::BTreeMap::>>::new(), + |mut acc, hunk| { + for (suspect, range) in hunk.suspects.clone() { + acc.entry(suspect).or_insert(Vec::new()).push(range); + } + + acc + }, + ); + + for (_, mut ranges) in ranges { + ranges.sort_by(|a, b| a.start.cmp(&b.start)); + + for window in ranges.windows(2) { + match window { + [a, b] => { + assert!(a.end <= b.start, "#{hunks_to_blame:#?}"); + } + _ => {} + } + } } } } diff --git a/gix-blame/src/file/mod.rs b/gix-blame/src/file/mod.rs index 1afa77723e0..dd7b7763ca4 100644 --- a/gix-blame/src/file/mod.rs +++ b/gix-blame/src/file/mod.rs @@ -16,10 +16,10 @@ pub(super) mod function; /// /// This is the core of the blame implementation as it matches regions in *Source File* to the *Blamed File*. fn process_change( - out: &mut Vec, new_hunks_to_blame: &mut Vec, offset: &mut Offset, suspect: ObjectId, + parent: ObjectId, hunk: Option, change: Option, ) -> (Option, Option) { @@ -40,6 +40,8 @@ fn process_change( match (hunk, change) { (Some(hunk), Some(Change::Unchanged(unchanged))) => { let Some(range_in_suspect) = hunk.suspects.get(&suspect) else { + // We don’t clone blame to `parent` as `suspect` has nothing to do with this + // `hunk`. new_hunks_to_blame.push(hunk); return (None, Some(Change::Unchanged(unchanged))); }; @@ -64,7 +66,7 @@ fn process_change( // Nothing to do with `hunk` except shifting it, // but `unchanged` needs to be checked against the next hunk to catch up. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); (None, Some(Change::Unchanged(unchanged))) } (false, false) => { @@ -93,7 +95,7 @@ fn process_change( // Nothing to do with `hunk` except shifting it, // but `unchanged` needs to be checked against the next hunk to catch up. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); (None, Some(Change::Unchanged(unchanged))) } } @@ -123,7 +125,7 @@ fn process_change( } Either::Right((before, after)) => { // requeue the left side `before` after offsetting it… - new_hunks_to_blame.push(before.shift_by(suspect, *offset)); + new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); // …and treat `after` as `new_hunk`, which contains the `added` range. after } @@ -132,20 +134,18 @@ fn process_change( *offset += added.end - added.start; *offset -= number_of_lines_deleted; - // The overlapping `added` section was successfully located. - out.push(BlameEntry::with_offset( - added.clone(), - suspect, - hunk_starting_at_added.offset_for(suspect), - )); - + // The overlapping `added` section was successfully located. // Re-split at the end of `added` to continue with what's after. match hunk_starting_at_added.split_at(suspect, added.end) { - Either::Left(_) => { + Either::Left(hunk) => { + new_hunks_to_blame.push(hunk); + // Nothing to split, so we are done with this hunk. (None, None) } - Either::Right((_, after)) => { + Either::Right((hunk, after)) => { + new_hunks_to_blame.push(hunk); + // Keep processing the unblamed range after `added` (Some(after), None) } @@ -162,17 +162,13 @@ fn process_change( Either::Left(hunk) => hunk, Either::Right((before, after)) => { // Keep looking for the left side of the unblamed portion. - new_hunks_to_blame.push(before.shift_by(suspect, *offset)); + new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); after } }; // We can 'blame' the overlapping area of `added` and `hunk`. - out.push(BlameEntry::with_offset( - added.start..range_in_suspect.end, - suspect, - hunk_starting_at_added.offset_for(suspect), - )); + new_hunks_to_blame.push(hunk_starting_at_added); // Keep processing `added`, it's portion past `hunk` may still contribute. (None, Some(Change::AddedOrReplaced(added, number_of_lines_deleted))) } @@ -183,18 +179,20 @@ fn process_change( // <---> (blamed) // <--> (new hunk) - out.push(BlameEntry::with_offset( - range_in_suspect.start..added.end, - suspect, - hunk.offset_for(suspect), - )); - *offset += added.end - added.start; *offset -= number_of_lines_deleted; match hunk.split_at(suspect, added.end) { - Either::Left(_) => (None, None), - Either::Right((_, after)) => (Some(after), None), + Either::Left(hunk) => { + new_hunks_to_blame.push(hunk); + + (None, None) + } + Either::Right((before, after)) => { + new_hunks_to_blame.push(before); + + (Some(after), None) + } } } (false, false) => { @@ -222,7 +220,7 @@ fn process_change( // <----> (added) // Retry `hunk` once there is overlapping changes to process. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); // Let hunks catchup with this change. ( @@ -237,11 +235,7 @@ fn process_change( // <---> (blamed) // Successfully blame the whole range. - out.push(BlameEntry::with_offset( - range_in_suspect.clone(), - suspect, - hunk.offset_for(suspect), - )); + new_hunks_to_blame.push(hunk); // And keep processing `added` with future `hunks` that might be affected by it. ( @@ -279,7 +273,7 @@ fn process_change( } Either::Right((before, after)) => { // `before` isn't affected by deletion, so keep it for later. - new_hunks_to_blame.push(before.shift_by(suspect, *offset)); + new_hunks_to_blame.push(before.clone_blame(suspect, parent).shift_by(parent, *offset)); // after will be affected by offset, and we will see if there are more changes affecting it. after } @@ -291,7 +285,8 @@ fn process_change( // | (line_number_in_destination) // Catchup with changes. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); + ( None, Some(Change::Deleted(line_number_in_destination, number_of_lines_deleted)), @@ -300,7 +295,7 @@ fn process_change( } (Some(hunk), None) => { // nothing to do - changes are exhausted, re-evaluate `hunk`. - new_hunks_to_blame.push(hunk.shift_by(suspect, *offset)); + new_hunks_to_blame.push(hunk.clone_blame(suspect, parent).shift_by(parent, *offset)); (None, None) } (None, Some(Change::Unchanged(_))) => { @@ -328,10 +323,10 @@ fn process_change( /// Consume `hunks_to_blame` and `changes` to pair up matches ranges (also overlapping) with each other. /// Once a match is found, it's pushed onto `out`. fn process_changes( - out: &mut Vec, hunks_to_blame: Vec, changes: Vec, suspect: ObjectId, + parent: ObjectId, ) -> Vec { let mut hunks_iter = hunks_to_blame.into_iter(); let mut changes_iter = changes.into_iter(); @@ -344,10 +339,10 @@ fn process_changes( loop { (hunk, change) = process_change( - out, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, hunk, change, ); @@ -407,19 +402,6 @@ impl UnblamedHunk { } } - fn offset_for(&self, suspect: ObjectId) -> Offset { - let range_in_suspect = self - .suspects - .get(&suspect) - .expect("Internal and we know suspect is present"); - - if self.range_in_blamed_file.start > range_in_suspect.start { - Offset::Added(self.range_in_blamed_file.start - range_in_suspect.start) - } else { - Offset::Deleted(range_in_suspect.start - self.range_in_blamed_file.start) - } - } - /// Transfer all ranges from the commit at `from` to the commit at `to`. fn pass_blame(&mut self, from: ObjectId, to: ObjectId) { if let Some(range_in_suspect) = self.suspects.remove(&from) { @@ -427,10 +409,13 @@ impl UnblamedHunk { } } - fn clone_blame(&mut self, from: ObjectId, to: ObjectId) { + // TODO + // Should this also accept `&mut self` as the other functions do? + fn clone_blame(mut self, from: ObjectId, to: ObjectId) -> Self { if let Some(range_in_suspect) = self.suspects.get(&from) { self.suspects.insert(to, range_in_suspect.clone()); } + self } fn remove_blame(&mut self, suspect: ObjectId) { @@ -439,36 +424,6 @@ impl UnblamedHunk { } impl BlameEntry { - /// Create a new instance by creating `range_in_blamed_file` after applying `offset` to `range_in_source_file`. - fn with_offset(range_in_source_file: Range, commit_id: ObjectId, offset: Offset) -> Self { - debug_assert!( - range_in_source_file.end > range_in_source_file.start, - "{range_in_source_file:?}" - ); - - match offset { - Offset::Added(added) => Self { - start_in_blamed_file: range_in_source_file.start + added, - start_in_source_file: range_in_source_file.start, - len: force_non_zero(range_in_source_file.len() as u32), - commit_id, - }, - Offset::Deleted(deleted) => { - debug_assert!( - range_in_source_file.start >= deleted, - "{range_in_source_file:?} {offset:?}" - ); - - Self { - start_in_blamed_file: range_in_source_file.start - deleted, - start_in_source_file: range_in_source_file.start, - len: force_non_zero(range_in_source_file.len() as u32), - commit_id, - } - } - } - } - /// Create an offset from a portion of the *Blamed File*. fn from_unblamed_hunk(unblamed_hunk: &UnblamedHunk, commit_id: ObjectId) -> Option { let range_in_source_file = unblamed_hunk.suspects.get(&commit_id)?; diff --git a/gix-blame/src/file/tests.rs b/gix-blame/src/file/tests.rs index c6ed47b29c3..165c0e5c768 100644 --- a/gix-blame/src/file/tests.rs +++ b/gix-blame/src/file/tests.rs @@ -16,23 +16,24 @@ fn new_unblamed_hunk(range_in_blamed_file: Range, suspect: ObjectId, offset } mod process_change { + use std::str::FromStr; + use super::*; - use crate::file::{force_non_zero, process_change, Change, Offset, UnblamedHunk}; - use crate::BlameEntry; + use crate::file::{process_change, Change, Offset, UnblamedHunk}; use gix_hash::ObjectId; #[test] fn nothing() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, None, ); @@ -44,16 +45,16 @@ mod process_change { #[test] fn added_hunk() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(0..3, 0)), ); @@ -67,30 +68,27 @@ mod process_change { ); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 0..3, + suspects: [(suspect, 0..3)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn added_hunk_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(2..3, 0)), ); @@ -103,37 +101,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 2, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 0..2, - suspects: [(suspect, 0..2)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 0..2, + suspects: [(suspect, 0..2), (parent, 0..2)].into() + }, + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect, 2..3)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(5); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(10..15, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(12..13, 0)), ); @@ -146,37 +141,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 12, - start_in_source_file: 12, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 10..12, - suspects: [(suspect, 5..7)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 10..12, + suspects: [(suspect, 10..12), (parent, 5..7)].into() + }, + UnblamedHunk { + range_in_blamed_file: 12..13, + suspects: [(suspect, 12..13)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(6)); } #[test] fn added_hunk_4() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 7..12 Some(new_unblamed_hunk(12..17, suspect, Offset::Added(5))), Some(Change::AddedOrReplaced(9..10, 0)), @@ -190,37 +182,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 14, - start_in_source_file: 9, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 12..14, - suspects: [(suspect, 7..9)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 12..14, + suspects: [(suspect, 7..9), (parent, 7..9)].into() + }, + UnblamedHunk { + range_in_blamed_file: 14..15, + suspects: [(suspect, 9..10)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_5() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::AddedOrReplaced(0..3, 1)), ); @@ -234,30 +223,27 @@ mod process_change { ); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 0..3, + suspects: [(suspect, 0..3)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(2)); } #[test] fn added_hunk_6() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 0..4 Some(new_unblamed_hunk(1..5, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(0..3, 1)), @@ -272,30 +258,27 @@ mod process_change { ); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 1, - start_in_source_file: 0, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 1..4, + suspects: [(suspect, 0..3)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(2)); } #[test] fn added_hunk_7() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(2); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 2..6 Some(new_unblamed_hunk(3..7, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(3..5, 1)), @@ -309,37 +292,34 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 4, - start_in_source_file: 3, - len: force_non_zero(2), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 3..4, - suspects: [(suspect, 0..1)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 3..4, + suspects: [(suspect, 2..3), (parent, 0..1)].into() + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 3..5)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn added_hunk_8() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 25..26 Some(new_unblamed_hunk(23..24, suspect, Offset::Deleted(2))), Some(Change::AddedOrReplaced(25..27, 1)), @@ -348,30 +328,27 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(25..27, 1))); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 23, - start_in_source_file: 25, - len: force_non_zero(1), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 23..24, + suspects: [(suspect, 25..26)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_9() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 21..22 Some(new_unblamed_hunk(23..24, suspect, Offset::Added(2))), Some(Change::AddedOrReplaced(18..22, 3)), @@ -380,30 +357,27 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, None); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 23, - start_in_source_file: 21, - len: force_non_zero(1), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 23..24, + suspects: [(suspect, 21..22)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn added_hunk_10() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 70..108 Some(new_unblamed_hunk(71..109, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(106..109, 0)), @@ -411,37 +385,34 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(106..109, 0))); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 107, - start_in_source_file: 106, - len: force_non_zero(2), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 71..107, - suspects: [(suspect, 70..106)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 71..107, + suspects: [(suspect, 70..106), (parent, 70..106)].into() + }, + UnblamedHunk { + range_in_blamed_file: 107..109, + suspects: [(suspect, 106..108)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn added_hunk_11() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 137..144 Some(new_unblamed_hunk(149..156, suspect, Offset::Added(12))), Some(Change::AddedOrReplaced(143..146, 0)), @@ -449,37 +420,34 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(143..146, 0))); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 155, - start_in_source_file: 143, - len: force_non_zero(1), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 149..155, - suspects: [(suspect, 137..143)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 149..155, + suspects: [(suspect, 137..143), (parent, 137..143)].into() + }, + UnblamedHunk { + range_in_blamed_file: 155..156, + suspects: [(suspect, 143..144)].into() + } + ] ); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn no_overlap() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Deleted(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 2..5 Some(new_unblamed_hunk(3..6, suspect, Offset::Added(1))), Some(Change::AddedOrReplaced(7..10, 1)), @@ -487,12 +455,11 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(7..10, 1))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 3..6, - suspects: [(suspect, 5..8)].into() + suspects: [(suspect, 2..5), (parent, 5..8)].into() }] ); assert_eq!(offset_in_destination, Offset::Deleted(3)); @@ -500,16 +467,16 @@ mod process_change { #[test] fn no_overlap_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 6..8 Some(new_unblamed_hunk(9..11, suspect, Offset::Added(3))), Some(Change::AddedOrReplaced(2..5, 0)), @@ -523,23 +490,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn no_overlap_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 5..15 Some(new_unblamed_hunk(4..15, suspect, Offset::Deleted(1))), Some(Change::AddedOrReplaced(4..5, 1)), @@ -553,23 +519,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn no_overlap_4() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 25..27 Some(new_unblamed_hunk(23..25, suspect, Offset::Deleted(2))), Some(Change::Unchanged(21..22)), @@ -583,23 +548,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } #[test] fn no_overlap_5() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 17..18 Some(new_unblamed_hunk(15..16, suspect, Offset::Deleted(2))), Some(Change::Deleted(20, 1)), @@ -607,12 +571,11 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::Deleted(20, 1))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 15..16, - suspects: [(suspect, 16..17)].into() + suspects: [(suspect, 17..18), (parent, 16..17)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(1)); @@ -620,16 +583,16 @@ mod process_change { #[test] fn no_overlap_6() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 22..24 Some(new_unblamed_hunk(23..25, suspect, Offset::Added(1))), Some(Change::Deleted(20, 1)), @@ -643,23 +606,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(1)); } #[test] fn enclosing_addition() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 5..8 Some(new_unblamed_hunk(2..5, suspect, Offset::Deleted(3))), Some(Change::AddedOrReplaced(3..12, 2)), @@ -668,30 +630,27 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::AddedOrReplaced(3..12, 2))); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 5, - len: force_non_zero(3), - commit_id: suspect + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 2..5, + suspects: [(suspect, 5..8)].into() }] ); - assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn enclosing_deletion() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 13..20 Some(new_unblamed_hunk(12..19, suspect, Offset::Deleted(1))), Some(Change::Deleted(15, 2)), @@ -705,12 +664,11 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 12..14, - suspects: [(suspect, 10..12)].into() + suspects: [(suspect, 13..15), (parent, 10..12)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(1)); @@ -718,16 +676,16 @@ mod process_change { #[test] fn enclosing_unchanged_lines() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(3); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, // range_in_destination: 109..113 Some(new_unblamed_hunk(110..114, suspect, Offset::Added(1))), Some(Change::Unchanged(109..172)), @@ -735,12 +693,11 @@ mod process_change { assert_eq!(hunk, None); assert_eq!(change, Some(Change::Unchanged(109..172))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 110..114, - suspects: [(suspect, 106..110)].into() + suspects: [(suspect, 109..113), (parent, 106..110)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(3)); @@ -748,16 +705,16 @@ mod process_change { #[test] fn unchanged_hunk() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::Unchanged(0..3)), ); @@ -770,35 +727,33 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(0)); } #[test] fn unchanged_hunk_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::Unchanged(0..7)), ); assert_eq!(hunk, None); assert_eq!(change, Some(Change::Unchanged(0..7))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 0..5, - suspects: [(suspect, 0..5)].into() + suspects: [(suspect, 0..5), (parent, 0..5)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(0)); @@ -806,16 +761,16 @@ mod process_change { #[test] fn unchanged_hunk_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Deleted(2); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(UnblamedHunk { range_in_blamed_file: 22..30, suspects: [(suspect, 21..29)].into(), @@ -831,35 +786,33 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(2)); } #[test] fn deleted_hunk() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(0..5, suspect, Offset::Added(0))), Some(Change::Deleted(5, 3)), ); assert_eq!(hunk, None); assert_eq!(change, Some(Change::Deleted(5, 3))); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, [UnblamedHunk { range_in_blamed_file: 0..5, - suspects: [(suspect, 0..5)].into() + suspects: [(suspect, 0..5), (parent, 0..5)].into() }] ); assert_eq!(offset_in_destination, Offset::Added(0)); @@ -867,16 +820,16 @@ mod process_change { #[test] fn deleted_hunk_2() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(2..16, suspect, Offset::Added(0))), Some(Change::Deleted(0, 4)), ); @@ -889,23 +842,22 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(4)); } #[test] fn deleted_hunk_3() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(0); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, Some(new_unblamed_hunk(2..16, suspect, Offset::Added(0))), Some(Change::Deleted(14, 4)), ); @@ -918,368 +870,345 @@ mod process_change { }) ); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!( new_hunks_to_blame, - [new_unblamed_hunk(2..14, suspect, Offset::Added(0))] + [UnblamedHunk { + range_in_blamed_file: 2..14, + suspects: [(suspect, 2..14), (parent, 2..14)].into() + }] ); assert_eq!(offset_in_destination, Offset::Deleted(4)); } #[test] fn addition_only() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, Some(Change::AddedOrReplaced(22..25, 1)), ); assert_eq!(hunk, None); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(3)); } #[test] fn deletion_only() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, Some(Change::Deleted(11, 5)), ); assert_eq!(hunk, None); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Deleted(4)); } #[test] fn unchanged_only() { - let mut lines_blamed = Vec::new(); let mut new_hunks_to_blame = Vec::new(); let mut offset_in_destination: Offset = Offset::Added(1); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let (hunk, change) = process_change( - &mut lines_blamed, &mut new_hunks_to_blame, &mut offset_in_destination, suspect, + parent, None, Some(Change::Unchanged(11..13)), ); assert_eq!(hunk, None); assert_eq!(change, None); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); assert_eq!(offset_in_destination, Offset::Added(1)); } } + mod process_changes { + use std::str::FromStr; + use crate::file::tests::new_unblamed_hunk; - use crate::file::{force_non_zero, process_changes, Change, Offset, UnblamedHunk}; - use crate::BlameEntry; + use crate::file::{process_changes, Change, Offset, UnblamedHunk}; use gix_hash::ObjectId; #[test] fn nothing() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let new_hunks_to_blame = process_changes(&mut lines_blamed, vec![], vec![], suspect); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); + let new_hunks_to_blame = process_changes(vec![], vec![], suspect, parent); - assert_eq!(lines_blamed, []); assert_eq!(new_hunks_to_blame, []); } #[test] fn added_hunk() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..4, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..4, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into(), + },] ); - assert_eq!(new_hunks_to_blame, []); } #[test] fn added_hunk_2() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..4, 0), Change::Unchanged(4..6)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] + new_hunks_to_blame, + [ + UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 0..2)].into(), + }, + ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(4))]); } #[test] fn added_hunk_3() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![ Change::Unchanged(0..2), Change::AddedOrReplaced(2..4, 0), Change::Unchanged(4..6), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 2, - len: force_non_zero(2), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, [ - new_unblamed_hunk(0..2, suspect, Offset::Added(0)), - new_unblamed_hunk(4..6, suspect, Offset::Added(2)) + UnblamedHunk { + range_in_blamed_file: 0..2, + suspects: [(suspect, 0..2), (parent, 0..2)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 2..4, + suspects: [(suspect, 2..4)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 2..4)].into(), + }, ] ); } #[test] fn added_hunk_4_0() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![ Change::AddedOrReplaced(0..1, 0), Change::AddedOrReplaced(1..4, 0), Change::Unchanged(4..6), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 0..1, + suspects: [(suspect, 0..1)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 1..4, + suspects: [(suspect, 1..4)].into(), }, - BlameEntry { - start_in_blamed_file: 1, - start_in_source_file: 1, - len: force_non_zero(3), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 0..2)].into(), } ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(4))]); } #[test] fn added_hunk_4_1() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..1, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect - }] + new_hunks_to_blame, + [ + UnblamedHunk { + range_in_blamed_file: 0..1, + suspects: [(suspect, 0..1)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 1..6, + suspects: [(suspect, 1..6), (parent, 0..5)].into(), + } + ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(1..6, suspect, Offset::Added(1))]); } #[test] fn added_hunk_4_2() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let suspect_2 = ObjectId::from_hex(b"2222222222222222222222222222222222222222").unwrap(); - let mut lines_blamed: Vec = vec![BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(2), - commit_id: suspect, - }]; let hunks_to_blame = vec![new_unblamed_hunk(2..6, suspect_2, Offset::Added(2))]; let changes = vec![Change::AddedOrReplaced(0..1, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect_2); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect_2, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(2), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect_2, 0..1)].into(), }, - BlameEntry { - start_in_blamed_file: 2, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect_2 + UnblamedHunk { + range_in_blamed_file: 3..6, + suspects: [(suspect_2, 1..4), (parent, 0..3)].into(), } ] ); - assert_eq!( - new_hunks_to_blame, - [new_unblamed_hunk(3..6, suspect_2, Offset::Added(3))] - ); } #[test] fn added_hunk_5() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..6, suspect, Offset::Added(0))]; let changes = vec![Change::AddedOrReplaced(0..4, 3), Change::Unchanged(4..6)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] + new_hunks_to_blame, + [ + UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 4..6), (parent, 3..5)].into(), + } + ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(1))]); } #[test] fn added_hunk_6() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(4..6, suspect, Offset::Added(1))]; let changes = vec![Change::AddedOrReplaced(0..3, 0), Change::Unchanged(3..5)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); - assert_eq!(lines_blamed, []); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(4..6, suspect, Offset::Added(4))]); + assert_eq!( + new_hunks_to_blame, + [UnblamedHunk { + range_in_blamed_file: 4..6, + suspects: [(suspect, 3..5), (parent, 0..2)].into(), + }] + ); } #[test] fn added_hunk_7() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let suspect_2 = ObjectId::from_hex(b"2222222222222222222222222222222222222222").unwrap(); - let mut lines_blamed: Vec = vec![BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect, - }]; - let hunks_to_blame = vec![new_unblamed_hunk(1..3, suspect_2, Offset::Added(1))]; + let suspect = ObjectId::from_hex(b"0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); + let hunks_to_blame = vec![new_unblamed_hunk(1..3, suspect, Offset::Added(1))]; let changes = vec![Change::AddedOrReplaced(0..1, 2)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect_2); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 1..2, + suspects: [(suspect, 0..1)].into(), }, - BlameEntry { - start_in_blamed_file: 1, - start_in_source_file: 0, - len: force_non_zero(1), - commit_id: suspect_2 + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect, 1..2), (parent, 2..3)].into(), } ] ); - assert_eq!( - new_hunks_to_blame, - [new_unblamed_hunk(2..3, suspect_2, Offset::Added(0))] - ); } #[test] fn added_hunk_8() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let mut lines_blamed = Vec::new(); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![new_unblamed_hunk(0..4, suspect, Offset::Added(0))]; let changes = vec![ Change::AddedOrReplaced(0..2, 0), Change::Unchanged(2..3), Change::AddedOrReplaced(3..4, 0), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(2), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 0..2, + suspects: [(suspect, 0..2)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 2..3, + suspects: [(suspect, 2..3), (parent, 0..1)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 3..4, + suspects: [(suspect, 3..4)].into(), }, - BlameEntry { - start_in_blamed_file: 3, - start_in_source_file: 3, - len: force_non_zero(1), - commit_id: suspect - } ] ); - assert_eq!(new_hunks_to_blame, [new_unblamed_hunk(2..3, suspect, Offset::Added(2))]); } #[test] fn added_hunk_9() { - let suspect = ObjectId::null(gix_hash::Kind::Sha1); - let mut lines_blamed: Vec = vec![BlameEntry { - start_in_blamed_file: 30, - start_in_source_file: 30, - len: force_non_zero(1), - commit_id: suspect, - }]; + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![ UnblamedHunk { range_in_blamed_file: 0..30, @@ -1295,72 +1224,106 @@ mod process_changes { Change::AddedOrReplaced(16..17, 0), Change::Unchanged(17..37), ]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); - - lines_blamed.sort_by(|a, b| a.start_in_blamed_file.cmp(&b.start_in_blamed_file)); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); assert_eq!( - lines_blamed, + new_hunks_to_blame, [ - BlameEntry { - start_in_blamed_file: 16, - start_in_source_file: 16, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 0..16, + suspects: [(suspect, 0..16), (parent, 0..16)].into() + }, + UnblamedHunk { + range_in_blamed_file: 16..17, + suspects: [(suspect, 16..17)].into() + }, + UnblamedHunk { + range_in_blamed_file: 17..30, + suspects: [(suspect, 17..30), (parent, 16..29)].into() }, - BlameEntry { - start_in_blamed_file: 30, - start_in_source_file: 30, - len: force_non_zero(1), - commit_id: suspect + UnblamedHunk { + range_in_blamed_file: 31..37, + suspects: [(suspect, 31..37), (parent, 30..36)].into() } ] ); + } + + #[test] + fn added_hunk_10() { + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); + let hunks_to_blame = vec![ + UnblamedHunk { + range_in_blamed_file: 1..3, + suspects: [(suspect, 1..3)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 5..7, + suspects: [(suspect, 5..7)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 8..10, + suspects: [(suspect, 8..10)].into(), + }, + ]; + let changes = vec![ + Change::Unchanged(0..6), + Change::AddedOrReplaced(6..9, 0), + Change::Unchanged(9..11), + ]; + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); + assert_eq!( new_hunks_to_blame, [ UnblamedHunk { - range_in_blamed_file: 0..16, - suspects: [(suspect, 0..16)].into() + range_in_blamed_file: 1..3, + suspects: [(suspect, 1..3), (parent, 1..3)].into(), }, UnblamedHunk { - range_in_blamed_file: 17..30, - suspects: [(suspect, 16..29)].into() + range_in_blamed_file: 5..6, + suspects: [(suspect, 5..6), (parent, 5..6)].into(), }, UnblamedHunk { - range_in_blamed_file: 31..37, - suspects: [(suspect, 30..36)].into() - } + range_in_blamed_file: 6..7, + suspects: [(suspect, 6..7)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 8..9, + suspects: [(suspect, 8..9)].into(), + }, + UnblamedHunk { + range_in_blamed_file: 9..10, + suspects: [(suspect, 9..10), (parent, 6..7)].into(), + }, ] ); } #[test] fn deleted_hunk() { - let mut lines_blamed = Vec::new(); - let suspect = ObjectId::null(gix_hash::Kind::Sha1); + let suspect = ObjectId::from_str("0000000000000000000000000000000000000000").unwrap(); + let parent = ObjectId::from_str("1111111111111111111111111111111111111111").unwrap(); let hunks_to_blame = vec![ new_unblamed_hunk(0..4, suspect, Offset::Added(0)), new_unblamed_hunk(4..7, suspect, Offset::Added(0)), ]; let changes = vec![Change::Deleted(0, 3), Change::AddedOrReplaced(0..4, 0)]; - let new_hunks_to_blame = process_changes(&mut lines_blamed, hunks_to_blame, changes, suspect); + let new_hunks_to_blame = process_changes(hunks_to_blame, changes, suspect, parent); - assert_eq!( - lines_blamed, - [BlameEntry { - start_in_blamed_file: 0, - start_in_source_file: 0, - len: force_non_zero(4), - commit_id: suspect - }] - ); assert_eq!( new_hunks_to_blame, - [UnblamedHunk { - range_in_blamed_file: 4..7, - suspects: [(suspect, 3..6)].into() - }] + [ + UnblamedHunk { + range_in_blamed_file: 0..4, + suspects: [(suspect, 0..4)].into() + }, + UnblamedHunk { + range_in_blamed_file: 4..7, + suspects: [(suspect, 4..7), (parent, 3..6)].into() + } + ] ); } } diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index 258a3457c4a..48926531c1a 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -234,6 +234,7 @@ mktest!( 1 ); mktest!(file_only_changed_in_branch, "file-only-changed-in-branch", 2); +mktest!(file_changed_in_two_branches, "file-changed-in-two-branches", 3); /// As of 2024-09-24, these tests are expected to fail. /// diff --git a/gix-blame/tests/fixtures/make_blame_repo.sh b/gix-blame/tests/fixtures/make_blame_repo.sh index 31e30c42e4d..9a62d45fc1a 100755 --- a/gix-blame/tests/fixtures/make_blame_repo.sh +++ b/gix-blame/tests/fixtures/make_blame_repo.sh @@ -181,6 +181,23 @@ git commit -q -m c13 git checkout main git merge branch-that-has-one-commit || true +echo -e "line 1\nline 2\n line 3" > file-changed-in-two-branches.txt +git add file-changed-in-two-branches.txt +git commit -q -m c14 + +git checkout -b branch-that-has-one-of-the-changes + +echo -e "line 1\nline 2\n line 3 changed" > file-changed-in-two-branches.txt +git add file-changed-in-two-branches.txt +git commit -q -m c14.1 + +git checkout main +echo -e "line 1 changed\nline 2\n line 3" > file-changed-in-two-branches.txt +git add file-changed-in-two-branches.txt +git commit -q -m c14.2 + +git merge branch-that-has-one-of-the-changes || true + git blame --porcelain simple.txt > .git/simple.baseline git blame --porcelain multiline-hunks.txt > .git/multiline-hunks.baseline git blame --porcelain deleted-lines.txt > .git/deleted-lines.baseline @@ -198,6 +215,7 @@ git blame --porcelain resolved-conflict.txt > .git/resolved-conflict.baseline git blame --porcelain file-in-one-chain-of-ancestors.txt > .git/file-in-one-chain-of-ancestors.baseline git blame --porcelain different-file-in-another-chain-of-ancestors.txt > .git/different-file-in-another-chain-of-ancestors.baseline git blame --porcelain file-only-changed-in-branch.txt > .git/file-only-changed-in-branch.baseline +git blame --porcelain file-changed-in-two-branches.txt > .git/file-changed-in-two-branches.baseline git blame --porcelain empty-lines-histogram.txt > .git/empty-lines-histogram.baseline From cbf7f515b10fba96d116c1404913cdc09eace9a3 Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 9 Jan 2025 21:57:38 +0100 Subject: [PATCH 2/2] Follow clippy suggestions --- gix-blame/src/file/function.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 1365b23bad7..357abe555fe 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -232,7 +232,7 @@ where std::collections::BTreeMap::>>::new(), |mut acc, hunk| { for (suspect, range) in hunk.suspects.clone() { - acc.entry(suspect).or_insert(Vec::new()).push(range); + acc.entry(suspect).or_default().push(range); } acc @@ -243,11 +243,8 @@ where ranges.sort_by(|a, b| a.start.cmp(&b.start)); for window in ranges.windows(2) { - match window { - [a, b] => { - assert!(a.end <= b.start, "#{hunks_to_blame:#?}"); - } - _ => {} + if let [a, b] = window { + assert!(a.end <= b.start, "#{hunks_to_blame:#?}"); } } }