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

2 files changed, 21 insertions, 16 deletions

diff --git a/core/src/mill/define/Module.scala b/core/src/mill/define/Module.scala
index 3f91b524..f72ec8ca 100644
--- a/core/src/mill/define/Module.scala
+++ b/core/src/mill/define/Module.scala
@@ -3,9 +3,12 @@ package mill.define
 import java.lang.reflect.Modifier
 
 import ammonite.ops.Path
+import mill.util.ParseArgs
 
 import scala.language.experimental.macros
 import scala.reflect.ClassTag
+import scala.reflect.NameTransformer.decode
+
 
 /**
   * `Module` is a class meant to be extended by `trait`s *only*, in order to
@@ -44,7 +47,7 @@ object Module{
     lazy val modules = traverse(Seq(_))
     lazy val segmentsToModules = modules.map(m => (m.millModuleSegments, m)).toMap
 
-    lazy val targets = traverse{_.millInternal.reflect[Target[_]]}.toSet
+    lazy val targets = traverse{_.millInternal.reflectAll[Target[_]]}.toSet
 
     lazy val segmentsToTargets = targets
       .map(t => (t.ctx.segments, t))
@@ -56,32 +59,30 @@ object Module{
     lazy val millModuleEnclosing = outer.millOuterCtx.enclosing
     lazy val millModuleLine = outer.millOuterCtx.lineNum
 
-    def reflect[T: ClassTag] = {
+    private def reflect[T: ClassTag](filter: (String) => Boolean): Array[T] = {
       val runtimeCls = implicitly[ClassTag[T]].runtimeClass
       for{
-        m <- outer.getClass.getMethods
+        m <- outer.getClass.getMethods.sortBy(_.getName)
+        n = decode(m.getName)
         if
-          !m.getName.contains('$') &&
+          filter(n) &&
+          ParseArgs.isLegalIdentifier(n) &&
           m.getParameterCount == 0 &&
           (m.getModifiers & Modifier.STATIC) == 0 &&
           (m.getModifiers & Modifier.ABSTRACT) == 0 &&
           runtimeCls.isAssignableFrom(m.getReturnType)
       } yield m.invoke(outer).asInstanceOf[T]
     }
-    def reflectNames[T: ClassTag] = {
-      val runtimeCls = implicitly[ClassTag[T]].runtimeClass
-      for{
-        m <- outer.getClass.getMethods
-        if
-          (m.getModifiers & Modifier.STATIC) == 0 &&
-          runtimeCls.isAssignableFrom(m.getReturnType)
-      } yield m.getName
-    }
+
+    def reflectAll[T: ClassTag]: Array[T] = reflect(Function.const(true))
+
+    def reflectSingle[T: ClassTag](label: String): Option[T] = reflect(_ == label).headOption
+
     // For some reason, this fails to pick up concrete `object`s nested directly within
     // another top-level concrete `object`. This is fine for now, since Mill's Ammonite
     // script/REPL runner always wraps user code in a wrapper object/trait
     def reflectNestedObjects[T: ClassTag] = {
-      (reflect[T] ++
+      (reflectAll[T] ++
         outer
           .getClass
           .getClasses
diff --git a/core/src/mill/util/ParseArgs.scala b/core/src/mill/util/ParseArgs.scala
index 274f6449..ae3b1685 100644
--- a/core/src/mill/util/ParseArgs.scala
+++ b/core/src/mill/util/ParseArgs.scala
@@ -112,9 +112,13 @@ object ParseArgs {
       case Parsed.Success(selector, _) => Right(selector)
     }
 
+  private val identChars = ('a' to 'z') ++ ('A' to 'Z') ++ ('0' to '9') ++ Seq('_', '-')
+  private val ident = P( CharsWhileIn(identChars) ).!
+
+  def isLegalIdentifier(identifier: String): Boolean =
+    (Start ~ ident ~ End).parse(identifier).isInstanceOf[Parsed.Success[_]]
+
   private def parseSelector(input: String) = {
-    val identChars = ('a' to 'z') ++ ('A' to 'Z') ++ ('0' to '9') ++ Seq('_', '-')
-    val ident = P( CharsWhileIn(identChars) ).!
     val ident2 = P( CharsWhileIn(identChars ++ ".") ).!
     val segment = P( ident ).map( Segment.Label)
     val crossSegment = P("[" ~ ident2.rep(1, sep = ",") ~ "]").map(Segment.Cross)