improving twirl (#473)

* improving twirl support: default imports, better `compileTwirl().classes` value

* twirl module doc edits

4 files changed, 93 insertions, 25 deletions

diff --git a/contrib/twirllib/src/mill/twirllib/TwirlModule.scala b/contrib/twirllib/src/mill/twirllib/TwirlModule.scala
index 2df70a1f..8f735f3a 100644
--- a/contrib/twirllib/src/mill/twirllib/TwirlModule.scala
+++ b/contrib/twirllib/src/mill/twirllib/TwirlModule.scala
@@ -33,8 +33,7 @@ trait TwirlModule extends mill.Module {
     )
   }
 
-  // REMIND currently it's not possible to override these default settings
-  private def twirlAdditionalImports: Seq[String] = Nil
+  def twirlAdditionalImports: Seq[String] = Nil
 
   private def twirlConstructorAnnotations: Seq[String] = Nil
 
diff --git a/contrib/twirllib/src/mill/twirllib/TwirlWorker.scala b/contrib/twirllib/src/mill/twirllib/TwirlWorker.scala
index b73ed5dc..a525a640 100644
--- a/contrib/twirllib/src/mill/twirllib/TwirlWorker.scala
+++ b/contrib/twirllib/src/mill/twirllib/TwirlWorker.scala
@@ -21,21 +21,43 @@ class TwirlWorker {
       case Some((sig, instance)) if sig == classloaderSig => instance
       case _ =>
         val cl = new URLClassLoader(twirlClasspath.map(_.toIO.toURI.toURL).toArray, null)
-        val twirlCompilerClass = cl.loadClass("play.twirl.compiler.TwirlCompiler")
+
+        // Switched to using the java api because of the hack-ish thing going on later.
+        //
+        // * we'll need to construct a collection of additional imports
+        // * it will need to consider the defaults
+        // * and add the user-provided additional imports
+        // * the default collection in scala api is a Seq[String]
+        // * but it is defined in a different classloader (namely in cl)
+        // * so we can not construct our own Seq and pass it to the method - it will be from our classloader, and not compatible
+        // * the java api has a Collection as the type for this param, for which it is much more doable to append things to it using reflection
+        //
+        // NOTE: I tried creating the cl classloader passing the current classloader as the parent:
+        //   val cl = new URLClassLoader(twirlClasspath.map(_.toIO.toURI.toURL).toArray, getClass.getClassLoader)
+        // in that case it was possible to cast the default to a Seq[String], construct our own Seq[String], and pass it to the method invoke- it was compatible.
+        // And the tests passed. But when run in a different mill project, I was getting exceptions like this:
+        // scala.reflect.internal.MissingRequirementError: object scala in compiler mirror not found.
+
+        val twirlCompilerClass = cl.loadClass("play.japi.twirl.compiler.TwirlCompiler")
+
+        // this one is only to get the codec: Codec parameter default value
+        val twirlScalaCompilerClass = cl.loadClass("play.twirl.compiler.TwirlCompiler")
+
         val compileMethod = twirlCompilerClass.getMethod("compile",
           classOf[java.io.File],
           classOf[java.io.File],
           classOf[java.io.File],
           classOf[java.lang.String],
-          cl.loadClass("scala.collection.Seq"),
-          cl.loadClass("scala.collection.Seq"),
+          cl.loadClass("java.util.Collection"),
+          cl.loadClass("java.util.List"),
           cl.loadClass("scala.io.Codec"),
           classOf[Boolean])
 
-        val defaultAdditionalImportsMethod = twirlCompilerClass.getMethod("compile$default$5")
-        val defaultConstructorAnnotationsMethod = twirlCompilerClass.getMethod("compile$default$6")
-        val defaultCodecMethod = twirlCompilerClass.getMethod("compile$default$7")
-        val defaultFlagMethod = twirlCompilerClass.getMethod("compile$default$8")
+        val arrayListClass = cl.loadClass("java.util.ArrayList")
+        val hashSetClass = cl.loadClass("java.util.HashSet")
+
+        val defaultAdditionalImportsMethod = twirlCompilerClass.getField("DEFAULT_IMPORTS")
+        val defaultCodecMethod = twirlScalaCompilerClass.getMethod("compile$default$7")
 
         val instance = new TwirlWorkerApi {
           override def compileTwirl(source: File,
@@ -46,14 +68,28 @@ class TwirlWorker {
                                     constructorAnnotations: Seq[String],
                                     codec: Codec,
                                     inclusiveDot: Boolean) {
+            val defaultAdditionalImports = defaultAdditionalImportsMethod.get(null) // unmodifiable collection
+            // copying it into a modifiable hash set and adding all additional imports
+            val allAdditionalImports =
+              hashSetClass
+                .getConstructor(cl.loadClass("java.util.Collection"))
+                .newInstance(defaultAdditionalImports)
+                .asInstanceOf[Object]
+            val hashSetAddMethod =
+              allAdditionalImports
+                .getClass
+                .getMethod("add", classOf[Object])
+            additionalImports.foreach(hashSetAddMethod.invoke(allAdditionalImports, _))
+
             val o = compileMethod.invoke(null, source,
               sourceDirectory,
               generatedDirectory,
               formatterType,
-              defaultAdditionalImportsMethod.invoke(null),
-              defaultConstructorAnnotationsMethod.invoke(null),
+              allAdditionalImports,
+              arrayListClass.newInstance().asInstanceOf[Object], // empty list seems to be the default
               defaultCodecMethod.invoke(null),
-              defaultFlagMethod.invoke(null))
+              Boolean.box(false)
+            )
           }
         }
         twirlInstanceCache = Some((classloaderSig, instance))
@@ -90,7 +126,7 @@ class TwirlWorker {
     sourceDirectories.foreach(compileTwirlDir)
 
     val zincFile = ctx.dest / 'zinc
-    val classesDir = ctx.dest / 'html
+    val classesDir = ctx.dest
 
     mill.eval.Result.Success(CompilationResult(zincFile, PathRef(classesDir)))
   }
diff --git a/contrib/twirllib/test/resources/hello-world/core/views/wrapper.scala.html b/contrib/twirllib/test/resources/hello-world/core/views/wrapper.scala.html
new file mode 100644
index 00000000..af1f5d8e
--- /dev/null
+++ b/contrib/twirllib/test/resources/hello-world/core/views/wrapper.scala.html
@@ -0,0 +1,5 @@
+@(content: Html)
+
+@defining("test") { className =>
+    <div class="@className">@content</div>
+}
\ No newline at end of file
diff --git a/contrib/twirllib/test/src/mill/twirllib/HelloWorldTests.scala b/contrib/twirllib/test/src/mill/twirllib/HelloWorldTests.scala
index 8ef6ee3e..804fd5a1 100644
--- a/contrib/twirllib/test/src/mill/twirllib/HelloWorldTests.scala
+++ b/contrib/twirllib/test/src/mill/twirllib/HelloWorldTests.scala
@@ -5,14 +5,18 @@ import mill.util.{TestEvaluator, TestUtil}
 import utest.framework.TestPath
 import utest.{TestSuite, Tests, assert, _}
 
+import scala.io.Codec
+
 object HelloWorldTests extends TestSuite {
 
   trait HelloBase extends TestUtil.BaseModule {
-    override def millSourcePath: Path = TestUtil.getSrcPathBase() / millOuterCtx.enclosing.split('.')
+    override def millSourcePath: Path =
+      TestUtil.getSrcPathBase() / millOuterCtx.enclosing.split('.')
   }
 
   trait HelloWorldModule extends mill.twirllib.TwirlModule {
     def twirlVersion = "1.0.0"
+    override def twirlAdditionalImports: Seq[String] = additionalImports
   }
 
   object HelloWorld extends HelloBase {
@@ -22,11 +26,13 @@ object HelloWorldTests extends TestSuite {
     }
   }
 
-  val resourcePath: Path = pwd / 'contrib / 'twirllib / 'test / 'resources / "hello-world"
+  val resourcePath
+    : Path = pwd / 'contrib / 'twirllib / 'test / 'resources / "hello-world"
 
-  def workspaceTest[T](m: TestUtil.BaseModule, resourcePath: Path = resourcePath)
-                      (t: TestEvaluator => T)
-                      (implicit tp: TestPath): T = {
+  def workspaceTest[T](
+      m: TestUtil.BaseModule,
+      resourcePath: Path = resourcePath
+  )(t: TestEvaluator => T)(implicit tp: TestPath): T = {
     val eval = new TestEvaluator(m)
     rm(m.millSourcePath)
     rm(eval.outPath)
@@ -36,14 +42,30 @@ object HelloWorldTests extends TestSuite {
   }
 
   def compileClassfiles: Seq[RelPath] = Seq[RelPath](
-    "hello.template.scala"
+    "hello.template.scala",
+    "wrapper.template.scala"
+  )
+
+  def expectedDefaultImports: Seq[String] = Seq(
+    "import _root_.play.twirl.api.TwirlFeatureImports._",
+    "import _root_.play.twirl.api.TwirlHelperImports._",
+    "import _root_.play.twirl.api.Html",
+    "import _root_.play.twirl.api.JavaScript",
+    "import _root_.play.twirl.api.Txt",
+    "import _root_.play.twirl.api.Xml"
+  )
+
+  def additionalImports: Seq[String] = Seq(
+    "mill.twirl.test.AdditionalImport1._",
+    "mill.twirl.test.AdditionalImport2._"
   )
 
   def tests: Tests = Tests {
     'twirlVersion - {
 
       'fromBuild - workspaceTest(HelloWorld) { eval =>
-        val Right((result, evalCount)) = eval.apply(HelloWorld.core.twirlVersion)
+        val Right((result, evalCount)) =
+          eval.apply(HelloWorld.core.twirlVersion)
 
         assert(
           result == "1.3.15",
@@ -54,20 +76,26 @@ object HelloWorldTests extends TestSuite {
     'compileTwirl - workspaceTest(HelloWorld) { eval =>
       val Right((result, evalCount)) = eval.apply(HelloWorld.core.compileTwirl)
 
-      val outputFiles = ls.rec(result.classes.path)
+      val outputFiles = ls.rec(result.classes.path).filter(_.name.endsWith(".scala"))
       val expectedClassfiles = compileClassfiles.map(
         eval.outPath / 'core / 'compileTwirl / 'dest / 'html / _
       )
+
       assert(
-        result.classes.path == eval.outPath / 'core / 'compileTwirl / 'dest / 'html,
+        result.classes.path == eval.outPath / 'core / 'compileTwirl / 'dest,
         outputFiles.nonEmpty,
         outputFiles.forall(expectedClassfiles.contains),
-        outputFiles.size == 1,
-        evalCount > 0
+        outputFiles.size == 2,
+        evalCount > 0,
+        outputFiles.forall { p =>
+          val lines = p.getLines(Codec.UTF8).map(_.trim)
+          (expectedDefaultImports ++ additionalImports.map(s => s"import $s")).forall(lines.contains)
+        }
       )
 
       // don't recompile if nothing changed
-      val Right((_, unchangedEvalCount)) = eval.apply(HelloWorld.core.compileTwirl)
+      val Right((_, unchangedEvalCount)) =
+        eval.apply(HelloWorld.core.compileTwirl)
 
       assert(unchangedEvalCount == 0)
     }