Simplify canPlace algorithm and fix instrumenter recursively placing …

…mutants (#1432)

- canPlace algorithm is simpler, only matching on certain blocks (we might've missed some types, but this is definitely a simpler implementation)
- Instrumenter now properly recursively places mutants. Before when a mutation switch was placed, it would not properly place mutations inside or above that node

core/src/main/scala/stryker4s/extension/TreeExtensions.scala

@@ -12,7 +12,6 @@ import scala.annotation.tailrec
 import scala.meta.*
 import scala.meta.transversers.SimpleTraverser
 import scala.reflect.ClassTag
-import scala.util.Try
 
 object TreeExtensions {
   @tailrec
@@ -56,11 +55,9 @@ object TreeExtensions {
       *
       * This function does not recursively go into the transformed tree
       */
-    final def transformOnce(fn: PartialFunction[Tree, Tree]): Try[Tree] = {
-      Try {
-        val onceTransformer = new OnceTransformer(fn)
-        onceTransformer(thisTree)
-      }
+    final def transformOnce(fn: PartialFunction[Tree, Tree]): Tree = {
+      val onceTransformer = new OnceTransformer(fn)
+      onceTransformer(thisTree)
     }
 
     /** Tries to transform a tree exactly once, returning None if the transformation was never applied
@@ -81,8 +78,7 @@ object TreeExtensions {
 
   private class OnceTransformer(fn: PartialFunction[Tree, Tree]) extends Transformer {
     override def apply(tree: Tree): Tree = {
-      val supered = super.apply(tree)
-      fn.applyOrElse(supered, identity[Tree])
+      fn.applyOrElse(tree, super.apply)
     }
   }
 

core/src/main/scala/stryker4s/mutants/TreeTraverser.scala

@@ -0,0 +1,40 @@
+package stryker4s.mutants
+
+import cats.syntax.all.*
+import stryker4s.extension.TreeExtensions.TreeIsInExtension
+import stryker4s.extension.mutationtype.ParentIsTypeLiteral
+
+import scala.meta.*
+
+trait TreeTraverser {
+
+  /** If the currently visiting node is a node where mutations can be placed, that node is returned, otherwise None
+    */
+  def canPlace(currentTree: Tree): Option[Term]
+
+}
+
+final class TreeTraverserImpl() extends TreeTraverser {
+
+  def canPlace(currentTree: Tree): Option[Term] = {
+    currentTree.parent
+      .filter {
+        case ParentIsTypeLiteral() => false
+        case name: Name            => !name.isDefinition
+        case t if t.is[Init]       => false
+
+        case d: Defn.Def if d.body == currentTree         => true
+        case d: Defn.Val if d.rhs == currentTree          => true
+        case d: Defn.Var if d.body == currentTree         => true
+        case _: Term.Block                                => true
+        case t: Term.Function if t.body == currentTree    => true
+        case t: Case if t.body == currentTree             => true
+        case t: Term.ForYield if t.body == currentTree    => true
+        case t: Template if t.stats.contains(currentTree) => true
+        case _                                            => false
+      }
+      .filterNot(_.isIn[Mod.Annot])
+      .as(currentTree)
+      .collect { case t: Term => t }
+  }
+}

core/src/main/scala/stryker4s/mutants/tree/MutantCollector.scala

@@ -4,15 +4,12 @@ import cats.syntax.align.*
 import cats.syntax.bifunctor.*
 import stryker4s.extension.TreeExtensions.*
 import stryker4s.model.{IgnoredMutationReason, MutatedCode, PlaceableTree}
-import stryker4s.mutants.Traverser
+import stryker4s.mutants.TreeTraverser
 import stryker4s.mutants.findmutants.MutantMatcher
 
 import scala.meta.Tree
 
-class MutantCollector(
-    traverser: Traverser,
-    matcher: MutantMatcher
-) {
+class MutantCollector(traverser: TreeTraverser, matcher: MutantMatcher) {
 
   def apply(tree: Tree): (Vector[(MutatedCode, IgnoredMutationReason)], Map[PlaceableTree, Mutations]) = {
 

core/src/main/scala/stryker4s/mutants/tree/MutantInstrumenter.scala

@@ -10,7 +10,7 @@ import stryker4s.model.*
 
 import scala.meta.*
 import scala.util.control.NonFatal
-import scala.util.{Failure, Success}
+import scala.util.{Failure, Success, Try}
 
 /** Instrument (place) mutants in a tree
   *
@@ -22,35 +22,40 @@ class MutantInstrumenter(options: InstrumenterOptions)(implicit log: Logger) {
 
   def instrumentFile(context: SourceContext, mutantMap: Map[PlaceableTree, MutantsWithId]): MutatedFile = {
 
-    val newTree = context.source
-      .transformOnce {
-        Function.unlift { originalTree =>
-          val p = PlaceableTree(originalTree)
-          mutantMap.get(p).map { case mutations =>
-            val mutableCases = mutations.map(mutantToCase)
-            val default = defaultCase(p, mutations.map(_.id).toNonEmptyList)
-
-            val cases = mutableCases :+ default
-
-            try
-              buildMatch(cases)
-            catch {
-              case NonFatal(e) =>
-                log.error(
-                  s"Failed to instrument mutants in `${context.path}`. Original statement: [${originalTree.syntax}]"
-                )
-                log.error(
-                  s"Failed mutation(s) '${mutations.map(_.id.value).mkString_(", ")}' at ${originalTree.pos.input}:${originalTree.pos.startLine + 1}:${originalTree.pos.startColumn + 1}."
-                )
-                log.error(
-                  "This is likely an issue on Stryker4s's end, please take a look at the debug logs",
-                  e
-                )
-                throw UnableToBuildPatternMatchException(context.path)
-            }
+    def instrumentWithMutants(mutantMap: Map[PlaceableTree, MutantsWithId]): PartialFunction[Tree, Tree] = {
+
+      Function.unlift { originalTree =>
+        val p = PlaceableTree(originalTree)
+        mutantMap.get(p).map { case mutations =>
+          val mutableCases = mutations.map(mutantToCase)
+
+          // Continue deeper into the tree (without the currently placed mutants)
+          val withDefaultsTransformed = PlaceableTree(p.tree.transformOnce(instrumentWithMutants(mutantMap - p)))
+          val default = defaultCase(withDefaultsTransformed, mutations.map(_.id).toNonEmptyList)
+
+          val cases = mutableCases :+ default
+
+          try
+            buildMatch(cases)
+          catch {
+            case NonFatal(e) =>
+              log.error(
+                s"Failed to instrument mutants in `${context.path}`. Original statement: [${originalTree.syntax}]"
+              )
+              log.error(
+                s"Failed mutation(s) '${mutations.map(_.id.value).mkString_(", ")}' at ${originalTree.pos.input}:${originalTree.pos.startLine + 1}:${originalTree.pos.startColumn + 1}."
+              )
+              log.error(
+                "This is likely an issue on Stryker4s's end, please take a look at the debug logs",
+                e
+              )
+              throw UnableToBuildPatternMatchException(context.path)
           }
         }
-      } match {
+      }
+    }
+
+    val newTree = Try(context.source.transformOnce(instrumentWithMutants(mutantMap))) match {
       case Success(tree)                  => tree
       case Failure(e: Stryker4sException) => throw e
       case Failure(e) =>

core/src/main/scala/stryker4s/run/Stryker4sRunner.scala

@@ -10,7 +10,7 @@ import stryker4s.log.{Logger, SttpLogWrapper}
 import stryker4s.model.CompilerErrMsg
 import stryker4s.mutants.findmutants.{MutantFinder, MutantMatcherImpl}
 import stryker4s.mutants.tree.{InstrumenterOptions, MutantCollector, MutantInstrumenter}
-import stryker4s.mutants.{Mutator, TraverserImpl}
+import stryker4s.mutants.{Mutator, TreeTraverserImpl}
 import stryker4s.report.*
 import stryker4s.report.dashboard.DashboardConfigProvider
 import stryker4s.run.process.ProcessRunner
@@ -32,7 +32,7 @@ abstract class Stryker4sRunner(implicit log: Logger) {
       resolveMutatesFileSource,
       new Mutator(
         new MutantFinder(),
-        new MutantCollector(new TraverserImpl(), new MutantMatcherImpl()),
+        new MutantCollector(new TreeTraverserImpl(), new MutantMatcherImpl()),
         instrumenter
       ),
       new MutantRunner(createTestRunnerPool, resolveFilesFileSource, new RollbackHandler(instrumenter), reporter),

core/src/test/scala/stryker4s/Stryker4sTest.scala

@@ -7,7 +7,7 @@ import stryker4s.files.ConfigFilesResolver
 import stryker4s.mutants.applymutants.ActiveMutationContext
 import stryker4s.mutants.findmutants.{MutantFinder, MutantMatcherImpl}
 import stryker4s.mutants.tree.{InstrumenterOptions, MutantCollector, MutantInstrumenter}
-import stryker4s.mutants.{Mutator, TraverserImpl}
+import stryker4s.mutants.{Mutator, TreeTraverserImpl}
 import stryker4s.report.{AggregateReporter, FinishedRunEvent}
 import stryker4s.run.threshold.SuccessStatus
 import stryker4s.run.{MutantRunner, RollbackHandler}
@@ -45,7 +45,7 @@ class Stryker4sTest extends Stryker4sIOSuite with MockitoIOSuite with Inside wit
         testSourceCollector,
         new Mutator(
           new MutantFinder(),
-          new MutantCollector(new TraverserImpl(), new MutantMatcherImpl()),
+          new MutantCollector(new TreeTraverserImpl(), new MutantMatcherImpl()),
           new MutantInstrumenter(InstrumenterOptions.sysContext(ActiveMutationContext.sysProps))
         ),
         testMutantRunner,

core/src/test/scala/stryker4s/extension/TreeExtensionsTest.scala

@@ -69,31 +69,31 @@ class TreeExtensionsTest extends Stryker4sSuite {
     it("should transform does not recursively transform new subtree") {
       val sut = q"def foo = 5"
 
-      val result = sut.transformOnce { case q"5" => q"5 + 1" }.get
+      val result = sut.transformOnce { case q"5" => q"5 + 1" }
 
       assert(result.isEqual(q"def foo = 5 + 1"), result)
     }
 
     it("should transform both appearances in the tree only once") {
       val sut = q"def foo = 5 + 5"
 
-      val result = sut.transformOnce { case q"5" => q"(5 * 2)" }.get
+      val result = sut.transformOnce { case q"5" => q"(5 * 2)" }
 
       assert(result.isEqual(q"def foo = (5 * 2) + (5 * 2)"), result)
     }
 
     it("should return the same tree if no transformation is applied") {
       val sut = q"def foo = 5"
 
-      val result = sut.transformOnce { case q"6" => q"6 + 1" }.get
+      val result = sut.transformOnce { case q"6" => q"6 + 1" }
 
       result should be theSameInstanceAs sut
     }
 
     it("should transform a parsed string and have changed syntax") {
       val sut = "val x: Int = 5".parse[Stat].get
 
-      val result = sut.transformOnce { case q"5" => q"6" }.get
+      val result = sut.transformOnce { case q"5" => q"6" }
 
       val expected = q"val x: Int = 6"
       assert(result.isEqual(expected), result)

core/src/test/scala/stryker4s/mutants/AddAllMutationsTest.scala

@@ -69,20 +69,20 @@ class AddAllMutationsTest extends Stryker4sIOSuite with LogMatchers {
             )
         } finally if (as.parCmds.isEmpty) finalize
       """,
-        5
+        11
       )
     }
 
-    // it("each case of pattern match") {
-    //   checkAllMutationsAreAdded(
-    //     q"""
-    //     foo match {
-    //       case _ => "break"
-    //       case _ if high == low => baz
-    //     }""",
-    //     2
-    //   )
-    // }
+    it("each case of pattern match") {
+      checkAllMutationsAreAdded(
+        q"""
+        foo match {
+          case _ => "break"
+          case _ if high == low => baz
+        }""",
+        2
+      )
+    }
 
     it("try-catch-finally") {
       checkAllMutationsAreAdded(
@@ -104,7 +104,7 @@ class AddAllMutationsTest extends Stryker4sIOSuite with LogMatchers {
 
       val mutator = new Mutator(
         new MutantFinderStub(source),
-        new MutantCollector(new TraverserImpl(), new MutantMatcherImpl()),
+        new MutantCollector(new TreeTraverserImpl(), new MutantMatcherImpl()),
         new MutantInstrumenter(InstrumenterOptions.testRunner)
       )
 

core/src/test/scala/stryker4s/mutants/MutatorTest.scala

@@ -17,7 +17,7 @@ class MutatorTest extends Stryker4sIOSuite with LogMatchers {
     implicit val conf: Config = Config.default
     val sut = new Mutator(
       new MutantFinder(),
-      new MutantCollector(new TraverserImpl(), new MutantMatcherImpl()),
+      new MutantCollector(new TreeTraverserImpl(), new MutantMatcherImpl()),
       new MutantInstrumenter(InstrumenterOptions.testRunner)
     )
 
@@ -80,7 +80,7 @@ class MutatorTest extends Stryker4sIOSuite with LogMatchers {
       implicit val conf: Config = Config.default
       val sut = new Mutator(
         new MutantFinder(),
-        new MutantCollector(new TraverserImpl, new MutantMatcherImpl),
+        new MutantCollector(new TreeTraverserImpl, new MutantMatcherImpl),
         new MutantInstrumenter(InstrumenterOptions.testRunner)
       )
       val files = Stream(FileUtil.getResource("scalaFiles/simpleFile.scala"))
@@ -96,7 +96,7 @@ class MutatorTest extends Stryker4sIOSuite with LogMatchers {
       implicit val conf: Config = Config.default.copy(excludedMutations = Set("EqualityOperator"))
       val sut = new Mutator(
         new MutantFinder(),
-        new MutantCollector(new TraverserImpl, new MutantMatcherImpl),
+        new MutantCollector(new TreeTraverserImpl, new MutantMatcherImpl),
         new MutantInstrumenter(InstrumenterOptions.testRunner)
       )
       val files = Stream(FileUtil.getResource("scalaFiles/simpleFile.scala"))
@@ -113,7 +113,7 @@ class MutatorTest extends Stryker4sIOSuite with LogMatchers {
       implicit val conf: Config = Config.default.copy(excludedMutations = Set("EqualityOperator"))
       val sut = new Mutator(
         new MutantFinder(),
-        new MutantCollector(new TraverserImpl, new MutantMatcherImpl),
+        new MutantCollector(new TreeTraverserImpl, new MutantMatcherImpl),
         new MutantInstrumenter(InstrumenterOptions.testRunner)
       )
       val files = Stream(FileUtil.getResource("fileTests/filledDir/src/main/scala/package/someFile.scala"))
@@ -130,7 +130,7 @@ class MutatorTest extends Stryker4sIOSuite with LogMatchers {
       implicit val conf: Config = Config.default.copy(excludedMutations = Set("EqualityOperator", "StringLiteral"))
       val sut = new Mutator(
         new MutantFinder(),
-        new MutantCollector(new TraverserImpl, new MutantMatcherImpl),
+        new MutantCollector(new TreeTraverserImpl, new MutantMatcherImpl),
         new MutantInstrumenter(InstrumenterOptions.testRunner)
       )
       val files = Stream(FileUtil.getResource("scalaFiles/simpleFile.scala"))