- Make `Discovered` logic use the public getter of a `val` field, instead of the private backing-field (which seems to end with a whitespace, and doesn't seem to exist for fields defined in anonynomous classes e.g. `new { val x = 1 }`

- Re-enable `mill.main.MainTests.nested`

- Share test graphs between `CrossModuleTests`, `DiscoveredTests` and `MainTests`

8 files changed, 169 insertions, 133 deletions

diff --git a/core/src/main/scala/mill/Main.scala b/core/src/main/scala/mill/Main.scala
index a4b81163..4012128a 100644
--- a/core/src/main/scala/mill/Main.scala
+++ b/core/src/main/scala/mill/Main.scala
@@ -96,7 +96,7 @@ object Main {
               case (label, child) if label == singleLabel => child
             } match{
               case Some(child) => resolve(tail, child, obj, rest, remainingCrossSelectors, newRevSelectorsSoFar)
-              case None => Left("Cannot resolve module " + renderSelector(newRevSelectorsSoFar))
+              case None => Left("Cannot resolve module " + renderSelector(newRevSelectorsSoFar.reverse))
             }
 
           case Mirror.Segment.Cross(cross) =>
diff --git a/core/src/main/scala/mill/discover/Discovered.scala b/core/src/main/scala/mill/discover/Discovered.scala
index 6bed7919..e3939d5b 100644
--- a/core/src/main/scala/mill/discover/Discovered.scala
+++ b/core/src/main/scala/mill/discover/Discovered.scala
@@ -34,6 +34,7 @@ object Discovered {
   implicit def apply[T]: Discovered[T] = macro applyImpl[T]
   def tupleLeft[T, V](items: List[(T, V)]) = items.map(_._1)
   def applyImpl[T: c.WeakTypeTag](c: Context): c.Expr[Discovered[T]] = {
+
     import c.universe._
     val tpe = c.weakTypeTag[T].tpe
     def rec(segments: List[Option[String]],
@@ -41,13 +42,11 @@ object Discovered {
 
       val r = new Router(c)
 
-
       val targets = for {
         m <- t.members.toList
         if m.isMethod &&
            m.typeSignature.paramLists.isEmpty &&
            m.typeSignature.resultType <:< c.weakTypeOf[Target[_]] &&
-           !m.name.toString.contains('$') &&
            !m.name.toString.contains(' ')
       } yield {
         val x = Ident(TermName(c.freshName()))
@@ -63,18 +62,19 @@ object Discovered {
       val crossChildren =
         if (!(t <:< c.weakTypeOf[Cross[Module]])) q"None"
         else {
-
           val TypeRef(_, _, Seq(arg)) = t
           val innerMirror = rec(None :: segments, arg)
           q"Some(((c: mill.define.Cross[_]) => mill.discover.Discovered.tupleLeft(c.items), $innerMirror))"
         }
       val childHierarchies = for{
         m <- t.members.toList
-        if (m.typeSignature <:< c.weakTypeOf[Module]) ||
-           (m.typeSignature <:< c.weakTypeOf[Cross[Module]])
+        if m.typeSignature.paramLists.isEmpty && m.isPublic
+        if (m.typeSignature.finalResultType <:< c.weakTypeOf[Module]) ||
+          (m.typeSignature.finalResultType <:< c.weakTypeOf[Cross[Module]])
+
       } yield {
         val name = m.name.toString.trim
-        q"($name, ${rec(Some(name) :: segments, m.typeSignature)})"
+        q"($name, ${rec(Some(name) :: segments, m.typeSignature.finalResultType)})"
       }
 
       val crossName = q"${TermName(c.freshName())}"
diff --git a/core/src/test/scala/mill/define/GraphTests.scala b/core/src/test/scala/mill/define/GraphTests.scala
index cbf0b189..52f144f0 100644
--- a/core/src/test/scala/mill/define/GraphTests.scala
+++ b/core/src/test/scala/mill/define/GraphTests.scala
@@ -13,6 +13,7 @@ object GraphTests extends TestSuite{
 
     val graphs = new TestGraphs()
     import graphs._
+    import TestGraphs._
 
     'topoSortedTransitiveTargets - {
       def check(targets: OSet[Task[_]], expected: OSet[Task[_]]) = {
diff --git a/core/src/test/scala/mill/discover/CrossModuleTests.scala b/core/src/test/scala/mill/discover/CrossModuleTests.scala
index 6c328a7e..a3a66daf 100644
--- a/core/src/test/scala/mill/discover/CrossModuleTests.scala
+++ b/core/src/test/scala/mill/discover/CrossModuleTests.scala
@@ -4,8 +4,9 @@ import mill.{Module, T}
 import mill.define.Cross
 import mill.discover.Mirror.{LabelledTarget, Segment}
 import mill.discover.Mirror.Segment.Label
+import mill.util.TestUtil.test
 import utest._
-
+import mill.util.TestGraphs._
 object CrossModuleTests extends TestSuite{
 
   val tests = Tests{
@@ -73,47 +74,46 @@ object CrossModuleTests extends TestSuite{
       }
     }
     'crossTargetDiscovery - {
-      object outer{
-        val crossed =
-          for(n <- Cross("2.10.6", "2.11.8", "2.12.4"))
-          yield new Module{ def scalaVersion = T{ n } }
-      }
-      val discovered = Discovered[outer.type]
 
-      val segments = discovered.targets(outer).map(_.segments)
+      val discovered = Discovered[singleCross.type].targets(singleCross)
+
+      val segments = discovered.map(_.segments)
       val expectedSegments = List(
-        List(Label("crossed"), Segment.Cross(List("2.10.6")), Label("scalaVersion")),
-        List(Label("crossed"), Segment.Cross(List("2.11.8")), Label("scalaVersion")),
-        List(Label("crossed"), Segment.Cross(List("2.12.4")), Label("scalaVersion"))
+        List(Label("cross"), Segment.Cross(List("210")), Label("suffix")),
+        List(Label("cross"), Segment.Cross(List("211")), Label("suffix")),
+        List(Label("cross"), Segment.Cross(List("212")), Label("suffix"))
       )
       assert(segments == expectedSegments)
-      val targets = discovered.targets(outer).map(_.target)
+      val targets = discovered.map(_.target)
       val expected = List(
-        outer.crossed("2.10.6").scalaVersion,
-        outer.crossed("2.11.8").scalaVersion,
-        outer.crossed("2.12.4").scalaVersion
+        singleCross.cross("210").suffix,
+        singleCross.cross("211").suffix,
+        singleCross.cross("212").suffix
       )
       assert(targets == expected)
     }
 
     'doubleCrossTargetDiscovery - {
-      object outer{
-        val crossed =
-          for{
-            n <- Cross("2.11.8", "2.12.4")
-            platform <- Cross("sjs0.6", "native0.3")
-          } yield new Module{ def suffix = T{ n + "_" + platform } }
-      }
-      val discovered = Discovered[outer.type]
-      val targets = discovered.targets(outer).map(_.target)
-      
+      val discovered = Discovered[doubleCross.type]
+      val targets = discovered.targets(doubleCross).map(_.target)
+
       val expected = List(
-        outer.crossed("sjs0.6", "2.11.8").suffix,
-        outer.crossed("native0.3", "2.11.8").suffix,
-        outer.crossed("sjs0.6", "2.12.4").suffix,
-        outer.crossed("native0.3", "2.12.4").suffix
+        doubleCross.cross("jvm", "210").suffix,
+        doubleCross.cross("js", "210").suffix,
+        doubleCross.cross("jvm", "211").suffix,
+        doubleCross.cross("js", "211").suffix,
+        doubleCross.cross("jvm", "212").suffix,
+        doubleCross.cross("js", "212").suffix,
+        doubleCross.cross("native", "212").suffix,
+
       )
       assert(targets == expected)
     }
+
+    'nestedCrosses - {
+      val discovered = Discovered[nestedCrosses.type].targets(nestedCrosses)
+      assert(discovered.length == 9)
+    }
   }
 }
+
diff --git a/core/src/test/scala/mill/discover/DiscoveredTests.scala b/core/src/test/scala/mill/discover/DiscoveredTests.scala
index 7fa030eb..e606b2a9 100644
--- a/core/src/test/scala/mill/discover/DiscoveredTests.scala
+++ b/core/src/test/scala/mill/discover/DiscoveredTests.scala
@@ -5,52 +5,33 @@ import java.io.InputStreamReader
 import mill.discover.Router.{ArgSig, EntryPoint}
 import utest._
 import mill.{Module, T}
-import mill.util.TestUtil.test
 import mill.discover.Mirror.Segment.Label
+import mill.util.TestGraphs.nestedModule
 object DiscoveredTests extends TestSuite{
 
   val tests = Tests{
 
     'targets - {
-      class CanNest extends Module{
-        val single = test()
-        val invisible: Any = test()
-        val invisible2: mill.define.Task[Int] = test()
-        val invisible3: mill.define.Task[_] = test()
-      }
-      object outer {
-        val single = test()
-        val invisible: Any = test()
-        object nested extends Module{
-          val single = test()
-          val invisible: Any = test()
-
-        }
-        val classInstance = new CanNest
-
-      }
-
-      val discovered = Discovered[outer.type]
-
+      val discovered = Discovered[nestedModule.type]
 
-      def flatten(h: Mirror[outer.type, _]): Seq[Any] = {
-        h.node(outer, Nil) :: h.children.flatMap{case (label, c) => flatten(c)}
+      def flatten(h: Mirror[nestedModule.type, _]): Seq[Any] = {
+        h.node(nestedModule, Nil) :: h.children.flatMap{case (label, c) => flatten(c)}
       }
       val flattenedHierarchy = flatten(discovered.mirror)
 
       val expectedHierarchy = Seq(
-        outer,
-        outer.classInstance,
-        outer.nested,
+        nestedModule,
+        nestedModule.classInstance,
+        nestedModule.nested,
       )
       assert(flattenedHierarchy == expectedHierarchy)
 
-      val mapped = discovered.targets(outer).map(x => x.segments -> x.target)
+      val mapped = discovered.targets(nestedModule).map(x => x.segments -> x.target)
 
       val expected = Seq(
-        (List(Label("classInstance"), Label("single")), outer.classInstance.single),
-        (List(Label("nested"), Label("single")), outer.nested.single),
-        (List(Label("single")), outer.single)
+        (List(Label("classInstance"), Label("single")), nestedModule.classInstance.single),
+        (List(Label("nested"), Label("single")), nestedModule.nested.single),
+        (List(Label("single")), nestedModule.single)
       )
       assert(mapped.toSet == expected.toSet)
     }
diff --git a/core/src/test/scala/mill/eval/EvaluationTests.scala b/core/src/test/scala/mill/eval/EvaluationTests.scala
index df18da15..d3c1861f 100644
--- a/core/src/test/scala/mill/eval/EvaluationTests.scala
+++ b/core/src/test/scala/mill/eval/EvaluationTests.scala
@@ -5,7 +5,7 @@ import mill.util.TestUtil.{Test, test}
 import mill.define.{Graph, Target, Task}
 import mill.{Module, T}
 import mill.discover.Discovered
-import mill.util.{OSet, TestUtil}
+import mill.util.{OSet, TestGraphs, TestUtil}
 import utest._
 import utest.framework.TestPath
 
@@ -50,8 +50,9 @@ object EvaluationTests extends TestSuite{
 
 
   val tests = Tests{
-    val graphs = new mill.util.TestGraphs()
+    val graphs = new TestGraphs()
     import graphs._
+    import TestGraphs._
     'evaluateSingle - {
 
       'singleton - {
diff --git a/core/src/test/scala/mill/main/MainTests.scala b/core/src/test/scala/mill/main/MainTests.scala
index b9f2020f..75403e17 100644
--- a/core/src/test/scala/mill/main/MainTests.scala
+++ b/core/src/test/scala/mill/main/MainTests.scala
@@ -3,6 +3,7 @@ package mill.main
 import mill.Module
 import mill.define.Task
 import mill.discover.{Discovered, Mirror}
+import mill.util.TestGraphs._
 import mill.util.TestUtil.test
 import utest._
 
@@ -28,83 +29,62 @@ object MainTests extends TestSuite{
       'neg3 - check(singleton, "", Left("Selector cannot be empty"))
     }
     'nested - {
-      class CanNest extends Module{
-        val single = test()
-      }
-      object outer {
-        val single = test()
-        object nested extends Module{
-          val single = test()
-        }
-        val classInstance = new CanNest
-      }
-      'pos1 - check(outer, "single", Right(outer.single))
-      'pos2 - check(outer, "nested.single", Right(outer.nested.single))
-      'pos3 - check(outer, "classInstance.single", Right(outer.classInstance.single))
-      'neg1 - check(outer, "doesntExist", Left("Cannot resolve task doesntExist"))
-      'neg2 - check(outer, "single.doesntExist", Left("Cannot resolve module single"))
-      'neg3 - check(outer, "nested.doesntExist", Left("Cannot resolve task nested.doesntExist"))
-      'neg4 - check(outer, "classInstance.doesntExist", Left("Cannot resolve task classInstance.doesntExist"))
+
+      'pos1 - check(nestedModule, "single", Right(nestedModule.single))
+      'pos2 - check(nestedModule, "nested.single", Right(nestedModule.nested.single))
+      'pos3 - check(nestedModule, "classInstance.single", Right(nestedModule.classInstance.single))
+      'neg1 - check(nestedModule, "doesntExist", Left("Cannot resolve task doesntExist"))
+      'neg2 - check(nestedModule, "single.doesntExist", Left("Cannot resolve module single"))
+      'neg3 - check(nestedModule, "nested.doesntExist", Left("Cannot resolve task nested.doesntExist"))
+      'neg4 - check(nestedModule, "classInstance.doesntExist", Left("Cannot resolve task classInstance.doesntExist"))
     }
     'cross - {
       'single - {
-        object outer{
-          val cross =
-            for(jarLabel <- mill.define.Cross("jarA", "jarB", "jarC"))
-            yield new mill.Module{
-              val target = test()
-            }
-        }
-        'pos1 - check(outer, "cross[jarA].target", Right(outer.cross("jarA").target))
-        'pos2 - check(outer, "cross[jarB].target", Right(outer.cross("jarB").target))
-        'neg1 - check(outer, "cross[jarA].doesntExist", Left("Cannot resolve task cross[jarA].doesntExist"))
+
+        'pos1 - check(singleCross, "cross[210].suffix", Right(singleCross.cross("210").suffix))
+        'pos2 - check(singleCross, "cross[211].suffix", Right(singleCross.cross("211").suffix))
+        'neg1 - check(singleCross, "cross[210].doesntExist", Left("Cannot resolve task cross[210].doesntExist"))
   //      'neg2 - check(outer, "cross[doesntExist].doesntExist", Left("Cannot resolve cross cross[doesntExist]"))
   //      'neg2 - check(outer, "cross[doesntExist].target", Left("Cannot resolve cross cross[doesntExist]"))
       }
       'double - {
-        object outer{
-          val cross =
-            for{
-              jarLabel <- mill.define.Cross("jarA", "jarB", "jarC")
-              tag <- mill.define.Cross("jvm", "js", "native")
-            } yield new mill.Module{
-              val target = test()
-            }
-        }
+
         'pos1 - check(
-          outer,
-          "cross[jvm,jarA].target",
-          Right(outer.cross("jvm", "jarA").target)
+          doubleCross,
+          "cross[jvm,210].suffix",
+          Right(doubleCross.cross("jvm", "210").suffix)
         )
         'pos2 - check(
-          outer,
-          "cross[jvm,jarB].target",
-          Right(outer.cross("jvm", "jarB").target)
+          doubleCross,
+          "cross[jvm,211].suffix",
+          Right(doubleCross.cross("jvm", "211").suffix)
         )
       }
       'nested - {
-        object outer{
-          val cross =
-            for(jarLabel <- mill.define.Cross("jarA", "jarB", "jarC"))
-            yield new mill.Module{
-              val cross2 =
-                for(tag <- mill.define.Cross("jvm", "js", "native"))
-                yield new mill.Module{
-                  val target = test()
-                }
-
-            }
+        'indirect - {
+          'pos1 - check(
+            indirectNestedCrosses,
+            "cross[210].cross2[js].suffix",
+            Right(indirectNestedCrosses.cross("210").cross2("js").suffix)
+          )
+          'pos2 - check(
+            indirectNestedCrosses,
+            "cross[211].cross2[jvm].suffix",
+            Right(indirectNestedCrosses.cross("211").cross2("jvm").suffix)
+          )
+        }
+        'direct - {
+          'pos1 - check(
+            nestedCrosses,
+            "cross[210].cross2[js].suffix",
+            Right(nestedCrosses.cross("210").cross2("js").suffix)
+          )
+          'pos2 - check(
+            nestedCrosses,
+            "cross[211].cross2[jvm].suffix",
+            Right(nestedCrosses.cross("211").cross2("jvm").suffix)
+          )
         }
