From b9ecfdbeb97de7554218f716a6e96e16418bb091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Pedro=20Henrique?= Date: Fri, 23 Aug 2024 21:19:13 -0300 Subject: [PATCH] fix: Issues on merge algorithm (#68) --- bin/src/control.rs | 2 + .../incorrect_merge_conflict/base.java | 15 +++ .../incorrect_merge_conflict/left.java | 16 +++ .../incorrect_merge_conflict/merge.java | 7 + .../incorrect_merge_conflict/right.java | 9 ++ .../unordered_with_non_labelled/merge.java | 14 +- merge/src/ordered_merge.rs | 122 ++++++++++++------ parsing/src/tree_sitter_parser.rs | 8 ++ 8 files changed, 144 insertions(+), 49 deletions(-) create mode 100644 bin/tests/scenarios/incorrect_merge_conflict/base.java create mode 100644 bin/tests/scenarios/incorrect_merge_conflict/left.java create mode 100644 bin/tests/scenarios/incorrect_merge_conflict/merge.java create mode 100644 bin/tests/scenarios/incorrect_merge_conflict/right.java diff --git a/bin/src/control.rs b/bin/src/control.rs index 036e855..3ce6d5f 100644 --- a/bin/src/control.rs +++ b/bin/src/control.rs @@ -50,10 +50,12 @@ pub fn run_tool_on_merge_scenario( right: &str, ) -> Result { if base == left { + log::info!("Early returning because base equals left"); return Ok(ExecutionResult::WithoutConflicts(right.to_string())); } if base == right { + log::info!("Early returning because base equals right"); return Ok(ExecutionResult::WithoutConflicts(left.to_string())); } diff --git a/bin/tests/scenarios/incorrect_merge_conflict/base.java b/bin/tests/scenarios/incorrect_merge_conflict/base.java new file mode 100644 index 0000000..fd94228 --- /dev/null +++ b/bin/tests/scenarios/incorrect_merge_conflict/base.java @@ -0,0 +1,15 @@ +public class Main { + public static void main() { + return new PagedIterable() { + public PagedIterator _iterator(int pageSize) { + return new PagedIterator(retrieve().with("since",since).asIterator("/repositories", GHRepository[].class, pageSize)) { + @Override + protected void wrapUp(GHRepository[] page) { + for (GHRepository c : page) + c.wrap(GitHub.this); + } + }; + } + }; + } +} diff --git a/bin/tests/scenarios/incorrect_merge_conflict/left.java b/bin/tests/scenarios/incorrect_merge_conflict/left.java new file mode 100644 index 0000000..2535508 --- /dev/null +++ b/bin/tests/scenarios/incorrect_merge_conflict/left.java @@ -0,0 +1,16 @@ +public class Main { + public static void main() { + return new PagedIterable() { + @Override + public PagedIterator _iterator(int pageSize) { + return new PagedIterator(retrieve().with("since",since).asIterator("/repositories", GHRepository[].class, pageSize)) { + @Override + protected void wrapUp(GHRepository[] page) { + for (GHRepository c : page) + c.wrap(GitHub.this); + } + }; + } + }; + } +} diff --git a/bin/tests/scenarios/incorrect_merge_conflict/merge.java b/bin/tests/scenarios/incorrect_merge_conflict/merge.java new file mode 100644 index 0000000..ad6b7ca --- /dev/null +++ b/bin/tests/scenarios/incorrect_merge_conflict/merge.java @@ -0,0 +1,7 @@ + public class Main { public static void main ( ) { return +<<<<<<< + new PagedIterable < GHRepository > ( ) { @ Override public PagedIterator < GHRepository > _iterator ( int pageSize ) { return new PagedIterator < GHRepository > ( retrieve ( ) . with ( "since" , since ) . asIterator ( "/repositories" , GHRepository [ ] . class , pageSize ) ) { @ Override protected void wrapUp ( GHRepository [ ] page ) { for ( GHRepository c : page ) c . wrap ( GitHub . this ) ; } } ; } } +======= + retrieve ( ) . with ( "since" , since ) . asPagedIterable ( "/repositories" , GHRepository [ ] . class , item -> item . wrap ( GitHub . this ) ) +>>>>>>> + ; } } diff --git a/bin/tests/scenarios/incorrect_merge_conflict/right.java b/bin/tests/scenarios/incorrect_merge_conflict/right.java new file mode 100644 index 0000000..3895e71 --- /dev/null +++ b/bin/tests/scenarios/incorrect_merge_conflict/right.java @@ -0,0 +1,9 @@ +public class Main { + public static void main() { + return retrieve().with("since",since) + .asPagedIterable( + "/repositories", + GHRepository[].class, + item -> item.wrap(GitHub.this) ); + } +} diff --git a/bin/tests/scenarios/unordered_with_non_labelled/merge.java b/bin/tests/scenarios/unordered_with_non_labelled/merge.java index 1fb9389..ce9067a 100644 --- a/bin/tests/scenarios/unordered_with_non_labelled/merge.java +++ b/bin/tests/scenarios/unordered_with_non_labelled/merge.java @@ -1,13 +1,7 @@ - public class Main { static { int + public class Main { static { int <<<<<<< -x + x = 0 ======= -y + y = 2 >>>>>>> - = -<<<<<<< -0 -======= -2 ->>>>>>> - ; } static { System . out . println ( "I'm a static block" ) ; } public Main ( ) { System . out . println ( "I'm a constructor" ) ; int y = 3 ; } static { System . out . println ( "I don't know what's going on" ) ; } } \ No newline at end of file + ; } static { System . out . println ( "I'm a static block" ) ; } public Main ( ) { System . out . println ( "I'm a constructor" ) ; int y = 3 ; } static { System . out . println ( "I don't know what's going on" ) ; } } diff --git a/merge/src/ordered_merge.rs b/merge/src/ordered_merge.rs index 30a8606..5314bde 100644 --- a/merge/src/ordered_merge.rs +++ b/merge/src/ordered_merge.rs @@ -42,9 +42,9 @@ pub fn ordered_merge<'a>( right_matching_in_left, matching_base_right, ) { - (true, Some(_), Some(_), Some(_), Some(_)) => { + (true, Some(_), Some(matching_base_left), Some(_), Some(_)) => { result_children.push(crate::merge( - cur_left, + matching_base_left.matching_node, cur_left, cur_right, base_left_matchings, @@ -68,6 +68,32 @@ pub fn ordered_merge<'a>( cur_left_option = children_left_it.next(); cur_right_option = children_right_it.next(); } + (true, Some(_), Some(matching_base_left), Some(_), None) => { + result_children.push(crate::merge( + matching_base_left.matching_node, + cur_left, + cur_right, + base_left_matchings, + base_right_matchings, + left_right_matchings, + )?); + + cur_left_option = children_left_it.next(); + cur_right_option = children_right_it.next(); + } + (true, Some(_), None, Some(_), Some(matching_base_right)) => { + result_children.push(crate::merge( + matching_base_right.matching_node, + cur_left, + cur_right, + base_left_matchings, + base_right_matchings, + left_right_matchings, + )?); + + cur_left_option = children_left_it.next(); + cur_right_option = children_right_it.next(); + } (false, Some(_), Some(_), None, Some(matching_base_right)) => { if !matching_base_right.is_perfect_match { result_children.push(MergedCSTNode::Conflict { @@ -139,13 +165,13 @@ pub fn ordered_merge<'a>( cur_right_option = children_right_it.next(); } (false, None, Some(matching_base_left), None, None) => { - result_children.push(cur_right.into()); - if !matching_base_left.is_perfect_match { result_children.push(MergedCSTNode::Conflict { left: Some(Box::new(cur_left.into())), - right: None, - }) + right: Some(Box::new(cur_right.into())), + }); + } else { + result_children.push(cur_right.into()); } cur_left_option = children_left_it.next(); @@ -160,12 +186,13 @@ pub fn ordered_merge<'a>( cur_left_option = children_left_it.next(); } (false, None, None, None, Some(matching_base_right)) => { - result_children.push(cur_left.into()); if !matching_base_right.is_perfect_match { result_children.push(MergedCSTNode::Conflict { - left: None, + left: Some(Box::new(cur_left.into())), right: Some(Box::new(cur_right.into())), }) + } else { + result_children.push(cur_left.into()); } cur_left_option = children_left_it.next(); @@ -181,13 +208,36 @@ pub fn ordered_merge<'a>( cur_right_option = children_right_it.next(); } (a, b, c, d, e) => { - return Err(MergeError::InvalidMatchingConfiguration( + log::warn!( + "Reached an Invalid Matching Configuration. {} {} {} {} {}", a, b.is_some(), c.is_some(), d.is_some(), - e.is_some(), - )); + e.is_some() + ); + log::debug!( + "Involved nodes {} AND {}", + cur_left.contents(), + cur_right.contents() + ); + log::debug!( + "Involved nodes parents {} AND {}", + left.contents(), + right.contents() + ); + + if cur_left.contents() == cur_right.contents() { + result_children.push(cur_left.into()) + } else { + result_children.push(MergedCSTNode::Conflict { + left: Some(Box::new(cur_left.into())), + right: Some(Box::new(cur_right.into())), + }) + } + + cur_left_option = children_left_it.next(); + cur_right_option = children_right_it.next(); } } } @@ -674,25 +724,22 @@ mod tests { assert_eq!( MergedCSTNode::NonTerminal { kind: "kind", - children: vec![ - MergedCSTNode::NonTerminal { + children: vec![MergedCSTNode::Conflict { + left: Some(Box::new(MergedCSTNode::NonTerminal { kind: "another_subtree", children: vec![MergedCSTNode::Terminal { kind: "kind_b", value: Cow::Borrowed("value_b"), }], - }, - MergedCSTNode::Conflict { - left: None, - right: Some(Box::new(MergedCSTNode::NonTerminal { - kind: "subtree", - children: vec![MergedCSTNode::Terminal { - kind: "kind_c", - value: Cow::Borrowed("value_c"), - }], - })), - }, - ], + })), + right: Some(Box::new(MergedCSTNode::NonTerminal { + kind: "subtree", + children: vec![MergedCSTNode::Terminal { + kind: "kind_c", + value: Cow::Borrowed("value_c"), + }], + })), + },], }, merged_tree ); @@ -700,25 +747,22 @@ mod tests { assert_eq!( MergedCSTNode::NonTerminal { kind: "kind", - children: vec![ - MergedCSTNode::NonTerminal { + children: vec![MergedCSTNode::Conflict { + left: Some(Box::new(MergedCSTNode::NonTerminal { + kind: "subtree", + children: vec![MergedCSTNode::Terminal { + kind: "kind_c", + value: Cow::Borrowed("value_c"), + }], + })), + right: Some(Box::new(MergedCSTNode::NonTerminal { kind: "another_subtree", children: vec![MergedCSTNode::Terminal { kind: "kind_b", value: Cow::Borrowed("value_b"), }], - }, - MergedCSTNode::Conflict { - left: Some(Box::new(MergedCSTNode::NonTerminal { - kind: "subtree", - children: vec![MergedCSTNode::Terminal { - kind: "kind_c", - value: Cow::Borrowed("value_c"), - }], - })), - right: None, - }, - ], + })), + },], }, merged_tree_swap ); diff --git a/parsing/src/tree_sitter_parser.rs b/parsing/src/tree_sitter_parser.rs index 9f697a9..c3b1945 100644 --- a/parsing/src/tree_sitter_parser.rs +++ b/parsing/src/tree_sitter_parser.rs @@ -82,6 +82,14 @@ impl From for ParserConfiguration { tree_sitter_java::language(), )), ); + + map.insert( + "variable_declarator", + Box::new(TreeSitterQuery::new( + r#"(variable_declarator (identifier) @name)"#, + tree_sitter_java::language(), + )), + ); map }, },