First pass at using a compiler plugin to remove the need for the `override` keyword when overriding a field within a `mill.Module`

This only applies to `mill.Module`s, not overrides elsewhere which still require the keyword. `mill.Module`s tend to have lots and lots of overriding, so the keyword is basically noise.

Also includes the necessary build changes to enable the locally-built Scalac plugin when compiling the test suite. Note that no changes are necessary for the executable assembly, because the `scalac-plugin.xml` will be included in the assembly and get picked up by the Ammonite scalac plugin classloader automatically

11 files changed, 164 insertions, 48 deletions

diff --git a/build.sbt b/build.sbt
index 57035a6e..7069a5c5 100644
--- a/build.sbt
+++ b/build.sbt
@@ -19,7 +19,17 @@ val sharedSettings = Seq(
     IO.write(file, """object amm extends App { ammonite.Main().run() }""")
     Seq(file)
   }.taskValue
+)
 
+val pluginSettings = Seq(
+  scalacOptions in Test ++= {
+    val jarFile = (packageBin in (plugin, Compile)).value
+    val addPlugin = "-Xplugin:" + jarFile.getAbsolutePath
+    // add plugin timestamp to compiler options to trigger recompile of
+    // main after editing the plugin. (Otherwise a 'clean' is needed.)
+    val dummy = "-Jdummy=" + jarFile.lastModified
+    Seq(addPlugin, dummy)
+  }
 )
 
 def bridge(bridgeVersion: String) = Project(
@@ -71,8 +81,10 @@ lazy val bridge2_12_3 = bridge("2.12.3")
 lazy val bridge2_12_4 = bridge("2.12.4")
 
 lazy val core = project
+  .dependsOn(plugin)
   .settings(
     sharedSettings,
+    pluginSettings,
     name := "mill-core",
     libraryDependencies ++= Seq(
       "org.scala-lang" % "scala-reflect" % scalaVersion.value % "provided",
@@ -84,25 +96,39 @@ lazy val core = project
     )
   )
 
+lazy val plugin = project
+  .settings(
+    sharedSettings,
+    libraryDependencies ++= Seq(
+      "org.scala-lang" % "scala-compiler" % scalaVersion.value,
+      "com.lihaoyi" %% "sourcecode" % "0.1.4"
+    ),
+    publishArtifact in Compile := false
+  )
+
 val bridgeProps = Def.task{
   val mapping = Map(
     "MILL_COMPILER_BRIDGE_2_10_6" -> (packageBin in (bridge2_10_6, Compile)).value.absolutePath,
     "MILL_COMPILER_BRIDGE_2_11_8" -> (packageBin in (bridge2_11_8, Compile)).value.absolutePath,
     "MILL_COMPILER_BRIDGE_2_11_11" -> (packageBin in (bridge2_11_11, Compile)).value.absolutePath,
     "MILL_COMPILER_BRIDGE_2_12_3" -> (packageBin in (bridge2_12_3, Compile)).value.absolutePath,
-    "MILL_COMPILER_BRIDGE_2_12_4" -> (packageBin in (bridge2_12_4, Compile)).value.absolutePath
+    "MILL_COMPILER_BRIDGE_2_12_4" -> (packageBin in (bridge2_12_4, Compile)).value.absolutePath,
+    "MILL_COMPILER_PLUGIN" -> (packageBin in (plugin, Compile)).value
   )
   for((k, v) <- mapping) yield s"-D$k=$v"
 }
+
 lazy val scalaplugin = project
   .dependsOn(core % "compile->compile;test->test")
   .settings(
     sharedSettings,
+    pluginSettings,
     name := "mill-scalaplugin",
     fork in Test := true,
     baseDirectory in Test := (baseDirectory in Test).value / "..",
     javaOptions in Test := bridgeProps.value.toSeq
   )
+
 lazy val bin = project
   .dependsOn(scalaplugin)
   .settings(
diff --git a/build.sc b/build.sc
index a7508941..04bc489b 100755
--- a/build.sc
+++ b/build.sc
@@ -3,32 +3,46 @@ import mill._
 import mill.scalaplugin._
 import mill.modules.Jvm.createAssembly
 
+
+object CompilerPlugin extends SbtScalaModule{
+  def scalaVersion = "2.12.4"
+  def basePath = pwd / 'plugin
+  def ivyDeps = Seq(
+    Dep.Java("org.scala-lang", "scala-compiler", scalaVersion()),
+    Dep("com.lihaoyi", "sourcecode", "0.1.4")
+  )
+}
+
 trait MillModule extends SbtScalaModule{ outer =>
   def scalaVersion = "2.12.4"
 
-  override def compileIvyDeps = Seq(Dep("com.lihaoyi", "acyclic", "0.1.7"))
-  override def scalacOptions = Seq("-P:acyclic:force")
-  override def scalacPluginIvyDeps = Seq(Dep("com.lihaoyi", "acyclic", "0.1.7"))
+  def compileIvyDeps = Seq(Dep("com.lihaoyi", "acyclic", "0.1.7"))
+  def scalacOptions = Seq("-P:acyclic:force")
+  def scalacPluginIvyDeps = Seq(Dep("com.lihaoyi", "acyclic", "0.1.7"))
 
   def testArgs = T{ Seq.empty[String] }
 
   object test extends this.Tests{
-    override def defaultCommandName() = "forkTest"
-    override def forkArgs = T{ testArgs() }
-    override def projectDeps =
+    def defaultCommandName() = "forkTest"
+    def forkArgs = T{ testArgs() }
+    def projectDeps =
       if (this == Core.test) Seq(Core)
       else Seq(outer, Core.test)
-    override def ivyDeps = Seq(Dep("com.lihaoyi", "utest", "0.6.0"))
+    def ivyDeps = Seq(Dep("com.lihaoyi", "utest", "0.6.0"))
     def testFramework = "mill.UTestFramework"
+    def scalacPluginClasspath = super.scalacPluginClasspath() ++ Seq(CompilerPlugin.jar())
+
   }
 }
 
 object Core extends MillModule {
-  override def compileIvyDeps = Seq(
+  def projectDeps = Seq(CompilerPlugin)
+
+  def compileIvyDeps = Seq(
     Dep.Java("org.scala-lang", "scala-reflect", scalaVersion())
   )
 
-  override def ivyDeps = Seq(
+  def ivyDeps = Seq(
     Dep("com.lihaoyi", "sourcecode", "0.1.4"),
     Dep("com.lihaoyi", "pprint", "0.5.3"),
     Dep.Point("com.lihaoyi", "ammonite", "1.0.3"),
@@ -61,7 +75,7 @@ val bridges = for{
 } yield new ScalaModule{
   def basePath = pwd / 'bridge
   def scalaVersion = crossVersion
-  override def allSources = T{
+  def allSources = T{
 
     val v = crossVersion.split('.').dropRight(1).mkString(".")
     val url =
@@ -77,7 +91,7 @@ val bridges = for{
 
     Seq(PathRef(curlDest / 'classes))
   }
-  override def ivyDeps = Seq(
+  def ivyDeps = Seq(
     Dep.Java("org.scala-lang", "scala-compiler", crossVersion),
     Dep.Java("org.scala-sbt", "compiler-interface", "1.0.5")
   )
@@ -85,16 +99,17 @@ val bridges = for{
 
 object ScalaPlugin extends MillModule {
 
-  override def projectDeps = Seq(Core)
+  def projectDeps = Seq(Core)
   def basePath = pwd / 'scalaplugin
 
-  override def testArgs = T{
+  def testArgs = T{
     val mapping = Map(
       "MILL_COMPILER_BRIDGE_2_10_6"  -> bridges("2.10.6").compile().classes.path,
       "MILL_COMPILER_BRIDGE_2_11_8"  -> bridges("2.11.8").compile().classes.path,
       "MILL_COMPILER_BRIDGE_2_11_11" -> bridges("2.11.11").compile().classes.path,
       "MILL_COMPILER_BRIDGE_2_12_3"  -> bridges("2.12.3").compile().classes.path,
       "MILL_COMPILER_BRIDGE_2_12_4"  -> bridges("2.12.4").compile().classes.path,
+      "MILL_COMPILER_PLUGIN" -> CompilerPlugin.compile().classes.path
     )
     for((k, v) <- mapping.toSeq) yield s"-D$k=$v"
   }
@@ -103,7 +118,7 @@ object ScalaPlugin extends MillModule {
 
 object Bin extends MillModule {
 
-  override def projectDeps = Seq(ScalaPlugin)
+  def projectDeps = Seq(ScalaPlugin)
   def basePath = pwd / 'bin
 
   def releaseAssembly = T{
@@ -116,7 +131,7 @@ object Bin extends MillModule {
     )
   }
 
-  override def prependShellScript =
+  def prependShellScript =
     "#!/usr/bin/env sh\n" +
     s"""exec java ${ScalaPlugin.testArgs().mkString(" ")} $$JAVA_OPTS -cp "$$0" mill.Main "$$@" """
 
diff --git a/core/src/main/scala/mill/define/Task.scala b/core/src/main/scala/mill/define/Task.scala
index e88ad8c6..8393acfb 100644
--- a/core/src/main/scala/mill/define/Task.scala
+++ b/core/src/main/scala/mill/define/Task.scala
@@ -58,7 +58,7 @@ object Target extends Applicative.Applyer[Task, Task, Result, Ctx]{
     import c.universe._
 
     c.Expr[Persistent[T]](
-      mill.define.Cacher.wrapCached(c)(
+      mill.plugin.Cacher.wrapCached(c)(
         q"new ${weakTypeOf[Persistent[T]]}(${Applicative.impl[Task, T, Ctx](c)(t).tree})"
       )
     )
@@ -74,7 +74,7 @@ object Target extends Applicative.Applyer[Task, Task, Result, Ctx]{
   def targetTaskImpl[T: c.WeakTypeTag](c: Context)(t: c.Expr[Task[T]]): c.Expr[Target[T]] = {
     import c.universe._
     c.Expr[Target[T]](
-      mill.define.Cacher.wrapCached(c)(
+      mill.plugin.Cacher.wrapCached(c)(
         q"new ${weakTypeOf[TargetImpl[T]]}($t, _root_.sourcecode.Enclosing())"
       )
     )
@@ -82,7 +82,7 @@ object Target extends Applicative.Applyer[Task, Task, Result, Ctx]{
   def targetImpl[T: c.WeakTypeTag](c: Context)(t: c.Expr[T]): c.Expr[Target[T]] = {
     import c.universe._
     c.Expr[Target[T]](
-      mill.define.Cacher.wrapCached(c)(
+      mill.plugin.Cacher.wrapCached(c)(
         q"new ${weakTypeOf[TargetImpl[T]]}(${Applicative.impl0[Task, T, Ctx](c)(q"mill.eval.Result.Success($t)").tree}, _root_.sourcecode.Enclosing())"
       )
     )
@@ -159,7 +159,7 @@ object Task {
   trait TaskModule extends Module {
     def defaultCommandName(): String
   }
-  trait Module extends mill.define.Cacher[Target]{
+  trait Module extends mill.plugin.Cacher[Target]{
     def wrapCached[T](t: Target[T], enclosing: String): Target[T] = t
   }
 
diff --git a/core/src/test/scala/mill/define/CacherTests.scala b/core/src/test/scala/mill/define/CacherTests.scala
index bacfa598..4b8e31ab 100644
--- a/core/src/test/scala/mill/define/CacherTests.scala
+++ b/core/src/test/scala/mill/define/CacherTests.scala
@@ -14,7 +14,7 @@ object CacherTests extends TestSuite{
   }
   object Middle extends Middle
   trait Middle extends Base{
-    override def value = T{ super.value() + 2}
+    def value = T{ super.value() + 2}
     def overriden = T{ super.value()}
   }
   object Terminal extends Terminal
@@ -49,6 +49,19 @@ object CacherTests extends TestSuite{
       eval(Terminal, Terminal.value) == 7,
       eval(Terminal, Terminal.overriden) == 1
     )
+    //    Doesn't fail, presumably compileError doesn't go far enough in the
+    //    compilation pipeline to hit the override checks
+    //
+    //    'overrideOutsideModuleFails - {
+    //      compileError("""
+    //        trait Foo{
+    //          def x = 1
+    //        }
+    //        object Bar extends Foo{
+    //          def x = 2
+    //        }
+    //      """)
+    //    }
   }
 }
 
diff --git a/plugin/src/main/resources/scalac-plugin.xml b/plugin/src/main/resources/scalac-plugin.xml
new file mode 100644
index 00000000..fb9a4404
--- /dev/null
+++ b/plugin/src/main/resources/scalac-plugin.xml
@@ -0,0 +1,4 @@
+<plugin>
+    <name>demo-plugin</name>
+    <classname>mill.plugin.AutoOverridePlugin</classname>
+</plugin>
\ No newline at end of file
diff --git a/plugin/src/main/scala/mill/plugin/AutoOverridePlugin.scala b/plugin/src/main/scala/mill/plugin/AutoOverridePlugin.scala
new file mode 100644
index 00000000..ab9d8db0
--- /dev/null
+++ b/plugin/src/main/scala/mill/plugin/AutoOverridePlugin.scala
@@ -0,0 +1,58 @@
+package mill.plugin
+
+import scala.reflect.internal.Flags
+import scala.tools.nsc.io.VirtualFile
+import scala.tools.nsc.util.BatchSourceFile
+import scala.tools.nsc.{Global, Phase}
+import scala.tools.nsc.plugins.{Plugin, PluginComponent}
+
+class AutoOverridePlugin(val global: Global) extends Plugin {
+  import global._
+  override def init(options: List[String],  error: String => Unit): Boolean = true
+
+  val name = "auto-override-plugin"
+  val description = "automatically inserts `override` keywords for you"
+  val components = List[PluginComponent](
+    new PluginComponent {
+
+      val global = AutoOverridePlugin.this.global
+      import global._
+
+      override val runsAfter = List("typer")
+      override val runsBefore = List("patmat")
+
+      val phaseName = "auto-override"
+
+      override def newPhase(prev: Phase) = new GlobalPhase(prev) {
+
+        def name: String = phaseName
+
+        def isCacher(owner: Symbol) = {
+          val baseClasses =
+            if (owner.isClass) Some(owner.asClass.baseClasses)
+            else if (owner.isModule) Some(owner.asModule.baseClasses)
+            else None
+          baseClasses.exists(_.exists(_.fullName == "mill.plugin.Cacher"))
+        }
+
+        def apply(unit: global.CompilationUnit): Unit = {
+          object AutoOverrider extends global.Transformer {
+            override def transform(tree: global.Tree) = tree match{
+              case d: DefDef
+                if d.symbol.overrideChain.count(!_.isAbstract) > 1
+                && !d.mods.isOverride
+                && isCacher(d.symbol.owner) =>
+
+                  d.symbol.flags = d.symbol.flags | Flags.OVERRIDE
+                  copyDefDef(d)(mods = d.mods | Flags.OVERRIDE)
+              case _ => super.transform(tree)
+
+            }
+          }
+
+          unit.body = AutoOverrider.transform(unit.body)
+        }
+      }
+    }
+  )
+}
\ No newline at end of file
diff --git a/core/src/main/scala/mill/define/Cacher.scala b/plugin/src/main/scala/mill/plugin/Cacher.scala
index 09200203..4c854df7 100644
--- a/core/src/main/scala/mill/define/Cacher.scala
+++ b/plugin/src/main/scala/mill/plugin/Cacher.scala
@@ -1,4 +1,4 @@
-package mill.define
+package mill.plugin
 
 import scala.collection.mutable
 import scala.reflect.macros.blackbox.Context
@@ -22,7 +22,7 @@ object Cacher{
     val owner = c.internal.enclosingOwner
     val ownerIsCacherClass =
       owner.owner.isClass &&
-      owner.owner.asClass.baseClasses.exists(_.fullName == "mill.define.Cacher")
+        owner.owner.asClass.baseClasses.exists(_.fullName == "mill.plugin.Cacher")
 
     if (ownerIsCacherClass && owner.isMethod) q"this.cachedTarget($t)"
     else c.abort(
diff --git a/scalaplugin/src/test/scala/mill/scalaplugin/AcyclicTests.scala b/scalaplugin/src/test/scala/mill/scalaplugin/AcyclicTests.scala
index f233fdf1..185cffc8 100644
--- a/scalaplugin/src/test/scala/mill/scalaplugin/AcyclicTests.scala
+++ b/scalaplugin/src/test/scala/mill/scalaplugin/AcyclicTests.scala
@@ -17,12 +17,12 @@ object AcyclicBuild{
 
       def version = "0.1.7"
       def scalaVersion = crossVersion
-      override def ivyDeps = Seq(
+      def ivyDeps = Seq(
         Dep.Java("org.scala-lang", "scala-compiler", scalaVersion())
       )
       object test extends this.Tests{
-        override def forkWorkingDir = pwd/'scalaplugin/'src/'test/'resource/'acyclic
-        override def ivyDeps = Seq(
+        def forkWorkingDir = pwd/'scalaplugin/'src/'test/'resource/'acyclic
+        def ivyDeps = Seq(
           Dep("com.lihaoyi", "utest", "0.6.0")
         )
         def testFramework = "utest.runner.Framework"
diff --git a/scalaplugin/src/test/scala/mill/scalaplugin/BetterFilesTests.scala b/scalaplugin/src/test/scala/mill/scalaplugin/BetterFilesTests.scala
index c59beca4..f4247094 100644
--- a/scalaplugin/src/test/scala/mill/scalaplugin/BetterFilesTests.scala
+++ b/scalaplugin/src/test/scala/mill/scalaplugin/BetterFilesTests.scala
@@ -11,7 +11,7 @@ import mill.util.JsonFormatters._
 object BetterFilesBuild{
   trait BetterFilesModule extends SbtScalaModule{ outer =>
     def scalaVersion = "2.12.4"
-    override def scalacOptions = Seq(
+    def scalacOptions = Seq(
       "-deprecation",                      // Emit warning and location for usages of deprecated APIs.
       "-encoding", "utf-8",                // Specify character encoding used by source files.
       "-explaintypes",                     // Explain type errors in more detail.
@@ -60,10 +60,10 @@ object BetterFilesBuild{
     )
     override def javacOptions = Seq("-source", "1.8", "-target", "1.8", "-Xlint")
     object test extends this.Tests{
-      override def projectDeps =
+      def projectDeps =
         if (this == Core.test) Seq(Core)
         else Seq(outer, Core.test)
-      override def ivyDeps = Seq(Dep("org.scalatest", "scalatest", "3.0.4"))
+      def ivyDeps = Seq(Dep("org.scalatest", "scalatest", "3.0.4"))
       def testFramework = "org.scalatest.tools.Framework"
     }
   }
@@ -71,19 +71,19 @@ object BetterFilesBuild{
     def basePath = BetterFilesTests.srcPath/"core"
   }
   object Akka extends BetterFilesModule{
-    override def projectDeps = Seq(Core)
+    def projectDeps = Seq(Core)
     def basePath = BetterFilesTests.srcPath/"akka"
-    override def ivyDeps = Seq(Dep("com.typesafe.akka", "akka-actor", "2.5.6"))
+    def ivyDeps = Seq(Dep("com.typesafe.akka", "akka-actor", "2.5.6"))
   }
   object ShapelessScanner extends BetterFilesModule{
-    override def projectDeps = Seq(Core)
+    def projectDeps = Seq(Core)
     def basePath = BetterFilesTests.srcPath/"shapeless"
-    override def ivyDeps = Seq(Dep("com.chuusai", "shapeless", "2.3.2"))
+    def ivyDeps = Seq(Dep("com.chuusai", "shapeless", "2.3.2"))
   }
   object Benchmarks extends BetterFilesModule{
-    override def projectDeps = Seq(Core)
+    def projectDeps = Seq(Core)
     def basePath = BetterFilesTests.srcPath/"benchmarks"
-    override def ivyDeps = Seq(
+    def ivyDeps = Seq(
       Dep.Java("commons-io", "commons-io", "2.5")
       // "fastjavaio" % "fastjavaio" % "1.0" from "https://github.com/williamfiset/FastJavaIO/releases/download/v1.0/fastjavaio.jar"
     )
diff --git a/scalaplugin/src/test/scala/mill/scalaplugin/HelloWorldTests.scala b/scalaplugin/src/test/scala/mill/scalaplugin/HelloWorldTests.scala
index 6e52b5ca..239f6871 100644
--- a/scalaplugin/src/test/scala/mill/scalaplugin/HelloWorldTests.scala
+++ b/scalaplugin/src/test/scala/mill/scalaplugin/HelloWorldTests.scala
@@ -18,15 +18,15 @@ trait HelloWorldModule extends ScalaModule {
 object HelloWorld extends HelloWorldModule
 
 object HelloWorldWithMain extends HelloWorldModule {
-  override def mainClass = Some("Main")
+  def mainClass = Some("Main")
 }
 
 object HelloWorldWarnUnused extends HelloWorldModule {
-  override def scalacOptions = T(Seq("-Ywarn-unused"))
+  def scalacOptions = T(Seq("-Ywarn-unused"))
 }
 
 object HelloWorldFatalWarnings extends HelloWorldModule {
-  override def scalacOptions = T(Seq("-Ywarn-unused", "-Xfatal-warnings"))
+  def scalacOptions = T(Seq("-Ywarn-unused", "-Xfatal-warnings"))
 }
 
 object HelloWorldTests extends TestSuite {
diff --git a/scalaplugin/src/test/scala/mill/scalaplugin/JawnTests.scala b/scalaplugin/src/test/scala/mill/scalaplugin/JawnTests.scala
index b48fd853..0d876660 100644
--- a/scalaplugin/src/test/scala/mill/scalaplugin/JawnTests.scala
+++ b/scalaplugin/src/test/scala/mill/scalaplugin/JawnTests.scala
@@ -14,15 +14,15 @@ object JawnBuild{
   class Jawn(crossVersion: String) extends Module{
     trait JawnModule extends SbtScalaModule{ outer =>
       def scalaVersion = crossVersion
-      override def scalacOptions = Seq(
+      def scalacOptions = Seq(
         "-deprecation",
         "-optimize",
         "-unchecked"
       )
       def testProjectDeps: Seq[TestScalaModule] = Nil
       object test extends this.Tests{
-        override def projectDeps = super.projectDeps ++ testProjectDeps
-        override def ivyDeps = Seq(
+        def projectDeps = super.projectDeps ++ testProjectDeps
+        def ivyDeps = Seq(
           Dep("org.scalatest", "scalatest", "3.0.3"),
           Dep("org.scalacheck", "scalacheck", "1.13.5")
         )
@@ -33,25 +33,25 @@ object JawnBuild{
       def basePath = JawnTests.srcPath/"parser"
     }
     object Util extends JawnModule{
-      override def projectDeps = Seq(Parser)
-      override def testProjectDeps = Seq(Parser.test)
+      def projectDeps = Seq(Parser)
+      def testProjectDeps = Seq(Parser.test)
       def basePath = JawnTests.srcPath/"util"
     }
     object Ast extends JawnModule{
-      override def projectDeps = Seq(Parser, Util)
-      override def testProjectDeps = Seq(Parser.test, Util.test)
+      def projectDeps = Seq(Parser, Util)
+      def testProjectDeps = Seq(Parser.test, Util.test)
       def basePath = JawnTests.srcPath/"ast"
     }
     class Support(name: String, ivyDeps0: Dep*) extends JawnModule{
-      override def projectDeps = Seq[ScalaModule](Parser)
+      def projectDeps = Seq[ScalaModule](Parser)
       def basePath = JawnTests.srcPath/"support"/"argonaut"
-      override def ivyDeps = ivyDeps0
+      def ivyDeps = ivyDeps0
     }
     object Argonaut extends Support("argonaut", Dep("io.argonaut", "argonaut", "6.2"))
     object Json4s extends Support("json4s", Dep("org.json4s", "json4s-ast", "3.5.2"))
 
     object Play extends Support("play"){
-      override def ivyDeps = mill.T{
+      def ivyDeps = mill.T{
         scalaBinaryVersion() match{
           case "2.10" => Seq(Dep("com.typesafe.play", "play-json", "2.4.11"))
           case "2.11" => Seq(Dep("com.typesafe.play", "play-json", "2.5.15"))