Clean up Resolve.scala and related code to improve rigor and error reporting

bsp/worker/src/mill/bsp/worker/MillBuildServer.scala

@@ -657,7 +657,7 @@ class MillBuildServer(
             val mainModule = new MainModule {
               override implicit def millDiscover: Discover[_] = Discover[this.type]
             }
-            val compileTargetName = (module.millModuleSegments ++ Segments(Label("compile"))).render
+            val compileTargetName = (module.millModuleSegments ++ Label("compile")).render
             log.debug(s"about to clean: ${compileTargetName}")
             val cleanTask = mainModule.clean(evaluator, Seq(compileTargetName): _*)
             val cleanResult = evaluator.evaluate(
@@ -674,7 +674,7 @@ class MillBuildServer(
             )
             else {
               val outPaths = evaluator.pathsResolver.resolveDest(
-                module.millModuleSegments ++ Seq(Label("compile"))
+                module.millModuleSegments ++ Label("compile")
               )
               val outPathSeq = Seq(outPaths.dest, outPaths.meta, outPaths.log)
 

contrib/scoverage/src/mill/contrib/scoverage/ScoverageReport.scala

@@ -3,7 +3,7 @@ package mill.contrib.scoverage
 import mill.contrib.scoverage.api.ScoverageReportWorkerApi.ReportType
 import mill.define.{Command, Module, Task}
 import mill.eval.Evaluator
-import mill.main.RunScript
+import mill.main.{ResolveTasks, RunScript}
 import mill.define.SelectMode
 import mill.{PathRef, T}
 import os.Path
@@ -81,17 +81,15 @@ trait ScoverageReport extends Module {
       sources: String,
       dataTargets: String
   ): Task[PathRef] = {
-    val sourcesTasks: Seq[Task[Seq[PathRef]]] = RunScript.resolveTasks(
-      mill.main.ResolveTasks,
+    val sourcesTasks: Seq[Task[Seq[PathRef]]] = ResolveTasks.resolve(
       evaluator,
       Seq(sources),
       SelectMode.Single
     ) match {
       case Left(err) => throw new Exception(err)
       case Right(tasks) => tasks.asInstanceOf[Seq[Task[Seq[PathRef]]]]
     }
-    val dataTasks: Seq[Task[PathRef]] = RunScript.resolveTasks(
-      mill.main.ResolveTasks,
+    val dataTasks: Seq[Task[PathRef]] = ResolveTasks.resolve(
       evaluator,
       Seq(dataTargets),
       SelectMode.Single

integration/failure/module-init-error/repo/build.sc

@@ -0,0 +1,21 @@
+import mill._, scalalib._
+
+def rootTarget = T{ println("Running rootTarget"); "rootTarget" }
+def rootCommand(s: String) = T.command{ println(s"Running rootCommand $s") }
+
+object foo extends Module{
+  def fooTarget = T{ println(s"Running fooTarget"); 123 }
+  def fooCommand(s: String) = T.command{ println(s"Running fooCommand $s") }
+  throw new Exception("Foo Boom")
+}
+
+object bar extends Module {
+  def barTarget = T { println(s"Running barTarget"); "abc" }
+  def barCommand(s: String) = T.command{ println(s"Running barCommand $s") }
+
+  object qux extends Module{
+    def quxTarget = T { println(s"Running quxTarget"); "xyz" }
+    def quxCommand(s: String) = T.command{ println(s"Running quxCommand $s") }
+    throw new Exception("Qux Boom")
+  }
+}

integration/failure/module-init-error/test/src/ModuleInitErrorTests.scala

@@ -0,0 +1,62 @@
+package mill.integration
+
+import mill.util.Util
+import utest._
+
+object ModuleInitErrorTests extends IntegrationTestSuite {
+  def captureOutErr = true
+  val tests = Tests {
+    initWorkspace()
+
+    test("rootTarget") {
+      // If we specify a target in the root module, we are not
+      // affected by the sub-modules failing to initialize
+      val res = evalStdout("rootTarget")
+      assert(res.isSuccess == true)
+      assert(res.out.contains("""Running rootTarget"""))
+    }
+    test("rootCommand") {
+      // If we specify a target in the root module, we are not
+      // affected by the sub-modules failing to initialize
+      val res = evalStdout("rootCommand", "hello")
+      assert(res.isSuccess == true)
+      assert(res.out.contains("""Running rootCommand hello"""))
+    }
+    test("fooTarget") {
+      val res = evalStdout("foo.fooTarget")
+      assert(res.isSuccess == false)
+      assert(fansi.Str(res.err).plainText.contains("""java.lang.Exception: Foo Boom"""))
+      // Make sure the stack trace is "short" and does not contain all the stack
+      // frames from the Mill launcher
+      assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
+    }
+    test("fooCommand") {
+      val res = evalStdout("foo.fooCommand", "hello")
+      assert(res.isSuccess == false)
+      assert(fansi.Str(res.err).plainText.contains("""java.lang.Exception: Foo Boom"""))
+      assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
+    }
+    test("barTarget") {
+      val res = evalStdout("bar.barTarget")
+      assert(res.isSuccess == true)
+      assert(res.out.contains("""Running barTarget"""))
+    }
+    test("barCommand") {
+      val res = evalStdout("bar.barCommand", "hello")
+      assert(res.isSuccess == true)
+      assert(res.out.contains("""Running barCommand hello"""))
+    }
+    test("quxTarget") {
+      val res = evalStdout("bar.qux.quxTarget")
+      assert(res.isSuccess == false)
+      assert(fansi.Str(res.err).plainText.contains("""java.lang.Exception: Qux Boom"""))
+      assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
+    }
+    test("quxCommand") {
+      val res = evalStdout("bar.qux.quxCommand", "hello")
+      assert(res.isSuccess == false)
+      assert(fansi.Str(res.err).plainText.contains("""java.lang.Exception: Qux Boom"""))
+      assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
+    }
+  }
+}

main/core/src/mill/define/BaseModule.scala

@@ -56,6 +56,6 @@ abstract class ExternalModule(implicit
     "External modules must be at a top-level static path, not " + millModuleEnclosing0.value
   )
   override implicit def millModuleSegments = {
-    Segments(millModuleEnclosing0.value.split('.').map(Segment.Label).toIndexedSeq: _*)
+    Segments(millModuleEnclosing0.value.split('.').map(Segment.Label).toIndexedSeq)
   }
 }

main/core/src/mill/define/Cross.scala

@@ -1,6 +1,7 @@
 package mill.define
 
 import language.experimental.macros
+import scala.collection.SeqView
 import scala.reflect.macros.blackbox
 
 object Cross {
@@ -269,18 +270,27 @@ object Cross {
 class Cross[M <: Cross.Module[_]](factories: Cross.Factory[M]*)(implicit ctx: mill.define.Ctx)
     extends mill.define.Module()(ctx) {
 
-  private val items: List[(List[Any], List[String], M)] = for {
+  // We lazily initialize the instances of `Cross.Module` only when they are
+  // requested, to avoid unexpected failures in one module initialization
+  // causing problems using others.
+  private class Lazy[T](t: () => T) {
+    lazy val value = t()
+  }
+
+  private val items: List[(List[Any], List[String], Lazy[M])] = for {
     factory <- factories.toList
     (crossSegments, (crossValues, make)) <-
       factory.crossSegmentsList.zip(factory.crossValuesListLists.zip(factory.makeList))
   } yield {
     val relPath = ctx.segment.pathSegments
-    val sub = make(
-      ctx
-        .withSegments(ctx.segments ++ Seq(ctx.segment))
-        .withMillSourcePath(ctx.millSourcePath / relPath)
-        .withSegment(Segment.Cross(crossSegments))
-        .withCrossValues(factories.flatMap(_.crossValuesRaw))
+    val sub = new Lazy(() =>
+      make(
+        ctx
+          .withSegments(ctx.segments ++ Seq(ctx.segment))
+          .withMillSourcePath(ctx.millSourcePath / relPath)
+          .withSegment(Segment.Cross(crossSegments))
+          .withCrossValues(factories.flatMap(_.crossValuesRaw))
+      )
     )
 
     (crossValues.toList, crossSegments.toList, sub)
@@ -293,25 +303,27 @@ class Cross[M <: Cross.Module[_]](factories: Cross.Factory[M]*)(implicit ctx: mi
    * A list of the cross modules, in
    * the order the original cross values were given in
    */
-  def crossModules: List[M] = items.collect { case (_, _, v) => v }
+  lazy val crossModules: Seq[M] = items.map { case (_, _, v) => v.value }
 
   /**
    * A mapping of the raw cross values to the cross modules, in
    * the order the original cross values were given in
    */
-  val valuesToModules: collection.Map[List[Any], M] =
-    items.map { case (values, segments, subs) => (values, subs) }.to(
-      collection.mutable.LinkedHashMap
-    )
+  val valuesToModules: collection.MapView[List[Any], M] = items
+    .map { case (values, segments, subs) => (values, subs) }
+    .to(collection.mutable.LinkedHashMap)
+    .view
+    .mapValues(_.value)
 
   /**
    * A mapping of the string-ified string segments to the cross modules, in
    * the order the original cross values were given in
    */
-  val segmentsToModules: collection.Map[List[String], M] =
-    items.map { case (values, segments, subs) => (segments, subs) }.to(
-      collection.mutable.LinkedHashMap
-    )
+  val segmentsToModules: collection.MapView[List[String], M] = items
+    .map { case (values, segments, subs) => (segments, subs) }
+    .to(collection.mutable.LinkedHashMap)
+    .view
+    .mapValues(_.value)
 
   /**
    * Fetch the cross module corresponding to the given cross values
@@ -321,9 +333,7 @@ class Cross[M <: Cross.Module[_]](factories: Cross.Factory[M]*)(implicit ctx: mi
   /**
    * Fetch the cross module corresponding to the given cross values
    */
-  def apply(arg0: Any, args: Any*): M = {
-    valuesToModules(arg0 :: args.toList)
-  }
+  def apply(arg0: Any, args: Any*): M = valuesToModules(arg0 :: args.toList)
 
   /**
    * Fetch the relevant cross module given the implicit resolver you have in

main/core/src/mill/define/ExpandBraces.scala

@@ -0,0 +1,57 @@
+package mill.define
+import fastparse._
+import fastparse.NoWhitespace.noWhitespaceImplicit
+
+object ExpandBraces {
+  private sealed trait Fragment
+  private object Fragment {
+    case class Keep(value: String) extends Fragment
+    case class Expand(values: List[List[Fragment]]) extends Fragment
+  }
+
+  def expandBraces(selectorString: String): Either[String, Seq[String]] = {
+    parse(selectorString, parser(_)) match {
+      case f: Parsed.Failure => Left(s"Parsing exception ${f.msg}")
+      case Parsed.Success(fragmentLists, _) =>
+        def processFragmentSequence(remaining: List[Fragment]): List[List[String]] =
+          remaining match {
+            case Nil => List(List())
+            case head :: tail =>
+              val tailStrings = processFragmentSequence(tail)
+              head match {
+                case Fragment.Keep(s) => tailStrings.map(s :: _)
+                case Fragment.Expand(fragmentLists) =>
+                  if (fragmentLists.length == 1) {
+                    for {
+                      lhs <- fragmentLists.flatMap(processFragmentSequence)
+                      rhs <- tailStrings
+                    } yield List("{") ::: lhs ::: List("}") ::: rhs
+                  } else for {
+                    lhs <- fragmentLists.flatMap(processFragmentSequence)
+                    rhs <- tailStrings
+                  } yield lhs ::: rhs
+              }
+          }
+
+        val res = processFragmentSequence(fragmentLists.toList).map(_.mkString)
+
+        Right(res)
+    }
+  }
+
+  private def plainChars[_p: P]: P[Fragment.Keep] =
+    P(CharsWhile(c => c != ',' && c != '{' && c != '}')).!.map(Fragment.Keep)
+
+  private def emptyExpansionBranch[_p: P] = P("").map(_ => List(Fragment.Keep("")))
+
+  private def toExpand[_p: P]: P[Fragment] =
+    P("{" ~ (braceParser.rep(1) | emptyExpansionBranch).rep(sep = ",") ~ "}").map(x =>
+      Fragment.Expand(x.toList.map(_.toList))
+    )
+
+  private def braceParser[_p: P]: P[Fragment] = P(toExpand | plainChars)
+
+  private def topLevelComma[_p: P] = P(",".!).map(Fragment.Keep(_))
+
+  private def parser[_p: P]: P[Seq[Fragment]] = P((braceParser | topLevelComma).rep(1) ~ End)
+}