Skip to content

Commit

Permalink
fix: Issues on merge algorithm (#68)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jpedroh
jpedroh authored Aug 24, 2024
1 parent feb07f5 commit b9ecfdb
Show file tree
Hide file tree
Showing 8 changed files with 144 additions and 49 deletions.
2 changes: 2 additions & 0 deletions bin/src/control.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,12 @@ pub fn run_tool_on_merge_scenario(
right: &str,
) -> Result<ExecutionResult, ExecutionError> {
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()));
}

Expand Down
15 changes: 15 additions & 0 deletions bin/tests/scenarios/incorrect_merge_conflict/base.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
public class Main {
public static void main() {
return new PagedIterable<GHRepository>() {
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);
}
};
}
};
}
}
16 changes: 16 additions & 0 deletions bin/tests/scenarios/incorrect_merge_conflict/left.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
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);
}
};
}
};
}
}
7 changes: 7 additions & 0 deletions bin/tests/scenarios/incorrect_merge_conflict/merge.java
View file
Original file line number Diff line number Diff line change
@@ -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 ) )
>>>>>>>
; } }
9 changes: 9 additions & 0 deletions bin/tests/scenarios/incorrect_merge_conflict/right.java
View file
Original file line number Diff line number Diff line change
@@ -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) );
}
}
14 changes: 4 additions & 10 deletions bin/tests/scenarios/unordered_with_non_labelled/merge.java
View file
Original file line number Diff line number Diff line change
@@ -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" ) ; } }
; } 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" ) ; } }
122 changes: 83 additions & 39 deletions merge/src/ordered_merge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
}
}
}
Expand Down Expand Up @@ -674,51 +724,45 @@ 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
);

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
);
Expand Down
8 changes: 8 additions & 0 deletions parsing/src/tree_sitter_parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ impl From<Language> for ParserConfiguration {
tree_sitter_java::language(),
)),
);

map.insert(
"variable_declarator",
Box::new(TreeSitterQuery::new(
r#"(variable_declarator (identifier) @name)"#,
tree_sitter_java::language(),
)),
);
map
},
},
Expand Down

0 comments on commit b9ecfdb

Please sign in to comment.