Do not filter user code in try with resources

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilter.java

@@ -17,6 +17,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
 import static org.pitest.bytecode.analysis.InstructionMatchers.debug;
@@ -38,6 +39,11 @@
 import static org.pitest.sequence.QueryStart.match;
 import static org.pitest.sequence.Result.result;
 
+/**
+ * Filters conditional logic and method calls generated for try-with-resources blocks
+ * Duplicate inlined mutations are left in place to be handled by the InlinedFinallyBlockFilter
+ * any mutants filtered here by accident will break the inlined filter logic.
+ */
 public class TryWithResourcesFilter extends RegionInterceptor {
 
   private static final boolean DEBUG = false;
@@ -47,33 +53,43 @@ public class TryWithResourcesFilter extends RegionInterceptor {
   private static final Slot<AbstractInsnNode> START = Slot.create(AbstractInsnNode.class);
   private static final Slot<AbstractInsnNode> END = Slot.create(AbstractInsnNode.class);
 
+  private static final SequenceMatcher<AbstractInsnNode> SUPPRESS =
+  javac11SuppressSequence()
+          .compile(QueryParams.params(AbstractInsnNode.class)
+                  .withIgnores(notAnInstruction().or(aLabel().and(isLabel(HANDLERS.read()).negate())))
+                  .withDebug(DEBUG)
+          );
 
   private static final SequenceMatcher<AbstractInsnNode> TRY_WITH_RESOURCES =
-          javac11()
+          // java 11+ generated blocks are checked individually
+          javac11CloseSequence()
+          // earlier versions of javac and ecj are handled as one continuous block
+          // however this will result in user code being incorrectly filtered
+          // this should be revisited
           .or(javac())
           .or(ecj())
           .compile(QueryParams.params(AbstractInsnNode.class)
                   .withIgnores(notAnInstruction().or(aLabel().and(isLabel(HANDLERS.read()).negate())))
                   .withDebug(DEBUG)
           );
 
-  private static SequenceQuery<AbstractInsnNode> javac11() {
+  private static SequenceQuery<AbstractInsnNode> javac11CloseSequence() {
     return any(AbstractInsnNode.class)
             .zeroOrMore(match(anyInstruction()))
             .then(closeSequence(true))
             .zeroOrMore(match(anyInstruction()))
             .then(isLabel(HANDLERS.read()).and(debug("handler")))
-            .then(ASTORE)
-            .then(ALOAD)
-            .then(closeSequence(false))
-            .then(GOTO)
+            .zeroOrMore(match(anyInstruction()));
+  }
+
+  private static SequenceQuery<AbstractInsnNode> javac11SuppressSequence() {
+    return any(AbstractInsnNode.class)
+            .zeroOrMore(match(anyInstruction()))
             .then(isLabel(HANDLERS.read()).and(debug("handler")))
             .then(ASTORE)
             .then(ALOAD)
             .then(ALOAD)
-            .then(addSuppressedMethodCall().and(debug("add suppressed")))
-            .then(ALOAD)
-            .then(ATHROW.and(recordPoint(END, true)))
+            .then(addSuppressedMethodCall().and(recordPoint(START,true)).and(debug("add suppressed")))
             .zeroOrMore(match(anyInstruction()));
   }
 
@@ -169,6 +185,7 @@ private static SequenceQuery<AbstractInsnNode> fullSequence(boolean record) {
             .then(omittedNullCheckSequence(false));
   }
 
+
   private static SequenceQuery<AbstractInsnNode> omittedNullCheckSequence(boolean record) {
     return QueryStart.match(ALOAD.and(recordPoint(START, record)))
             .then(IFNULL)
@@ -193,10 +210,10 @@ private static SequenceQuery<AbstractInsnNode> optimalSequence(boolean record) {
 
   private static SequenceQuery<AbstractInsnNode> closeSequence(boolean record) {
     // javac may (or may not) generate a null check before the close
-    return match(closeMethodCall().and(recordPoint(START, record)))
+    return match(closeMethodCall().and(recordPoint(START, record)).and(recordPoint(END, record)))
             .or(match(IFNULL.and(recordPoint(START, record)))
                     .then(ALOAD)
-                    .then(closeMethodCall()));
+                    .then(closeMethodCall().and(recordPoint(END, record))));
   }
 
 
@@ -212,11 +229,24 @@ protected List<Region> computeRegions(MethodTree method) {
             .map(t -> t.handler)
             .collect(Collectors.toList());
 
+
+    return Stream.concat(suppress(method, handlers), tryWithResources(method, handlers))
+            .collect(Collectors.toList());
+  }
+
+  private Stream<Region> tryWithResources(MethodTree method, List<LabelNode> handlers) {
     Context context = Context.start(DEBUG);
     context = context.store(HANDLERS.write(), handlers);
     return TRY_WITH_RESOURCES.contextMatches(method.instructions(), context).stream()
-            .map(c -> new Region(c.retrieve(START.read()).get(), c.retrieve(END.read()).get()))
-            .collect(Collectors.toList());
+            .map(c -> new Region(c.retrieve(START.read()).get(), c.retrieve(END.read()).get()));
+  }
+
+  private Stream<Region> suppress(MethodTree method, List<LabelNode> handlers) {
+    Context context = Context.start(DEBUG);
+    context = context.store(HANDLERS.write(), handlers);
+    return SUPPRESS.contextMatches(method.instructions(), context).stream()
+            .map(c -> c.retrieve(START.read()).get())
+            .map(n -> new Region(n, n));
   }
 
 

pitest-entry/src/test/java/com/example/trywithresources/NoThrowAutoClosableResource.java

@@ -0,0 +1,8 @@
+package com.example.trywithresources;
+
+public class NoThrowAutoClosableResource implements AutoCloseable {
+
+    @Override
+    public void close() {
+    }
+}

pitest-entry/src/test/java/com/example/trywithresources/TryWithResourcesNoThrow.java

@@ -0,0 +1,25 @@
+package com.example.trywithresources;
+
+import java.util.Map;
+
+public class TryWithResourcesNoThrow {
+
+    private final Map<String, ThreadLocal<Integer>> threadLocalMap;
+
+    TryWithResourcesNoThrow(final Map<String, ThreadLocal<Integer>> threadLocalMap) {
+        this.threadLocalMap = threadLocalMap;
+    }
+
+    public int usingTryWithResources() {
+        final ThreadLocal<Integer> threadLocal = threadLocalMap.get("Value1");
+        try (final NoThrowAutoClosableResource myAutoClosable = new NoThrowAutoClosableResource()) {
+            return threadLocal.get();
+        } finally {
+            System.out.println("mutate me");
+            threadLocal.remove();
+        }
+    }
+
+
+}
+

pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/FilterTester.java

@@ -29,6 +29,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+@Deprecated
 public class FilterTester {
 
   private static final Collection<String> COMPILERS = Arrays.asList("javac", "javac11", "ecj", "aspectj");

pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterFactoryTest.java

@@ -0,0 +1,62 @@
+package org.pitest.mutationtest.build.intercept.javafeatures;
+
+import com.example.trywithresources.NoThrowAutoClosableResource;
+import com.example.trywithresources.SimpleCloseCall;
+import com.example.trywithresources.TryWithResourcesNoThrow;
+import org.junit.Test;
+import org.pitest.classinfo.ClassName;
+import org.pitest.mutationtest.engine.gregor.mutators.NullMutateEverything;
+import org.pitest.verifier.interceptors.InterceptorVerifier;
+import org.pitest.verifier.interceptors.VerifierStart;
+import twr.example6.TryWithNestedTryExample;
+
+import java.io.ByteArrayOutputStream;
+
+import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallTo;
+
+public class TryWithResourcesFilterFactoryTest {
+
+    InterceptorVerifier v = VerifierStart.forInterceptorFactory(new TryWithResourcesFilterFactory())
+            .usingMutator(new NullMutateEverything());
+
+    @Test
+    public void filtersAutoGeneratedCloseMethods() {
+        v.forClass(TryWithResourcesNoThrow.class)
+                .forCodeMatching(methodCallTo(ClassName.fromClass(NoThrowAutoClosableResource.class), "close").asPredicate())
+                .mutantsAreGenerated()
+                .allMutantsAreFiltered()
+                .verify();
+    }
+
+    @Test
+    public void doesNotFilterManualCloseCalls() {
+        v.forClass(SimpleCloseCall.class)
+                .forCodeMatching(methodCallTo(ClassName.fromClass(ByteArrayOutputStream.class), "close").asPredicate())
+                .mutantsAreGenerated()
+                .noMutantsAreFiltered()
+                .verify();
+
+    }
+
+    @Test
+    public void doesNotFilterUserCode() {
+        v.forClass(TryWithResourcesNoThrow.class)
+                .forCodeMatching(methodCallTo(ClassName.fromClass(ThreadLocal.class), "remove").asPredicate())
+                .mutantsAreGenerated()
+                .noMutantsAreFiltered()
+                .verify();
+
+    }
+
+    @Test
+    public void filtersAddSuppressed() {
+        // using TryWithNestedTryExample here just for variety, not essential to the test
+        v.forClass(TryWithNestedTryExample.class)
+                .forCodeMatching(methodCallTo(ClassName.fromClass(Throwable.class), "addSuppressed").asPredicate())
+                .mutantsAreGenerated()
+                .allMutantsAreFiltered()
+                .verify();
+
+    }
+
+}