From 5562234ff2bd865dbdb83b774647115b6ce51ef5 Mon Sep 17 00:00:00 2001 From: Joao Duarte Date: Thu, 2 May 2024 21:03:26 -0300 Subject: [PATCH] fix: correct some issues on the matching algorithm --- matching/src/lib.rs | 60 ++------------------------ matching/src/matching_configuration.rs | 4 +- matching/src/ordered/mod.rs | 5 --- matching/src/unordered/unique_label.rs | 35 ++++++++++----- merge/src/merge.rs | 21 ++++++++- 5 files changed, 52 insertions(+), 73 deletions(-) diff --git a/matching/src/lib.rs b/matching/src/lib.rs index 286d2af..bda7aa1 100644 --- a/matching/src/lib.rs +++ b/matching/src/lib.rs @@ -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), @@ -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) - ) - } } diff --git a/matching/src/matching_configuration.rs b/matching/src/matching_configuration.rs index e4921b0..bf42b81 100644 --- a/matching/src/matching_configuration.rs +++ b/matching/src/matching_configuration.rs @@ -18,13 +18,15 @@ impl From 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), diff --git a/matching/src/ordered/mod.rs b/matching/src/ordered/mod.rs index 403d17b..b4b8810 100644 --- a/matching/src/ordered/mod.rs +++ b/matching/src/ordered/mod.rs @@ -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(); diff --git a/matching/src/unordered/unique_label.rs b/matching/src/unordered/unique_label.rs index d89b63a..738e4b6 100644 --- a/matching/src/unordered/unique_label.rs +++ b/matching/src/unordered/unique_label.rs @@ -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) { ( @@ -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); + } } } } diff --git a/merge/src/merge.rs b/merge/src/merge.rs index 7ec2144..6f12709 100644 --- a/merge/src/merge.rs +++ b/merge/src/merge.rs @@ -15,6 +15,18 @@ pub fn merge<'a>( base_right_matchings: &'a Matchings<'a>, left_right_matchings: &'a Matchings<'a>, ) -> Result, 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) @@ -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) + } } }