Merge pull request #919 from hcoles/records

Filter junk mutations from java records

README.md

@@ -8,6 +8,10 @@ Read all about it at http://pitest.org
 
 ## Releases
 
+### current snapshot
+
+* #919 Filter junk mutations in java records
+
 ### 1.6.8
 
 * #917 - Add method to retrieve all mutator ids for pitclipse and other tooling

pitest-entry/src/main/java/org/pitest/bytecode/analysis/ClassTree.java

@@ -3,6 +3,7 @@
 import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Function;
@@ -11,6 +12,7 @@
 import org.objectweb.asm.tree.AnnotationNode;
 import org.objectweb.asm.tree.ClassNode;
 import org.objectweb.asm.tree.MethodNode;
+import org.objectweb.asm.tree.RecordComponentNode;
 import org.objectweb.asm.util.Textifier;
 import org.objectweb.asm.util.TraceClassVisitor;
 import org.pitest.classinfo.ClassName;
@@ -57,6 +59,14 @@ public List<AnnotationNode> annotations() {
     return annotaions;
   }
 
+  public List<RecordComponentNode> recordComponents() {
+    if (rawNode.recordComponents == null) {
+      return Collections.emptyList();
+    }
+
+    return rawNode.recordComponents;
+  }
+
   private static Function<MethodNode, MethodTree> toTree(final ClassName name) {
     return a -> new MethodTree(name,a);
   }

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/RecordFilter.java

@@ -0,0 +1,112 @@
+package org.pitest.mutationtest.build.intercept.javafeatures;
+
+import org.objectweb.asm.Type;
+import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.InvokeDynamicInsnNode;
+import org.objectweb.asm.tree.RecordComponentNode;
+import org.pitest.bytecode.analysis.ClassTree;
+import org.pitest.bytecode.analysis.MethodTree;
+import org.pitest.mutationtest.build.InterceptorType;
+import org.pitest.mutationtest.build.MutationInterceptor;
+import org.pitest.mutationtest.engine.Mutater;
+import org.pitest.mutationtest.engine.MutationDetails;
+
+import java.util.Collection;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+
+public class RecordFilter implements MutationInterceptor {
+
+    private boolean isRecord;
+    private ClassTree currentClass;
+
+    @Override
+    public InterceptorType type() {
+        return InterceptorType.FILTER;
+    }
+
+    @Override
+    public void begin(ClassTree clazz) {
+        isRecord = "java/lang/Record".equals(clazz.rawNode().superName);
+        currentClass = clazz;
+    }
+
+    @Override
+    public Collection<MutationDetails> intercept(Collection<MutationDetails> mutations, Mutater m) {
+        if (isRecord) {
+            return mutations.stream()
+                    .filter(makeMethodFilter(currentClass))
+                    .collect(Collectors.toList());
+        }
+        return mutations;
+    }
+
+    private Predicate<MutationDetails> makeMethodFilter(ClassTree currentClass) {
+        Set<String> accessorNames = currentClass.recordComponents().stream()
+                .map(this::toName)
+                .collect(Collectors.toSet());
+        return m -> !accessorNames.contains(m.getMethod().name()) && !isStandardMethod(accessorNames.size(), m);
+    }
+
+    private boolean isStandardMethod(int numberOfComponents, MutationDetails m) {
+        return isRecordInit(m, numberOfComponents)
+                || isRecordEquals(m)
+                || isRecordHashCode(m)
+                || isRecordToString(m);
+    }
+
+    private boolean isRecordInit(MutationDetails m, int numberOfComponents) {
+        // constructors with the same airty as the generated ones, but different
+        // types won't get mutated. They're probably rare enough that this doesn't matter.
+        int airty = Type.getArgumentTypes(m.getId().getLocation().getMethodDesc()).length;
+        return m.getMethod().name().equals("<init>") && airty == numberOfComponents;
+    }
+
+    private boolean isRecordEquals(MutationDetails m) {
+        return m.getId().getLocation().getMethodDesc().equals("(Ljava/lang/Object;)Z")
+                && m.getMethod().name().equals("equals")
+                && hasDynamicObjectMethodsCall(m);
+    }
+
+    private boolean isRecordHashCode(MutationDetails m) {
+        return m.getId().getLocation().getMethodDesc().equals("()I")
+                && m.getMethod().name().equals("hashCode")
+                && hasDynamicObjectMethodsCall(m);
+    }
+
+    private boolean isRecordToString(MutationDetails m) {
+        return m.getId().getLocation().getMethodDesc().equals("()Ljava/lang/String;")
+                && m.getMethod().name().equals("toString")
+                && hasDynamicObjectMethodsCall(m);
+    }
+
+    private boolean hasDynamicObjectMethodsCall(MutationDetails mutation) {
+        // java/lang/runtime/ObjectMethods was added to support records and can be used as a marker
+        // for an auth generated equals method. It's not likely that a custom method would
+        // contain a dynamic call to it
+        Optional<MethodTree> method = currentClass.method(mutation.getId().getLocation());
+        return method.filter(m -> m.instructions().stream().anyMatch(this::isInvokeDynamicCallToObjectMethods))
+                .isPresent();
+    }
+
+    private boolean isInvokeDynamicCallToObjectMethods(AbstractInsnNode node) {
+        if (node instanceof InvokeDynamicInsnNode) {
+            InvokeDynamicInsnNode call = (InvokeDynamicInsnNode) node;
+            return call.bsm.getOwner().equals("java/lang/runtime/ObjectMethods")
+                && call.bsm.getName().equals("bootstrap");
+
+        }
+        return false;
+    }
+
+    private String toName(RecordComponentNode recordComponentNode) {
+       return recordComponentNode.name;
+    }
+
+    @Override
+    public void end() {
+        currentClass = null;
+    }
+}

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/RecordFilterFactory.java

