Collapse `LocalDef` into `T{...}` macro, make it more flexible so it only caches `def`s (and complains about `val`s) if it's defined top-level in a class/trait/object, for easy overriding.

If it's in some helper function somewhere, overriding doesn't matter, and it's up to the user to figure out how to construct a target graph without redundancy

3 files changed, 27 insertions, 52 deletions

diff --git a/core/src/main/scala/forge/Target.scala b/core/src/main/scala/forge/Target.scala
index ae5b488b..681d9d2f 100644
--- a/core/src/main/scala/forge/Target.scala
+++ b/core/src/main/scala/forge/Target.scala
@@ -2,7 +2,7 @@ package forge
 
 
 import ammonite.ops.{ls, mkdir}
-import forge.util.{Args, LocalDef, PathRef}
+import forge.util.{Args, PathRef}
 import play.api.libs.json.{Format, JsValue, Json}
 
 import scala.annotation.compileTimeOnly
@@ -34,10 +34,8 @@ abstract class Target[T] extends Target.Ops[T]{
 object Target{
   trait Cacher{
     private[this] val cacherLazyMap = mutable.Map.empty[sourcecode.Enclosing, Target[_]]
-    protected[this] def T[T](t: T)
-                            (implicit l: LocalDef): Target[T] = macro localDefImpl[T]
-    protected[this] def T[T](t: => Target[T])
-                            (implicit c: sourcecode.Enclosing, l: LocalDef): Target[T] = {
+    protected[this] def cachedTarget[T](t: => Target[T])
+                                      (implicit c: sourcecode.Enclosing): Target[T] = {
       cacherLazyMap.getOrElseUpdate(c, t).asInstanceOf[Target[T]]
     }
   }
@@ -46,15 +44,12 @@ object Target{
     val inputs = Nil
     def evaluate(args: Args)  = t0
   }
-  def apply[T](t: Target[T]): Target[T] = t
+  def apply[T](t: Target[T]): Target[T] = macro impl0[T]
   def apply[T](t: T): Target[T] = macro impl[T]
-  def localDefImpl[T: c.WeakTypeTag](c: Context)
-                                    (t: c.Expr[T])
-                                    (l: c.Expr[LocalDef]): c.Expr[Target[T]] = {
-    impl(c)(t)
+  def impl0[T: c.WeakTypeTag](c: Context)(t: c.Expr[Target[T]]): c.Expr[Target[T]] = {
+    wrapCached(c)(t.tree)
   }
-  def impl[T: c.WeakTypeTag](c: Context)
-                            (t: c.Expr[T]): c.Expr[Target[T]] = {
+  def impl[T: c.WeakTypeTag](c: Context)(t: c.Expr[T]): c.Expr[Target[T]] = {
     import c.universe._
     val bound = collection.mutable.Buffer.empty[(c.Tree, Symbol)]
     val OptionGet = c.universe.typeOf[Target[_]].member(TermName("apply"))
@@ -78,14 +73,25 @@ object Target{
 
     val bindings = symbols.map(c.internal.valDef(_))
 
-    val newTargetTree = q"forge.zipMap(..$exprs){ (..$bindings) => $transformed }"
-
-    val embedded =
-      if (!(c.prefix.tree.tpe <:< typeOf[Cacher])) newTargetTree
-      else q"${c.prefix}.T($newTargetTree)"
-
-
-    c.Expr[Target[T]](embedded)
+    wrapCached(c)(q"forge.zipMap(..$exprs){ (..$bindings) => $transformed }")
+  }
+  def wrapCached[T](c: Context)(t: c.Tree) = {
+    import c.universe._
+    val owner = c.internal.enclosingOwner
+    val ownerIsCacherClass = owner.owner.asClass.baseClasses.exists(_.fullName == "forge.Target.Cacher")
+
+    if (ownerIsCacherClass && !owner.isMethod){
+      c.abort(
+        c.enclosingPosition,
+        "T{} members defined in a Cacher class/trait/object body must be defs"
+      )
+    }else{
+      val embedded =
+        if (!ownerIsCacherClass) t
+        else q"this.cachedTarget($t)"
+
+      c.Expr[Target[T]](embedded)
+    }
   }
 
   abstract class Ops[T]{ this: Target[T] =>
diff --git a/core/src/main/scala/forge/util/LocalDef.scala b/core/src/main/scala/forge/util/LocalDef.scala
deleted file mode 100644
index 2a58bbd2..00000000
--- a/core/src/main/scala/forge/util/LocalDef.scala
+++ /dev/null
@@ -1,28 +0,0 @@
-package forge.util
-
-import scala.reflect.macros.blackbox
-import language.experimental.macros
-class LocalDef
-object LocalDef {
-  implicit def default: LocalDef = macro enclosing
-  def enclosing(c: blackbox.Context): c.Expr[LocalDef] = {
-
-    import c.universe._
-    val current = c.internal.enclosingOwner
-
-    if (
-      !current.isMethod ||
-      // We can't do this right now because it causes recursive method errors
-      // current.asMethod.paramLists.nonEmpty ||
-      !(current.owner.isClass || current.owner.isModuleClass)
-    ) {
-      c.abort(
-        c.enclosingPosition,
-        "T{} can only be used directly within a zero-arg method defined in a class body"
-      )
-    }else{
-
-      c.Expr[LocalDef](q"""new forge.util.LocalDef()""")
-    }
-  }
-}
diff --git a/core/src/test/scala/forge/CacherTests.scala b/core/src/test/scala/forge/CacherTests.scala
index c6157a4a..de8265bd 100644
--- a/core/src/test/scala/forge/CacherTests.scala
+++ b/core/src/test/scala/forge/CacherTests.scala
@@ -48,13 +48,10 @@ object CacherTests extends TestSuite{
     )
     'errors{
       val expectedMsg =
-        "T{} can only be used directly within a zero-arg method defined in a class body"
+        "T{} members defined in a Cacher class/trait/object body must be defs"
 
       val err1 = compileError("object Foo extends Target.Cacher{ val x = T{1} }")
       assert(err1.msg == expectedMsg)
-
-      val err2 = compileError("object Foo extends Target.Cacher{ def x = {def y = T{1}} }")
-      assert(err2.msg == expectedMsg)
     }
   }
 }