Consider signatures of method before and after erasure in ExtractAPI

The signatures of methods that have value classes as arguments or return
type change during the erasure phase. Because we only registered
signatures before the erasure, we missed some API changes when a class
was changed to a value class (or a value class changed to a class).

This commit fixes this problem by recording the signatures of method
before and after erasure.

Fixes sbt/sbt#1171

compile/interface/src/main/scala/xsbt/Compat.scala

@@ -1,6 +1,6 @@
 package xsbt
 
-import scala.tools.nsc.Global
+import scala.tools.nsc.{ Global, Phase }
 import scala.tools.nsc.symtab.Flags
 
 /**
@@ -45,6 +45,11 @@ abstract class Compat {
   val Nullary = global.NullaryMethodType
   val ScalaObjectClass = definitions.ScalaObjectClass
 
+  implicit def withExitingPostErasure(global: Global) = new WithExitingPostErasure(global)
+  class WithExitingPostErasure(global: Global) {
+    def exitingPostErasure[T](op: => T) = global afterPostErasure op
+  }
+
   private[this] final class MiscCompat {
     // in 2.9, nme.LOCALCHILD was renamed to tpnme.LOCAL_CHILD
     def tpnme = nme

compile/interface/src/main/scala/xsbt/ExtractAPI.scala

@@ -178,9 +178,9 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
 
   private def viewer(s: Symbol) = (if (s.isModule) s.moduleClass else s).thisType
   private def printMember(label: String, in: Symbol, t: Type) = println(label + " in " + in + " : " + t + " (debug: " + debugString(t) + " )")
-  private def defDef(in: Symbol, s: Symbol) =
+  private def defDef(in: Symbol, s: Symbol): List[xsbti.api.Def] =
     {
-      def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): xsbti.api.Def =
+      def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): List[xsbti.api.Def] =
         {
           def parameterList(syms: List[Symbol]): xsbti.api.ParameterList =
             {
@@ -192,13 +192,50 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
               assert(typeParams.isEmpty)
               assert(valueParameters.isEmpty)
               build(base, typeParameters(in, typeParams0), Nil)
-            case MethodType(params, resultType) =>
-              build(resultType, typeParams, parameterList(params) :: valueParameters)
+            case mType @ MethodType(params, resultType) =>
+              // The types of a method's parameters change between phases: For instance, if a
+              // parameter is a subtype of AnyVal, then it won't have the same type before and after
+              // erasure. Therefore we record the type of parameters before AND after erasure to
+              // make sure that we don't miss some API changes.
+              //   class A(val x: Int) extends AnyVal
+              //   def foo(a: A): Int = A.x <- has type (LA)I before erasure
+              //                            <- has type (I)I after erasure
+              // If we change A from value class to normal class, we need to recompile all clients
+              // of def foo.
+              val beforeErasure = parameterList(params) :: valueParameters
+              val afterErasure  = global exitingPostErasure (parameterList(mType.params) :: valueParameters)
+
+              build(resultType, typeParams, beforeErasure) ++ build(resultType, typeParams, afterErasure)
             case Nullary(resultType) => // 2.9 and later
               build(resultType, typeParams, valueParameters)
             case returnType =>
-              val t2 = processType(in, dropConst(returnType))
-              new xsbti.api.Def(valueParameters.reverse.toArray, t2, typeParams, simpleName(s), getAccess(s), getModifiers(s), annotations(in, s))
+              def makeDef(retTpe: xsbti.api.Type): xsbti.api.Def =
+                new xsbti.api.Def(
+                  valueParameters.reverse.toArray,
+                  retTpe,
+                  typeParams,
+                  simpleName(s),
+                  getAccess(s),
+                  getModifiers(s),
+                  annotations(in, s))
+
+              // The return type of a method may change before and after erasure. Consider the
+              // following method:
+              //   class A(val x: Int) extends AnyVal
+              //   def foo(x: Int): A = new A(x) <- has type (I)LA before erasure
+              //                                 <- has type (I)I after erasure
+              // If we change A from value class to normal class, we need to recompile all clients
+              // of def foo.
+              val beforeErasure = processType(in, dropConst(returnType))
+              val afterErasure = {
+                val erasedReturn = dropConst(global exitingPostErasure viewer(in).memberInfo(s)) map {
+                  case MethodType(_, r) => r
+                  case other            => other
+                }
+                processType(in, erasedReturn)
+              }
+
+              makeDef(beforeErasure) :: makeDef(afterErasure) :: Nil
           }
         }
       def parameterS(s: Symbol): xsbti.api.MethodParameter =
@@ -292,22 +329,22 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
     defs
   }
 
-  private def definition(in: Symbol, sym: Symbol): Option[xsbti.api.Definition] =
+  private def definition(in: Symbol, sym: Symbol): List[xsbti.api.Definition] =
     {
-      def mkVar = Some(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _)))
-      def mkVal = Some(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _)))
+      def mkVar = List(fieldDef(in, sym, false, new xsbti.api.Var(_, _, _, _, _)))
+      def mkVal = List(fieldDef(in, sym, true, new xsbti.api.Val(_, _, _, _, _)))
       if (isClass(sym))
-        if (ignoreClass(sym)) None else Some(classLike(in, sym))
+        if (ignoreClass(sym)) Nil else List(classLike(in, sym))
       else if (sym.isNonClassType)
-        Some(typeDef(in, sym))
+        List(typeDef(in, sym))
       else if (sym.isVariable)
-        if (isSourceField(sym)) mkVar else None
+        if (isSourceField(sym)) mkVar else Nil
       else if (sym.isStable)
-        if (isSourceField(sym)) mkVal else None
+        if (isSourceField(sym)) mkVal else Nil
       else if (sym.isSourceMethod && !sym.isSetter)
-        if (sym.isGetter) mkVar else Some(defDef(in, sym))
+        if (sym.isGetter) mkVar else defDef(in, sym)
       else
-        None
+        Nil
     }
   private def ignoreClass(sym: Symbol): Boolean =
     sym.isLocalClass || sym.isAnonymousClass || sym.fullName.endsWith(LocalChild.toString)
@@ -530,4 +567,4 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
       if (annots.isEmpty) processType(in, at.underlying) else annotated(in, annots, at.underlying)
     }
 
-}
+}

sbt/src/sbt-test/source-dependencies/value-class/build.sbt

@@ -0,0 +1,2 @@
+incOptions := incOptions.value.withRecompileAllFraction(1.0)
+

sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala

@@ -0,0 +1 @@
+class A(val x: Int)

sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala

@@ -0,0 +1 @@
+class A(val x: Int) extends AnyVal

sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala

@@ -0,0 +1,3 @@
+class B {
+  def foo(a: A): Int = 1
+}

sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala

@@ -0,0 +1,3 @@
+class B {
+  def bar: A = new A(0)
+}

sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala

@@ -0,0 +1,3 @@
+object C extends App {
+  println(new B().foo(null))
+}

sbt/src/sbt-test/source-dependencies/value-class/changes/C1.scala

@@ -0,0 +1,3 @@
+object C extends App {
+  println(new B().bar.x)
+}

sbt/src/sbt-test/source-dependencies/value-class/test

@@ -0,0 +1,30 @@
+$ copy-file changes/A0.scala src/main/scala/A.scala
+$ copy-file changes/B0.scala src/main/scala/B.scala
+$ copy-file changes/C0.scala src/main/scala/C.scala
+
+# A is a normal class. B.foo accepts a parameter of type A. C calls B.foo, giving it `null`.
+> compile
+> run
+
+# Make A a value class.
+$ copy-file changes/A1.scala src/main/scala/A.scala
+
+# The code no longer compiles because B.foo no longer accepts `null` as an argument.
+# This means that we have invalidated C.scala, as expected!
+-> compile
+
+$ copy-file changes/A0.scala src/main/scala/A.scala
+$ copy-file changes/B1.scala src/main/scala/B.scala
+$ copy-file changes/C1.scala src/main/scala/C.scala
+
+# A is a normal class. B.bar takes no arguments and returns an instance of A. C calls B.bar.
+> compile
+> run
+
+# Make A a value class.
+$ copy-file changes/A1.scala src/main/scala/A.scala
+
+# The code compiles. It will run iff C is recompiled because the signature of B.bar has changed,
+# because A is now a value class.
+> run
+