fix: assertions remover use now add statements with a filter (#410)

* fix: assertions remover use now add statements with a filter

* fix: isAssert, match when the parent is a block or an assertion

* fix: use the original configuration to set up the targetedModule

* fix: rely now on the name of the package of the invocation rather than on the toString()

* fix: check not null and use pre-build factory instead of factory of gumtree

* test: update test according to new resources

dspot/src/main/java/fr/inria/diversify/dspot/Amplification.java

@@ -86,7 +86,7 @@ public void amplification(CtType<?> classTest, int maxIteration) throws IOExcept
 	 */
 	public void amplification(CtType<?> classTest, List<CtMethod<?>> methods, int maxIteration) throws IOException, InterruptedException, ClassNotFoundException {
 		List<CtMethod<?>> tests = methods.stream()
-				.filter(mth -> AmplificationChecker.isTest(mth, this.configuration.getInputProgram().getRelativeTestSourceCodeDir()))
+				.filter(AmplificationChecker::isTest)
 				.collect(Collectors.toList());
 		if (tests.isEmpty()) {
 			LOGGER.warn("No test has been found into {}", classTest.getQualifiedName());

dspot/src/main/java/fr/inria/diversify/dspot/assertGenerator/AssertionRemover.java

@@ -13,6 +13,7 @@
 import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.reference.CtTypeReference;
+import spoon.reflect.visitor.filter.TypeFilter;
 
 /**
  * Created by Benjamin DANGLOT
@@ -22,7 +23,7 @@
 @SuppressWarnings("unchecked")
 public class AssertionRemover {
 
-    private final int [] counter = new int[]{0};
+    private final int[] counter = new int[]{0};
 
     /**
      * Removes all assertions from a test.
@@ -44,15 +45,21 @@ public CtMethod<?> removeAssertion(CtMethod<?> testMethod) {
      */
     public void removeAssertion(CtInvocation<?> invocation) {
         final Factory factory = invocation.getFactory();
-        invocation.getArguments().forEach(argument -> {
+        final TypeFilter<CtStatement> statementTypeFilter = new TypeFilter<CtStatement>(CtStatement.class) {
+            @Override
+            public boolean matches(CtStatement element) {
+                return element.equals(invocation);
+            }
+        };
+        for (CtExpression<?> argument : invocation.getArguments()) {
             CtExpression clone = ((CtExpression) argument).clone();
             if (clone instanceof CtUnaryOperator) {
                 clone = ((CtUnaryOperator) clone).getOperand();
             }
             if (clone instanceof CtStatement) {
                 clone.getTypeCasts().clear();
-                invocation.insertBefore((CtStatement) clone);
-            } else if (! (clone instanceof CtLiteral || clone instanceof CtVariableRead)) {
+                invocation.getParent(CtStatementList.class).insertBefore(statementTypeFilter, (CtStatement) clone);
+            } else if (!(clone instanceof CtLiteral || clone instanceof CtVariableRead)) {
                 CtTypeReference<?> typeOfParameter = clone.getType();
                 if (clone.getType().equals(factory.Type().NULL_TYPE)) {
                     typeOfParameter = factory.Type().createReference(Object.class);
@@ -62,16 +69,16 @@ public void removeAssertion(CtInvocation<?> invocation) {
                         typeOfParameter.getSimpleName() + "_" + counter[0]++,
                         clone
                 );
-                invocation.insertBefore(localVariable);
+                invocation.getParent(CtStatementList.class).insertBefore(statementTypeFilter, localVariable);
             }
-        });
+        }
         // must find the first statement list to remove the invocation from it, e.g. the block that contains the assertions
         // the assertion can be inside other stuff, than directly in the block
-        CtElement currentParent = invocation;
-        while (! (currentParent.getParent() instanceof CtStatementList)) {
-            currentParent = currentParent.getParent();
+        CtElement topStatement = invocation;
+        while (!(topStatement.getParent() instanceof CtStatementList)) {
+            topStatement = topStatement.getParent();
         }
-        ((CtStatementList) currentParent.getParent()).removeStatement((CtStatement) currentParent);
+        ((CtStatementList) topStatement.getParent()).removeStatement((CtStatement) topStatement);
     }
 
 }

dspot/src/main/java/fr/inria/diversify/dspot/selector/ChangeDetectorSelector.java

@@ -58,12 +58,13 @@ public void init(InputConfiguration configuration) {
             inputConfiguration = new InputConfiguration(configurationPath);
             InputProgram inputProgram = InputConfiguration.initInputProgram(inputConfiguration);
             inputConfiguration.setInputProgram(inputProgram);
-            this.pathToChangedVersionOfProgram = pathToFolder +
-                    DSpotUtils.shouldAddSeparator.apply(pathToFolder) +
-                    (inputConfiguration.getProperty("targetModule") != null ?
-                            inputConfiguration.getProperty("targetModule") +
-                                    DSpotUtils.shouldAddSeparator.apply(pathToFolder)
-                            : "");
+            this.pathToChangedVersionOfProgram = pathToFolder;
+            if (this.configuration.getProperty("targetModule") != null) {
+                this.pathToChangedVersionOfProgram += DSpotUtils.shouldAddSeparator.apply(pathToFolder) +
+                        this.configuration.getProperty("targetModule") +
+                        DSpotUtils.shouldAddSeparator.apply(pathToFolder);
+                configuration.getProperties().setProperty("targetModule", this.configuration.getProperty("targetModule"));
+            }
             inputProgram.setProgramDir(this.pathToChangedVersionOfProgram);
             Initializer.initialize(inputConfiguration, inputProgram);
         } catch (Exception e) {

dspot/src/main/java/fr/inria/diversify/utils/AmplificationChecker.java

@@ -15,6 +15,7 @@
 import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.filter.TypeFilter;
 
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -44,15 +45,25 @@ public static boolean isAssert(CtInvocation invocation) {
                 );
     }
 
+    private final static List<String> ASSERTIONS_PACKAGES =
+            Arrays.asList("org.junit", "com.google.common.truth", "org.assertj", "junit");
+
     private static boolean _isAssert(CtInvocation invocation) {
-        // simplification of this method, rely on the name of the method, also,
-        // we checks that this invocation is not an invocation to a method that contains assertion
+        // simplification of this method.
+        // We rely on the package of the declaring type of the invocation
         // in this case, we will match it
-        final String nameOfMethodCalled = invocation.getExecutable().getSimpleName();
-        return nameOfMethodCalled.toLowerCase().contains("assert") ||
-                nameOfMethodCalled.toLowerCase().startsWith("fail") ||
-                invocation.toString().toLowerCase().contains("assert") ||
-                invocation.toString().toLowerCase().contains("catchexception");// eu.codearte.catch-exception.catch-exception
+        final String qualifiedNameOfPackage;
+        if (invocation.getExecutable().getDeclaringType().getPackage() == null) {
+            if (invocation.getExecutable().getDeclaringType().getTopLevelType() != null) {
+                qualifiedNameOfPackage = invocation.getExecutable().getDeclaringType().getTopLevelType().getPackage().getQualifiedName();
+            } else {
+                return false;
+            }
+        } else {
+            qualifiedNameOfPackage = invocation.getExecutable().getDeclaringType().getPackage().getQualifiedName();
+        }
+        return ASSERTIONS_PACKAGES.stream()
+                .anyMatch(qualifiedNameOfPackage::startsWith);
     }
 
     public static boolean canBeAdded(CtInvocation invocation) {
@@ -84,6 +95,9 @@ public boolean matches(CtMethod<?> element) {
     };
 
     public static boolean isTest(CtMethod<?> candidate) {
+        if (candidate == null) {
+            return false;
+        }
         CtClass<?> parent = candidate.getParent(CtClass.class);
         // if the test method has @Ignore, is not a test
         if (candidate.getAnnotation(org.junit.Ignore.class) != null) {
@@ -130,6 +144,7 @@ public boolean matches(CtInvocation element) {
 
     /**
      * checks if the given test class inherit from {@link junit.framework.TestCase}, <i>i.e.</i> is JUnit3 test class.
+     *
      * @param testClass
      * @return true if the given test class inherit from {@link junit.framework.TestCase}, false otherwise
      */
@@ -170,6 +185,7 @@ public static boolean isTestJUnit4(CtType<?> classTest) {
                 );
     }
 
+    @Deprecated
     public static boolean isTest(CtMethod candidate, String relativePath) {
         try {
             if (!relativePath.isEmpty() && candidate.getPosition() != null

dspot/src/main/java/fr/inria/stamp/diff/SelectorOnDiff.java

@@ -8,18 +8,20 @@
 import gumtree.spoon.diff.Diff;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import spoon.reflect.declaration.CtClass;
 import spoon.reflect.declaration.CtMethod;
+import spoon.reflect.declaration.CtParameter;
 import spoon.reflect.declaration.CtType;
 import spoon.reflect.factory.Factory;
 import spoon.reflect.reference.CtExecutableReference;
+import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.filter.TypeFilter;
 
 import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -43,7 +45,7 @@ public class SelectorOnDiff {
      * @param configuration of the project under amplification. This configuration must contain the following properties:
      *                      baseSha: with the commit sha of the base branch
      *                      project: with the path to the base project
-     *                     folderPath: with the path to the changed project
+     *                      folderPath: with the path to the changed project
      * @return a map that associates the full qualified name of test classes to their test methods to be amplified.
      */
     public static Map<String, List<String>> findTestMethodsAccordingToADiff(InputConfiguration configuration) {
@@ -80,6 +82,7 @@ public static Map<String, List<String>> findTestMethodsAccordingToADiff(InputCon
     /**
      * Constructor. Please, have look to {@link  fr.inria.stamp.diff.SelectorOnDiff#findTestMethodsAccordingToADiff(InputConfiguration)}.
      * The usage of this constructor and the method  {@link  fr.inria.stamp.diff.SelectorOnDiff#findTestMethods()} is discouraged.
+     *
      * @param configuration
      * @param factory
      * @param baseSha
@@ -102,6 +105,7 @@ public SelectorOnDiff(InputConfiguration configuration,
      * This method does the same job than {@link  fr.inria.stamp.diff.SelectorOnDiff#findTestMethodsAccordingToADiff(InputConfiguration)} but use an instance.
      * It is more convenient to use the static method {@link  fr.inria.stamp.diff.SelectorOnDiff#findTestMethodsAccordingToADiff(InputConfiguration)}
      * which instantiate and set specific value rather than use this method.
+     *
      * @return a map that associates the full qualified name of test classes to their test methods to be amplified.
      */
     @SuppressWarnings("unchecked")
@@ -226,6 +230,7 @@ public boolean matches(CtExecutableReference element) {
                     }
                 }).stream()
                 .map(ctExecutableReference -> ctExecutableReference.getParent(CtMethod.class))
+                .filter(Objects::nonNull)
                 .filter(AmplificationChecker::isTest)
                 .filter(ctMethod -> !(modifiedTestMethods.contains(ctMethod)))
                 .collect(Collectors.toList());
@@ -234,26 +239,38 @@ public boolean matches(CtExecutableReference element) {
     public Set<CtMethod> getModifiedMethods(Set<String> modifiedJavaFiles) {
         return modifiedJavaFiles.stream()
                 .flatMap(s ->
-                        getModifiedMethods(pathToFirstVersion + s.substring(1), pathToSecondVersion + s.substring(1)).stream()
+                        getModifiedMethods(pathToFirstVersion + s.substring(1),
+                                pathToSecondVersion + s.substring(1)
+                        )
                 ).collect(Collectors.toSet());
     }
 
-    public Set<CtMethod> getModifiedMethods(String pathFile1, String pathFile2) {
+    public Stream<CtMethod> getModifiedMethods(String pathFile1, String pathFile2) {
         try {
             final File file1 = new File(pathFile1);
             final File file2 = new File(pathFile2);
             if (!file1.exists() || !file2.exists()) {
-                return Collections.emptySet();
+                return Stream.of();
             }
             Diff result = (new AstComparator()).compare(file1, file2);
             return result.getRootOperations()
                     .stream()
                     .map(operation -> operation.getSrcNode().getParent(CtMethod.class))
-                    .filter(Objects::nonNull) // it seems that gumtree can return null value
-                    .collect(Collectors.toSet());
+                    .filter(Objects::nonNull)
+                    .map(method -> {
+                        final CtClass<?> ctClass = factory.Class().get(method.getDeclaringType().getQualifiedName());
+                        final CtTypeReference<?>[] ctTypeReferences = (CtTypeReference<?>[]) method.getParameters()
+                                .stream()
+                                .map(parameter -> ((CtParameter) parameter).getType())
+                                .toArray(value -> new CtTypeReference<?>[value]);
+                        return ctClass.getMethod(method.getType(),
+                                method.getSimpleName(),
+                                ctTypeReferences
+                        );
+                    }).filter(Objects::nonNull); // it seems that gumtree can return null value;
         } catch (Exception ignored) {
             // if something bad happen, we do not care, we go for next file
-            return Collections.emptySet();
+            return Stream.of();
         }
     }
 

dspot/src/test/java/fr/inria/diversify/dspot/assertGenerator/AssertionRemoverTest.java

@@ -104,6 +104,6 @@ public void testOnDifferentKindOfAssertions() throws Exception {
 		final CtMethod<?> testMethod = testClass.getMethodsByName("test").get(0);
 		final CtMethod<?> removedAssertion = assertionRemover.removeAssertion(testMethod);
 		System.out.println(removedAssertion);
-		assertEquals(2, removedAssertion.getBody().getStatements().size());
+		assertEquals(3, removedAssertion.getBody().getStatements().size());
 	}
 }

dspot/src/test/java/fr/inria/diversify/utils/AmplificationCheckerTest.java

@@ -44,10 +44,11 @@ public void testIsAssert() throws Exception {
 			isAssert method should be match all the kind of assertions:
 				For now, the supported types are:
 					assert*** (from org.junit)
-					assertThat (from google.truth)
+					assertThat / isEqualsTo (from google.truth)
+					then / hasSameClassAs (from assertj)
 
 			see src/test/resources/sample/src/test/java/fr/inria/helper/TestWithMultipleAsserts.java
-			Also, the isAssert method will math invocation on methods that contain assertions
+			Also, the isAssert method will match invocation on methods that contain assertions
 		 */
 
         CtClass<?> classTest = Utils.getFactory().Class().get("fr.inria.helper.TestWithMultipleAsserts");
@@ -58,4 +59,14 @@ public void testIsAssert() throws Exception {
         assertEquals(11, collect.size());
     }
 
+    @Test
+    public void testIsAssert2() throws Exception {
+        final CtClass<?> testClass = Utils.findClass("fr.inria.assertionremover.TestClassWithAssertToBeRemoved");
+        final List<CtInvocation> invocations = testClass.getMethodsByName("test1")
+                .get(0)
+                .getElements(new TypeFilter<>(CtInvocation.class));
+        final List<CtInvocation> collect = invocations.stream().filter(AmplificationChecker::isAssert).collect(Collectors.toList());
+        System.out.println(collect);
+        assertEquals(3, collect.size());
+    }
 }

dspot/src/test/resources/sample/src/test/java/fr/inria/helper/TestWithMultipleAsserts.java

@@ -20,6 +20,11 @@ public void test() {
         System.out.println("");
         System.out.println("");
         then(0).isInstanceOf(int.class).hasSameClassAs(0);
+        notVerify();
+    }
+
+    private void notVerify() {
+        // empty
     }
 
     private void verify(String s) {