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 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..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 @@ -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,50 @@ 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); + + // 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 isInStaticInitializer() { - return a -> a.getId().getLocation().getMethodName().equals(""); + 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 static Predicate isPrivateStatic() { - return a -> ((a.rawNode().access & Opcodes.ACC_STATIC) != 0) - && ((a.rawNode().access & Opcodes.ACC_PRIVATE) != 0); - } + 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; + } - private Predicate isConstructor() { - return a -> a.rawNode().name.equals(""); + 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 +133,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/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/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..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,10 @@ 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; import org.junit.Test; import org.pitest.mutationtest.engine.MutationDetails; import org.pitest.mutationtest.engine.gregor.mutators.NullMutateEverything; @@ -9,7 +13,6 @@ import java.util.function.Predicate; - public class StaticInitializerInterceptorTest { InterceptorVerifier v = VerifierStart.forInterceptorFactory(new StaticInitializerInterceptorFactory()) @@ -17,8 +20,8 @@ public class StaticInitializerInterceptorTest { @Test - public void shouldNotFilterMutationsInClassWithoutStaticInitializer() { - v.forClass(NoStaticInializer.class) + public void doesNotFilterMutationsInClassWithoutStaticInitializer() { + v.forClass(NoStaticInitializer.class) .forAnyCode() .mutantsAreGenerated() .noMutantsAreFiltered() @@ -26,8 +29,8 @@ public void shouldNotFilterMutationsInClassWithoutStaticInitializer() { } @Test - public void shouldFilterMutationsInStaticInitializer() { - v.forClass(HasStaticInializer.class) + public void filtersMutationsInStaticInitializer() { + v.forClass(HasStaticInitializer.class) .forMethod("") .forAnyCode() .mutantsAreGenerated() @@ -36,7 +39,7 @@ public void shouldFilterMutationsInStaticInitializer() { } @Test - public void shouldMarkMutationsInPrivateMethodsCalledFromStaticInitializer() { + public void filtersMutationsInPrivateMethodsCalledFromStaticInitializer() { v.forClass(HasPrivateCallsFromStaticInializer.class) .forMethod("a") .forAnyCode() @@ -46,8 +49,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 +58,8 @@ public void shouldNotMarkMutationsInPackageDefaultMethodsCalledFromStaticInitial .verify(); } - @Test - public void shouldNotMarkMutationsInPrivateStaticMethodsNotInvolvedInInit() { + public void doesNotFilterMutationsInPrivateStaticMethodsNotInvolvedInInit() { v.forClass(HasOtherPrivateStaticMethods.class) .forMethod("b") .forAnyCode() @@ -67,7 +69,7 @@ public void shouldNotMarkMutationsInPrivateStaticMethodsNotInvolvedInInit() { } @Test - public void shouldNotMarkMutationsInOverriddenMethodsNotInvolvedInStaticInit() { + public void doesNotFilterMutationsInOverriddenMethodsNotInvolvedInStaticInit() { v.forClass(HasOverloadedMethodsThatAreNotUsedInStaticInitialization.class) .forMutantsMatching(inMethod("a", "(I)V")) .mutantsAreGenerated() @@ -84,19 +86,69 @@ 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(); + } + + @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); } + 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 +164,7 @@ private static void a() { } } -class HasDefaultCallsFromStaticInializer { +class HasDefaultCallsFromStaticInitializer { static { a(); }