Skip to content

Commit

Permalink
filter lombok autogenerated null checks
Browse files Browse the repository at this point in the history
  • Loading branch information
hcoles committed Oct 23, 2023
1 parent 7aa83f9 commit 3c5a210
Show file tree
Hide file tree
Showing 7 changed files with 214 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}

}

Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

public class FirstLineInterceptorFactoryTest {

GroovyFilterFactory underTest = new GroovyFilterFactory();
FirstLineInterceptorFactory underTest = new FirstLineInterceptorFactory();

@Test
public void isOnChain() {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
Binary file not shown.

0 comments on commit 3c5a210

Please sign in to comment.