Merge pull request #923 from hcoles/breaking_code_simplification

Refactorings that may break plugin compatibility

pitest-aggregator/src/main/java/org/pitest/aggregate/BlockCoverageDataLoader.java

@@ -8,7 +8,6 @@
 import org.pitest.coverage.BlockCoverage;
 import org.pitest.coverage.BlockLocation;
 import org.pitest.mutationtest.engine.Location;
-import org.pitest.mutationtest.engine.MethodName;
 
 class BlockCoverageDataLoader extends DataLoader<BlockCoverage> {
 
@@ -29,7 +28,7 @@ class BlockCoverageDataLoader extends DataLoader<BlockCoverage> {
   protected BlockCoverage mapToData(final Map<String, Object> map) {
     final String method = (String) map.get(METHOD);
     final Location location = new Location(ClassName.fromString((String) map.get(CLASSNAME)),
-        MethodName.fromString(method.substring(0, method.indexOf(OPEN_PAREN))), method.substring(method.indexOf(OPEN_PAREN)));
+        method.substring(0, method.indexOf(OPEN_PAREN)), method.substring(method.indexOf(OPEN_PAREN)));
 
     final BlockLocation blockLocation = new BlockLocation(location, Integer.parseInt((String) map.get(NUMBER)),
         Integer.parseInt((String) map.get(FIRST_INSN)),Integer.parseInt((String) map.get(LAST_INSN)));

pitest-aggregator/src/main/java/org/pitest/aggregate/MutationResultDataLoader.java

@@ -10,7 +10,6 @@
 import org.pitest.mutationtest.MutationResult;
 import org.pitest.mutationtest.MutationStatusTestPair;
 import org.pitest.mutationtest.engine.Location;
-import org.pitest.mutationtest.engine.MethodName;
 import org.pitest.mutationtest.engine.MutationDetails;
 import org.pitest.mutationtest.engine.MutationIdentifier;
 
@@ -35,7 +34,7 @@ class MutationResultDataLoader extends DataLoader<MutationResult> {
 
   @Override
   protected MutationResult mapToData(final Map<String, Object> map) {
-    final Location location = new Location(ClassName.fromString((String) map.get(MUTATED_CLASS)), MethodName.fromString((String) map.get(MUTATED_METHOD)),
+    final Location location = new Location(ClassName.fromString((String) map.get(MUTATED_CLASS)), (String) map.get(MUTATED_METHOD),
         (String) map.get(METHOD_DESCRIPTION));
 
     final MutationIdentifier id = new MutationIdentifier(location, Arrays.asList(Integer.valueOf((String) map.get(INDEX))), (String) map.get(MUTATOR));

pitest-entry/src/main/java/org/pitest/bytecode/analysis/MethodTree.java

@@ -9,7 +9,6 @@
 import org.objectweb.asm.tree.MethodNode;
 import org.pitest.classinfo.ClassName;
 import org.pitest.mutationtest.engine.Location;
-import org.pitest.mutationtest.engine.MethodName;
 
 public class MethodTree {
 
@@ -27,7 +26,7 @@ public MethodNode rawNode() {
   }
 
   public Location asLocation() {
-    return Location.location(this.owner,MethodName.fromString(this.rawNode.name), this.rawNode.desc);
+    return Location.location(this.owner, this.rawNode.name, this.rawNode.desc);
   }
 
   /**

pitest-entry/src/main/java/org/pitest/coverage/execute/Receive.java

@@ -4,7 +4,6 @@
 import org.pitest.coverage.BlockLocation;
 import org.pitest.coverage.CoverageResult;
 import org.pitest.mutationtest.engine.Location;
-import org.pitest.mutationtest.engine.MethodName;
 import org.pitest.testapi.Description;
 import org.pitest.util.Id;
 import org.pitest.util.ReceiveStrategy;
@@ -55,7 +54,7 @@ private void handleProbes(final SafeDataInputStream is) {
     final int first = is.readInt();
     final int last = is.readInt();
     final Location loc = Location.location(this.classIdToName.get(classId),
-        MethodName.fromString(methodName), methodSig);
+        methodName, methodSig);
     for (int i = first; i != (last + 1); i++) {
       // nb, convert from classwide id to method scoped index within
       // BlockLocation

pitest-entry/src/main/java/org/pitest/coverage/export/DefaultCoverageExporter.java

@@ -48,7 +48,7 @@ private void writeLineCoverage(final BlockCoverage each, final Writer out) {
         out,
         "<block classname='" + l.getClassName().asJavaName() + "'"
             + " method='"
-            + StringUtil.escapeBasicHtmlChars(l.getMethodName().name()) + StringUtil.escapeBasicHtmlChars(l.getMethodDesc())
+            + StringUtil.escapeBasicHtmlChars(l.getMethodName()) + StringUtil.escapeBasicHtmlChars(l.getMethodDesc())
             + "' number='" + each.getBlock().getBlock()
             + "' firstInstruction='" + each.getBlock().getFirstInsnInBlock()
             + "' lastInstruction='" + each.getBlock().getLastInsnInBlock()

pitest-entry/src/main/java/org/pitest/mutationtest/build/DefaultTestPrioritiser.java

@@ -41,7 +41,6 @@ public List<TestInfo> assignTests(MutationDetails mutation) {
   }
 
   private Collection<TestInfo> pickTests(MutationDetails mutation) {
-    if (!mutation.isInStaticInitializer()) {
       if (mutation.getId().getIndexes().size() > 1) {
         HashSet<TestInfo> ret = new HashSet<>();
         for (int each : mutation.getId().getIndexes()) {
@@ -59,10 +58,6 @@ private Collection<TestInfo> pickTests(MutationDetails mutation) {
             .getTestsForInstructionLocation(new InstructionLocation(new BlockLocation(mutation.getId().getLocation(), mutation.getBlock(), -1, -1),
                 mutation.getId().getFirstIndex() - 1));
       }
-    } else {
-      LOG.warning("Using untargetted tests");
-      return this.coverage.getTestsForClass(mutation.getClassName());
-    }
   }
 
   private List<TestInfo> prioritizeTests(ClassName clazz,

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/equivalent/EqualsPerformanceShortcutFilter.java

@@ -21,7 +21,6 @@
 import org.pitest.mutationtest.build.InterceptorType;
 import org.pitest.mutationtest.build.MutationInterceptor;
 import org.pitest.mutationtest.engine.Location;
-import org.pitest.mutationtest.engine.MethodName;
 import org.pitest.mutationtest.engine.Mutater;
 import org.pitest.mutationtest.engine.MutationDetails;
 import org.pitest.sequence.QueryParams;
@@ -110,7 +109,7 @@ private Predicate<MutationDetails> inEqualsMethod() {
     return a -> {
       final Location loc = a.getId().getLocation();
       return loc.getMethodDesc().equals("(Ljava/lang/Object;)Z")
-          && loc.getMethodName().equals(MethodName.fromString("equals"));
+          && loc.getMethodName().equals("equals");
     };
   }
 

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/equivalent/NullFlatMapFilter.java

@@ -131,7 +131,7 @@ private boolean calledOnlyFromFlatMap(Location location) {
 
     private boolean callsTarget(MethodTree target, MethodTree method) {
         return method.instructions().stream()
-                .anyMatch(callTo(target.asLocation().getClassName(), target.asLocation().getMethodName().name(), target.asLocation().getMethodDesc())
+                .anyMatch(callTo(target.asLocation().getClassName(), target.asLocation().getMethodName(), target.asLocation().getMethodDesc())
                         .or(dynamicCallTo(target.asLocation())));
     }
 
@@ -161,7 +161,7 @@ private static Predicate<AbstractInsnNode> dynamicCallTo(Location desc) {
             if ( t instanceof InvokeDynamicInsnNode) {
                 InvokeDynamicInsnNode call = (InvokeDynamicInsnNode) t;
                 return Arrays.stream(call.bsmArgs)
-                        .anyMatch(isHandle(desc.getClassName(), desc.getMethodName().name(), desc.getMethodDesc()));
+                        .anyMatch(isHandle(desc.getClassName(), desc.getMethodName(), desc.getMethodDesc()));
             }
             return false;
         };

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/EnumConstructorFilter.java

@@ -37,7 +37,7 @@ public Collection<MutationDetails> intercept(
     }
 
     private Predicate<MutationDetails> isInEnumConstructor() {
-        return m -> isEnum && m.getMethod().name().equals("<init>");
+        return m -> isEnum && m.getMethod().equals("<init>");
     }
 
 

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/InlinedFinallyBlockFilter.java

@@ -1,10 +1,21 @@
 package org.pitest.mutationtest.build.intercept.javafeatures;
 
-import static java.util.function.Predicate.isEqual;
-import static org.pitest.functional.FCollection.bucket;
-import static org.pitest.functional.FCollection.map;
-import static org.pitest.functional.FCollection.mapTo;
-import static org.pitest.functional.prelude.Prelude.not;
+import org.objectweb.asm.tree.AbstractInsnNode;
+import org.objectweb.asm.tree.LabelNode;
+import org.pitest.bytecode.analysis.ClassTree;
+import org.pitest.bytecode.analysis.MethodTree;
+import org.pitest.mutationtest.build.InterceptorType;
+import org.pitest.mutationtest.build.MutationInterceptor;
+import org.pitest.mutationtest.engine.Mutater;
+import org.pitest.mutationtest.engine.MutationDetails;
+import org.pitest.mutationtest.engine.MutationIdentifier;
+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.util.Log;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -16,15 +27,24 @@
 import java.util.Set;
 import java.util.function.Function;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 
-import org.pitest.bytecode.analysis.ClassTree;
-import org.pitest.functional.FCollection;
-import org.pitest.mutationtest.build.InterceptorType;
-import org.pitest.mutationtest.build.MutationInterceptor;
-import org.pitest.mutationtest.engine.Mutater;
-import org.pitest.mutationtest.engine.MutationDetails;
-import org.pitest.mutationtest.engine.MutationIdentifier;
-import org.pitest.util.Log;
+import static java.util.function.Predicate.isEqual;
+import static org.objectweb.asm.Opcodes.ARETURN;
+import static org.objectweb.asm.Opcodes.ATHROW;
+import static org.objectweb.asm.Opcodes.DRETURN;
+import static org.objectweb.asm.Opcodes.FRETURN;
+import static org.objectweb.asm.Opcodes.IRETURN;
+import static org.objectweb.asm.Opcodes.LRETURN;
+import static org.objectweb.asm.Opcodes.RETURN;
+import static org.pitest.bytecode.analysis.InstructionMatchers.anyInstruction;
+import static org.pitest.bytecode.analysis.InstructionMatchers.isInstruction;
+import static org.pitest.bytecode.analysis.InstructionMatchers.notAnInstruction;
+import static org.pitest.bytecode.analysis.InstructionMatchers.opCode;
+import static org.pitest.functional.FCollection.bucket;
+import static org.pitest.functional.FCollection.map;
+import static org.pitest.functional.FCollection.mapTo;
+import static org.pitest.functional.prelude.Prelude.not;
 
 /**
  * Detects mutations on same line, but within different code blocks. This
@@ -37,54 +57,99 @@ public class InlinedFinallyBlockFilter implements MutationInterceptor {
 
   private static final Logger LOG = Log.getLogger();
 
+  private static final boolean DEBUG = false;
+
+  private static final Slot<AbstractInsnNode> MUTATED_INSTRUCTION = Slot.create(AbstractInsnNode.class);
+  private static final Slot<List> HANDLERS = Slot.create(List.class);
+
+  static final SequenceMatcher<AbstractInsnNode> IS_IN_HANDLER = QueryStart
+          .any(AbstractInsnNode.class)
+          .then(handlerLabel(HANDLERS))
+          .zeroOrMore(QueryStart.match(doesNotEndBlock()))
+          .then(isInstruction(MUTATED_INSTRUCTION.read()))
+          .zeroOrMore(QueryStart.match(anyInstruction()))
+          .compile(QueryParams.params(AbstractInsnNode.class)
+                  .withIgnores(notAnInstruction())
+                  .withDebug(DEBUG)
+          );
+
+  private static Match<AbstractInsnNode> doesNotEndBlock() {
+     return endsBlock().negate();
+  }
+
+  private static Match<AbstractInsnNode> endsBlock() {
+    return opCode(RETURN)
+            .or(opCode(ARETURN))
+            .or(opCode(DRETURN))
+            .or(opCode(FRETURN))
+            .or(opCode(IRETURN))
+            .or(opCode(LRETURN))
+            .or(opCode(ATHROW)); // dubious if this is needed
+  }
+
+  private static Match<AbstractInsnNode> handlerLabel(Slot<List> handlers) {
+    return (c,t) -> {
+      if (t instanceof LabelNode) {
+        LabelNode label = (LabelNode) t;
+        List<LabelNode> labels = c.retrieve(handlers.read()).get();
+        return labels.contains(label);
+      }
+      return false;
+    };
+  }
+
+  private ClassTree currentClass;
+
   @Override
   public InterceptorType type() {
     return InterceptorType.FILTER;
   }
 
   @Override
   public void begin(ClassTree clazz) {
-    // no-opp
+    currentClass = clazz;
   }
 
   @Override
   public Collection<MutationDetails> intercept(
       Collection<MutationDetails> mutations, Mutater m) {
-    final List<MutationDetails> combined = new ArrayList<>(
+
+    List<MutationDetails> combined = new ArrayList<>(
         mutations.size());
-    final Map<LineMutatorPair, Collection<MutationDetails>> mutatorLinebuckets = bucket(
+    Map<LineMutatorPair, Collection<MutationDetails>> mutatorLineBuckets = bucket(
         mutations, toLineMutatorPair());
 
-    for (final Entry<LineMutatorPair, Collection<MutationDetails>> each : mutatorLinebuckets
+    for (final Entry<LineMutatorPair, Collection<MutationDetails>> each : mutatorLineBuckets
         .entrySet()) {
       if (each.getValue().size() > 1) {
-        checkForInlinedCode(combined, each);
+        checkForInlinedCode(combined, each.getValue());
       } else {
         combined.addAll(each.getValue());
       }
     }
 
-    /* FIXME tests rely on order of returned mutants */
     combined.sort(compareLineNumbers());
     return combined;
   }
 
   @Override
   public void end() {
-    // no-opp
+    currentClass = null;
   }
 
   private static Comparator<MutationDetails> compareLineNumbers() {
     return Comparator.comparingInt(MutationDetails::getLineNumber);
   }
 
-  private void checkForInlinedCode(final Collection<MutationDetails> combined,
-      final Entry<LineMutatorPair, Collection<MutationDetails>> each) {
+  private void checkForInlinedCode(Collection<MutationDetails> mutantsToReturn,
+                                  Collection<MutationDetails> similarMutantsOnSameLine) {
+
+    final List<MutationDetails> mutationsInHandlerBlock = similarMutantsOnSameLine.stream()
+            .filter(this::isInFinallyBlock)
+            .collect(Collectors.toList());
 
-    final List<MutationDetails> mutationsInHandlerBlock = FCollection
-        .filter(each.getValue(), MutationDetails::isInFinallyBlock);
     if (!isPossibleToCorrectInlining(mutationsInHandlerBlock)) {
-      combined.addAll(each.getValue());
+      mutantsToReturn.addAll(similarMutantsOnSameLine);
       return;
     }
 
@@ -94,16 +159,34 @@ private void checkForInlinedCode(final Collection<MutationDetails> combined,
     // check that we have at least on mutation in a different block
     // to the base one (is this not implied by there being only 1 mutation in
     // the handler ????)
-    final List<Integer> ids = map(each.getValue(), MutationDetails::getBlock);
+    final List<Integer> ids = map(similarMutantsOnSameLine, MutationDetails::getBlock);
     if (ids.stream().anyMatch(not(isEqual(firstBlock)))) {
-      combined.add(makeCombinedMutant(each.getValue()));
+      mutantsToReturn.add(makeCombinedMutant(similarMutantsOnSameLine));
     } else {
-      combined.addAll(each.getValue());
+      mutantsToReturn.addAll(similarMutantsOnSameLine);
     }
   }
 
-  private boolean isPossibleToCorrectInlining(
-      final List<MutationDetails> mutationsInHandlerBlock) {
+  private boolean isInFinallyBlock(MutationDetails m) {
+    MethodTree method = currentClass.method(m.getId().getLocation()).get();
+    List<LabelNode> handlers = method.rawNode().tryCatchBlocks.stream()
+            .filter(t -> t.type == null)
+            .map(t -> t.handler)
+            .collect(Collectors.toList());
+
+    if (handlers.isEmpty()) {
+      return false;
+    }
+
+    AbstractInsnNode mutatedInstruction = method.instruction(m.getInstructionIndex());
+
+    Context<AbstractInsnNode> context = Context.start(method.instructions(), DEBUG);
+    context.store(MUTATED_INSTRUCTION.write(), mutatedInstruction);
+    context.store(HANDLERS.write(), handlers);
+    return IS_IN_HANDLER.matches(method.instructions(), context);
+  }
+
+  private boolean isPossibleToCorrectInlining(List<MutationDetails> mutationsInHandlerBlock) {
     if (mutationsInHandlerBlock.size() > 1) {
       LOG.warning("Found more than one mutation similar on same line in a finally block. Can't correct for inlining.");
       return false;
@@ -112,10 +195,9 @@ private boolean isPossibleToCorrectInlining(
     return !mutationsInHandlerBlock.isEmpty();
   }
 
-  private static MutationDetails makeCombinedMutant(
-      final Collection<MutationDetails> value) {
-    final MutationDetails first = value.iterator().next();
-    final Set<Integer> indexes = new HashSet<>();
+  private static MutationDetails makeCombinedMutant(Collection<MutationDetails> value) {
+    MutationDetails first = value.iterator().next();
+    Set<Integer> indexes = new HashSet<>();
     mapTo(value, MutationDetails::getFirstIndex, indexes);
 
     final MutationIdentifier id = new MutationIdentifier(first.getId()
@@ -125,9 +207,9 @@ private static MutationDetails makeCombinedMutant(
         first.getLineNumber(), first.getBlock());
   }
 
-
   private static Function<MutationDetails, LineMutatorPair> toLineMutatorPair() {
-    return a -> new LineMutatorPair(a.getLineNumber(), a.getMutator());
+    // bucket by combination of mutator and description
+    return a -> new LineMutatorPair(a.getLineNumber(), a.getMutator() + a.getDescription());
   }
 
 }

pitest-entry/src/main/java/org/pitest/mutationtest/build/intercept/javafeatures/LineMutatorPair.java

@@ -2,12 +2,12 @@
 
 import java.util.Objects;
 
-public class LineMutatorPair {
+final class LineMutatorPair {
 
   private final int    lineNumber;
   private final String mutator;
 
-  public LineMutatorPair(final int lineNumber, final String mutator) {
+  LineMutatorPair(final int lineNumber, final String mutator) {
     this.lineNumber = lineNumber;
     this.mutator = mutator;
   }