Set compiler phase runsBefore to tailcalls (#191)

* Set compiler phase `runsBefore` to `tailcalls`

`runsAfter` is set to `fields`. However, in reality the phase
assembly algorithm used in 2.12.8 and before, also 2.13.0, places the
`GenJavadoc` phase right after `specialize`, which is valid, but probably
not intended.

With some changes in the phase assembly algorithm in 2.13.1, the
`GenJavadoc` phase was now placed right after `fields`, which caused
tests to fail.

* Only set runsBefore for 2.12.10+ and 2.13.1+

Author: lrytz

plugin/src/main/scala/com/typesafe/genjavadoc/Plugin.scala

@@ -61,9 +61,31 @@ class GenJavadocPlugin(val global: Global) extends Plugin {
 
     override val global: GT = GenJavadocPlugin.this.global
 
-    val isPreFields =
-      nsc.Properties.versionNumberString.startsWith("2.11.")
-    override val runsAfter = List(if(isPreFields) "uncurry" else "fields")
+    private[this] val Version = s"""(\\d+)\\.(\\d+)\\.(\\d+).*""".r
+    private[this] val (min, pat) = nsc.Properties.versionNumberString match {
+      case Version("2", b, c) => (b.toInt, c.toInt)
+      case v =>
+        reporter.warning(NoPosition, s"Unexpected Scala version in GenJavadoc: $v")
+        (-1, -1)
+    }
+
+    override val runsAfter: List[String] = List(if (min <= 11) "uncurry" else "fields")
+
+    // This used to be `Nil`. In 2.12.9 and earlier, and also 2.13.0, the phase assembly algorithm
+    // would in fact place the genjavadoc phase much later than `fields`, after `specialize`
+    // (see https://github.com/scala/scala-dev/issues/647#issuecomment-525650681).
+    //
+    // This was never intended, so we tried adding `runsBefore tailcalls`. However, because the
+    // compiler's own phase have ambiguous ordering, the 2.12.9 / 2.13.0 algorithm changed the order
+    // of other phases (see https://github.com/lightbend/genjavadoc/pull/191#issuecomment-532185154).
+    //
+    // In 2.12.10 and 2.13.1, the algorithm was updated (scala/scala#8393), and setting
+    // `runsBefore tailcalls` is now possible. The order of the compiler's own phases is made
+    // unambiguous in scala/scala#8426 and 8427.
+    override val runsBefore: List[String] =
+      if (min == 12 && pat >= 10 || min == 13 && pat >= 1 || min > 13) List("tailcalls")
+      else Nil
+
     val phaseName = "GenJavadoc"
 
     def newTransformer(unit: CompilationUnit) = new GenJavadocTransformer(unit)

plugin/src/test/resources/expected_output/basic/akka/rk/buh/is/it/A.java

@@ -84,6 +84,11 @@ public  class C implements akka.rk.buh.is.it.X {
     public   C ()  { throw new RuntimeException(); }
     public  int i ()  { throw new RuntimeException(); }
   }
+  /** tailrecced */
+  public final class TR {
+    public   TR ()  { throw new RuntimeException(); }
+    public  void tr (java.lang.String r)  { throw new RuntimeException(); }
+  }
   /**
    * object A.C.C1
    */

plugin/src/test/resources/input/basic/test.scala

@@ -183,6 +183,14 @@ class A {
      */
     object E
   }
+
+  /** tailrecced */
+  final class TR {
+    @scala.annotation.tailrec def tr(r: String): Unit = r match {
+      case _ => tr(r)
+    }
+  }
+
 }
 
 /**

plugin/src/test/resources/patches/2.12.0.patch

@@ -15,7 +15,7 @@
    }
    /**
     * class A.B
-@@ -98,8 +98,8 @@
+@@ -103,8 +103,8 @@
     * class A.C
     */
    static public  class C1 {
@@ -25,7 +25,7 @@
    }
    /**
     * object C1
-@@ -137,6 +137,11 @@
+@@ -142,6 +142,11 @@
    static public  java.lang.String stattic ()  { throw new RuntimeException(); }
    static public  java.lang.Object x ()  { throw new RuntimeException(); }
    /**
@@ -37,7 +37,7 @@
     * varargs
     * @param s (undocumented)
     * @return (undocumented)
-@@ -209,9 +214,4 @@
+@@ -214,9 +219,4 @@
     */
    public  int testthrows () throws java.lang.IllegalArgumentException, java.lang.NullPointerException { throw new RuntimeException(); }
    public  scala.runtime.Null$ getNull ()  { throw new RuntimeException(); }

plugin/src/test/resources/patches/2.12.1.patch

@@ -15,7 +15,7 @@
    }
    /**
     * class A.B
-@@ -98,8 +98,8 @@
+@@ -103,8 +103,8 @@
     * class A.C
     */
    static public  class C1 {
@@ -25,7 +25,7 @@
    }
    /**
     * object C1
-@@ -137,6 +137,11 @@
+@@ -142,6 +142,11 @@
    static public  java.lang.String stattic ()  { throw new RuntimeException(); }
    static public  java.lang.Object x ()  { throw new RuntimeException(); }
    /**
@@ -37,7 +37,7 @@
     * varargs
     * @param s (undocumented)
     * @return (undocumented)
-@@ -209,9 +214,4 @@
+@@ -214,9 +219,4 @@
     */
    public  int testthrows () throws java.lang.IllegalArgumentException, java.lang.NullPointerException { throw new RuntimeException(); }
    public  scala.runtime.Null$ getNull ()  { throw new RuntimeException(); }

