Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: incorrect matching of Java import declarations #52

Merged
merged 8 commits into from
May 2, 2024
Original file line number Diff line number Diff line change
@@ -1 +1 @@
package de . fosd . jdime . common ; import de . fosd . jdime . common . operations . AddOperation ; import AST . * ; import de . fosd . jdime . common . operations . ConflictOperation ; public class ASTNodeArtifact extends Artifact < ASTNodeArtifact > { private ASTNodeArtifact ( final ASTNode < ? > astnode ) { assert ( astnode != null ) ; this . astnode = astnode ; this . initializeChildren ( ) ; } public ASTNodeArtifact ( final FileArtifact artifact ) { assert ( artifact != null ) ; setRevision ( artifact . getRevision ( ) ) ; ASTNode < ? > astnode ; if ( artifact . isEmpty ( ) ) { astnode = new ASTNode < > ( ) ; } else { Program p = initProgram ( ) ; p . addSourceFile ( artifact . getPath ( ) ) ; astnode = p ; } this . astnode = astnode ; this . initializeChildren ( ) ; renumberTree ( ) ; } }
package de . fosd . jdime . common ; import AST . * ; import de . fosd . jdime . common . operations . ConflictOperation ; import de . fosd . jdime . common . operations . AddOperation ; public class ASTNodeArtifact extends Artifact < ASTNodeArtifact > { private ASTNodeArtifact ( final ASTNode < ? > astnode ) { assert ( astnode != null ) ; this . astnode = astnode ; this . initializeChildren ( ) ; } public ASTNodeArtifact ( final FileArtifact artifact ) { assert ( artifact != null ) ; setRevision ( artifact . getRevision ( ) ) ; ASTNode < ? > astnode ; if ( artifact . isEmpty ( ) ) { astnode = new ASTNode < > ( ) ; } else { Program p = initProgram ( ) ; p . addSourceFile ( artifact . getPath ( ) ) ; astnode = p ; } this . astnode = astnode ; this . initializeChildren ( ) ; renumberTree ( ) ; } }
14 changes: 14 additions & 0 deletions bin/tests/scenarios/import_declarations_grouping_node/base.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package br.fosd.jdime.stats;

import java.text.DecimalFormat;
import java.util.HashMap;
import java.util.TreeSet;

import org.apache.commons.lang3.ClassUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;

import de.fosd.jdime.common.LangElem;

public class ASTStats {
}
13 changes: 13 additions & 0 deletions bin/tests/scenarios/import_declarations_grouping_node/left.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package br.fosd.jdime.stats;

import java.text.DecimalFormat;
import java.util.HashMap;
import java.util.TreeSet;

import de.fosd.jdime.common.LangElem;
import org.apache.commons.lang3.ClassUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;

