Merge pull request #1310 from hcoles/feature/filter_defensive_copy

filter mutations to defensive unmodifiable collection returns
hcoles · Feb 13, 2024 · 7be0282 · 7be0282
2 parents 0c79a82 + e80f824
commit 7be0282
Show file tree

Hide file tree

Showing 7 changed files with 224 additions and 4 deletions.
diff --git a/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java b/pitest-entry/src/main/java/org/pitest/bytecode/analysis/InstructionMatchers.java
@@ -188,16 +188,19 @@ public static  Match<AbstractInsnNode> methodDescEquals(final String desc) {
   }
 
   public static  Match<AbstractInsnNode> methodCallTo(final ClassName owner, final String name) {
+    return methodCallTo(owner, c -> c.equals(name));
+  }
+
+  public static  Match<AbstractInsnNode> methodCallTo(final ClassName owner, Predicate<String> name) {
     return (c, t) -> {
       if ( t instanceof MethodInsnNode ) {
         final MethodInsnNode call = (MethodInsnNode) t;
-        return result( call.name.equals(name) && call.owner.equals(owner.asInternalName()), c);
+        return result( name.test(call.name) && call.owner.equals(owner.asInternalName()), c);
       }
       return result(false, c);
     };
   }
 
-
   public static  Match<AbstractInsnNode> isInstruction(final SlotRead<AbstractInsnNode> target) {
     return (c, t) -> result(c.retrieve(target).get() == t, c);
   }

diff --git a/.../java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java b/.../java/org/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollection.java
@@ -0,0 +1,55 @@
+package org.pitest.mutationtest.build.intercept.defensive;
+
+import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.LabelNode;
+import org.pitest.bytecode.analysis.MethodTree;
+import org.pitest.classinfo.ClassName;
+import org.pitest.mutationtest.build.intercept.Region;
+import org.pitest.mutationtest.build.intercept.RegionInterceptor;
+import org.pitest.sequence.Context;
+import org.pitest.sequence.Match;
+import org.pitest.sequence.QueryParams;
+import org.pitest.sequence.QueryStart;
+import org.pitest.sequence.SequenceMatcher;
+import org.pitest.sequence.Slot;
+import org.pitest.sequence.SlotWrite;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
+import static org.pitest.bytecode.analysis.InstructionMatchers.isA;
+import static org.pitest.bytecode.analysis.InstructionMatchers.methodCallTo;
+import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.ARETURN;
+import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKESTATIC;
+import static org.pitest.sequence.Result.result;
+
+public class ReturnUnmodifiableCollection extends RegionInterceptor {
+
+    static final Slot<AbstractInsnNode> MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class);
+
+    static final SequenceMatcher<AbstractInsnNode> DEFENSIVE_RETURN = QueryStart
+            .any(AbstractInsnNode.class)
+            .then(INVOKESTATIC.and(methodCallTo(ClassName.fromClass(Collections.class), n -> n.startsWith("unmodifiable"))).and(store(MUTATED_INSTRUCTION.write())))
+            .then(ARETURN)
+            .zeroOrMore(QueryStart.match(anyInstruction()))
+            .compile(QueryParams.params(AbstractInsnNode.class)
+                    .withIgnores(notAnInstruction().or(isA(LabelNode.class)))
+            );
+
+
+    @Override
+    protected List<Region> computeRegions(MethodTree method) {
+        Context context = Context.start();
+        return DEFENSIVE_RETURN.contextMatches(method.instructions(), context).stream()
+                .map(c -> c.retrieve(MUTATED_INSTRUCTION.read()).get())
+                .map(n -> new Region(n, n))
+                .collect(Collectors.toList());
+    }
+
+    private static Match<AbstractInsnNode> store(SlotWrite<AbstractInsnNode> slot) {
+        return (c,n) -> result(true, c.store(slot, n));
+    }
+}
diff --git a/...rg/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactory.java b/...rg/pitest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactory.java
@@ -0,0 +1,25 @@
+package org.pitest.mutationtest.build.intercept.defensive;
+
+import org.pitest.mutationtest.build.InterceptorParameters;
+import org.pitest.mutationtest.build.MutationInterceptor;
+import org.pitest.mutationtest.build.MutationInterceptorFactory;
+import org.pitest.plugin.Feature;
+
+public class ReturnUnmodifiableCollectionFactory implements MutationInterceptorFactory {
+    @Override
+    public MutationInterceptor createInterceptor(InterceptorParameters params) {
+        return new ReturnUnmodifiableCollection();
+    }
+
+    @Override
+    public Feature provides() {
+        return Feature.named("DEFENSIVERETURN")
+                .withOnByDefault(true)
+                .withDescription(description());
+    }
+
+    @Override
+    public String description() {
+        return "Filter mutations to defensive return wrappers such as unmodifiableCollection";
+    }
+}
diff --git a/...main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory b/...main/resources/META-INF/services/org.pitest.mutationtest.build.MutationInterceptorFactory
@@ -24,5 +24,7 @@ org.pitest.mutationtest.build.intercept.equivalent.EquivalentReturnMutationFilte
 org.pitest.mutationtest.build.intercept.exclude.FirstLineInterceptorFactory
 org.pitest.mutationtest.build.intercept.equivalent.DivisionByMinusOneFilterFactory
 org.pitest.mutationtest.build.intercept.lombok.LombokFilter
+org.pitest.mutationtest.build.intercept.defensive.ReturnUnmodifiableCollectionFactory
+
 
 org.pitest.plugin.export.MutantExportFactory
