From 0993c1c7cc1103c70fb364db77dcc3e776027e7d Mon Sep 17 00:00:00 2001
From: Guillaume Martres <smarter@ubuntu.com>
Date: Sat, 23 Jan 2016 17:50:57 +0100
Subject: [PATCH] Always invalidate API when return type is a value class

Before this commit, we did not do the invalidation for methods with
multiple parameter list, the comment above `hasValueClassAsReturnType`
said:

  Note: We only inspect the "outermost type" (i.e. no recursion) because
  we don't need to inspect after erasure a function that would, for
  instance, return a function that returns a subtype of AnyVal.

But this is wrong: a method with signature:
  def foo(a: A)(b: B): C
is erased to:
  def foo(a: A, b: B): C
and not, as the comment in the code suggest, to:
  def foo(a: A): B => C
so we do need to inspect the final result type of methods, because they
can be value classes that will be erased to their underlying value.
---
 .../src/main/scala/xsbt/ExtractAPI.scala      | 18 ++++++-----------
 .../value-class/changes/B2.scala              |  3 +++
 .../value-class/changes/C2.scala              |  3 +++
 .../source-dependencies/value-class/test      | 20 +++++++++++++++++++
 4 files changed, 32 insertions(+), 12 deletions(-)
 create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/B2.scala
 create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/C2.scala

diff --git a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala
index 31389218b..7a0a78b2c 100644
--- a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala
+++ b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala
@@ -208,20 +208,14 @@ class ExtractAPI[GlobalType <: CallbackGlobal](val global: GlobalType,
         s.asMethod.paramss.flatten map (_.info) exists (t => isAnyValSubtype(t.typeSymbol))
       }
 
-      // Note: We only inspect the "outermost type" (i.e. no recursion) because we don't need to
-      // inspect after erasure a function that would, for instance, return a function that returns
-      // a subtype of AnyVal.
-      val hasValueClassAsReturnType: Boolean = {
-        val tpe = viewer(in).memberInfo(s)
-        tpe match {
-          case PolyType(_, base) => isAnyValSubtype(base.typeSymbol)
-          case MethodType(_, resultType) => isAnyValSubtype(resultType.typeSymbol)
-          case Nullary(resultType) => isAnyValSubtype(resultType.typeSymbol)
-          case resultType => isAnyValSubtype(resultType.typeSymbol)
-        }
+      def hasValueClassAsReturnType(tpe: Type): Boolean = tpe match {
+        case PolyType(_, base) => hasValueClassAsReturnType(base)
+        case MethodType(_, resultType) => hasValueClassAsReturnType(resultType)
+        case Nullary(resultType) => hasValueClassAsReturnType(resultType)
+        case resultType => isAnyValSubtype(resultType.typeSymbol)
       }
 
-      val inspectPostErasure = hasValueClassAsParameter || hasValueClassAsReturnType
+      val inspectPostErasure = hasValueClassAsParameter || hasValueClassAsReturnType(viewer(in).memberInfo(s))
 
       def build(t: Type, typeParams: Array[xsbti.api.TypeParameter], valueParameters: List[xsbti.api.ParameterList]): List[xsbti.api.Def] =
         {
diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/B2.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/B2.scala
new file mode 100644
index 000000000..fe1136389
--- /dev/null
+++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/B2.scala
@@ -0,0 +1,3 @@
+class B {
+  def bar(dummy: String)(dummy2: String): A = new A(0)
+}
diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/C2.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/C2.scala
new file mode 100644
index 000000000..17b70f957
--- /dev/null
+++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/C2.scala
@@ -0,0 +1,3 @@
+object C extends App {
+  println(new B().bar("")("").x)
+}
diff --git a/sbt/src/sbt-test/source-dependencies/value-class/test b/sbt/src/sbt-test/source-dependencies/value-class/test
index f78acb2d8..268274bf6 100644
--- a/sbt/src/sbt-test/source-dependencies/value-class/test
+++ b/sbt/src/sbt-test/source-dependencies/value-class/test
@@ -1,3 +1,4 @@
+## Case 1: value class as parameter of method
 $ 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
@@ -13,6 +14,8 @@ $ copy-file changes/A1.scala src/main/scala/A.scala
 # This means that we have invalidated C.scala, as expected!
 -> compile
 
+
+## Case 2: value class as return type of method with no parameter lists
 $ 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
@@ -28,3 +31,20 @@ $ copy-file changes/A1.scala src/main/scala/A.scala
 # because A is now a value class.
 > run
 
+
+## Case 3: value class as return type of method with multiple parameter lists
+$ copy-file changes/A0.scala src/main/scala/A.scala
+$ copy-file changes/B2.scala src/main/scala/B.scala
+$ copy-file changes/C2.scala src/main/scala/C.scala
+
+# A is a normal class. B.bar takes two dummy 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
+