-//        'pos1 - check(
-//          outer,
-//          "cross[jarA].cross2[js].target",
-//          Right(outer.cross(List("jarA")).cross2(List("js")).target)
-//        )
-//        'pos2 - check(
-//          outer,
-//          "cross[jarB].cross2[jvm].target",
-//          Right(outer.cross(List("jarB")).cross2(List("jvm")).target)
-//        )
       }
     }
 
diff --git a/core/src/test/scala/mill/util/TestGraphs.scala b/core/src/test/scala/mill/util/TestGraphs.scala
index af25aabb..edea69a4 100644
--- a/core/src/test/scala/mill/util/TestGraphs.scala
+++ b/core/src/test/scala/mill/util/TestGraphs.scala
@@ -1,6 +1,18 @@
 package mill.util
 import TestUtil.test
 import mill.{Module, T}
+
+/**
+  * Example dependency graphs for us to use in our test suite.
+  *
+  * The graphs using `test()` live in the `class` and need to be instantiated
+  * every time you use them, because they are mutable (you can poke at the
+  * `test`'s `counter`/`failure`/`exception` fields to test various graph
+  * evaluation scenarios.
+  *
+  * The immutable graphs, used for testing discovery & target resolution,
+  * live in the companion object.
+  */
 class TestGraphs(){
   // single
   object singleton {
@@ -91,7 +103,8 @@ class TestGraphs(){
     }
     val j = test(test(i), test(i, f), test(f))
   }
-
+}
+object TestGraphs{
   //        _ left _
   //       /        \
   //  task1 -------- right
@@ -134,5 +147,65 @@ class TestGraphs(){
     def right = T{ task1() + left() + 1 }
     val task2 = T.task{ left() + right() }
   }
-}
 
