fix: incorrect matching of Java import declarations (#52)

bin/tests/scenarios/fancy_argument_types_matching_issue/merge.java

@@ -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  ( ) ; } }

bin/tests/scenarios/import_declarations_grouping_node/base.java

@@ -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 {
+}

bin/tests/scenarios/import_declarations_grouping_node/left.java

@@ -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 {
+}

bin/tests/scenarios/import_declarations_grouping_node/merge.java

@@ -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  { }

bin/tests/scenarios/import_declarations_grouping_node/right.java

@@ -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 {
+}

bin/tests/scenarios/imports_are_merged_correctly/merge.java

@@ -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 ;

bin/tests/scenarios/jdime_matching_issue/merge.java

@@ -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  ( ) ) ; } }

bin/tests/scenarios/overload_methods_with_spread_parameter/merge.java

@@ -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 ) ; } } }

matching/src/matching_configuration.rs

@@ -24,6 +24,7 @@ impl From<Language> for MatchingConfiguration<'_> {
                     "constructor_declaration",
                     "field_declaration",
                     "method_declaration",
+                    "import_declaration",
                 ]
                 .into(),
                 handlers: MatchingHandlers::from(Language::Java),

matching/src/unordered/mod.rs

@@ -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)
             }
         }

model/src/cst_node.rs

@@ -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)]

parsing_handlers/src/java/mod.rs

@@ -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])
 }

parsing_handlers/src/java/tweak_import_declarations.rs

@@ -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);
+    }
+}