Merge pull request #1268 from hcoles/feature/lombok_null_checks

filter lombok autogenerated null checks

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

@@ -19,15 +19,19 @@
 import org.objectweb.asm.tree.IincInsnNode;
 import org.objectweb.asm.tree.JumpInsnNode;
 import org.objectweb.asm.tree.LabelNode;
+import org.objectweb.asm.tree.LdcInsnNode;
 import org.objectweb.asm.tree.LineNumberNode;
 import org.objectweb.asm.tree.MethodInsnNode;
+import org.objectweb.asm.tree.TypeInsnNode;
 import org.objectweb.asm.tree.VarInsnNode;
 import org.pitest.classinfo.ClassName;
 import org.pitest.sequence.Match;
 import org.pitest.sequence.Slot;
 import org.pitest.sequence.SlotRead;
 import org.pitest.sequence.SlotWrite;
 
+import java.util.function.Predicate;
+
 public class InstructionMatchers {
 
   public static Match<AbstractInsnNode> anyInstruction() {
@@ -40,7 +44,28 @@ public static Match<AbstractInsnNode> anyInstruction() {
   public static Match<AbstractInsnNode> notAnInstruction() {
      return isA(LineNumberNode.class).or(isA(FrameNode.class));
   }
-
+
+  public static Match<AbstractInsnNode> newCall(ClassName target) {
+    final String clazz = target.asInternalName();
+    return (c, t) -> {
+      if ( t instanceof TypeInsnNode ) {
+        final TypeInsnNode call = (TypeInsnNode) t;
+        return result(call.getOpcode() == Opcodes.NEW && call.desc.equals(clazz), c);
+      }
+      return result(false, c);
+    };
+  }
+
+  public static Match<AbstractInsnNode> ldcString(Predicate<String> match) {
+    return (c, t) -> {
+      if ( t instanceof LdcInsnNode) {
+        final LdcInsnNode ldc = (LdcInsnNode) t;
+        return result(ldc.cst instanceof String && match.test((String) ldc.cst), c);
+      }
+      return result(false, c);
+    };
+  }
+
   public static Match<AbstractInsnNode> opCode(final int opcode) {
     return (c, a) -> result(a.getOpcode() == opcode, c);
   }

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/lombok/LombokFilter.java

@@ -0,0 +1,28 @@
+package org.pitest.mutationtest.build.intercept.lombok;
+
+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 LombokFilter implements MutationInterceptorFactory {
+
+    @Override
+    public String description() {
+        return "Lombok junk mutations filter";
+    }
+
+    @Override
+    public MutationInterceptor createInterceptor(InterceptorParameters params) {
+        return new LombokNullFilter();
+    }
+
+    @Override
+    public Feature provides() {
+        return Feature.named("lombok")
+                .withDescription("Filters out junk mutations generated by lombok")
+                .withOnByDefault(true);
+    }
+
+}
+

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/lombok/LombokNullFilter.java

@@ -0,0 +1,86 @@
+package org.pitest.mutationtest.build.intercept.lombok;
+
+import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.LabelNode;
+import org.pitest.bytecode.analysis.MethodTree;
+import org.pitest.classinfo.ClassName;
+import org.pitest.mutationtest.build.intercept.Region;
+import org.pitest.mutationtest.build.intercept.RegionInterceptor;
+import org.pitest.sequence.Context;
+import org.pitest.sequence.Match;
+import org.pitest.sequence.QueryParams;
+import org.pitest.sequence.QueryStart;
+import org.pitest.sequence.SequenceMatcher;
+import org.pitest.sequence.Slot;
+import org.pitest.sequence.SlotWrite;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import java.util.stream.Collectors;
+
+import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
+import static org.pitest.bytecode.analysis.InstructionMatchers.isA;
+import static org.pitest.bytecode.analysis.InstructionMatchers.ldcString;
+import static org.pitest.bytecode.analysis.InstructionMatchers.newCall;
+import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.ALOAD;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.ATHROW;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.DUP;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.IFNONNULL;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKESPECIAL;
+import static org.pitest.sequence.Result.result;
+
+public class LombokNullFilter extends RegionInterceptor {
+
+    static final Slot<AbstractInsnNode> START = Slot.create(AbstractInsnNode.class);
+    static final Slot<AbstractInsnNode> END = Slot.create(AbstractInsnNode.class);
+
+    static final SequenceMatcher<AbstractInsnNode> NULL_CHECK = QueryStart
+            .any(AbstractInsnNode.class)
+            .then(ALOAD.and(store(START.write())))
+            .then(IFNONNULL)
+            .then(newCall(ClassName.fromClass(NullPointerException.class)))
+            .then(DUP)
+            // perhaps requiring the string here is too brittle? But makes it less likely to pick up non lombok generated null checks
+            .then(ldcString(s -> s.endsWith("is marked non-null but is null")))
+            .then(INVOKESPECIAL)
+            .then(ATHROW.and(store(END.write())))
+            .zeroOrMore(QueryStart.match(anyInstruction()))
+            .compile(QueryParams.params(AbstractInsnNode.class)
+                    .withIgnores(notAnInstruction().or(isA(LabelNode.class)))
+            );
+
+    private static Match<AbstractInsnNode> store(SlotWrite<AbstractInsnNode> slot) {
+        return (c,n) -> result(true, c.store(slot, n));
+    }
+
+
+    protected List<Region> computeRegions(MethodTree method) {
+        // Should really be checking that the null check if for one
+        // of the annotated parameters, but can likely get away with
+        // this since hand rolled nulls checks are not commonly mixed with
+        // lombok code
+        if (!hasLombokNonNullAnnotation(method)) {
+            return Collections.emptyList();
+        }
+
+        Context context = Context.start();
+        return NULL_CHECK.contextMatches(method.instructions(), context).stream()
+                .map(c -> new Region(c.retrieve(START.read()).get(), c.retrieve(END.read()).get()))
+                .collect(Collectors.toList());
+    }
+
+    private boolean hasLombokNonNullAnnotation(MethodTree method) {
+        if (method.rawNode().invisibleParameterAnnotations == null) {
+            return false;
+        }
+
+        return Arrays.stream(method.rawNode().invisibleParameterAnnotations)
+                .filter( l -> l != null) // asm is nasty
+                .flatMap(l -> l.stream().filter(node -> node.desc.equals("Llombok/NonNull;")))
+                .findAny()
+                .isPresent();
+    }
+}

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

@@ -23,5 +23,6 @@ org.pitest.mutationtest.build.intercept.equivalent.EqualsPerformanceShortcutFilt
 org.pitest.mutationtest.build.intercept.equivalent.EquivalentReturnMutationFilter
 org.pitest.mutationtest.build.intercept.exclude.FirstLineInterceptorFactory
 org.pitest.mutationtest.build.intercept.equivalent.DivisionByMinusOneFilterFactory
+org.pitest.mutationtest.build.intercept.lombok.LombokFilter
 
 org.pitest.plugin.export.MutantExportFactory

pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/exclude/FirstLineInterceptorFactoryTest.java

@@ -7,7 +7,7 @@
 
 public class FirstLineInterceptorFactoryTest {
 
-    GroovyFilterFactory underTest = new GroovyFilterFactory();
+    FirstLineInterceptorFactory underTest = new FirstLineInterceptorFactory();
 
     @Test
     public void isOnChain() {
@@ -16,16 +16,16 @@ public void isOnChain() {
     }
 
     @Test
-    public void isOnByDefault() {
+    public void isOffByDefault() {
         FactoryVerifier.confirmFactory(underTest)
-                .isOnByDefault();
+                .isOffByDefault();
     }
 
 
     @Test
-    public void featureIsCalledFGroovy() {
+    public void featureIsCalledNoFirstLine() {
         FactoryVerifier.confirmFactory(underTest)
-                .featureName().isEqualTo("fgroovy");
+                .featureName().isEqualTo("nofirstline");
     }
 
     @Test

pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/lombok/LombokNullFilterTest.java

@@ -0,0 +1,68 @@
+package org.pitest.mutationtest.build.intercept.lombok;
+
+import org.junit.Test;
+import org.pitest.bytecode.analysis.OpcodeMatchers;
+import org.pitest.mutationtest.build.InterceptorType;
+import org.pitest.mutationtest.build.MutationInterceptorFactory;
+import org.pitest.mutationtest.engine.gregor.mutators.NullMutateEverything;
+import org.pitest.verifier.interceptors.FactoryVerifier;
+import org.pitest.verifier.interceptors.InterceptorVerifier;
+import org.pitest.verifier.interceptors.VerifierStart;
+
+public class LombokNullFilterTest {
+
+    private MutationInterceptorFactory underTest = new LombokFilter();
+    InterceptorVerifier v = VerifierStart.forInterceptorFactory(underTest)
+            .usingMutator(new NullMutateEverything());
+
+    @Test
+    public void isOnChain() {
+        FactoryVerifier.confirmFactory(underTest)
+                .isOnChain();
+    }
+
+    @Test
+    public void isOnByDefault() {
+        FactoryVerifier.confirmFactory(underTest)
+                .isOnByDefault();
+    }
+
+    @Test
+    public void featureIsCalledLombok() {
+        FactoryVerifier.confirmFactory(underTest)
+                .featureName().isEqualTo("lombok");
+    }
+
+    @Test
+    public void createsFilters() {
+        FactoryVerifier.confirmFactory(underTest)
+                .createsInterceptorsOfType(InterceptorType.FILTER);
+    }
+
+    @Test
+    public void filtersMutationsToLombokNotNullCode() {
+        v.usingResourceFolder("lombok")
+                .forClass("ExampleNotNull")
+                .forCodeMatching(OpcodeMatchers.IFNONNULL.asPredicate())
+                .mutantsAreGenerated()
+                .allMutantsAreFiltered()
+                .verify();
+    }
+
+    @Test
+    public void doesNotFilterHandRolledNullChecks() {
+        v.forClass(HandRolledNullChecks.class)
+                .forAnyCode()
+                .mutantsAreGenerated()
+                .noMutantsAreFiltered()
+                .verify();
+    }
+}
+
+class HandRolledNullChecks {
+    void mutateMe(String a, String b) {
+        if (a == null) {
+            throw new NullPointerException("a is marked non-null but is null");
+        }
+    }
+}