@@ -0,0 +1,27 @@
+package org.pitest.mutationtest.build.intercept.javafeatures;
+
+import org.pitest.mutationtest.build.InterceptorParameters;
+import org.pitest.mutationtest.build.MutationInterceptor;
+import org.pitest.mutationtest.build.MutationInterceptorFactory;
+import org.pitest.plugin.Feature;
+
+public class RecordFilterFactory implements MutationInterceptorFactory {
+
+    @Override
+    public String description() {
+        return "Record junk mutation filter";
+    }
+
+    @Override
+    public MutationInterceptor createInterceptor(InterceptorParameters params) {
+        return new RecordFilter();
+    }
+
+    @Override
+    public Feature provides() {
+        return Feature.named("FRECORD")
+                .withOnByDefault(true)
+                .withDescription("Filters mutations in compiler generated record code");
+    }
+
+}

pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory

@@ -9,6 +9,7 @@ org.pitest.mutationtest.build.intercept.javafeatures.ImplicitNullCheckFilterFact
 org.pitest.mutationtest.build.intercept.javafeatures.MethodReferenceNullCheckFilterFactory
 org.pitest.mutationtest.build.intercept.javafeatures.ForEachLoopFilterFactory
 org.pitest.mutationtest.build.intercept.javafeatures.EnumConstructorFilterFactory
+org.pitest.mutationtest.build.intercept.javafeatures.RecordFilterFactory
 org.pitest.mutationtest.build.intercept.logging.LoggingCallsFilterFactory
 org.pitest.mutationtest.build.intercept.timeout.InfiniteForLoopFilterFactory
 org.pitest.mutationtest.build.intercept.timeout.InfiniteIteratorLoopFilterFactory

pitest-entry/src/test/java/org/pitest/mutationtest/build/MutationDiscoveryTest.java

@@ -211,6 +211,19 @@ public void filtersEquivalentReturnValsMutants() {
     assertThat(actual).isEmpty();
   }
 