+
+  class CanNest extends Module{
+    def single = T{ 1 }
+    def invisible: Any = T{ 2 }
+    def invisible2: mill.define.Task[Int] = T{ 3 }
+    def invisible3: mill.define.Task[_] = T{ 4 }
+  }
+  object nestedModule extends Module{
+    def single = T{ 5 }
+    def invisible: Any = T{ 6 }
+    object nested extends Module{
+      def single = T{ 7 }
+      def invisible: Any = T{ 8 }
+
+    }
+    val classInstance = new CanNest
+
+  }
+
+
+  object singleCross{
+    val cross =
+      for(scalaVersion <- mill.define.Cross("210", "211", "212"))
+      yield new mill.Module{
+        def suffix = T{ scalaVersion }
+      }
+  }
+  object doubleCross{
+    val cross =
+      for{
+        scalaVersion <- mill.define.Cross("210", "211", "212")
+        platform <- mill.define.Cross("jvm", "js", "native")
+        if !(platform == "native" && scalaVersion != "212")
+      } yield new Module{
+        def suffix = T{ scalaVersion + "_" + platform }
+      }
+  }
+
+  object indirectNestedCrosses{
+    val cross = mill.define.Cross("210", "211", "212").map(new cross(_))
+    class cross(scalaVersion: String) extends mill.Module{
+      val cross2 =
+        for(platform <- mill.define.Cross("jvm", "js", "native"))
+        yield new mill.Module{
+          def suffix = T{ scalaVersion + "_" + platform }
+        }
+    }
+  }
+
+  object nestedCrosses{
+    val cross =
+      for(scalaVersion <- mill.define.Cross("210", "211", "212"))
+      yield new mill.Module{
+        val cross2 =
+          for(platform <- mill.define.Cross("jvm", "js", "native"))
+          yield new mill.Module{
+            def suffix = T{ scalaVersion + "_" + platform }
+          }
+      }
+  }
+}