Skip to content

Commit

Permalink
fix: correct some issues on the matching algorithm
Browse files Browse the repository at this point in the history
  • Loading branch information
jpedroh committed May 3, 2024
1 parent 6d4d62a commit 5562234
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 73 deletions.
60 changes: 4 additions & 56 deletions matching/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ pub fn calculate_matchings<'a>(
right: &'a model::CSTNode,
config: &'a MatchingConfiguration<'a>,
) -> Matchings<'a> {
if left.kind() != right.kind() {
return Matchings::empty();
}

match (left, right) {
(
model::CSTNode::NonTerminal(non_terminal_left),
Expand Down Expand Up @@ -113,60 +117,4 @@ mod tests {
matchings.get_matching_entry(&left, &right)
)
}

#[test]
fn two_terminal_nodes_have_a_match_with_score_zero_if_they_have_different_kind() {
let left = CSTNode::Terminal(Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind_a",
value: "value",
start_position: Point { row: 0, column: 0 },
end_position: Point { row: 0, column: 5 },
is_block_end_delimiter: false,
});
let right = CSTNode::Terminal(Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind_b",
value: "value",
start_position: Point { row: 0, column: 0 },
end_position: Point { row: 0, column: 5 },
is_block_end_delimiter: false,
});

let matching_configuration = MatchingConfiguration::default();
let matchings = calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
Some(&MatchingEntry::new(0, false)),
matchings.get_matching_entry(&left, &right)
)
}

#[test]
fn two_terminal_nodes_have_a_match_with_score_zero_if_they_have_different_kind_and_value() {
let left = CSTNode::Terminal(Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind_a",
value: "value_a",
start_position: Point { row: 0, column: 0 },
end_position: Point { row: 0, column: 7 },
is_block_end_delimiter: false,
});
let right = CSTNode::Terminal(Terminal {
id: uuid::Uuid::new_v4(),
kind: "kind_b",
value: "value_a",
start_position: Point { row: 0, column: 0 },
end_position: Point { row: 0, column: 7 },
is_block_end_delimiter: false,
});

let matching_configuration = MatchingConfiguration::default();
let matchings = calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
Some(&MatchingEntry::new(0, false)),
matchings.get_matching_entry(&left, &right)
)
}
}
4 changes: 3 additions & 1 deletion matching/src/matching_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ impl From<Language> for MatchingConfiguration<'_> {
fn from(language: Language) -> Self {
match language {
Language::Java => MatchingConfiguration {
delimiters: ["{", "}"].into(),
delimiters: ["{", "}", ";"].into(),
kinds_with_label: [
"compact_constructor_declaration",
"constructor_declaration",
"field_declaration",
"method_declaration",
"import_declaration",
"class_declaration",
"interface_declaration",
]
.into(),
handlers: MatchingHandlers::from(Language::Java),
Expand Down
5 changes: 0 additions & 5 deletions matching/src/ordered/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@ pub fn calculate_matchings<'a>(
.compute_matching_score(left, right)
.unwrap_or((left.kind() == right.kind()).into());

// Node roots do not match, early return
if root_matching == 0 {
return Matchings::empty();
}

let m = children_left.len();
let n = children_right.len();

Expand Down
35 changes: 25 additions & 10 deletions matching/src/unordered/unique_label.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{matching_configuration::MatchingConfiguration, MatchingEntry, Matchi
pub fn calculate_matchings<'a>(
left: &'a CSTNode,
right: &'a CSTNode,
matching_configuration: &'a MatchingConfiguration<'a>,
config: &'a MatchingConfiguration<'a>,
) -> crate::Matchings<'a> {
match (left, right) {
(
Expand All @@ -28,15 +28,30 @@ pub fn calculate_matchings<'a>(

for child_left in children_left {
for child_right in children_right {
let child_matchings =
crate::calculate_matchings(child_left, child_right, matching_configuration);

if let Some(matching_entry) =
child_matchings.get_matching_entry(child_left, child_right)
{
if matching_entry.score >= 1 {
sum += matching_entry.score;
result.extend(child_matchings);
let is_same_identifier = config
.handlers
.compute_matching_score(child_left, child_right)
.unwrap_or_else(|| {
log::warn!(
"Could not find an identifier while matching {} {}",
child_left.contents(),
child_right.contents()
);

(child_left.kind() == child_right.kind()).into()
});

if is_same_identifier == 1 {
let child_matchings =
crate::calculate_matchings(child_left, child_right, config);

if let Some(matching_entry) =
child_matchings.get_matching_entry(child_left, child_right)
{
if matching_entry.score >= 1 {
sum += matching_entry.score;
result.extend(child_matchings);
}
}
}
}
Expand Down
21 changes: 20 additions & 1 deletion merge/src/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ pub fn merge<'a>(
base_right_matchings: &'a Matchings<'a>,
left_right_matchings: &'a Matchings<'a>,
) -> Result<MergedCSTNode<'a>, MergeError> {
if left.kind() != right.kind() {
log::debug!(
"Error while merging\n left: {}\n right:{}",
left.contents(),
right.contents()
);
return Err(MergeError::NodesWithDifferentKinds(
left.kind().to_string(),
right.kind().to_string(),
));
}

match (base, left, right) {
(CSTNode::Terminal(a_base), CSTNode::Terminal(a_left), CSTNode::Terminal(a_right)) => {
merge_terminals(a_base, a_left, a_right)
Expand All @@ -38,7 +50,14 @@ pub fn merge<'a>(
)?)
}
}
(_, _, _) => Err(MergeError::MergingTerminalWithNonTerminal),
(_, _, _) => {
log::debug!(
"Error while merging NonTerminal with Terminal {} and {}",
left.contents(),
right.contents()
);
Err(MergeError::MergingTerminalWithNonTerminal)
}
}
}

Expand Down

0 comments on commit 5562234

Please sign in to comment.