From 1ac5993c10c75de05756623ff1d9f0df172c8be1 Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Wed, 26 Mar 2025 11:43:35 +0000 Subject: [PATCH] treat try with resources bytecode as multiple blocks --- .../javafeatures/TryWithResourcesFilter.java | 56 +++++++++++++---- .../NoThrowAutoClosableResource.java | 8 +++ .../TryWithResourcesNoThrow.java | 25 ++++++++ .../intercept/javafeatures/FilterTester.java | 1 + .../TryWithResourcesFilterFactoryTest.java | 62 +++++++++++++++++++ 5 files changed, 139 insertions(+), 13 deletions(-) create mode 100644 pitest-entry/src/test/java/com/example/trywithresources/NoThrowAutoClosableResource.java create mode 100644 pitest-entry/src/test/java/com/example/trywithresources/TryWithResourcesNoThrow.java create mode 100644 pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterFactoryTest.java 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 b9279e288..5a59047d2 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 @@ -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,9 +53,19 @@ public class TryWithResourcesFilter extends RegionInterceptor { private static final Slot START = Slot.create(AbstractInsnNode.class); private static final Slot END = Slot.create(AbstractInsnNode.class); + private static final SequenceMatcher SUPPRESS = + javac11SuppressSequence() + .compile(QueryParams.params(AbstractInsnNode.class) + .withIgnores(notAnInstruction().or(aLabel().and(isLabel(HANDLERS.read()).negate()))) + .withDebug(DEBUG) + ); private static final SequenceMatcher 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) @@ -57,23 +73,23 @@ public class TryWithResourcesFilter extends RegionInterceptor { .withDebug(DEBUG) ); - private static SequenceQuery javac11() { + private static SequenceQuery 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 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 fullSequence(boolean record) { .then(omittedNullCheckSequence(false)); } + private static SequenceQuery omittedNullCheckSequence(boolean record) { return QueryStart.match(ALOAD.and(recordPoint(START, record))) .then(IFNULL) @@ -193,10 +210,10 @@ private static SequenceQuery optimalSequence(boolean record) { private static SequenceQuery 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 computeRegions(MethodTree method) { .map(t -> t.handler) .collect(Collectors.toList()); + + return Stream.concat(suppress(method, handlers), tryWithResources(method, handlers)) + .collect(Collectors.toList()); + } + + private Stream tryWithResources(MethodTree method, List 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 suppress(MethodTree method, List 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)); } diff --git a/pitest-entry/src/test/java/com/example/trywithresources/NoThrowAutoClosableResource.java b/pitest-entry/src/test/java/com/example/trywithresources/NoThrowAutoClosableResource.java new file mode 100644 index 000000000..a3a010347 --- /dev/null +++ b/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() { + } +} \ No newline at end of file diff --git a/pitest-entry/src/test/java/com/example/trywithresources/TryWithResourcesNoThrow.java b/pitest-entry/src/test/java/com/example/trywithresources/TryWithResourcesNoThrow.java new file mode 100644 index 000000000..080315c60 --- /dev/null +++ b/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> threadLocalMap; + + TryWithResourcesNoThrow(final Map> threadLocalMap) { + this.threadLocalMap = threadLocalMap; + } + + public int usingTryWithResources() { + final ThreadLocal threadLocal = threadLocalMap.get("Value1"); + try (final NoThrowAutoClosableResource myAutoClosable = new NoThrowAutoClosableResource()) { + return threadLocal.get(); + } finally { + System.out.println("mutate me"); + threadLocal.remove(); + } + } + + +} + 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 e15952c3f..edb92e8c4 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 @@ -29,6 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat; +@Deprecated public class FilterTester { private static final Collection COMPILERS = Arrays.asList("javac", "javac11", "ecj", "aspectj"); diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterFactoryTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/javafeatures/TryWithResourcesFilterFactoryTest.java new file mode 100644 index 000000000..5a7212027 --- /dev/null +++ b/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(); + + } + +} \ No newline at end of file