Allow hyphens in module and task names (#362)

* Allow bacticked tasks

* Prevent stack overflow

* Test for illegal bacticked identifiers

* Filter out illegal backticked identifiers

The only legal identifiers are aplanumeric, unserscore (_), and
hyphens (-).

* Remove unused method that is invalid

* Document valid characters for module/task names

7 files changed, 89 insertions, 0 deletions

diff --git a/main/test/src/mill/define/BasePathTests.scala b/main/test/src/mill/define/BasePathTests.scala
index 1f5b4037..d5167081 100644
--- a/main/test/src/mill/define/BasePathTests.scala
+++ b/main/test/src/mill/define/BasePathTests.scala
@@ -14,6 +14,12 @@ object BasePathTests extends TestSuite{
     'singleton - {
       check(testGraphs.singleton)(identity)
     }
+    'backtickIdentifiers - {
+      check(testGraphs.bactickIdentifiers)(
+        _.`nested-module`,
+        "nested-module"
+      )
+    }
     'separateGroups - {
       check(TestGraphs.triangleTask)(identity)
     }
diff --git a/main/test/src/mill/define/DiscoverTests.scala b/main/test/src/mill/define/DiscoverTests.scala
index cd9939f0..248d6afe 100644
--- a/main/test/src/mill/define/DiscoverTests.scala
+++ b/main/test/src/mill/define/DiscoverTests.scala
@@ -14,6 +14,9 @@ object DiscoverTests extends TestSuite{
     'singleton - {
       check(testGraphs.singleton)(_.single)
     }
+    'backtickIdentifiers {
+      check(testGraphs.bactickIdentifiers)(_.`up-target`, _.`a-down-target`, _.`nested-module`.`nested-target`)
+    }
     'separateGroups - {
       check(TestGraphs.triangleTask)(_.left, _.right)
     }
diff --git a/main/test/src/mill/define/GraphTests.scala b/main/test/src/mill/define/GraphTests.scala
index 7e6680be..224ce59f 100644
--- a/main/test/src/mill/define/GraphTests.scala
+++ b/main/test/src/mill/define/GraphTests.scala
@@ -25,6 +25,10 @@ object GraphTests extends TestSuite{
         targets = Agg(singleton.single),
         expected = Agg(singleton.single)
       )
+      'backtickIdentifiers - check(
+        targets = Agg(bactickIdentifiers.`a-down-target`),
+        expected = Agg(bactickIdentifiers.`up-target`, bactickIdentifiers.`a-down-target`)
+      )
       'pair - check(
         targets = Agg(pair.down),
         expected = Agg(pair.up, pair.down)
@@ -95,6 +99,14 @@ object GraphTests extends TestSuite{
         Agg(_.single),
         Agg(singleton.single -> 1)
       )
+      'backtickIdentifiers - check(bactickIdentifiers)(
+        _.`a-down-target`,
+        Agg(_.`up-target`, _.`a-down-target`),
+        Agg(
+          bactickIdentifiers.`up-target` -> 1,
+          bactickIdentifiers.`a-down-target` -> 1
+        )
+      )
       'pair - check(pair)(
         _.down,
         Agg(_.up, _.down),
diff --git a/main/test/src/mill/eval/EvaluationTests.scala b/main/test/src/mill/eval/EvaluationTests.scala
index 9c215086..75a5bbe3 100644
--- a/main/test/src/mill/eval/EvaluationTests.scala
+++ b/main/test/src/mill/eval/EvaluationTests.scala
@@ -67,6 +67,18 @@ object EvaluationTests extends TestSuite{
         // After incrementing the counter, it forces re-evaluation
         check(single, expValue = 1, expEvaled = Agg(single))
       }
+      'backtickIdentifiers - {
+        import graphs.bactickIdentifiers._
+        val check = new Checker(bactickIdentifiers)
+
+        check(`a-down-target`, expValue = 0, expEvaled = Agg(`up-target`, `a-down-target`))
+
+        `a-down-target`.counter += 1
+        check(`a-down-target`, expValue = 1, expEvaled = Agg(`a-down-target`))
+
+        `up-target`.counter += 1
+        check(`a-down-target`, expValue = 2, expEvaled = Agg(`up-target`, `a-down-target`))
+      }
       'pair - {
         import pair._
         val check = new Checker(pair)