diff --git a/...itest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java b/...itest/mutationtest/build/intercept/defensive/ReturnUnmodifiableCollectionFactoryTest.java
@@ -0,0 +1,135 @@
+package org.pitest.mutationtest.build.intercept.defensive;
+
+import org.junit.Test;
+import org.pitest.mutationtest.build.InterceptorType;
+import org.pitest.mutationtest.build.MutationInterceptorFactory;
+import org.pitest.mutationtest.engine.gregor.mutators.NullMutateEverything;
+import org.pitest.verifier.interceptors.FactoryVerifier;
+import org.pitest.verifier.interceptors.InterceptorVerifier;
+import org.pitest.verifier.interceptors.VerifierStart;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.pitest.bytecode.analysis.OpcodeMatchers.INVOKESTATIC;
+
+
+public class ReturnUnmodifiableCollectionFactoryTest {
+    private final MutationInterceptorFactory underTest = new ReturnUnmodifiableCollectionFactory();
+    InterceptorVerifier v = VerifierStart.forInterceptorFactory(underTest)
+            .usingMutator(new NullMutateEverything());
+
+    @Test
+    public void isOnChain() {
+        FactoryVerifier.confirmFactory(underTest)
+                .isOnChain();
+    }
+
+    @Test
+    public void isOnByDefault() {
+        FactoryVerifier.confirmFactory(underTest)
+                .isOnByDefault();
+    }
+
+    @Test
+    public void featureIsCalledLombok() {
+        FactoryVerifier.confirmFactory(underTest)
+                .featureName().isEqualTo("defensivereturn");
+    }
+
+    @Test
+    public void createsFilters() {
+        FactoryVerifier.confirmFactory(underTest)
+                .createsInterceptorsOfType(InterceptorType.FILTER);
+    }
+
+
+    @Test
+    public void filtersMutationsToReturnUnmodifiableSet() {
+        v.forClass(HasUnmodifiableSetReturn.class)
+                .forCodeMatching(INVOKESTATIC.asPredicate())
+                .allMutantsAreFiltered()
+                .verify();
+    }
+
+    @Test
+    public void filtersMutationsToReturnUnmodifiableList() {
+        v.forClass(HasUnmodifiableListReturn.class)
+                .forCodeMatching(INVOKESTATIC.asPredicate())
+                .allMutantsAreFiltered()
+                .verify();
+    }
+
+    @Test
+    public void filtersMutationsToReturnUnmodifiableMap() {
+        v.forClass(HasUnmodifiableMapReturn.class)
+                .forCodeMatching(INVOKESTATIC.asPredicate())
+                .allMutantsAreFiltered()
+                .verify();
+    }
+
+    @Test
+    public void doesNotFilterOtherCode() {
+        v.forClass(HasUnmodifiableSetReturn.class)
+                .forCodeMatching(INVOKESTATIC.asPredicate().negate())
+                .noMutantsAreFiltered()
+                .verify();
+    }
+
+    @Test
+    public void doesNotFilterOtherCallsToUnModifiableSet() {
+        v.forClass(HasUnmodifiableSetNonReturn.class)
+                .forAnyCode()
+                .noMutantsAreFiltered()
+                .verify();
+    }
+}
+
+class HasUnmodifiableSetReturn {
+    private final Set<String> s = new HashSet<>();
+
+    public Set<String> mutateMe(int i) {
+        if (i != 1) {
+            return Collections.unmodifiableSet(s);
+        }
+
+        return s;
+    }
+}
+
+class HasUnmodifiableListReturn {
+    private final List<String> s = new ArrayList<>();
+
+    public List<String> mutateMe(int i) {
+        if (i != 1) {
+            return Collections.unmodifiableList(s);
+        }
+
+        return s;
+    }
+}
+
+class HasUnmodifiableMapReturn {
+
+    public Map<String,String> mutateMe(Map<String,String> m) {
+        return Collections.unmodifiableMap(m);
+    }
+}
+
+class HasUnmodifiableSetNonReturn {
+    private final Set<String> s = new HashSet<>();
+    private Set<String> copy;
+
+
+    public Set<String> dontMutateME(int i) {
+        if (i != 1) {
+            copy = Collections.unmodifiableSet(s);
+        }
+
+        return s;
+    }
+}
diff --git a/...g/pitest/mutationtest/build/intercept/equivalent/EqualsPerformanceShortcutFilterTest.java b/...g/pitest/mutationtest/build/intercept/equivalent/EqualsPerformanceShortcutFilterTest.java
@@ -17,7 +17,6 @@
 import org.pitest.mutationtest.engine.gregor.MethodMutatorFactory;
 import org.pitest.mutationtest.engine.gregor.config.Mutator;
 import org.pitest.mutationtest.engine.gregor.mutators.returns.BooleanFalseReturnValsMutator;
-import org.pitest.mutationtest.engine.gregor.mutators.returns.PrimitiveReturnsMutator;
 
 public class EqualsPerformanceShortcutFilterTest {
 

diff --git a/pitest/src/main/java/org/pitest/util/StreamUtil.java b/pitest/src/main/java/org/pitest/util/StreamUtil.java
@@ -5,6 +5,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.nio.Buffer;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.ReadableByteChannel;
@@ -35,7 +36,7 @@ private static void copy(final InputStream input, final OutputStream output)
     final WritableByteChannel dest = Channels.newChannel(output);
     final ByteBuffer buffer = ByteBuffer.allocateDirect(16 * 1024);
     while (src.read(buffer) != -1) {
-      buffer.flip();
+      ((Buffer)buffer).flip();
       dest.write(buffer);
       buffer.compact();
     }