Skip to content

Commit

Permalink
refactor: remove base from merge signature
Browse files Browse the repository at this point in the history
  • Loading branch information
jpedroh committed May 25, 2024
1 parent 83a66ab commit 554a544
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 82 deletions.
1 change: 0 additions & 1 deletion bin/src/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 16 additions & 14 deletions merge/src/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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,
Expand All @@ -50,7 +60,7 @@ pub fn merge<'a>(
)?)
}
}
(_, _, _) => {
(_, _) => {
log::debug!(
"Error while merging NonTerminal with Terminal {} and {}",
left.contents(),
Expand All @@ -75,14 +85,6 @@ mod tests {
#[test]
fn test_can_not_merge_terminal_with_non_terminal() -> Result<(), Box<dyn std::error::Error>> {
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",
Expand Down
78 changes: 17 additions & 61 deletions merge/src/merge_terminals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MergedCSTNode<'a>, 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,
}),
}
}

Expand All @@ -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,
Expand All @@ -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(),
Expand Down Expand Up @@ -108,7 +92,7 @@ mod tests {
};

assert_merge_is_correct_and_idempotent_with_respect_to_parent_side(
&base,
Some(&base),
&left,
&right,
&MergedCSTNode::Terminal {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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())
);
}
}
2 changes: 0 additions & 2 deletions merge/src/ordered_merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 0 additions & 4 deletions merge/src/unordered_merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit 554a544

Please sign in to comment.