plugin/src/test/resources/patches/2.12.10.patch

@@ -15,7 +15,7 @@
    }
    /**
     * class A.B
-@@ -98,8 +98,8 @@
+@@ -103,8 +103,8 @@
     * class A.C
     */
    static public  class C1 {
@@ -25,7 +25,7 @@
    }
    /**
     * object C1
-@@ -137,6 +137,11 @@
+@@ -142,6 +142,11 @@
    static public  java.lang.String stattic ()  { throw new RuntimeException(); }
    static public  java.lang.Object x ()  { throw new RuntimeException(); }
    /**
@@ -37,7 +37,7 @@
     * varargs
     * @param s (undocumented)
     * @return (undocumented)
-@@ -209,9 +214,4 @@
+@@ -214,9 +219,4 @@
     */
    public  int testthrows () throws java.lang.IllegalArgumentException, java.lang.NullPointerException { throw new RuntimeException(); }
    public  scala.runtime.Null$ getNull ()  { throw new RuntimeException(); }

plugin/src/test/resources/patches/2.12.11.patch

@@ -15,7 +15,7 @@
    }
    /**
     * class A.B
-@@ -98,8 +98,8 @@
+@@ -103,8 +103,8 @@
     * class A.C
     */
    static public  class C1 {
@@ -25,7 +25,7 @@
    }
    /**
     * object C1
-@@ -137,6 +137,11 @@
+@@ -142,6 +142,11 @@
    static public  java.lang.String stattic ()  { throw new RuntimeException(); }
    static public  java.lang.Object x ()  { throw new RuntimeException(); }
    /**
@@ -37,7 +37,7 @@
     * varargs
     * @param s (undocumented)
     * @return (undocumented)
-@@ -209,9 +214,4 @@
+@@ -214,9 +219,4 @@
     */
    public  int testthrows () throws java.lang.IllegalArgumentException, java.lang.NullPointerException { throw new RuntimeException(); }
    public  scala.runtime.Null$ getNull ()  { throw new RuntimeException(); }

plugin/src/test/resources/patches/2.12.2.patch

@@ -15,7 +15,7 @@
    }
    /**
     * class A.B
-@@ -98,8 +98,8 @@
+@@ -103,8 +103,8 @@
     * class A.C
     */
    static public  class C1 {
@@ -25,7 +25,7 @@
    }
    /**
     * object C1
-@@ -137,6 +137,11 @@
+@@ -142,6 +142,11 @@
    static public  java.lang.String stattic ()  { throw new RuntimeException(); }
    static public  java.lang.Object x ()  { throw new RuntimeException(); }
    /**
@@ -37,7 +37,7 @@
     * varargs
     * @param s (undocumented)
     * @return (undocumented)
-@@ -209,9 +214,4 @@
+@@ -214,9 +219,4 @@
     */
    public  int testthrows () throws java.lang.IllegalArgumentException, java.lang.NullPointerException { throw new RuntimeException(); }
    public  scala.runtime.Null$ getNull ()  { throw new RuntimeException(); }

plugin/src/test/resources/patches/2.12.3.patch

@@ -15,7 +15,7 @@
    }
    /**
     * class A.B
-@@ -98,8 +98,8 @@
+@@ -103,8 +103,8 @@
     * class A.C
     */
    static public  class C1 {
@@ -25,7 +25,7 @@
    }
    /**
     * object C1
-@@ -137,6 +137,11 @@
+@@ -142,6 +142,11 @@
    static public  java.lang.String stattic ()  { throw new RuntimeException(); }
    static public  java.lang.Object x ()  { throw new RuntimeException(); }
    /**
@@ -37,7 +37,7 @@
     * varargs
     * @param s (undocumented)
     * @return (undocumented)
-@@ -209,9 +214,4 @@
+@@ -214,9 +219,4 @@
     */
    public  int testthrows () throws java.lang.IllegalArgumentException, java.lang.NullPointerException { throw new RuntimeException(); }
    public  scala.runtime.Null$ getNull ()  { throw new RuntimeException(); }

plugin/src/test/resources/patches/2.12.4.patch

@@ -15,7 +15,7 @@
    }
    /**
     * class A.B
-@@ -98,8 +98,8 @@
+@@ -103,8 +103,8 @@
     * class A.C
     */
    static public  class C1 {
@@ -25,7 +25,7 @@
    }
    /**
     * object C1
-@@ -137,6 +137,11 @@
+@@ -142,6 +142,11 @@
    static public  java.lang.String stattic ()  { throw new RuntimeException(); }
    static public  java.lang.Object x ()  { throw new RuntimeException(); }
    /**
@@ -37,7 +37,7 @@
     * varargs
     * @param s (undocumented)
     * @return (undocumented)
-@@ -209,9 +214,4 @@
+@@ -214,9 +219,4 @@
     */
    public  int testthrows () throws java.lang.IllegalArgumentException, java.lang.NullPointerException { throw new RuntimeException(); }
    public  scala.runtime.Null$ getNull ()  { throw new RuntimeException(); }