diff --git a/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java b/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java index d7d5351f6..d8b2a8daa 100644 --- a/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java +++ b/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java @@ -19,8 +19,10 @@ 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; @@ -28,6 +30,8 @@ import org.pitest.sequence.SlotRead; import org.pitest.sequence.SlotWrite; +import java.util.function.Predicate; + public class InstructionMatchers { public static Match anyInstruction() { @@ -40,7 +44,28 @@ public static Match anyInstruction() { public static Match notAnInstruction() { return isA(LineNumberNode.class).or(isA(FrameNode.class)); } - + + public static Match 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 ldcString(Predicate 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 opCode(final int opcode) { return (c, a) -> result(a.getOpcode() == opcode, c); } diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/lombok/LombokFilter.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/lombok/LombokFilter.java new file mode 100644 index 000000000..b6ea7a261 --- /dev/null +++ b/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); + } + +} + diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/lombok/LombokNullFilter.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/lombok/LombokNullFilter.java new file mode 100644 index 000000000..7bfdbe102 --- /dev/null +++ b/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 START = Slot.create(AbstractInsnNode.class); + static final Slot END = Slot.create(AbstractInsnNode.class); + + static final SequenceMatcher 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 store(SlotWrite slot) { + return (c,n) -> result(true, c.store(slot, n)); + } + + + protected List 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(); + } +} diff --git a/pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory b/pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory index 688a5e911..3a8988333 100755 --- a/pitest-entry/src/main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory +++ b/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 diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/exclude/FirstLineInterceptorFactoryTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/exclude/FirstLineInterceptorFactoryTest.java index 6a418b333..8485aec1c 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/exclude/FirstLineInterceptorFactoryTest.java +++ b/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 diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/lombok/LombokNullFilterTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/lombok/LombokNullFilterTest.java new file mode 100644 index 000000000..e3c863e94 --- /dev/null +++ b/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"); + } + } +} \ No newline at end of file diff --git a/pitest-entry/src/test/resources/sampleClasses/lombok/ExampleNotNull.class.bin b/pitest-entry/src/test/resources/sampleClasses/lombok/ExampleNotNull.class.bin new file mode 100644 index 000000000..bf7244bcc Binary files /dev/null and b/pitest-entry/src/test/resources/sampleClasses/lombok/ExampleNotNull.class.bin differ