From 554a5449a81182f943215ad544b91d8fe664a887 Mon Sep 17 00:00:00 2001 From: Joao Duarte Date: Sat, 25 May 2024 14:09:14 -0400 Subject: [PATCH] refactor: remove base from merge signature --- bin/src/control.rs | 1 - merge/src/merge.rs | 30 +++++++------- merge/src/merge_terminals.rs | 78 ++++++++---------------------------- merge/src/ordered_merge.rs | 2 - merge/src/unordered_merge.rs | 4 -- 5 files changed, 33 insertions(+), 82 deletions(-) diff --git a/bin/src/control.rs b/bin/src/control.rs index 0c5adbc..f26d47c 100644 --- a/bin/src/control.rs +++ b/bin/src/control.rs @@ -74,7 +74,6 @@ pub fn run_tool_on_merge_scenario( matching::calculate_matchings(&left_tree, &right_tree, &matching_configuration); let result = merge::merge( - &base_tree, &left_tree, &right_tree, &matchings_left_base, diff --git a/merge/src/merge.rs b/merge/src/merge.rs index 6f12709..62ed72e 100644 --- a/merge/src/merge.rs +++ b/merge/src/merge.rs @@ -8,7 +8,6 @@ use model::CSTNode; use crate::merged_cst_node::MergedCSTNode; pub fn merge<'a>( - base: &'a CSTNode<'a>, left: &'a CSTNode<'a>, right: &'a CSTNode<'a>, base_left_matchings: &'a Matchings<'a>, @@ -27,11 +26,22 @@ pub fn merge<'a>( )); } - match (base, left, right) { - (CSTNode::Terminal(a_base), CSTNode::Terminal(a_left), CSTNode::Terminal(a_right)) => { - merge_terminals(a_base, a_left, a_right) + match (left, right) { + (CSTNode::Terminal(a_left), CSTNode::Terminal(a_right)) => { + let matching_base_left = base_left_matchings.find_matching_for(left); + let matching_base_right = base_right_matchings.find_matching_for(right); + assert_eq!(matching_base_left, matching_base_right); + + let base = matching_base_left + .map(|matching| matching.matching_node) + .and_then(|node| match node { + CSTNode::Terminal(terminal) => Some(terminal), + _ => None, + }); + + merge_terminals(base, a_left, a_right) } - (CSTNode::NonTerminal(_), CSTNode::NonTerminal(a_left), CSTNode::NonTerminal(a_right)) => { + (CSTNode::NonTerminal(a_left), CSTNode::NonTerminal(a_right)) => { if a_left.are_children_unordered && a_right.are_children_unordered { Ok(unordered_merge( a_left, @@ -50,7 +60,7 @@ pub fn merge<'a>( )?) } } - (_, _, _) => { + (_, _) => { log::debug!( "Error while merging NonTerminal with Terminal {} and {}", left.contents(), @@ -75,14 +85,6 @@ mod tests { #[test] fn test_can_not_merge_terminal_with_non_terminal() -> Result<(), Box> { let error = merge( - &CSTNode::Terminal(Terminal { - id: uuid::Uuid::new_v4(), - kind: "kind", - start_position: Point { row: 0, column: 0 }, - end_position: Point { row: 0, column: 7 }, - value: "value", - is_block_end_delimiter: false, - }), &CSTNode::Terminal(Terminal { id: uuid::Uuid::new_v4(), kind: "kind", diff --git a/merge/src/merge_terminals.rs b/merge/src/merge_terminals.rs index 7eeed52..68801bc 100644 --- a/merge/src/merge_terminals.rs +++ b/merge/src/merge_terminals.rs @@ -3,39 +3,23 @@ use model::cst_node::Terminal; use crate::{MergeError, MergedCSTNode}; pub fn merge_terminals<'a>( - base: &'a Terminal<'a>, + base: Option<&'a Terminal<'a>>, left: &'a Terminal<'a>, right: &'a Terminal<'a>, ) -> Result, MergeError> { - // Nodes of different kind, early return - if left.kind != right.kind { - return Err(MergeError::NodesWithDifferentKinds( - left.kind.to_string(), - right.kind.to_string(), - )); + if left.value == right.value { + return Ok(left.to_owned().into()); } - // Unchanged - if left.value == base.value && right.value == base.value { - Ok(base.to_owned().into()) - // Changed in both - } else if left.value != base.value && right.value != base.value { - match diffy::merge(base.value, left.value, right.value) { - Ok(value) => Ok(MergedCSTNode::Terminal { - kind: base.kind, - value, - }), - Err(value) => Ok(MergedCSTNode::Terminal { - kind: base.kind, - value, - }), - } - // Only left changed - } else if left.value != base.value { - Ok(left.to_owned().into()) - // Only right changed - } else { - Ok(right.to_owned().into()) + match diffy::merge(base.map_or("", |node| node.value), left.value, right.value) { + Ok(value) => Ok(MergedCSTNode::Terminal { + kind: left.kind, + value, + }), + Err(value) => Ok(MergedCSTNode::Terminal { + kind: left.kind, + value, + }), } } @@ -46,7 +30,7 @@ mod tests { use model::{cst_node::Terminal, Point}; fn assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - base: &Terminal, + base: Option<&Terminal>, parent_a: &Terminal, parent_b: &Terminal, expected_merge: &MergedCSTNode, @@ -72,7 +56,7 @@ mod tests { }; assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - &node, + Some(&node), &node, &node, &node.clone().into(), @@ -108,7 +92,7 @@ mod tests { }; assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - &base, + Some(&base), &left, &right, &MergedCSTNode::Terminal { @@ -147,7 +131,7 @@ mod tests { }; assert_eq!( - merge_terminals(&base, &left, &right)?, + merge_terminals(Some(&base), &left, &right)?, MergedCSTNode::Terminal { kind: "kind", value: "<<<<<<< ours\nleft_value||||||| original\nvalue=======\nright_value>>>>>>> theirs\n".to_string() @@ -178,38 +162,10 @@ mod tests { }; assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - &base_and_left, + Some(&base_and_left), &base_and_left, &changed_parent, &changed_parent.clone().into(), ) } - - #[test] - fn i_get_an_error_if_i_try_to_merge_nodes_of_different_kinds() { - let kind_a = Terminal { - id: uuid::Uuid::new_v4(), - kind: "kind_a", - start_position: Point { row: 0, column: 0 }, - end_position: Point { row: 0, column: 7 }, - value: "value", - is_block_end_delimiter: false, - }; - let kind_b = Terminal { - id: uuid::Uuid::new_v4(), - kind: "kind_b", - start_position: Point { row: 0, column: 0 }, - end_position: Point { row: 0, column: 7 }, - value: "value_right", - is_block_end_delimiter: false, - }; - - let result = merge_terminals(&kind_a, &kind_a, &kind_b); - - assert!(result.is_err()); - assert_eq!( - result.unwrap_err(), - MergeError::NodesWithDifferentKinds("kind_a".to_string(), "kind_b".to_string()) - ); - } } diff --git a/merge/src/ordered_merge.rs b/merge/src/ordered_merge.rs index eff9d01..0d375fd 100644 --- a/merge/src/ordered_merge.rs +++ b/merge/src/ordered_merge.rs @@ -43,7 +43,6 @@ pub fn ordered_merge<'a>( ) { (true, Some(_), Some(_), Some(_), Some(_)) => { result_children.push(crate::merge( - cur_left, cur_left, cur_right, base_left_matchings, @@ -56,7 +55,6 @@ pub fn ordered_merge<'a>( } (true, Some(_), None, Some(_), None) => { result_children.push(crate::merge( - cur_left, cur_left, cur_right, base_left_matchings, diff --git a/merge/src/unordered_merge.rs b/merge/src/unordered_merge.rs index 7df782f..6acf8b3 100644 --- a/merge/src/unordered_merge.rs +++ b/merge/src/unordered_merge.rs @@ -48,7 +48,6 @@ pub fn unordered_merge<'a>( } (None, Some(right_matching)) => { result_children.push(merge( - left_child, left_child, right_matching.matching_node, base_left_matchings, @@ -71,7 +70,6 @@ pub fn unordered_merge<'a>( } (Some(_), Some(right_matching)) => { result_children.push(merge( - left_child, left_child, right_matching.matching_node, base_left_matchings, @@ -99,7 +97,6 @@ pub fn unordered_merge<'a>( } (None, Some(matching_left_right)) => { result_children.push(merge( - right_child, matching_left_right.matching_node, right_child, base_left_matchings, @@ -119,7 +116,6 @@ pub fn unordered_merge<'a>( } (Some(_), Some(matching_left_right)) => { result_children.push(merge( - right_child, matching_left_right.matching_node, right_child, base_left_matchings,