public class ASTStats {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package br . fosd . jdime . stats ; import java . text . DecimalFormat ; import java . util . HashMap ; import java . util . TreeSet ; import de . fosd . jdime . common . LangElem ; import java . util . logging . Level ; import java . util . logging . Logger ; public class ASTStats { }
12 changes: 12 additions & 0 deletions bin/tests/scenarios/import_declarations_grouping_node/right.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package br.fosd.jdime.stats;

import java.text.DecimalFormat;
import java.util.HashMap;
import java.util.TreeSet;
import java.util.logging.Level;
import java.util.logging.Logger;

import de.fosd.jdime.common.LangElem;

public class ASTStats {
}
Original file line number Diff line number Diff line change
@@ -1 +1 @@
package de . fosd . jdime . merge ; import java . util . List ; import AST . * ; import de . fosd . jdime . operations . AddOperation ; import de . fosd . jdime . operations . ConflictOperation ; import de . fosd . jdime . operations . MergeOperation ; import static de . fosd . jdime . artifact . Artifacts . root ; import static de . fosd . jdime . strdump . DumpMode . PLAINTEXT_TREE ;
package de . fosd . jdime . merge ; import java . util . List ; import AST . * ; import de . fosd . jdime . operations . AddOperation ; import de . fosd . jdime . operations . ConflictOperation ; import de . fosd . jdime . operations . MergeOperation ; import static de . fosd . jdime . artifact . Artifacts . root ; import static de . fosd . jdime . strdump . DumpMode . PLAINTEXT_TREE ;
2 changes: 1 addition & 1 deletion bin/tests/scenarios/jdime_matching_issue/merge.java
Original file line number Diff line number Diff line change
@@ -1 +1 @@
package de . fosd . jdime . artifact ; import java . security . MessageDigest ; public abstract class Artifact < T extends Artifact < T > > implements Comparable < T > , StatisticsInterface { public boolean hasChanges ( Revision revision ) { if ( this . revision . equals ( revision ) ) { return false ; } if ( ! hasMatching ( revision ) ) { return true ; } T match = getMatching ( revision ) . getMatchingArtifact ( this ) ; return getTreeSize ( ) != match . getTreeSize ( ) || ! getTreeHash ( ) . equals ( match . getTreeHash ( ) ) ; } }
package de . fosd . jdime . artifact ; import java . security . MessageDigest ; public abstract class Artifact < T extends Artifact < T > > implements Comparable < T > , StatisticsInterface { public boolean hasChanges ( Revision revision ) { if ( this . revision . equals ( revision ) ) { return false ; } if ( ! hasMatching ( revision ) ) { return true ; } T match = getMatching ( revision ) . getMatchingArtifact ( this ) ; return getTreeSize ( ) != match . getTreeSize ( ) || ! getTreeHash ( ) . equals ( match . getTreeHash ( ) ) ; } }
Original file line number Diff line number Diff line change
@@ -1 +1 @@
package de . fosd . jdime ; import java . io . File ; import java . net . URISyntaxException ; import java . net . URL ; import java . util . Arrays ; import org . junit . BeforeClass ; import static org . junit . Assert . assertNotNull ; import static org . junit . Assert . assertTrue ; import static org . junit . Assert . fail ; public class JDimeTest { protected static File file ( File parent , String child ) { File f = new File ( parent , child ) ; assertTrue ( f + " does not exist." , f . exists ( ) ) ; return f ; } protected static File file ( File parent , String name , String ... names ) { if ( names != null ) { String path = String . format ( "%s/%s" , name , String . join ( "/" , names ) ) ; return file ( parent , path ) ; } else { return file ( parent , name ) ; } } protected static File file ( String path ) { URL res = JDimeTest . class . getResource ( path ) ; assertNotNull ( "The file " + path + " was not found." , res ) ; try { return new File ( res . toURI ( ) ) ; } catch ( URISyntaxException e ) { fail ( e . getMessage ( ) ) ; return null ; } } protected static File file ( String name , String ... names ) { if ( names != null ) { String path = String . format ( "/%s/%s" , name , String . join ( "/" , names ) ) ; return file ( path ) ; } else { return file ( "/" + name ) ; } } }
package de . fosd . jdime ; import java . io . File ; import java . net . URISyntaxException ; import java . net . URL ; import java . util . Arrays ; import org . junit . BeforeClass ; import static org . junit . Assert . assertNotNull ; import static org . junit . Assert . assertTrue ; import static org . junit . Assert . fail ; public class JDimeTest { protected static File file ( File parent , String child ) { File f = new File ( parent , child ) ; assertTrue ( f + " does not exist." , f . exists ( ) ) ; return f ; } protected static File file ( File parent , String name , String ... names ) { if ( names != null ) { String path = String . format ( "%s/%s" , name , String . join ( "/" , names ) ) ; return file ( parent , path ) ; } else { return file ( parent , name ) ; } } protected static File file ( String path ) { URL res = JDimeTest . class . getResource ( path ) ; assertNotNull ( "The file " + path + " was not found." , res ) ; try { return new File ( res . toURI ( ) ) ; } catch ( URISyntaxException e ) { fail ( e . getMessage ( ) ) ; return null ; } } protected static File file ( String name , String ... names ) { if ( names != null ) { String path = String . format ( "/%s/%s" , name , String . join ( "/" , names ) ) ; return file ( path ) ; } else { return file ( "/" + name ) ; } } }
1 change: 1 addition & 0 deletions matching/src/matching_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ impl From<Language> for MatchingConfiguration<'_> {
"constructor_declaration",
"field_declaration",
"method_declaration",
"import_declaration",
]
.into(),
handlers: MatchingHandlers::from(Language::Java),
Expand Down
12 changes: 10 additions & 2 deletions matching/src/unordered/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,18 @@ pub fn calculate_matchings<'a>(
match (left, right) {
(model::CSTNode::NonTerminal(left_nt), model::CSTNode::NonTerminal(right_nt)) => {
if all_children_labeled(left_nt, config) && all_children_labeled(right_nt, config) {
log::debug!("Using unique label matching.");
log::debug!(
"Matching children of \"{}\" with \"{}\" using unique label matching.",
left.kind(),
right.kind()
);
unique_label::calculate_matchings(left, right, config)
} else {
log::debug!("Using assignment problem matching.");
log::debug!(
"Matching children of \"{}\" with \"{}\" using assignment problem matching.",
left.kind(),
right.kind()
);
assignment_problem::calculate_matchings(left, right, config)
}
}
Expand Down
14 changes: 14 additions & 0 deletions model/src/cst_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,20 @@ impl CSTNode<'_> {
CSTNode::NonTerminal(node) => node.contents(),
}
}