+  @Test
+  public void filterMutantsInJavaRecords() {
+    this.data.setDetectInlinedCode(true);
+
+    final ClassName clazz = ClassName.fromString("records/PureRecord_javac");
+    final Collection<MutationDetails> actual = findMutants(clazz);
+    assertThat(actual).isEmpty();
+
+    this.data.setFeatures(Collections.singletonList("-FRECORD"));
+    final Collection<MutationDetails> actualWithoutFilter = findMutants(clazz);
+    assertThat(actualWithoutFilter).isNotEmpty();
+  }
+
   public static class AnnotatedToAvoidMethod {
     public int a() {
       return 1;

pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/FilterTester.java

@@ -134,6 +134,17 @@ public void assertFiltersNoMutationsMatching(Predicate<MutationDetails> match, C
     softly.assertAll();
   }
 
+  public void assertFiltersNoMutationsMatching(Predicate<MutationDetails> match, String sample) {
+    final GregorMutater mutator = mutateFromResourceDir();
+    final SoftAssertions softly = new SoftAssertions();
+
+    for (final Sample s : samples(sample)) {
+      assertFiltersNoMatchingMutants(match, mutator, s, softly);
+    }
+
+    softly.assertAll();
+  }
+
   public void assertFiltersMutationsMatching(Predicate<MutationDetails> match, Class<?> clazz) {
     final Sample s = makeSampleForCurrentCompiler(clazz);
 

pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/NotARecord.java

@@ -0,0 +1,13 @@
+package org.pitest.mutationtest.build.intercept.javafeatures;
+
+public class NotARecord {
+    private final String value;
+
+    public NotARecord(String value) {
+        this.value = value;
+    }
+
+    public final String value() {
+        return value;
+    }
+}

pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/RecordFilterTest.java

@@ -0,0 +1,75 @@
+package org.pitest.mutationtest.build.intercept.javafeatures;
+
+import org.junit.Test;
+import org.pitest.mutationtest.build.InterceptorType;
+import org.pitest.mutationtest.engine.MutationDetails;
+import org.pitest.mutationtest.engine.gregor.config.Mutator;
+
+import java.util.function.Predicate;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class RecordFilterTest {
+    private static final String             PATH      = "records/{0}_{1}";
+
+    RecordFilter testee = new RecordFilter();
+    FilterTester verifier = new FilterTester(PATH, this.testee, Mutator.all());
+
+    @Test
+    public void declaresTypeAsFilter() {
+        assertThat(this.testee.type()).isEqualTo(InterceptorType.FILTER);
+    }
+
+    @Test
+    public void findsNoMutantsInEmptyRecord() {
+        this.verifier.assertLeavesNMutants(0, "NoDataRecord");
+    }
+
+    @Test
+    public void findsNoMutantsInPureRecord() {
+        this.verifier.assertLeavesNMutants(0, "PureRecord");
+    }
+
+    @Test
+    public void findsMutantsInNormalClass() {
+        this.verifier.assertFiltersNoMutationsMatching(m -> true, NotARecord.class);
+    }
+
+    @Test
+    public void mutatesNonRecordMethods() {
+        this.verifier.assertFiltersNoMutationsMatching(inMethodCalled("extraMethod"), "CustomRecord");
+    }
+
+    @Test
+    public void mutatesCustomConstructors() {
+        this.verifier.assertFiltersNoMutationsMatching(inMethodCalled("<init>").and(removesSysOutCall()),
+                "RecordWithCustomConstructor");
+    }
+
+    @Test
+    public void mutatesCustomEqualsMethods() {
+        this.verifier.assertFiltersNoMutationsMatching(inMethodCalled("equals"),
+                "RecordWithCustomEquals");
+    }
+
+    @Test
+    public void mutatesCustomHashCodeMethods() {
+        this.verifier.assertFiltersNoMutationsMatching(inMethodCalled("hashCode"),
+                "RecordWithCustomHashCode");
+    }
+
+    @Test
+    public void mutatesCustomToStringMethods() {
+        this.verifier.assertFiltersNoMutationsMatching(inMethodCalled("toString"),
+                "RecordWithCustomToString");
+    }
+
+    private Predicate<MutationDetails> removesSysOutCall() {
+        return m -> m.getDescription().contains("PrintStream::println");
+    }
+
+    private Predicate<MutationDetails> inMethodCalled(String name) {
+        return m -> m.getMethod().name().equals(name);
+    }
+
+}