diff --git a/main/test/src/mill/eval/FailureTests.scala b/main/test/src/mill/eval/FailureTests.scala
index 244b084d..22021079 100644
--- a/main/test/src/mill/eval/FailureTests.scala
+++ b/main/test/src/mill/eval/FailureTests.scala
@@ -80,6 +80,39 @@ object FailureTests extends TestSuite{
         expectedRawValues = Seq(Result.Skipped)
       )
     }
+    'evaluateBacktickIdentifiers - {
+      val check = new TestEvaluator(bactickIdentifiers)
+      import bactickIdentifiers._
+      check.fail(
+        `a-down-target`,
+        expectedFailCount = 0,
+        expectedRawValues = Seq(Result.Success(0))
+      )
+
+      `up-target`.failure = Some("lols")
+
+      check.fail(
+        `a-down-target`,
+        expectedFailCount = 1,
+        expectedRawValues = Seq(Result.Skipped)
+      )
+
+      `up-target`.failure = None
+
+      check.fail(
+        `a-down-target`,
+        expectedFailCount = 0,
+        expectedRawValues = Seq(Result.Success(0))
+      )
+
+      `up-target`.exception = Some(new IndexOutOfBoundsException())
+
+      check.fail(
+        `a-down-target`,
+        expectedFailCount = 1,
+        expectedRawValues = Seq(Result.Skipped)
+      )
+    }
     'multipleUsesOfDest - {
       object build extends TestUtil.BaseModule {
         // Using `T.ctx(  ).dest` twice in a single task is ok
diff --git a/main/test/src/mill/main/MainTests.scala b/main/test/src/mill/main/MainTests.scala
index f9bf7aec..e836099c 100644
--- a/main/test/src/mill/main/MainTests.scala
+++ b/main/test/src/mill/main/MainTests.scala
@@ -34,6 +34,20 @@ object MainTests extends TestSuite{
       'neg6 - check("single.doesntExist", Left("Task single is not a module and has no children."))
       'neg7 - check("", Left("Selector cannot be empty"))
     }
+    'backtickIdentifiers - {
+      val check = MainTests.check(bactickIdentifiers) _
+      'pos1 - check("up-target", Right(Seq(_.`up-target`)))
+      'pos2 - check("a-down-target", Right(Seq(_.`a-down-target`)))
+      'neg1 - check("uptarget", Left("Cannot resolve uptarget. Did you mean up-target?"))
+      'neg2 - check("upt-arget", Left("Cannot resolve upt-arget. Did you mean up-target?"))
+      'neg3 - check("up-target.doesntExist", Left("Task up-target is not a module and has no children."))
+      'neg4 - check("", Left("Selector cannot be empty"))
+      'neg5 - check("invisible&", Left("Cannot resolve invisible. Try `mill resolve _` to see what's available."))
+      'nested - {
+        'pos - check("nested-module.nested-target", Right(Seq(_.`nested-module`.`nested-target`)))
+        'neg - check("nested-module.doesntExist", Left("Cannot resolve nested-module.doesntExist. Try `mill resolve nested-module._` to see what's available."))
+      }
+    }
     'nested - {
       val check = MainTests.check(nestedModule) _
       'pos1 - check("single", Right(Seq(_.single)))
diff --git a/main/test/src/mill/util/TestGraphs.scala b/main/test/src/mill/util/TestGraphs.scala
index 83e03576..d3b35ddc 100644
--- a/main/test/src/mill/util/TestGraphs.scala
+++ b/main/test/src/mill/util/TestGraphs.scala
@@ -20,6 +20,15 @@ class TestGraphs(){
     val single = test()
   }
 
+  object bactickIdentifiers extends TestUtil.BaseModule {
+    val `up-target` = test()
+    val `a-down-target` = test(`up-target`)
+    val `invisible&` = test()
+    object `nested-module` extends TestUtil.BaseModule {
+      val `nested-target` = test()
+    }
+  }
+
   // up---down
   object pair extends TestUtil.BaseModule{
     val up = test()