diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/ForEachLoopFilter.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/ForEachLoopFilter.java index 607f8005e..3443b0c02 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/ForEachLoopFilter.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/ForEachLoopFilter.java @@ -183,7 +183,7 @@ public Collection intercept( private Predicate mutatesIteratorLoopPlumbing() { return a -> { final int instruction = a.getInstructionIndex(); - final MethodTree method = ForEachLoopFilter.this.currentClass.methods().stream() + final MethodTree method = currentClass.methods().stream() .filter(MethodMatchers.forLocation(a.getId().getLocation())) .findFirst() .get(); diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilter.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilter.java index bfa980923..b5003b7ca 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilter.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilter.java @@ -1,10 +1,7 @@ package org.pitest.mutationtest.build.intercept.javafeatures; -import java.util.Collection; -import java.util.HashSet; -import java.util.Set; -import java.util.function.Predicate; - +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.LabelNode; import org.pitest.bytecode.analysis.ClassTree; import org.pitest.bytecode.analysis.MethodTree; import org.pitest.functional.FCollection; @@ -13,10 +10,129 @@ import org.pitest.mutationtest.build.MutationInterceptor; import org.pitest.mutationtest.engine.Mutater; import org.pitest.mutationtest.engine.MutationDetails; +import org.pitest.sequence.Context; +import org.pitest.sequence.Match; +import org.pitest.sequence.QueryParams; +import org.pitest.sequence.SequenceMatcher; +import org.pitest.sequence.SequenceQuery; +import org.pitest.sequence.Slot; + +import java.util.Collection; +import java.util.function.Predicate; + +import static org.objectweb.asm.Opcodes.ALOAD; +import static org.objectweb.asm.Opcodes.ASTORE; +import static org.objectweb.asm.Opcodes.ATHROW; +import static org.objectweb.asm.Opcodes.GOTO; +import static org.objectweb.asm.Opcodes.IFNONNULL; +import static org.objectweb.asm.Opcodes.IFNULL; +import static org.objectweb.asm.Opcodes.IF_ACMPEQ; +import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction; +import static org.pitest.bytecode.analysis.InstructionMatchers.isA; +import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallNamed; +import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction; +import static org.pitest.bytecode.analysis.InstructionMatchers.opCode; +import static org.pitest.bytecode.analysis.InstructionMatchers.recordTarget; +import static org.pitest.sequence.QueryStart.any; +import static org.pitest.sequence.QueryStart.match; public class TryWithResourcesFilter implements MutationInterceptor { - private Set lines; + private static final boolean DEBUG = false; + + private static final Slot MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class); + private static final Slot FOUND = Slot.create(Boolean.class); + + private static SequenceQuery javac11() { + return any(AbstractInsnNode.class) + .zeroOrMore(match(anyInstruction())) + .then(aLabel()) + .then(anALoad()) + .then(closeCall()) + .then(aLabel()) + .then(aGoto()) + .then(aLabel()) + .then(anAStore()) + .then(anALoad()) + .then(anALoad()) + .then(addSuppressedCall()) + .zeroOrMore(match(anyInstruction())); + } + + private static SequenceQuery javac8() { + return any(AbstractInsnNode.class) + .zeroOrMore(match(anyInstruction())) + .then(ifNull()) + .then(anALoad()) + .then(ifNull()) + .then(aLabel()) + .then(anALoad()) + .then(closeCall()) + .then(aLabel()) + .then(aGoto()) + .then(aLabel()) + .then(anAStore()) + .then(aLabel()) + .then(anALoad()) + .then(anALoad()) + .then(addSuppressedCall()) + .then(aLabel()) + .then(aGoto()) + .then(aLabel()) + .then(anALoad()) + .then(closeCall()) + .zeroOrMore(match(anyInstruction())); + } + + private static SequenceQuery ecj() { + return any(AbstractInsnNode.class) + .zeroOrMore(match(anyInstruction())) + .then(closeCall()) + .then(aLabel()) + .then(anALoad()) + .then(opCode(ATHROW).and(mutationPoint())) + .then(aLabel()) + .then(anAStore()) + .then(anALoad()) + .then(ifNonNull()) + .then(anALoad()) + .then(anAStore()) + .then(aGoto()) + .then(aLabel()) + .zeroOrMore(match(anyInstruction())); + } + + private static SequenceQuery ecjAddSuppressedCheck() { + return any(AbstractInsnNode.class) + .zeroOrMore(match(anyInstruction())) + .then(ifNonNull()) + .then(anALoad()) + .then(anAStore()) + .then(aGoto()) + .then(aLabel()) + .then(anALoad()) + .then(anALoad()) + .then(opCode(IF_ACMPEQ).and(mutationPoint())) + .then(anALoad()) + .then(anALoad()) + .then(addSuppressedCall()) + .then(aLabel()) + .zeroOrMore(match(anyInstruction())); + } + + + private static final SequenceMatcher TRY_WITH_RESOURCES = match(Match.never()) + .or(javac11()) + .or(javac8()) + .or(ecj()) + .or(ecjAddSuppressedCheck()) + .then(containMutation(FOUND)) + .compile(QueryParams.params(AbstractInsnNode.class) + .withIgnores(notAnInstruction()) + .withDebug(DEBUG) + ); + + private ClassTree currentClass; @Override public InterceptorType type() { @@ -25,29 +141,76 @@ public InterceptorType type() { @Override public void begin(ClassTree clazz) { - this.lines = new HashSet<>(); - for (final MethodTree each : clazz.methods()) { - checkMehod(each,this.lines); - } - } - - private void checkMehod(MethodTree each, Set lines) { - each.rawNode().accept(new TryWithResourcesMethodVisitor(lines)); + this.currentClass = clazz; } @Override public Collection intercept( - Collection mutations, Mutater m) { - return FCollection.filter(mutations, Prelude.not(isOnMarkedLine())); + Collection mutations, Mutater m) { + return FCollection.filter(mutations, Prelude.not(mutatesTryWithResourcesScaffolding())); } - private Predicate isOnMarkedLine() { - return a -> TryWithResourcesFilter.this.lines.contains(a.getClassLine().getLineNumber()); + private Predicate mutatesTryWithResourcesScaffolding() { + return a -> { + int instruction = a.getInstructionIndex(); + MethodTree method = currentClass.method(a.getId().getLocation()) + .orElseThrow(() -> new IllegalStateException("Could not find method for mutant " + a)); + + // performance hack + if (method.rawNode().tryCatchBlocks.size() <= 1) { + return false; + } + + AbstractInsnNode mutatedInstruction = method.instruction(instruction); + + Context context = Context.start(method.instructions(), DEBUG); + context.store(MUTATED_INSTRUCTION.write(), mutatedInstruction); + return TRY_WITH_RESOURCES.matches(method.instructions(), context); + }; } @Override public void end() { + this.currentClass = null; + } + + private static Match aLabel() { + return isA(LabelNode.class); + } + private static Match anALoad() { + return opCode(ALOAD).and(mutationPoint()); + } + + private static Match aGoto() { + return opCode(GOTO).and(mutationPoint()); + } + + private static Match addSuppressedCall() { + return methodCallNamed("addSuppressed").and(mutationPoint()); + } + + private static Match anAStore() { + return opCode(ASTORE).and(mutationPoint()); + } + + private static Match closeCall() { + return methodCallNamed("close").and(mutationPoint()); + } + + private static Match ifNonNull() { + return opCode(IFNONNULL).and(mutationPoint()); + } + + private static Match ifNull() { + return opCode(IFNULL).and(mutationPoint()); + } + + private static Match mutationPoint() { + return recordTarget(MUTATED_INSTRUCTION.read(), FOUND.write()); + } + private static Match containMutation(final Slot found) { + return (c, t) -> c.retrieve(found.read()).isPresent(); } } diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesMethodVisitor.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesMethodVisitor.java deleted file mode 100644 index c8168a903..000000000 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesMethodVisitor.java +++ /dev/null @@ -1,229 +0,0 @@ -/* - * Copyright 2014 Artem Khvastunov - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and limitations under the License. - */ -package org.pitest.mutationtest.build.intercept.javafeatures; - -import org.objectweb.asm.Label; -import org.objectweb.asm.MethodVisitor; -import org.objectweb.asm.Opcodes; -import org.pitest.bytecode.ASMVersion; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Set; - -import static org.objectweb.asm.Opcodes.ALOAD; -import static org.objectweb.asm.Opcodes.ASTORE; -import static org.objectweb.asm.Opcodes.ATHROW; -import static org.objectweb.asm.Opcodes.GOTO; -import static org.objectweb.asm.Opcodes.IFNONNULL; -import static org.objectweb.asm.Opcodes.IFNULL; -import static org.objectweb.asm.Opcodes.IF_ACMPEQ; -import static org.objectweb.asm.Opcodes.INVOKEINTERFACE; -import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL; - -/** - * Example of code that was generated by 1.7 compiler for try-with-resources - * block: - * - * 
- * 
- * } finally {
- *   if (closeable != null) { // IFNULL
- *     if (localThrowable2 != null) { // IFNULL
- *       try {
- *         closeable.close(); // INVOKEVIRTUAL or INVOKEINTERFACE
- *       } catch (Throwable x2) {
- *         localThrowable2.addSuppressed(x2); // INVOKEVIRTUAL
- *       }
- *     } else {
- *       closeable.close(); // INVOKEVIRTUAL or INVOKEINTERFACE
- *     }
- *   }
- * } // ATHROW
- * 
- *
- * - * This class considers that only auto generated code may have such sequence - * without any line change. Such an approach make sense only for javac - * compiler. - *

- * Eclipse Java Compiler as well as aspectj - * have its own opinion how to compile try-with-resources block: - * - * 

- * 
- * } finally {
- *   if (throwable1 == null) { // IFNONNULL
- *     throwable1 = throwable2;
- *   } else {
- *     if (throwable1 != throwable2) { // IF_ACMPEQ
- *       throwable1.addSuppressed(throwable2); // INVOKEVIRTUAL
- *     }
- *   }
- * } // ATHROW
- * 
- *
- * - * @author Artem Khvastunov <contact@artspb.me> - */ -class TryWithResourcesMethodVisitor extends MethodVisitor { - - private static final List JAVAC_CLASS_INS_SEQUENCE = Arrays - .asList( - ASTORE, // store - // throwable - ALOAD, - IFNULL, // closeable - // != - // null - ALOAD, - IFNULL, // localThrowable2 - // != - // null - ALOAD, - INVOKEVIRTUAL, - GOTO, // closeable.close() - ASTORE, // Throwable - // x2 - ALOAD, - ALOAD, - INVOKEVIRTUAL, - GOTO, // localThrowable2.addSuppressed(x2) - ALOAD, - INVOKEVIRTUAL, // closeable.close() - ALOAD, - ATHROW); // throw - // throwable - - private static final List JAVAC_INTERFACE_INS_SEQUENCE = Arrays - .asList( - ASTORE, // store - // throwable - ALOAD, - IFNULL, // closeable - // != - // null - ALOAD, - IFNULL, // localThrowable2 - // != - // null - ALOAD, - INVOKEINTERFACE, - GOTO, // closeable.close() - ASTORE, // Throwable - // x2 - ALOAD, - ALOAD, - INVOKEVIRTUAL, - GOTO, // localThrowable2.addSuppressed(x2) - ALOAD, - INVOKEINTERFACE, // closeable.close() - ALOAD, - ATHROW); // throw - // throwable - - private static final List ECJ_INS_SEQUENCE = Arrays - .asList( - ASTORE, // store - // throwable2 - ALOAD, - IFNONNULL, // if - // (throwable1 - // == - // null) - ALOAD, - ASTORE, - GOTO, // throwable1 - // = - // throwable2; - ALOAD, - ALOAD, - IF_ACMPEQ, // if - // (throwable1 - // != - // throwable2) - // { - ALOAD, - ALOAD, - INVOKEVIRTUAL, // throwable1.addSuppressed(throwable2) - ALOAD, - ATHROW); // throw - // throwable1 - - private final Set lines; - - private final List opcodesStack = new ArrayList<>(); - private int currentLineNumber; - - /** - * @param lines - * to store detected line numbers - */ - TryWithResourcesMethodVisitor(final Set lines) { - super(ASMVersion.ASM_VERSION); - this.lines = lines; - } - - @Override - public void visitLineNumber(int line, Label start) { - prepareToStartTracking(); - this.currentLineNumber = line; - super.visitLineNumber(line, start); - } - - @Override - public void visitVarInsn(int opcode, int var) { - this.opcodesStack.add(opcode); - super.visitVarInsn(opcode, var); - } - - @Override - public void visitJumpInsn(int opcode, Label label) { - this.opcodesStack.add(opcode); - super.visitJumpInsn(opcode, label); - } - - @Override - public void visitMethodInsn(int opcode, String owner, String name, - String desc, boolean itf) { - this.opcodesStack.add(opcode); - super.visitMethodInsn(opcode, owner, name, desc, itf); - } - - @Override - public void visitInsn(int opcode) { - if (opcode == Opcodes.ATHROW) { - this.opcodesStack.add(opcode); - finishTracking(); - } - super.visitInsn(opcode); - } - - private void finishTracking() { - if (JAVAC_CLASS_INS_SEQUENCE.equals(this.opcodesStack) - || JAVAC_INTERFACE_INS_SEQUENCE.equals(this.opcodesStack) - || ECJ_INS_SEQUENCE.equals(this.opcodesStack)) { - this.lines.add(this.currentLineNumber); - } - prepareToStartTracking(); - } - - private void prepareToStartTracking() { - if (!this.opcodesStack.isEmpty()) { - this.opcodesStack.clear(); - } - } -} diff --git a/pitest-entry/src/test/java/com/example/trywithresources/SimpleCloseCall.java b/pitest-entry/src/test/java/com/example/trywithresources/SimpleCloseCall.java new file mode 100644 index 000000000..9ac442d43 --- /dev/null +++ b/pitest-entry/src/test/java/com/example/trywithresources/SimpleCloseCall.java @@ -0,0 +1,21 @@ +package com.example.trywithresources; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; + +public class SimpleCloseCall { + public static void main(String[] args) { + ByteArrayOutputStream os = new ByteArrayOutputStream(); + try { + os.flush(); + } catch (IOException e) { + throw new RuntimeException(e); + } finally { + try { + os.close(); + } catch(IOException e) { + throw new RuntimeException(e); + } + } + } +} diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/FilterTester.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/FilterTester.java index 7d780770a..5a12183ba 100755 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/FilterTester.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/FilterTester.java @@ -2,6 +2,7 @@ import org.assertj.core.api.Condition; import org.assertj.core.api.SoftAssertions; +import org.objectweb.asm.util.Textifier; import org.pitest.bytecode.analysis.ClassTree; import org.pitest.bytecode.analysis.MethodTree; import org.pitest.classinfo.ClassByteArraySource; @@ -31,8 +32,7 @@ public class FilterTester { - private static final Collection COMPILERS = Arrays.asList("javac", - "ecj", "aspectj"); + private static final Collection COMPILERS = Arrays.asList("javac", "javac11", "ecj", "aspectj"); private final String path; private final ClassByteArraySource source = new ResourceFolderByteArraySource(); @@ -92,8 +92,9 @@ public void assertLeavesNMutants(int n, String sample) { final Collection actual = filter(s.clazz, mutations, mutator); softly.assertThat(actual) - .describedAs("Wrong number of mutants with " + s.compiler) + .describedAs("Wrong number of mutants with " + s.compiler + " for class \n" + s.clazz) .hasSize(n); + } softly.assertAll(); diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterTest.java index c3baf9770..3e61b9b1a 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; +import com.example.trywithresources.SimpleCloseCall; import org.junit.Test; import org.pitest.mutationtest.build.InterceptorType; import org.pitest.mutationtest.engine.gregor.config.Mutator; @@ -44,4 +45,9 @@ public void shouldWorkWithTwoClosables() { this.verifier.assertLeavesNMutants(1, "TryWithTwoCloseableExample"); } + @Test + public void doesNotFilterProgrammerAddedCloseCalls() { + this.verifier.assertLeavesNMutants(3, "SimpleCloseCall"); + } + } diff --git a/pitest-entry/src/test/resources/sampleClasses/trywithresources/SimpleCloseCall_javac11.class.bin b/pitest-entry/src/test/resources/sampleClasses/trywithresources/SimpleCloseCall_javac11.class.bin new file mode 100644 index 000000000..bc622f314 Binary files /dev/null and b/pitest-entry/src/test/resources/sampleClasses/trywithresources/SimpleCloseCall_javac11.class.bin differ diff --git a/pitest-entry/src/test/resources/sampleClasses/trywithresources/TryExample_javac11.bin b/pitest-entry/src/test/resources/sampleClasses/trywithresources/TryExample_javac11.bin new file mode 100644 index 000000000..17a3d56c9 Binary files /dev/null and b/pitest-entry/src/test/resources/sampleClasses/trywithresources/TryExample_javac11.bin differ diff --git a/pitest-entry/src/test/resources/sampleClasses/trywithresources/TryWithInterfaceExample_javac11.class.bin b/pitest-entry/src/test/resources/sampleClasses/trywithresources/TryWithInterfaceExample_javac11.class.bin new file mode 100644 index 000000000..331de5cc4 Binary files /dev/null and b/pitest-entry/src/test/resources/sampleClasses/trywithresources/TryWithInterfaceExample_javac11.class.bin differ