pub fn start_position(&self) -> Point {
match self {
CSTNode::Terminal(node) => node.start_position,
CSTNode::NonTerminal(node) => node.start_position,
}
}

pub fn end_position(&self) -> Point {
match self {
CSTNode::Terminal(node) => node.end_position,
CSTNode::NonTerminal(node) => node.end_position,
}
}
}

#[derive(Debug, Default, Clone)]
Expand Down
4 changes: 3 additions & 1 deletion parsing_handlers/src/java/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
mod tweak_import_declarations;

use crate::ParsingHandlers;

pub fn get_default_java_parsing_handlers() -> ParsingHandlers {
ParsingHandlers::new(vec![])
ParsingHandlers::new(vec![tweak_import_declarations::tweak_import_declarations])
}
95 changes: 95 additions & 0 deletions parsing_handlers/src/java/tweak_import_declarations.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
use model::{cst_node::NonTerminal, CSTNode};

pub fn tweak_import_declarations(root: CSTNode<'_>) -> CSTNode<'_> {
if root.kind() != "program" {
return root.to_owned();
}

match root {
CSTNode::Terminal(_) => root,
CSTNode::NonTerminal(program) => {
let import_declaration_children: Vec<CSTNode> = program
.children
.iter()
.filter(|node| node.kind() == "import_declaration")
.cloned()
.collect();

if import_declaration_children.is_empty() {
return CSTNode::NonTerminal(program);
}

let import_declarations_start = import_declaration_children
.first()
.unwrap()
.start_position();

let import_declarations_end =
import_declaration_children.last().unwrap().end_position();

let import_declarations = CSTNode::NonTerminal(NonTerminal {
id: uuid::Uuid::new_v4(),
kind: "import_declarations",
children: import_declaration_children,
start_position: import_declarations_start,
end_position: import_declarations_end,
are_children_unordered: true,
});

let first_import_declaration_index = program
.children
.iter()
.position(|node| node.kind() == "import_declaration")
.unwrap();
let last_import_declaration_index = program
.children
.iter()
.rposition(|node| node.kind() == "import_declaration")
.unwrap();

let mut new_program_children: Vec<CSTNode<'_>> = vec![];
new_program_children.extend_from_slice(
&program.children.iter().as_slice()[..first_import_declaration_index],
);
new_program_children.push(import_declarations);
new_program_children.extend_from_slice(
&program.children.iter().as_slice()[last_import_declaration_index + 1..],
);

CSTNode::NonTerminal(NonTerminal {
id: program.id,
kind: program.kind,
start_position: program.start_position,
end_position: program.end_position,
children: new_program_children,
are_children_unordered: program.are_children_unordered,
})
}
}
}

#[cfg(test)]
mod tests {
use model::{cst_node::Terminal, CSTNode};

#[test]
fn if_the_root_is_not_a_program_we_just_return_it() {
let root = CSTNode::Terminal(Terminal {
kind: "terminal",
value: "not_a_program",
..Default::default()
});

assert_eq!(super::tweak_import_declarations(root.clone()), root);
}

#[test]
fn if_somehow_the_root_is_a_terminal_node_we_just_return_it() {
let root = CSTNode::Terminal(Terminal {
kind: "program",
..Default::default()
});

assert_eq!(super::tweak_import_declarations(root.clone()), root);
}
}
Loading