Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move signature logic to its own class #507

Merged
merged 2 commits into from
May 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ final class ClassfileParser private (in: BufferReader, pool: ConstantPool) {
val attrEnd = in.bp + attrLen
pool.getName(attrIndex) match {
case "Deprecated" => m.isDeprecated = true
case "Signature" => m.signature = pool.getName(in.nextChar)
case "Signature" => m.signature = Signature(pool.getName(in.nextChar))
case _ => ()
}
in.bp = attrEnd
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ sealed abstract class MemberInfo(val owner: ClassInfo, val bytecodeName: String,
extends InfoLike
{
final var isDeprecated: Boolean = false
final var signature: String = "" // Includes generics. 'descriptor' is the erased version.
final var signature: Signature = Signature.none // Includes generics. 'descriptor' is the erased version.

def nonAccessible: Boolean

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,12 @@ sealed abstract class Problem extends ProblemRef {
case ReversedMissingMethodProblem(ref) => s"${ref.memberString} is present only in $affectedVersion version"
case FinalMethodProblem(ref) => s"${ref.methodString} is declared final in $affectedVersion version"
case IncompatibleResultTypeProblem(ref, newmeth) => s"${ref.methodString} has a different result type in $affectedVersion version, where it is ${newmeth.tpe.resultType} rather than ${ref.tpe.resultType}"
case IncompatibleSignatureProblem(ref, newmeth) => s"${ref.methodString} has a different generic signature in $affectedVersion version, where it is ${orMIA(newmeth.signature)} rather than ${orMIA(ref.signature)}. See https://github.com/lightbend/mima#incompatiblesignatureproblem"
case IncompatibleSignatureProblem(ref, newmeth) => s"${ref.methodString} has a different generic signature in $affectedVersion version, where it is ${newmeth.signature} rather than ${ref.signature}. See https://github.com/lightbend/mima#incompatiblesignatureproblem"
case DirectAbstractMethodProblem(ref) => s"${ref.methodString} does not have a correspondent in $affectedVersion version"
case ReversedAbstractMethodProblem(ref) => s"in $affectedVersion version there is ${ref.methodString}, which does not have a correspondent"
case UpdateForwarderBodyProblem(ref) => s"in $affectedVersion version, classes mixing ${ref.owner.fullName} needs to update body of ${ref.shortMethodString}"
case InheritedNewAbstractMethodProblem(absmeth, ref) => s"${absmeth.methodString} is inherited by class ${ref.owner.bytecodeName} in $affectedVersion version."
}

// a method that takes no parameters and returns Object can have no signature
private def orMIA(s: String) = if (s.isEmpty) "<missing>" else s
}

// Template problems
Expand Down
76 changes: 76 additions & 0 deletions core/src/main/scala/com/typesafe/tools/mima/core/Signature.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package com.typesafe.tools.mima.core

import scala.annotation.tailrec

class Signature(private val signature: String) {
import Signature._

lazy val canonicalized = {
signature.headOption match {
case None | Some('(') => signature
case _ =>
val (formalTypeParameters, rest) = FormalTypeParameter.parseList(signature.drop(1))
val replacements = formalTypeParameters.map(_.identifier).zipWithIndex
replacements.foldLeft(signature) { case (sig, (from, to)) =>
sig
.replace(s"<${from}:", s"<__${to}__:")
.replace(s";${from}:", s";__${to}__:")
.replace(s"T${from};", s"__${to}__") }
}
}

def matches(newer: Signature, isConstructor: Boolean): Boolean = {
return (signature == newer.signature) ||
(isConstructor && hasMatchingCtorSig(newer.signature)) ||
canonicalized == newer.canonicalized
}

// Special case for scala#7975
private def hasMatchingCtorSig(newer: String): Boolean =
newer.isEmpty || // ignore losing signature on constructors
signature.endsWith(newer.tail) // ignore losing the 1st (outer) param (.tail drops the leading '(')

// a method that takes no parameters and returns Object can have no signature
override def toString = if (signature.isEmpty) "<missing>" else signature
}

object Signature {
def apply(signature: String): Signature = new Signature(signature)

val none = Signature("")

case class FormalTypeParameter(identifier: String, bound: String)
object FormalTypeParameter {
def parseList(in: String, listSoFar: List[FormalTypeParameter] = Nil): (List[FormalTypeParameter], String) = {
in(0) match {
case '>' => (listSoFar, in.drop(1))
case o => {
val (next, rest) = parseOne(in)
parseList(rest, listSoFar :+ next)
}
}
}
def parseOne(in: String): (FormalTypeParameter, String) = {
val identifier = in.takeWhile(_ != ':')
val boundAndRest = in.dropWhile(_ != ':').drop(1)
val (bound, rest) = splitBoundAndRest(boundAndRest)
(FormalTypeParameter(identifier, bound), rest)
}
@tailrec
private def splitBoundAndRest(in: String, boundSoFar: String = "", depth: Int = 0): (String, String) = {
if (depth > 0) {
in(0) match {
case '>' => splitBoundAndRest(in.drop(1), boundSoFar + '>', depth - 1)
case '<' => splitBoundAndRest(in.drop(1), boundSoFar + '<', depth + 1)
case o => splitBoundAndRest(in.drop(1), boundSoFar + o, depth)
}
} else {
in(0) match {
case '<' => splitBoundAndRest(in.drop(1), boundSoFar + '<', depth + 1)
case ';' => (boundSoFar, in.drop(1))
case o => splitBoundAndRest(in.drop(1), boundSoFar + o, depth)
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.typesafe.tools.mima.lib.analyze.method

import scala.annotation.tailrec

import com.typesafe.tools.mima.core._

private[analyze] object MethodChecker {
Expand Down Expand Up @@ -44,69 +42,9 @@ private[analyze] object MethodChecker {
}
}

private def hasMatchingDescriptorAndSignature(oldmeth: MethodInfo, newmeth: MethodInfo): Boolean = {
private def hasMatchingDescriptorAndSignature(oldmeth: MethodInfo, newmeth: MethodInfo): Boolean =
oldmeth.descriptor == newmeth.descriptor &&
hasMatchingSignature(oldmeth.signature, newmeth.signature, newmeth.bytecodeName)
}

def hasMatchingSignature(oldsig: String, newsig: String, bytecodeName: String): Boolean = {
def canonicalize(signature: String): String = {
signature.headOption match {
case None | Some('(') => signature
case _ =>
val (formalTypeParameters, rest) = FormalTypeParameter.parseList(signature.drop(1))
val replacements = formalTypeParameters.map(_.identifier).zipWithIndex
replacements.foldLeft(signature) { case (sig, (from, to)) =>
sig
.replace(s"<${from}:", s"<__${to}__:")
.replace(s";${from}:", s";__${to}__:")
.replace(s"T${from};", s"__${to}__") }
}
}

oldsig == newsig ||
canonicalize(oldsig) == canonicalize(newsig) || // Special case for scala#7975
bytecodeName == MemberInfo.ConstructorName && hasMatchingCtorSig(oldsig, newsig)
}

case class FormalTypeParameter(identifier: String, bound: String)
object FormalTypeParameter {
def parseList(in: String, listSoFar: List[FormalTypeParameter] = Nil): (List[FormalTypeParameter], String) = {
in(0) match {
case '>' => (listSoFar, in.drop(1))
case o => {
val (next, rest) = parseOne(in)
parseList(rest, listSoFar :+ next)
}
}
}
def parseOne(in: String): (FormalTypeParameter, String) = {
val identifier = in.takeWhile(_ != ':')
val boundAndRest = in.dropWhile(_ != ':').drop(1)
val (bound, rest) = splitBoundAndRest(boundAndRest)
(FormalTypeParameter(identifier, bound), rest)
}
@tailrec
private def splitBoundAndRest(in: String, boundSoFar: String = "", depth: Int = 0): (String, String) = {
if (depth > 0) {
in(0) match {
case '>' => splitBoundAndRest(in.drop(1), boundSoFar + '>', depth - 1)
case '<' => splitBoundAndRest(in.drop(1), boundSoFar + '<', depth + 1)
case o => splitBoundAndRest(in.drop(1), boundSoFar + o, depth)
}
} else {
in(0) match {
case '<' => splitBoundAndRest(in.drop(1), boundSoFar + '<', depth + 1)
case ';' => (boundSoFar, in.drop(1))
case o => splitBoundAndRest(in.drop(1), boundSoFar + o, depth)
}
}
}
}

private def hasMatchingCtorSig(oldsig: String, newsig: String): Boolean =
newsig.isEmpty || // ignore losing signature on constructors
oldsig.endsWith(newsig.tail) // ignore losing the 1st (outer) param (.tail drops the leading '(')
oldmeth.signature.matches(newmeth.signature, newmeth.bytecodeName == MemberInfo.ConstructorName)

private def checkExisting1v1(oldmeth: MethodInfo, newmeth: MethodInfo) = {
if (newmeth.isLessVisibleThan(oldmeth))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package com.typesafe.tools.mima.lib.analyze.method

import com.typesafe.tools.mima.core.MemberInfo
package com.typesafe.tools.mima.core

import org.scalatest.matchers.should.Matchers
import org.scalatest.wordspec.AnyWordSpec

final class MethodCheckerSpec extends AnyWordSpec with Matchers {
final class SignatureSpec extends AnyWordSpec with Matchers {
val promiseSig =
"Lscala/concurrent/Promise<" +
"Lscala/Function1<" +
Expand All @@ -14,33 +12,33 @@ final class MethodCheckerSpec extends AnyWordSpec with Matchers {
">;" +
">;"

val `signature_in_2.12.8` = s"(Lakka/http/impl/engine/server/GracefulTerminatorStage;$promiseSig)V"
val `signature_in_2.12.9` = s"($promiseSig)V"
val `signature_in_2.12.8` = Signature(s"(Lakka/http/impl/engine/server/GracefulTerminatorStage;$promiseSig)V")
val `signature_in_2.12.9` = Signature(s"($promiseSig)V")

"The method checker" should {
"allow dropping the first parameter of the Signature attribute of a constructor" in {
// Assuming the descriptor is the same,
// dropping the first parameter of the Signature attribute
// can only be explained by going from a Scala version that does not have
// the fix in scala#7975 (2.12.8, 2.13.0) to one that does
assert(MethodChecker.hasMatchingSignature(`signature_in_2.12.8`, `signature_in_2.12.9`, MemberInfo.ConstructorName))
assert(`signature_in_2.12.8`.matches(`signature_in_2.12.9`, true))
}

"reject adding the first parameter of the Signature attribute of a constructor back" in {
assert(!MethodChecker.hasMatchingSignature(`signature_in_2.12.9`, `signature_in_2.12.8`, MemberInfo.ConstructorName))
assert(!`signature_in_2.12.9`.matches(`signature_in_2.12.8`, true))
}

"allow renaming a generic parameter" in {
assert(MethodChecker.hasMatchingSignature(
"<U:Ljava/lang/Object;>(TU;Lscala/collection/immutable/List<TU;>;)Lscala/Option<TU;>;",
"<T:Ljava/lang/Object;>(TT;Lscala/collection/immutable/List<TT;>;)Lscala/Option<TT;>;",
"foo"))
val withU = Signature("<U:Ljava/lang/Object;>(TU;Lscala/collection/immutable/List<TU;>;)Lscala/Option<TU;>;")
val withT = Signature("<T:Ljava/lang/Object;>(TT;Lscala/collection/immutable/List<TT;>;)Lscala/Option<TT;>;")

assert(withU.matches(withT, false))
}
}

"The signature parser" should {
"parse a signature with generic bounds that themselves have generics" in {
val (types, rest) = MethodChecker.FormalTypeParameter.parseList(
val (types, rest) = Signature.FormalTypeParameter.parseList(
"T:Ljava/lang/Object;U:Lscala/collection/immutable/List<TT;>;>(TT;Lscala/collection/immutable/List<TU;>;)Lscala/Option<TT;>;>")
types.length should be(2)
types(0).identifier should be("T")
Expand Down