Skip to content

Commit

Permalink
fix(matching): Incorrect calculation of perfect node matching
Browse files Browse the repository at this point in the history
  • Loading branch information
jpedroh committed Jun 15, 2024
1 parent 26dbc22 commit f64fc75
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 48 deletions.
13 changes: 6 additions & 7 deletions matching/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ pub fn calculate_matchings<'a>(
let is_perfect_match = kind_left == kind_right && value_left == value_right;
Matchings::from_single(
UnorderedPair(left, right),
MatchingEntry::new(is_perfect_match.into(), is_perfect_match),
MatchingEntry::new(left, right, is_perfect_match.into()),
)
}
(_, _) => Matchings::from_single(UnorderedPair(left, right), MatchingEntry::new(0, false)),
(_, _) => Matchings::empty(),
}
}

Expand Down Expand Up @@ -85,7 +85,7 @@ mod tests {
let matchings = calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
Some(&MatchingEntry::new(1, true)),
Some(&MatchingEntry::new(&left, &right, 1)),
matchings.get_matching_entry(&left, &right)
)
}
Expand All @@ -112,9 +112,8 @@ mod tests {
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)
)
let left_right_matching = matchings.get_matching_entry(&left, &right).unwrap();
assert_eq!(0, left_right_matching.score);
assert!(!left_right_matching.is_perfect_match);
}
}
6 changes: 4 additions & 2 deletions matching/src/matching_entry.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
use model::CSTNode;

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct MatchingEntry {
pub score: usize,
pub is_perfect_match: bool,
}

impl MatchingEntry {
pub fn new(score: usize, is_perfect_match: bool) -> Self {
pub fn new(left: &CSTNode, right: &CSTNode, score: usize) -> Self {
MatchingEntry {
score,
is_perfect_match,
is_perfect_match: (2 * score) == (left.get_tree_size() + right.get_tree_size()),
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion matching/src/matchings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ mod tests {
});

let mut matchings = HashMap::new();
matchings.insert(UnorderedPair(&a_node, &a_node), MatchingEntry::new(1, true));
matchings.insert(
UnorderedPair(&a_node, &a_node),
MatchingEntry::new(&a_node, &a_node, 1),
);

assert_eq!(
Some(Matching {
Expand Down
53 changes: 23 additions & 30 deletions matching/src/ordered/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,7 @@ pub fn calculate_matchings<'a>(

let mut matchings = Matchings::from_single(
UnorderedPair(left, right),
MatchingEntry::new(
matrix_m[m][n] + root_matching,
left.contents() == right.contents(),
),
MatchingEntry::new(left, right, matrix_m[m][n] + root_matching),
);

while i >= 1 && j >= 1 {
Expand All @@ -108,7 +105,7 @@ pub fn calculate_matchings<'a>(

#[cfg(test)]
mod tests {
use crate::{matching_entry::MatchingEntry, *};
use crate::MatchingConfiguration;
use model::{
cst_node::{NonTerminal, Terminal},
language, CSTNode, Language, Point,
Expand Down Expand Up @@ -144,10 +141,10 @@ mod tests {
let matching_configuration = MatchingConfiguration::default();
let matchings = super::calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
Some(&MatchingEntry::new(1, true)),
matchings.get_matching_entry(&child, &child)
)
let child_matching = matchings.get_matching_entry(&child, &child);
assert!(child_matching.is_some());
assert_eq!(1, child_matching.unwrap().score);
assert!(child_matching.unwrap().is_perfect_match)
}

#[test]
Expand Down Expand Up @@ -188,11 +185,9 @@ mod tests {

let matching_configuration = MatchingConfiguration::from(Language::Java);
let matchings = super::calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
None,
matchings.get_matching_entry(&left_child, &right_child)
)
assert!(matchings
.get_matching_entry(&left_child, &right_child)
.is_none())
}

#[test]
Expand Down Expand Up @@ -234,10 +229,9 @@ mod tests {
let matching_configuration = MatchingConfiguration::from(language::Language::Java);
let matchings = super::calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
Some(&MatchingEntry::new(2, false)),
matchings.get_matching_entry(&left, &right)
)
let left_right_matchings = matchings.get_matching_entry(&left, &right).unwrap();
assert_eq!(2, left_right_matchings.score);
assert!(!left_right_matchings.is_perfect_match);
}

#[test]
Expand Down Expand Up @@ -271,10 +265,9 @@ mod tests {
let matching_configuration = MatchingConfiguration::from(language::Language::Java);
let matchings = super::calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
Some(&MatchingEntry::new(2, true)),
matchings.get_matching_entry(&left, &right)
)
let left_right_matchings = matchings.get_matching_entry(&left, &right).unwrap();
assert_eq!(2, left_right_matchings.score);
assert!(left_right_matchings.is_perfect_match);
}

#[test]
Expand Down Expand Up @@ -317,14 +310,14 @@ mod tests {
let matching_configuration = MatchingConfiguration::default();
let matchings = super::calculate_matchings(&left, &right, &matching_configuration);

assert_eq!(
Some(&MatchingEntry::new(2, true)),
matchings.get_matching_entry(&intermediate, &intermediate)
);
let intermediate_matching = matchings
.get_matching_entry(&intermediate, &intermediate)
.unwrap();
assert_eq!(2, intermediate_matching.score);
assert!(intermediate_matching.is_perfect_match);

assert_eq!(
Some(&MatchingEntry::new(3, true)),
matchings.get_matching_entry(&left, &right)
)
let left_right_matching = matchings.get_matching_entry(&left, &right).unwrap();
assert_eq!(3, left_right_matching.score);
assert!(left_right_matching.is_perfect_match);
}
}
5 changes: 1 addition & 4 deletions matching/src/unordered/assignment_problem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,7 @@ fn solve_assignment_problem<'a>(

result.extend(Matchings::from_single(
UnorderedPair(left, right),
MatchingEntry {
score: max_matching as usize + 1,
is_perfect_match: left.contents() == right.contents(),
},
MatchingEntry::new(left, right, max_matching as usize + 1),
));

result
Expand Down
5 changes: 1 addition & 4 deletions matching/src/unordered/unique_label.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ pub fn calculate_matchings<'a>(

result.extend(Matchings::from_single(
UnorderedPair(left, right),
MatchingEntry {
score: sum + root_matching,
is_perfect_match: left.contents() == right.contents(),
},
MatchingEntry::new(left, right, sum + root_matching),
));

result
Expand Down
16 changes: 16 additions & 0 deletions model/src/cst_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,22 @@ impl CSTNode<'_> {
CSTNode::NonTerminal(node) => node.end_position,
}
}

fn get_subtree_size(&self) -> usize {
match self {
CSTNode::Terminal(_) => 0,
CSTNode::NonTerminal(node) => node
.children
.iter()
.fold(node.children.len(), |acc, child| {
acc + child.get_subtree_size()
}),
}
}

pub fn get_tree_size(&self) -> usize {
return self.get_subtree_size() + 1;

Check failure on line 65 in model/src/cst_node.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement
}
}

#[derive(Debug, Default, Clone)]
Expand Down

0 comments on commit f64fc75

Please sign in to comment.