diff --git a/bin/src/control.rs b/bin/src/control.rs index 0cdb218..600fee5 100644 --- a/bin/src/control.rs +++ b/bin/src/control.rs @@ -87,6 +87,7 @@ pub fn run_tool_on_merge_scenario( log::info!("Starting merge of the trees"); 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 62ed72e..6f12709 100644 --- a/merge/src/merge.rs +++ b/merge/src/merge.rs @@ -8,6 +8,7 @@ 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>, @@ -26,22 +27,11 @@ pub fn merge<'a>( )); } - 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) + match (base, left, right) { + (CSTNode::Terminal(a_base), CSTNode::Terminal(a_left), CSTNode::Terminal(a_right)) => { + merge_terminals(a_base, a_left, a_right) } - (CSTNode::NonTerminal(a_left), CSTNode::NonTerminal(a_right)) => { + (CSTNode::NonTerminal(_), CSTNode::NonTerminal(a_left), CSTNode::NonTerminal(a_right)) => { if a_left.are_children_unordered && a_right.are_children_unordered { Ok(unordered_merge( a_left, @@ -60,7 +50,7 @@ pub fn merge<'a>( )?) } } - (_, _) => { + (_, _, _) => { log::debug!( "Error while merging NonTerminal with Terminal {} and {}", left.contents(), @@ -85,6 +75,14 @@ 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 68801bc..7eeed52 100644 --- a/merge/src/merge_terminals.rs +++ b/merge/src/merge_terminals.rs @@ -3,23 +3,39 @@ use model::cst_node::Terminal; use crate::{MergeError, MergedCSTNode}; pub fn merge_terminals<'a>( - base: Option<&'a Terminal<'a>>, + base: &'a Terminal<'a>, left: &'a Terminal<'a>, right: &'a Terminal<'a>, ) -> Result, MergeError> { - if left.value == right.value { - return Ok(left.to_owned().into()); + // Nodes of different kind, early return + if left.kind != right.kind { + return Err(MergeError::NodesWithDifferentKinds( + left.kind.to_string(), + right.kind.to_string(), + )); } - 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, - }), + // 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()) } } @@ -30,7 +46,7 @@ mod tests { use model::{cst_node::Terminal, Point}; fn assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - base: Option<&Terminal>, + base: &Terminal, parent_a: &Terminal, parent_b: &Terminal, expected_merge: &MergedCSTNode, @@ -56,7 +72,7 @@ mod tests { }; assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - Some(&node), + &node, &node, &node, &node.clone().into(), @@ -92,7 +108,7 @@ mod tests { }; assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - Some(&base), + &base, &left, &right, &MergedCSTNode::Terminal { @@ -131,7 +147,7 @@ mod tests { }; assert_eq!( - merge_terminals(Some(&base), &left, &right)?, + merge_terminals(&base, &left, &right)?, MergedCSTNode::Terminal { kind: "kind", value: "<<<<<<< ours\nleft_value||||||| original\nvalue=======\nright_value>>>>>>> theirs\n".to_string() @@ -162,10 +178,38 @@ mod tests { }; assert_merge_is_correct_and_idempotent_with_respect_to_parent_side( - Some(&base_and_left), + &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 0d375fd..eff9d01 100644 --- a/merge/src/ordered_merge.rs +++ b/merge/src/ordered_merge.rs @@ -43,6 +43,7 @@ pub fn ordered_merge<'a>( ) { (true, Some(_), Some(_), Some(_), Some(_)) => { result_children.push(crate::merge( + cur_left, cur_left, cur_right, base_left_matchings, @@ -55,6 +56,7 @@ 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 6acf8b3..7df782f 100644 --- a/merge/src/unordered_merge.rs +++ b/merge/src/unordered_merge.rs @@ -48,6 +48,7 @@ pub fn unordered_merge<'a>( } (None, Some(right_matching)) => { result_children.push(merge( + left_child, left_child, right_matching.matching_node, base_left_matchings, @@ -70,6 +71,7 @@ pub fn unordered_merge<'a>( } (Some(_), Some(right_matching)) => { result_children.push(merge( + left_child, left_child, right_matching.matching_node, base_left_matchings, @@ -97,6 +99,7 @@ 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, @@ -116,6 +119,7 @@ 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,