Skip to content

Commit

Permalink
Merge pull request #997 from hcoles/bug/try-with-resource-empty-return
Browse files Browse the repository at this point in the history 
Bug/try with resource empty return
  • Loading branch information
@hcoles
hcoles authored Mar 10, 2022
2 parents 0c1daf2 + c8e8088 commit 5c2455d
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 28 deletions.
85 changes: 58 additions & 27 deletions ...va/org/pitest/mutationtest/build/intercept/equivalent/EquivalentReturnMutationFilter.java
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package org.pitest.mutationtest.build.intercept.equivalent;

import static org.objectweb.asm.Opcodes.ALOAD;
import static org.objectweb.asm.Opcodes.ASTORE;
import static org.pitest.bytecode.analysis.InstructionMatchers.aVariableAccess;
import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
import static org.pitest.bytecode.analysis.InstructionMatchers.getStatic;
import static org.pitest.bytecode.analysis.InstructionMatchers.isA;
import static org.pitest.bytecode.analysis.InstructionMatchers.isInstruction;
import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallNamed;
import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallTo;
import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction;
import static org.pitest.bytecode.analysis.InstructionMatchers.opCode;
import static org.pitest.bytecode.analysis.InstructionMatchers.variableMatches;

import java.util.Arrays;
import java.util.Collection;
Expand All @@ -23,6 +28,7 @@
import org.pitest.bytecode.analysis.ClassTree;
import org.pitest.bytecode.analysis.MethodMatchers;
import org.pitest.bytecode.analysis.MethodTree;
import org.pitest.classinfo.ClassName;
import org.pitest.functional.FCollection;
import org.pitest.functional.prelude.Prelude;
import org.pitest.mutationtest.build.CompoundMutationInterceptor;
Expand Down Expand Up @@ -219,6 +225,7 @@ public void end() {
class EmptyReturnsFilter implements MutationInterceptor {

private static final Slot<AbstractInsnNode> MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class);
private static final Slot<Integer> LOCAL_VAR = Slot.create(Integer.class);

static final SequenceQuery<AbstractInsnNode> CONSTANT_ZERO = QueryStart
.match(isZeroConstant())
Expand All @@ -230,13 +237,28 @@ class EmptyReturnsFilter implements MutationInterceptor {
static final SequenceMatcher<AbstractInsnNode> ZERO_VALUES = QueryStart
.any(AbstractInsnNode.class)
.zeroOrMore(QueryStart.match(anyInstruction()))
.then(CONSTANT_ZERO.or(CONSTANT_FALSE))
.then(CONSTANT_ZERO.or(CONSTANT_FALSE).or(QueryStart.match(loadsEmptyReturnOntoStack())))
.then(isInstruction(MUTATED_INSTRUCTION.read()))
.zeroOrMore(QueryStart.match(anyInstruction()))
.compile(QueryParams.params(AbstractInsnNode.class)
.withIgnores(notAnInstruction().or(isA(LabelNode.class)))
);

static final SequenceMatcher<AbstractInsnNode> INDIRECT_ZERO_VALUES = QueryStart
.any(AbstractInsnNode.class)
.zeroOrMore(QueryStart.match(anyInstruction()))
.then(loadsEmptyReturnOntoStack())
.then(aStoreTo(LOCAL_VAR))
// match anything that doesn't overwrite the local var
// possible we will get issues here if there is a jump instruction
// to get to the point that the empty value is returned.
.zeroOrMore(QueryStart.match(aStoreTo(LOCAL_VAR).negate()))
.then(opCode(ALOAD).and(variableMatches(LOCAL_VAR.read())))
.then(isInstruction(MUTATED_INSTRUCTION.read()))
.zeroOrMore(QueryStart.match(anyInstruction()))
.compile(QueryParams.params(AbstractInsnNode.class)
.withIgnores(notAnInstruction().or(isA(LabelNode.class)))
);

private static final Set<String> MUTATOR_IDS = new HashSet<>();
private static final Set<Integer> ZERO_CONSTANTS = new HashSet<>();
Expand All @@ -257,7 +279,7 @@ public InterceptorType type() {
return InterceptorType.FILTER;
}

@Override
@Override
public void begin(ClassTree clazz) {
this.currentClass = clazz;
}
Expand All @@ -268,6 +290,10 @@ public Collection<MutationDetails> intercept(
return FCollection.filter(mutations, Prelude.not(isEquivalent(m)));
}

private static Match<AbstractInsnNode> aStoreTo(Slot<Integer> variable) {
return opCode(ASTORE).and(aVariableAccess(variable.write()));
}

private static Match<AbstractInsnNode> isZeroConstant() {
return (context,node) -> ZERO_CONSTANTS.contains(node.getOpcode());
}
Expand All @@ -285,35 +311,16 @@ public boolean test(MutationDetails a) {
.findFirst()
.get();
final int mutatedInstruction = a.getInstructionIndex();
return returnsZeroValue(method, mutatedInstruction)
|| returnsEmptyString(method, mutatedInstruction)
|| returns(method, mutatedInstruction, "java/util/Optional","empty")
|| returns(method, mutatedInstruction, "java/util/stream/Stream","empty")
|| returns(method, mutatedInstruction, "java/util/Collections","emptyList")
|| returns(method, mutatedInstruction, "java/util/Collections","emptyMap")
|| returns(method, mutatedInstruction, "java/util/Collections","emptySet")
|| returns(method, mutatedInstruction, "java/util/List","of")
|| returns(method, mutatedInstruction, "java/util/Set","of");
return returnsZeroValue(ZERO_VALUES, method, mutatedInstruction)
|| returnsZeroValue(INDIRECT_ZERO_VALUES, method, mutatedInstruction)
|| returnsEmptyString(method, mutatedInstruction);
}

private Boolean returnsZeroValue(MethodTree method,
int mutatedInstruction) {
private Boolean returnsZeroValue(SequenceMatcher<AbstractInsnNode> sequence, MethodTree method,
int mutatedInstruction) {
final Context<AbstractInsnNode> context = Context.start(method.instructions(), false);
context.store(MUTATED_INSTRUCTION.write(), method.instruction(mutatedInstruction));
return ZERO_VALUES.matches(method.instructions(), context);
}

private boolean returns(MethodTree method, int mutatedInstruction, String owner, String name) {
final AbstractInsnNode node = method.realInstructionBefore(mutatedInstruction);
if (node instanceof MethodInsnNode ) {
final MethodInsnNode call = (MethodInsnNode) node;
return call.owner.equals(owner) && call.name.equals(name) && takesNoArguments(call.desc);
}
return false;
}

private boolean takesNoArguments(String desc) {
return Type.getArgumentTypes(desc).length == 0;
return sequence.matches(method.instructions(), context);
}

private boolean returnsEmptyString(MethodTree method,
Expand All @@ -329,6 +336,30 @@ private boolean returnsEmptyString(MethodTree method,
};
}

private static Match<AbstractInsnNode> loadsEmptyReturnOntoStack() {
return noArgsCall("java/util/Optional", "empty")
.or(noArgsCall("java/util/stream/Stream", "empty"))
.or(noArgsCall("java/util/Collections", "emptyList"))
.or(noArgsCall("java/util/Collections", "emptyMap"))
.or(noArgsCall("java/util/Collections", "emptySet"))
.or(noArgsCall("java/util/List", "of"))
.or(noArgsCall("java/util/Set", "of"));
}

private static Match<AbstractInsnNode> noArgsCall(String owner, String name) {
return methodCallTo(ClassName.fromString(owner), name).and(takesNoArgs());
}

private static Match<AbstractInsnNode> takesNoArgs() {
return (c,node) -> {
if (node instanceof MethodInsnNode ) {
final MethodInsnNode call = (MethodInsnNode) node;
return Type.getArgumentTypes(call.desc).length == 0;
}
return false;
};
}

@Override
public void end() {
this.currentClass = null;
Expand Down
18 changes: 18 additions & 0 deletions ...est/java/com/example/emptyreturns/AlreadyReturnsEmptyOptionalInTryWithResourcesBlock.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.example.emptyreturns;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Optional;

public class AlreadyReturnsEmptyOptionalInTryWithResourcesBlock {
public Optional<String> a() throws IOException {
try(ByteArrayOutputStream os = new ByteArrayOutputStream()) {
Double.parseDouble("12");
if (os.size() > 42) {
return Optional.empty();
}
return Optional.of("foo");
}
}

}
37 changes: 36 additions & 1 deletion ...rg/pitest/mutationtest/build/intercept/equivalent/EquivalentReturnMutationFilterTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package org.pitest.mutationtest.build.intercept.equivalent;

import com.example.emptyreturns.AlreadyReturnsEmptyOptionalInTryWithResourcesBlock;
import org.junit.Ignore;
import org.junit.Test;
import org.pitest.mutationtest.build.InterceptorType;
import org.pitest.mutationtest.build.MutationInterceptor;
import org.pitest.mutationtest.build.intercept.javafeatures.FilterTester;
import org.pitest.mutationtest.engine.gregor.mutators.returns.EmptyObjectReturnValsMutator;
import org.pitest.mutationtest.engine.gregor.mutators.returns.NullReturnValsMutator;
import org.pitest.mutationtest.engine.gregor.mutators.returns.PrimitiveReturnsMutator;
import org.pitest.util.CurrentRuntime;

import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -17,14 +20,15 @@
import java.util.stream.Stream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assume.assumeTrue;
import static org.pitest.mutationtest.engine.gregor.mutators.returns.BooleanFalseReturnValsMutator.FALSE_RETURNS;
import static org.pitest.mutationtest.engine.gregor.mutators.returns.BooleanTrueReturnValsMutator.TRUE_RETURNS;

public class EquivalentReturnMutationFilterTest {

MutationInterceptor testee = new EquivalentReturnMutationFilter().createInterceptor(null);

FilterTester verifier = new FilterTester("", this.testee, PrimitiveReturnsMutator.PRIMITIVE_RETURNS
FilterTester verifier = new FilterTester("emptyReturns/{0}_{1}", this.testee, PrimitiveReturnsMutator.PRIMITIVE_RETURNS
, EmptyObjectReturnValsMutator.EMPTY_RETURNS
, NullReturnValsMutator.NULL_RETURNS
, FALSE_RETURNS
Expand Down Expand Up @@ -190,6 +194,26 @@ public void filtersEquivalentOptionalMutants() {
verifier.assertFiltersNMutationFromClass(1, AlreadyReturnsEmptyOptional.class);
}

@Test
public void filtersEquivalentOptionalMutantsInTryBlocks() {
verifier.assertFiltersNMutationFromClass(1, AlreadyReturnsEmptyOptionalInTryBlock.class);
}

@Test
public void filtersEquivalentOptionalMutantsInTryWithResourcesBlocks() {
// skip test if we are running/compiling with java 8 as out analysis can't yet
// handle the bytecode
assumeTrue(CurrentRuntime.version() >= 9);
verifier.assertFiltersNMutationFromClass(1, AlreadyReturnsEmptyOptionalInTryWithResourcesBlock.class);
}

@Test
@Ignore("need more complex analysis")
public void filtersEquivalentOptionalMutantsInTryWithResourcesBlocksForOtherCompilers() {
// javac sample is for java 8
verifier.assertFiltersNMutationFromSample(1, "AlreadyReturnsEmptyOptionalInTryWithResourcesBlock");
}

@Test
public void filtersEquivalentStreamMutants() {
verifier.assertFiltersNMutationFromClass(1, AlreadyReturnsEmptyStream.class);
Expand Down Expand Up @@ -412,6 +436,17 @@ public Optional<String> a() {
}
}

class AlreadyReturnsEmptyOptionalInTryBlock {
public Optional<String> a() {
try {
Double.parseDouble("12");
return Optional.empty();
} catch (Exception ex) {
return Optional.of("foo");
}
}
}

class AlreadyReturnsEmptyStream {
public Stream<String> a() {
return Stream.empty();
Expand Down
17 changes: 17 additions & 0 deletions pitest-entry/src/test/java/org/pitest/util/CurrentRuntime.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.pitest.util;

public class CurrentRuntime {

public static int version() {
String version = System.getProperty("java.version").replace("-ea", "");
if (version.startsWith("1.")) {
version = version.substring(2, 3);
} else {
int dot = version.indexOf(".");
if (dot != -1) {
version = version.substring(0, dot);
}
}
return Integer.parseInt(version);
}
}
Binary file added ...leClasses/emptyReturns/AlreadyReturnsEmptyOptionalInTryWithResourcesBlock_javac.class.bin
View file
Binary file not shown.

0 comments on commit 5c2455d

Please sign in to comment.