From 46058029b5ec0c809f29f77dcb59e8a25166d69f Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 4 Nov 2015 11:16:53 +0100 Subject: [PATCH] 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 --- .../src/main/scala/xsbt/Compat.scala | 7 +- .../src/main/scala/xsbt/ExtractAPI.scala | 69 ++++++++++++++----- .../source-dependencies/value-class/build.sbt | 2 + .../value-class/changes/A0.scala | 1 + .../value-class/changes/A1.scala | 1 + .../value-class/changes/B0.scala | 3 + .../value-class/changes/B1.scala | 3 + .../value-class/changes/C0.scala | 3 + .../value-class/changes/C1.scala | 3 + .../source-dependencies/value-class/test | 30 ++++++++ 10 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/build.sbt create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/changes/C1.scala create mode 100644 sbt/src/sbt-test/source-dependencies/value-class/test diff --git a/compile/interface/src/main/scala/xsbt/Compat.scala b/compile/interface/src/main/scala/xsbt/Compat.scala index 74116c0af..016658238 100644 --- a/compile/interface/src/main/scala/xsbt/Compat.scala +++ b/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 diff --git a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala b/compile/interface/src/main/scala/xsbt/ExtractAPI.scala index 5c2cbf61b..2caea4ba4 100644 --- a/compile/interface/src/main/scala/xsbt/ExtractAPI.scala +++ b/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) } -} \ No newline at end of file +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/build.sbt b/sbt/src/sbt-test/source-dependencies/value-class/build.sbt new file mode 100644 index 000000000..e850154ac --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/build.sbt @@ -0,0 +1,2 @@ +incOptions := incOptions.value.withRecompileAllFraction(1.0) + diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala new file mode 100644 index 000000000..ad5bf4c56 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/A0.scala @@ -0,0 +1 @@ +class A(val x: Int) diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala new file mode 100644 index 000000000..dbaa1c3f0 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/A1.scala @@ -0,0 +1 @@ +class A(val x: Int) extends AnyVal diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala new file mode 100644 index 000000000..0dba978c3 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/B0.scala @@ -0,0 +1,3 @@ +class B { + def foo(a: A): Int = 1 +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala new file mode 100644 index 000000000..c7b689c5e --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/B1.scala @@ -0,0 +1,3 @@ +class B { + def bar: A = new A(0) +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala new file mode 100644 index 000000000..c656b25a4 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/C0.scala @@ -0,0 +1,3 @@ +object C extends App { + println(new B().foo(null)) +} diff --git a/sbt/src/sbt-test/source-dependencies/value-class/changes/C1.scala b/sbt/src/sbt-test/source-dependencies/value-class/changes/C1.scala new file mode 100644 index 000000000..ad8384312 --- /dev/null +++ b/sbt/src/sbt-test/source-dependencies/value-class/changes/C1.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 new file mode 100644 index 000000000..f78acb2d8 --- /dev/null +++ b/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 +