From ff45d24103684342a47ec2ed42565356a116f18d Mon Sep 17 00:00:00 2001 From: Iurii Malchenko Date: Fri, 2 Nov 2018 09:30:52 +0200 Subject: improving twirl (#473) * improving twirl support: default imports, better `compileTwirl().classes` value * twirl module doc edits --- .../twirllib/src/mill/twirllib/TwirlModule.scala | 3 +- .../twirllib/src/mill/twirllib/TwirlWorker.scala | 58 +++++++++-- .../hello-world/core/views/wrapper.scala.html | 5 + .../test/src/mill/twirllib/HelloWorldTests.scala | 52 +++++++--- docs/pages/9 - Contrib Modules.md | 115 ++++----------------- 5 files changed, 115 insertions(+), 118 deletions(-) create mode 100644 contrib/twirllib/test/resources/hello-world/core/views/wrapper.scala.html 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 => +
@content
+} \ 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) } diff --git a/docs/pages/9 - Contrib Modules.md b/docs/pages/9 - Contrib Modules.md index cd02f2c2..4d811cde 100644 --- a/docs/pages/9 - Contrib Modules.md +++ b/docs/pages/9 - Contrib Modules.md @@ -161,14 +161,16 @@ Also note that twirl templates get compiled into scala code, so you also need to ```scala import $ivy.`com.lihaoyi::mill-contrib-twirllib:0.3.2`, mill.twirllib._ object app extends ScalaModule with TwirlModule { - +// ... } ``` #### Configuration options -* ` def twirlVersion: T[String]` (mandatory) - the version of the twirl compiler to use, like "1.3.15" - +* `def twirlVersion: T[String]` (mandatory) - the version of the twirl compiler to use, like "1.3.15" +* `def twirlAdditionalImports: Seq[String] = Nil` - the additional imports that will be added by twirl compiler to the top + of all templates + #### Details The following filesystem layout is expected: @@ -197,107 +199,34 @@ object app extends ScalaModule with TwirlModule { } ``` -#### Caveats - -There is a couple of caveats, to be aware of, as of now (in `v0.3.2`). - -##### Packages -First, if you structure your twirl templates into packages, like this: -```text -build.sc -app/ - src/hello/ - Main.scala - views/ - hello/ - another/ - view1.scala.html - view2.scala.html -``` - -the generated sources in the `out` directory will look like this: -```text -build.sc -out/app/compileTwirl/dest/ - hello/ - another/ - html/ - view1.template.scala - view2.template.scala -``` - -Looking at the `mill show app.compileTwirl` in this setup shows this: -``` -{ - ... - "classes": "ref: ... : .../out/app/compileTwirl/dest/html" -} -``` - -Basically it means that currently `TwirlModule` expects all templates to be html and with no packages. -So adding this directly to the generated sources will not exactly work as expected (as there might not even be a `out/app/compileTwirl/dest/html` directory -at all, unless you have templates in the default package). - -The workaround is simple, though: +To add additional imports to all of the twirl templates: ```scala object app extends ScalaModule with TwirlModule { def twirlVersion = "1.3.15" - override def generatedSources = T{ - val classes = compileTwirl().classes - Seq(classes.copy(path = classes.path / up)) // we just move one dir up - } + override def twirlAdditionalImports = Seq("my.additional.stuff._", "my.other.stuff._") + def generatedSources = T{ Seq(compileTwirl().classes) } } ``` -This should cover the problem with templates under packages, and also should make other-than-html -templates available as well. - -##### Default imports - -Another problem is with some default imports that the twirl sbt plugin assumes, but it seems not to work with `TwirlModule`. - -If you reference `Html` in your templates, like - +as the result all templates will get this line at the top: ```scala -// wrapper.scala.html -@(content: Html) -
- @content -
+@import "my.additional.stuff._" +@import "my.other.stuff._" ``` -the template will not compile. You'll need to add this import: -``` -@import play.twirl.api._ -``` - -in the template that uses twirl classes. - -Another one is `@defining`, which might be used like this: -``` -@defining({ - val calculatedClass = { - // do some calculations here - } - calculatedClass -}) { calculatedClass => -
stuff 1
-
stuff 2
-} -``` - -You'll need this import: +Besides that, twirl compiler has default imports, at the moment these: ```scala -@import play.twirl.api.TwirlFeatureImports._ -``` - -At some point `TwirlModule` might get support for the additional "default" imports, which will make this much easier, -but right now it is unimplemented +Seq( + "_root_.play.twirl.api.TwirlFeatureImports._", + "_root_.play.twirl.api.TwirlHelperImports._", + "_root_.play.twirl.api.Html", + "_root_.play.twirl.api.JavaScript", + "_root_.play.twirl.api.Txt", + "_root_.play.twirl.api.Xml" +) +``` -```scala - // REMIND currently it's not possible to override these default settings - private def twirlAdditionalImports: Seq[String] = Nil -``` +These imports will always be added to every template. You don't need to list them if you override `twirlAdditionalImports`. #### Example There's an [example project](https://github.com/lihaoyi/cask/tree/master/example/twirl) -- cgit v1.2.3