diff --git a/README.md b/README.md index a1404463a..714127ce9 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ Read all about it at http://pitest.org * #1025 - Rework String Switch filtering * #1027 - Rework assert filtering and remove legacy filter mechanism +* #903 - Filter mutants in singleton constructors ### 1.8.0 diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/EnumConstructorFilter.java b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/EnumConstructorFilter.java index 4f62ac2f7..0a851a635 100755 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/EnumConstructorFilter.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/EnumConstructorFilter.java @@ -13,6 +13,10 @@ /** * Filters out mutations in Enum constructors, these are called only once * per instance so are effectively static initializers. + * + * This filter overlaps with the StaticInitializerInterceptor, and could + * probably be removed. Left in place for now as it is computationally less + * expensive. */ public class EnumConstructorFilter implements MutationInterceptor { 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 199ed2345..7e9390ba0 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 @@ -29,7 +29,7 @@ * for static initialisation if it is * * 1. In a static initializer (i.e ) - * 2. In a private static method called directly from + * 2. In a private static method or constructor called directly from * * TODO A better analysis would include private static methods called indirectly from * and would exclude methods called from location other than . @@ -74,7 +74,7 @@ private void analyseClass(ClassTree tree) { final Predicate matchingCalls = Prelude.or(selfCalls); final Predicate initOnlyMethods = Prelude.or(tree.methods().stream() - .filter(isPrivateStatic()) + .filter(isPrivateStatic().or(isConstructor())) .filter(matchingCalls) .map(AnalysisFunctions.matchMutationsInMethod()) .collect(Collectors.toList()) @@ -84,7 +84,6 @@ private void analyseClass(ClassTree tree) { } } - private static Predicate isInStaticInitializer() { return a -> a.getId().getLocation().getMethodName().equals(""); } @@ -94,6 +93,10 @@ private static Predicate isPrivateStatic() { && ((a.rawNode().access & Opcodes.ACC_PRIVATE) != 0); } + private Predicate isConstructor() { + return a -> a.rawNode().name.equals(""); + } + private static Predicate matchesCall(final MethodInsnNode call) { return a -> a.rawNode().name.equals(call.name) && a.rawNode().desc.equals(call.desc); diff --git a/pitest-entry/src/test/java/com/example/staticinitializers/SingletonWithWorkInInitializer.java b/pitest-entry/src/test/java/com/example/staticinitializers/SingletonWithWorkInInitializer.java new file mode 100644 index 000000000..530791457 --- /dev/null +++ b/pitest-entry/src/test/java/com/example/staticinitializers/SingletonWithWorkInInitializer.java @@ -0,0 +1,18 @@ +package com.example.staticinitializers; + +public class SingletonWithWorkInInitializer { + int num = 6; + + private static final SingletonWithWorkInInitializer INSTANCE = new SingletonWithWorkInInitializer(); + + private SingletonWithWorkInInitializer() { + } + + public static SingletonWithWorkInInitializer getInstance() { + return INSTANCE; + } + + public boolean isMember6() { + return 6 == num; + } +} \ No newline at end of file diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/build/MutationDiscoveryTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/build/MutationDiscoveryTest.java index ba8f314ad..badb9b4af 100755 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/build/MutationDiscoveryTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/build/MutationDiscoveryTest.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.logging.Logger; +import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; @@ -198,7 +199,9 @@ public void shouldFilterMutationsToForEachLoops() { public void shouldFilterMutationsToEnumConstructors() { final Collection actual = findMutants(AnEnum.class); - this.data.setFeatures(Collections.singletonList("-FENUM")); + // both the enum and static initializer filters pick up + // enum constructors. Both left in place for now, but enum one could perhaps be removed + this.data.setFeatures(asList("-FENUM", "-FSTATI")); final Collection actualWithoutFilter = findMutants(AnEnum.class); assertThat(actual.size()).isLessThan(actualWithoutFilter.size()); 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 2a9cb3ea6..de060682e 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,129 +1,163 @@ package org.pitest.mutationtest.build.intercept.staticinitializers; +import com.example.staticinitializers.SingletonWithWorkInInitializer; import org.junit.Test; -import org.pitest.mutationtest.build.intercept.javafeatures.FilterTester; import org.pitest.mutationtest.engine.MutationDetails; import org.pitest.mutationtest.engine.gregor.mutators.NullMutateEverything; +import org.pitest.verifier.interceptors.InterceptorVerifier; +import org.pitest.verifier.interceptors.VerifierStart; import java.util.function.Predicate; public class StaticInitializerInterceptorTest { - StaticInitializerInterceptor testee = new StaticInitializerInterceptor(); - - FilterTester verifier = new FilterTester("", this.testee, NullMutateEverything.asList()); - - - @Test - public void shouldNotFilterMutationsInClassWithoutStaticInitializer() { - verifier.assertFiltersNMutationFromClass(0, NoStaticInializer.class); - } - - @Test - public void shouldFilterMutationsInStaticInitializer() { - verifier.assertFiltersMutationsMatching(inMethodNamed(""), HasStaticInializer.class); - } - - @Test - public void shouldMarkMutationsInPrivateMethodsCalledFromStaticInitializer() { - verifier.assertFiltersMutationsMatching(inMethodNamed("a"), HasPrivateCallsFromStaticInializer.class); - } + InterceptorVerifier v = VerifierStart.forInterceptorFactory(new StaticInitializerInterceptorFactory()) + .usingMutator(new NullMutateEverything()); + + + @Test + public void shouldNotFilterMutationsInClassWithoutStaticInitializer() { + v.forClass(NoStaticInializer.class) + .forAnyCode() + .mutantsAreGenerated() + .noMutantsAreFiltered() + .verify(); + } + + @Test + public void shouldFilterMutationsInStaticInitializer() { + v.forClass(HasStaticInializer.class) + .forMethod("") + .forAnyCode() + .mutantsAreGenerated() + .allMutantsAreFiltered() + .verify(); + } + + @Test + public void shouldMarkMutationsInPrivateMethodsCalledFromStaticInitializer() { + v.forClass(HasPrivateCallsFromStaticInializer.class) + .forMethod("a") + .forAnyCode() + .mutantsAreGenerated() + .allMutantsAreFiltered() + .verify(); + } + + @Test + public void shouldNotMarkMutationsInPackageDefaultMethodsCalledFromStaticInitializer() { + v.forClass(HasDefaultCallsFromStaticInializer.class) + .forMethod("a") + .forAnyCode() + .mutantsAreGenerated() + .noMutantsAreFiltered() + .verify(); + } + + + @Test + public void shouldNotMarkMutationsInPrivateStaticMethodsNotInvolvedInInit() { + v.forClass(HasOtherPrivateStaticMethods.class) + .forMethod("b") + .forAnyCode() + .mutantsAreGenerated() + .noMutantsAreFiltered() + .verify(); + } + + @Test + public void shouldNotMarkMutationsInOverriddenMethodsNotInvolvedInStaticInit() { + v.forClass(HasOverloadedMethodsThatAreNotUsedInStaticInitialization.class) + .forMutantsMatching(inMethod("a", "(I)V")) + .mutantsAreGenerated() + .noMutantsAreFiltered() + .verify(); + } @Test - public void shouldNotMarkMutationsInPackageDefaultMethodsCalledFromStaticInitializer() { - verifier.assertFiltersNoMutationsMatching(inMethodNamed("a"), HasDefaultCallsFromStaticInializer.class); + public void filtersMutantsInSingletonConstructor() { + v.forClass(SingletonWithWorkInInitializer.class) + .forMutantsMatching(inMethod("", "()V")) + .mutantsAreGenerated() + .allMutantsAreFiltered() + .verify(); } - - @Test - public void shouldNotMarkMutationsInPrivateStaticMethodsNotInvolvedInInit() { - verifier.assertFiltersNoMutationsMatching(inMethodNamed("b"), HasOtherPrivateStaticMethods.class); - } - - @Test - public void shouldNotMarkMutationsInOverriddenMethodsNotInvolvedInStaticInit() { - verifier.assertFiltersNoMutationsMatching(inMethod("a", "(I)V"), HasOverloadedMethodsThatAreNotUsedInStaticInitialization.class); - } - - private Predicate inMethodNamed(String name) { - return m -> m.getMethod().equals(name); - } - - private Predicate inMethod(String name, String desc) { - return m -> m.getMethod().equals(name) && m.getId().getLocation().getMethodDesc().equals(desc); - } + private Predicate inMethod(String name, String desc) { + return m -> m.getMethod().equals(name) && m.getId().getLocation().getMethodDesc().equals(desc); + } } class NoStaticInializer { - { - System.out.println("NOT static code"); - } + { + System.out.println("NOT static code"); + } } class HasStaticInializer { - static { - System.out.println("static code"); - } + static { + System.out.println("static code"); + } } class HasPrivateCallsFromStaticInializer { - static { - a(); - } + static { + a(); + } - private static void a() { - System.out.println("static code"); - } + private static void a() { + System.out.println("static code"); + } } class HasDefaultCallsFromStaticInializer { - static { - a(); - } + static { + a(); + } - static void a() { - System.out.println("NOT guaranteed to be static code"); - } + static void a() { + System.out.println("NOT guaranteed to be static code"); + } } class HasOtherPrivateStaticMethods { - static { - a(); - } + static { + a(); + } - private static void a() { + private static void a() { - } + } - public static void entryPoint(int i) { - b(i); - } + public static void entryPoint(int i) { + b(i); + } - private static void b(int i) { - System.out.println("NOT static code"); - } + private static void b(int i) { + System.out.println("NOT static code"); + } } class HasOverloadedMethodsThatAreNotUsedInStaticInitialization { - static { - a(); - } + static { + a(); + } - private static void a() { + private static void a() { - } + } - public static void entryPoint(int i) { - a(i); - } + public static void entryPoint(int i) { + a(i); + } - // same name, different sig - private static void a(int i) { - System.out.println("NOT static code"); - } + // same name, different sig + private static void a(int i) { + System.out.println("NOT static code"); + } }