From 746baa42b8ce2b5e0264effbfff803e119f5468f Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Mon, 1 Aug 2022 15:24:50 +0100 Subject: [PATCH 1/3] expand filtering of static initializer mutants Expands the analysis to catch any mutants in methods in a call tree from the static initializer, which consists only of other private methods. --- .../StaticInitializerInterceptor.java | 107 +++++++++++------- .../staticinitializers/BrokenChain.java | 27 +++++ .../SecondLevelPrivateMethods.java | 16 +++ .../SingletonWithWorkInInitializer.java | 5 + .../ThirdLevelPrivateMethods.java | 28 +++++ .../MutationCoverageReportSystemTest.java | 5 +- .../StaticInitializerInterceptorTest.java | 71 +++++++++--- 7 files changed, 199 insertions(+), 60 deletions(-) create mode 100644 pitest-entry/src/test/java/com/example/staticinitializers/BrokenChain.java create mode 100644 pitest-entry/src/test/java/com/example/staticinitializers/SecondLevelPrivateMethods.java create mode 100644 pitest-entry/src/test/java/com/example/staticinitializers/ThirdLevelPrivateMethods.java diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java index 7e9390ba0..01798dc08 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java @@ -1,26 +1,28 @@ package org.pitest.mutationtest.build.intercept.staticinitializers; -import java.util.Collection; -import java.util.List; -import java.util.Optional; -import java.util.function.Function; -import java.util.function.Predicate; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import org.objectweb.asm.Opcodes; import org.objectweb.asm.tree.AbstractInsnNode; import org.objectweb.asm.tree.MethodInsnNode; -import org.pitest.bytecode.analysis.AnalysisFunctions; import org.pitest.bytecode.analysis.ClassTree; import org.pitest.bytecode.analysis.MethodTree; import org.pitest.classinfo.ClassName; -import org.pitest.functional.prelude.Prelude; import org.pitest.mutationtest.build.InterceptorType; import org.pitest.mutationtest.build.MutationInterceptor; +import org.pitest.mutationtest.engine.Location; import org.pitest.mutationtest.engine.Mutater; import org.pitest.mutationtest.engine.MutationDetails; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; + /** * Identifies and marks mutations in code that is active during class * Initialisation. @@ -29,10 +31,8 @@ * for static initialisation if it is * * 1. In a static initializer (i.e ) - * 2. In a private static method or constructor called directly from + * 2. In a private method or constructor called from or another private method in the call tree * - * TODO A better analysis would include private static methods called indirectly from - * and would exclude methods called from location other than . * */ class StaticInitializerInterceptor implements MutationInterceptor { @@ -64,42 +64,45 @@ private void analyseClass(ClassTree tree) { final Optional clinit = tree.methods().stream().filter(nameEquals("")).findFirst(); if (clinit.isPresent()) { - final List> selfCalls = - clinit.get().instructions().stream() - .flatMap(is(MethodInsnNode.class)) - .filter(calls(tree.name())) - .map(StaticInitializerInterceptor::matchesCall) - .collect(Collectors.toList()); - - final Predicate matchingCalls = Prelude.or(selfCalls); - - final Predicate initOnlyMethods = Prelude.or(tree.methods().stream() - .filter(isPrivateStatic().or(isConstructor())) - .filter(matchingCalls) - .map(AnalysisFunctions.matchMutationsInMethod()) - .collect(Collectors.toList()) - ); - - this.isStaticInitCode = Prelude.or(isInStaticInitializer(), initOnlyMethods); - } - } - private static Predicate isInStaticInitializer() { - return a -> a.getId().getLocation().getMethodName().equals(""); + // We can't see if a method *call* is private from the call site + // so collect a set of private methods within the class first + Set privateMethods = tree.methods().stream() + .filter(m -> m.isPrivate()) + .map(MethodTree::asLocation) + .collect(Collectors.toSet()); + + Map> callTree = tree.methods().stream() + .filter(m -> m.isPrivate() || m.rawNode().name.equals("")) + .flatMap(m -> callsFor(tree, m).stream().map(c -> new Call(m.asLocation(), c))) + .filter(c -> privateMethods.contains(c.to())) + .collect(Collectors.groupingBy(Call::from)); + + Set visited = new HashSet<>(); + + visit(callTree, visited, clinit.get().asLocation()); + + this.isStaticInitCode = m -> visited.contains(m.getId().getLocation()); + } } - private static Predicate isPrivateStatic() { - return a -> ((a.rawNode().access & Opcodes.ACC_STATIC) != 0) - && ((a.rawNode().access & Opcodes.ACC_PRIVATE) != 0); + private List callsFor(ClassTree tree, MethodTree m) { + return m.instructions().stream() + .flatMap(is(MethodInsnNode.class)) + .filter(calls(tree.name())) + .map(this::asLocation) + .collect(Collectors.toList()); } - private Predicate isConstructor() { - return a -> a.rawNode().name.equals(""); + private void visit(Map> callTree, Set visited, Location l) { + visited.add(l); + for (Call each : callTree.getOrDefault(l, Collections.emptyList())) { + visit(callTree, visited, each.to()); + } } - private static Predicate matchesCall(final MethodInsnNode call) { - return a -> a.rawNode().name.equals(call.name) - && a.rawNode().desc.equals(call.desc); + private Location asLocation(MethodInsnNode call) { + return Location.location(ClassName.fromString(call.owner), call.name, call.desc); } private Predicate calls(final ClassName self) { @@ -125,3 +128,21 @@ public InterceptorType type() { } } + +class Call { + private final Location from; + private final Location to; + + Call(Location from, Location to) { + this.from = from; + this.to = to; + } + + Location from() { + return from; + } + + Location to() { + return to; + } +} \ No newline at end of file diff --git a/pitest-entry/src/test/java/com/example/staticinitializers/BrokenChain.java b/pitest-entry/src/test/java/com/example/staticinitializers/BrokenChain.java new file mode 100644 index 000000000..0e691634d --- /dev/null +++ b/pitest-entry/src/test/java/com/example/staticinitializers/BrokenChain.java @@ -0,0 +1,27 @@ +package com.example.staticinitializers; + +public class BrokenChain { + static { + dontMutate1(); + } + + private static void dontMutate1() { + dontMutate2(); + System.out.println("mutate me"); + } + + private static void dontMutate2() { + mutateMe(); + System.out.println("mutate me"); + } + + // chain broken here by public method + public static void mutateMe() { + mutateMe2(); + System.out.println("mutate me"); + } + + private static void mutateMe2() { + System.out.println("mutate me"); + } +} diff --git a/pitest-entry/src/test/java/com/example/staticinitializers/SecondLevelPrivateMethods.java b/pitest-entry/src/test/java/com/example/staticinitializers/SecondLevelPrivateMethods.java new file mode 100644 index 000000000..3ec240a25 --- /dev/null +++ b/pitest-entry/src/test/java/com/example/staticinitializers/SecondLevelPrivateMethods.java @@ -0,0 +1,16 @@ +package com.example.staticinitializers; + +public class SecondLevelPrivateMethods { + static { + dontMutate1(); + } + + private static void dontMutate1() { + dontMutate2(); + System.out.println("mutate me"); + } + + private static void dontMutate2() { + System.out.println("mutate me"); + } +} diff --git a/pitest-entry/src/test/java/com/example/staticinitializers/SingletonWithWorkInInitializer.java b/pitest-entry/src/test/java/com/example/staticinitializers/SingletonWithWorkInInitializer.java index 530791457..6630d4463 100644 --- a/pitest-entry/src/test/java/com/example/staticinitializers/SingletonWithWorkInInitializer.java +++ b/pitest-entry/src/test/java/com/example/staticinitializers/SingletonWithWorkInInitializer.java @@ -6,6 +6,7 @@ public class SingletonWithWorkInInitializer { private static final SingletonWithWorkInInitializer INSTANCE = new SingletonWithWorkInInitializer(); private SingletonWithWorkInInitializer() { + doNotMutateMethodCalledFromConstructor(); } public static SingletonWithWorkInInitializer getInstance() { @@ -15,4 +16,8 @@ public static SingletonWithWorkInInitializer getInstance() { public boolean isMember6() { return 6 == num; } + + private void doNotMutateMethodCalledFromConstructor() { + System.out.println("do not mutate"); + } } \ No newline at end of file diff --git a/pitest-entry/src/test/java/com/example/staticinitializers/ThirdLevelPrivateMethods.java b/pitest-entry/src/test/java/com/example/staticinitializers/ThirdLevelPrivateMethods.java new file mode 100644 index 000000000..131fadba7 --- /dev/null +++ b/pitest-entry/src/test/java/com/example/staticinitializers/ThirdLevelPrivateMethods.java @@ -0,0 +1,28 @@ +package com.example.staticinitializers; + +public class ThirdLevelPrivateMethods { + + static { + dontMutate1(); + dontMutate2(); + } + + private static void dontMutate1() { + dontMutate2(); + System.out.println("mutate me"); + } + + private static void dontMutate2() { + dontMutate3(); + System.out.println("mutate me"); + } + + private static void dontMutate3() { + dontMutate4(); + System.out.println("mutate me"); + } + + private static void dontMutate4() { + System.out.println("mutate me"); + } +} diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/MutationCoverageReportSystemTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/MutationCoverageReportSystemTest.java index d1cd43094..db22fec1c 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/MutationCoverageReportSystemTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/MutationCoverageReportSystemTest.java @@ -389,7 +389,7 @@ public void shouldMutateNonPrivateStaticMethodsCalledFromInitializerOnly() { } @Test - public void willMutatePriveMethodsCalledInChainFromInitializer() { + public void doesNotMutatePrivateMethodsCalledInChainFromInitializer() { setMutators("VOID_METHOD_CALLS"); this.data @@ -397,8 +397,7 @@ public void willMutatePriveMethodsCalledInChainFromInitializer() { createAndRun(); - // would prefer removed here - verifyResults(NO_COVERAGE); + verifyResults(); } @Test diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java index de060682e..8610268fe 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java @@ -1,6 +1,9 @@ package org.pitest.mutationtest.build.intercept.staticinitializers; +import com.example.staticinitializers.BrokenChain; +import com.example.staticinitializers.SecondLevelPrivateMethods; import com.example.staticinitializers.SingletonWithWorkInInitializer; +import com.example.staticinitializers.ThirdLevelPrivateMethods; import org.junit.Test; import org.pitest.mutationtest.engine.MutationDetails; import org.pitest.mutationtest.engine.gregor.mutators.NullMutateEverything; @@ -9,7 +12,6 @@ import java.util.function.Predicate; - public class StaticInitializerInterceptorTest { InterceptorVerifier v = VerifierStart.forInterceptorFactory(new StaticInitializerInterceptorFactory()) @@ -17,8 +19,8 @@ public class StaticInitializerInterceptorTest { @Test - public void shouldNotFilterMutationsInClassWithoutStaticInitializer() { - v.forClass(NoStaticInializer.class) + public void doesNotFilterMutationsInClassWithoutStaticInitializer() { + v.forClass(NoStaticInitializer.class) .forAnyCode() .mutantsAreGenerated() .noMutantsAreFiltered() @@ -26,8 +28,8 @@ public void shouldNotFilterMutationsInClassWithoutStaticInitializer() { } @Test - public void shouldFilterMutationsInStaticInitializer() { - v.forClass(HasStaticInializer.class) + public void filtersMutationsInStaticInitializer() { + v.forClass(HasStaticInitializer.class) .forMethod("") .forAnyCode() .mutantsAreGenerated() @@ -36,7 +38,7 @@ public void shouldFilterMutationsInStaticInitializer() { } @Test - public void shouldMarkMutationsInPrivateMethodsCalledFromStaticInitializer() { + public void filtersMutationsInPrivateMethodsCalledFromStaticInitializer() { v.forClass(HasPrivateCallsFromStaticInializer.class) .forMethod("a") .forAnyCode() @@ -46,8 +48,8 @@ public void shouldMarkMutationsInPrivateMethodsCalledFromStaticInitializer() { } @Test - public void shouldNotMarkMutationsInPackageDefaultMethodsCalledFromStaticInitializer() { - v.forClass(HasDefaultCallsFromStaticInializer.class) + public void doesNotFilterMutationsInPackageDefaultMethodsCalledFromStaticInitializer() { + v.forClass(HasDefaultCallsFromStaticInitializer.class) .forMethod("a") .forAnyCode() .mutantsAreGenerated() @@ -55,9 +57,8 @@ public void shouldNotMarkMutationsInPackageDefaultMethodsCalledFromStaticInitial .verify(); } - @Test - public void shouldNotMarkMutationsInPrivateStaticMethodsNotInvolvedInInit() { + public void doesNotFilterMutationsInPrivateStaticMethodsNotInvolvedInInit() { v.forClass(HasOtherPrivateStaticMethods.class) .forMethod("b") .forAnyCode() @@ -67,7 +68,7 @@ public void shouldNotMarkMutationsInPrivateStaticMethodsNotInvolvedInInit() { } @Test - public void shouldNotMarkMutationsInOverriddenMethodsNotInvolvedInStaticInit() { + public void doesNotFilterMutationsInOverriddenMethodsNotInvolvedInStaticInit() { v.forClass(HasOverloadedMethodsThatAreNotUsedInStaticInitialization.class) .forMutantsMatching(inMethod("a", "(I)V")) .mutantsAreGenerated() @@ -84,19 +85,61 @@ public void filtersMutantsInSingletonConstructor() { .verify(); } + @Test + public void filtersMutantsCalledFromPrivateSingletonConstructor() { + v.forClass(SingletonWithWorkInInitializer.class) + .forMutantsMatching(inMethodStartingWith("doNotMutate")) + .mutantsAreGenerated() + .allMutantsAreFiltered() + .verify(); + } + + @Test + public void filtersPrivateMethodsCalledIndirectly() { + v.forClass(SecondLevelPrivateMethods.class) + .forMutantsMatching(inMethodStartingWith("dontMutate")) + .mutantsAreGenerated() + .allMutantsAreFiltered() + .verify(); + } + + @Test + public void filtersPrivateMethodsCalledIndirectlyInLongChain() { + v.forClass(ThirdLevelPrivateMethods.class) + .forMutantsMatching(inMethodStartingWith("dontMutate")) + .mutantsAreGenerated() + .allMutantsAreFiltered() + .verify(); + } + + @Test + public void doesNotFilterPrivateMethodsWhenChainBrokenByPublicMethod() { + v.forClass(BrokenChain.class) + .forMutantsMatching(inMethodStartingWith("mutateMe")) + .mutantsAreGenerated() + .noMutantsAreFiltered() + .verify(); + } + + private Predicate inMethod(String name, String desc) { return m -> m.getMethod().equals(name) && m.getId().getLocation().getMethodDesc().equals(desc); } + private Predicate inMethodStartingWith(String stub) { + return m -> m.getMethod().startsWith(stub); + } + + } -class NoStaticInializer { +class NoStaticInitializer { { System.out.println("NOT static code"); } } -class HasStaticInializer { +class HasStaticInitializer { static { System.out.println("static code"); } @@ -112,7 +155,7 @@ private static void a() { } } -class HasDefaultCallsFromStaticInializer { +class HasDefaultCallsFromStaticInitializer { static { a(); } From c9fa24cd4a52d56835c658c1ba56f5527fa5a7cc Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Mon, 1 Aug 2022 16:03:57 +0100 Subject: [PATCH 2/3] avoid method call cycles --- .../StaticInitializerInterceptor.java | 5 +++++ .../MethodsCallsEachOtherInLoop.java | 19 +++++++++++++++++++ .../StaticInitializerInterceptorTest.java | 9 +++++++++ 3 files changed, 33 insertions(+) create mode 100644 pitest-entry/src/test/java/com/example/staticinitializers/MethodsCallsEachOtherInLoop.java diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java index 01798dc08..4ca7ec4f8 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptor.java @@ -95,6 +95,11 @@ private List callsFor(ClassTree tree, MethodTree m) { } private void visit(Map> callTree, Set visited, Location l) { + // avoid stack overflow if methods call each other in a cycle + if (visited.contains(l)) { + return; + } + visited.add(l); for (Call each : callTree.getOrDefault(l, Collections.emptyList())) { visit(callTree, visited, each.to()); diff --git a/pitest-entry/src/test/java/com/example/staticinitializers/MethodsCallsEachOtherInLoop.java b/pitest-entry/src/test/java/com/example/staticinitializers/MethodsCallsEachOtherInLoop.java new file mode 100644 index 000000000..aea40207e --- /dev/null +++ b/pitest-entry/src/test/java/com/example/staticinitializers/MethodsCallsEachOtherInLoop.java @@ -0,0 +1,19 @@ +package com.example.staticinitializers; + +public class MethodsCallsEachOtherInLoop { + static { + a(true); + } + + private static void a(boolean bool) { + if (bool) { + b(false); + } + } + + private static void b(boolean bool) { + if (bool) { + a(false); + } + } +} diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java index 8610268fe..78b4831a4 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/intercept/staticinitializers/StaticInitializerInterceptorTest.java @@ -1,6 +1,7 @@ package org.pitest.mutationtest.build.intercept.staticinitializers; import com.example.staticinitializers.BrokenChain; +import com.example.staticinitializers.MethodsCallsEachOtherInLoop; import com.example.staticinitializers.SecondLevelPrivateMethods; import com.example.staticinitializers.SingletonWithWorkInInitializer; import com.example.staticinitializers.ThirdLevelPrivateMethods; @@ -121,6 +122,14 @@ public void doesNotFilterPrivateMethodsWhenChainBrokenByPublicMethod() { .verify(); } + @Test + public void analysisDoesNotGetStuckInInfiniteLoop() { + v.forClass(MethodsCallsEachOtherInLoop.class) + .forMutantsMatching(inMethodStartingWith("a")) + .mutantsAreGenerated() + .allMutantsAreFiltered() + .verify(); + } private Predicate inMethod(String name, String desc) { return m -> m.getMethod().equals(name) && m.getId().getLocation().getMethodDesc().equals(desc); From fdccf2f833a48fd503311aba597de71aaae1845f Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Tue, 2 Aug 2022 10:02:58 +0100 Subject: [PATCH 3/3] update readme for static initializer filtering --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index c4f403b77..42592735a 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ Read all about it at http://pitest.org ### 1.9.4 (unreleased) * #1063 - Improve filtering of equivalent return mutants +* #1066 - Expand static initializer filtering ### 1.9.3