Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

filter lombok autogenerated null checks #1268

Merged
merged 1 commit into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.