Skip to content

Commit

Permalink
Merge pull request #507 from raboof/move-signature-logic
Browse files Browse the repository at this point in the history
Move signature logic to its own class
  • Loading branch information
mergify[bot] authored May 26, 2020
2 parents f417373 + c321096 commit 4546f6a
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 83 deletions.
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

0 comments on commit 4546f6a

Please sign in to comment.