Emit mixin forwarders for JUnit-annotated trait methods by default

JUnit 4 does not support default methods. For better user experience,
this commit makes the compiler generate mixin forwarders for inherited
trait methods that carry a JUnit annotation.

The -Yjunit-trait-methods-no-forwarders flag disables this behavior.

This supersedes the scala-js/scala-2.12-junit-mixin-plugin compiler
plugin.

10 files changed, 36 insertions, 21 deletions

diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala
index 7ef606b6ef..047ea3c751 100644
--- a/src/compiler/scala/tools/nsc/settings/Warnings.scala
+++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala
@@ -25,7 +25,7 @@ trait Warnings {
   // currently considered too noisy for general use
   val warnUnusedImport     = BooleanSetting("-Ywarn-unused-import", "Warn when imports are unused.")
 
-  val nowarnDefaultJunitMethods = BooleanSetting("-Ynowarn-default-junit-methods", "Don't warn when a JUnit @Test method is generated as a default method (not supported in JUnit 4).")
+  val junitTraitMethodsNoForwarders = BooleanSetting("-Yjunit-trait-methods-no-forwarders", "Don't generate forwarders for JUnit annotated trait methods (JUnit 4 does not support default methods).")
 
   // Experimental lint warnings that are turned off, but which could be turned on programmatically.
   // They are not activated by -Xlint and can't be enabled on the command line because they are not
diff --git a/src/compiler/scala/tools/nsc/transform/Mixin.scala b/src/compiler/scala/tools/nsc/transform/Mixin.scala
index f8e6b630d1..58f32b9aca 100644
--- a/src/compiler/scala/tools/nsc/transform/Mixin.scala
+++ b/src/compiler/scala/tools/nsc/transform/Mixin.scala
@@ -286,12 +286,16 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL {
                 case _ => false
               }
 
-              if (existsCompetingMethod(clazz.baseClasses))
+              def generateJUnitForwarder: Boolean = {
+                !settings.junitTraitMethodsNoForwarders &&
+                  member.annotations.nonEmpty &&
+                  JUnitAnnotations.exists(annot => annot.exists && member.hasAnnotation(annot))
+              }
+
+              if (existsCompetingMethod(clazz.baseClasses) || generateJUnitForwarder)
                 genForwarder(required = true)
               else if (settings.XgenMixinForwarders)
                 genForwarder(required = false)
-              else if (!settings.nowarnDefaultJunitMethods && JUnitTestClass.exists && member.hasAnnotation(JUnitTestClass))
-                warning(member.pos, "JUnit tests in traits that are compiled as default methods are not executed by JUnit 4. JUnit 5 will fix this issue.")
             }
 
           case _        =>
diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
index d0539dfd42..113ac4cd09 100644
--- a/src/reflect/scala/reflect/internal/Definitions.scala
+++ b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -1151,7 +1151,6 @@ trait Definitions extends api.StandardDefinitions {
     lazy val ElidableMethodClass        = requiredClass[scala.annotation.elidable]
     lazy val ImplicitNotFoundClass      = requiredClass[scala.annotation.implicitNotFound]
     lazy val ImplicitAmbiguousClass     = getClassIfDefined("scala.annotation.implicitAmbiguous")
-    lazy val JUnitTestClass             = getClassIfDefined("org.junit.Test")
     lazy val MigrationAnnotationClass   = requiredClass[scala.annotation.migration]
     lazy val ScalaStrictFPAttr          = requiredClass[scala.annotation.strictfp]
     lazy val SwitchClass                = requiredClass[scala.annotation.switch]
@@ -1193,6 +1192,8 @@ trait Definitions extends api.StandardDefinitions {
     lazy val MethodTargetClass          = requiredClass[meta.companionMethod]    // TODO: module, moduleClass? package, packageObject?
     lazy val LanguageFeatureAnnot       = requiredClass[meta.languageFeature]
 
+    lazy val JUnitAnnotations = List("Test", "Ignore", "Before", "After", "BeforeClass", "AfterClass").map(n => getClassIfDefined("org.junit." + n))
+
     // Language features
     lazy val languageFeatureModule      = getRequiredModule("scala.languageFeature")
 
diff --git a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
index f55b33959a..90c026a738 100644
--- a/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
+++ b/src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
@@ -379,7 +379,6 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     definitions.ElidableMethodClass
     definitions.ImplicitNotFoundClass
     definitions.ImplicitAmbiguousClass
-    definitions.JUnitTestClass
     definitions.MigrationAnnotationClass
     definitions.ScalaStrictFPAttr
     definitions.SwitchClass
@@ -417,6 +416,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     definitions.ClassTargetClass
     definitions.MethodTargetClass
     definitions.LanguageFeatureAnnot
+    definitions.JUnitAnnotations
     definitions.languageFeatureModule
     definitions.metaAnnotations
     definitions.AnnotationDefaultAttr
diff --git a/test/files/neg/nowarnDefaultJunitMethods.check b/test/files/neg/nowarnDefaultJunitMethods.check
deleted file mode 100644
index 7efdcc299a..0000000000
--- a/test/files/neg/nowarnDefaultJunitMethods.check
+++ /dev/null
@@ -1,6 +0,0 @@
-C_1.scala:2: warning: JUnit tests in traits that are compiled as default methods are not executed by JUnit 4. JUnit 5 will fix this issue.
-  @org.junit.Test def foo = 0
-                      ^
-error: No warnings can be incurred under -Xfatal-warnings.
-one warning found
-one error found
diff --git a/test/files/neg/nowarnDefaultJunitMethods.flags b/test/files/neg/nowarnDefaultJunitMethods.flags
deleted file mode 100644
index 85d8eb2ba2..0000000000
--- a/test/files/neg/nowarnDefaultJunitMethods.flags
+++ /dev/null
@@ -1 +0,0 @@
--Xfatal-warnings
diff --git a/test/files/neg/nowarnDefaultJunitMethods/C_1.scala b/test/files/neg/nowarnDefaultJunitMethods/C_1.scala
deleted file mode 100644
index e2565a48bc..0000000000
--- a/test/files/neg/nowarnDefaultJunitMethods/C_1.scala
+++ /dev/null
@@ -1,5 +0,0 @@
-trait T {
-  @org.junit.Test def foo = 0
-}
-
-class C extends T
diff --git a/test/files/neg/nowarnDefaultJunitMethods/Test.java b/test/files/neg/nowarnDefaultJunitMethods/Test.java
deleted file mode 100644
index e8d64c2cc8..0000000000
--- a/test/files/neg/nowarnDefaultJunitMethods/Test.java
+++ /dev/null
@@ -1,3 +0,0 @@
-package org.junit;
-
-public @interface Test { }
diff --git a/test/files/run/junitForwarders/C_1.scala b/test/files/run/junitForwarders/C_1.scala
new file mode 100644
index 0000000000..2af2026a61
--- /dev/null
+++ b/test/files/run/junitForwarders/C_1.scala
@@ -0,0 +1,15 @@
+trait T {
+  @org.junit.Test def foo = 0
+}
+
+class C extends T
+
+object Test extends App {
+  def check(c: Class[_], e: String) = {
+    val s = c.getDeclaredMethods.sortBy(_.getName).map(m => s"${m.getName} - ${m.getDeclaredAnnotations.mkString(", ")}").mkString(";")
+    assert(s == e, s"found: $s\nexpected: $e")
+  }
+  check(classOf[C], "foo - @org.junit.Test()")
+  // TODO scala-dev#213: should `foo$` really carry the @Test annotation?
+  check(classOf[T], "$init$ - ;foo - @org.junit.Test();foo$ - @org.junit.Test()")
+}
diff --git a/test/files/run/junitForwarders/Test.java b/test/files/run/junitForwarders/Test.java
new file mode 100644
index 0000000000..57c4d5b544
--- /dev/null
+++ b/test/files/run/junitForwarders/Test.java
@@ -0,0 +1,10 @@
+package org.junit;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.METHOD})
+public @interface Test { }