diff --git a/core/src/main/scala/com/typesafe/tools/mima/lib/analyze/method/MethodChecker.scala b/core/src/main/scala/com/typesafe/tools/mima/lib/analyze/method/MethodChecker.scala index af0de470..3d62e864 100644 --- a/core/src/main/scala/com/typesafe/tools/mima/lib/analyze/method/MethodChecker.scala +++ b/core/src/main/scala/com/typesafe/tools/mima/lib/analyze/method/MethodChecker.scala @@ -1,5 +1,7 @@ package com.typesafe.tools.mima.lib.analyze.method +import scala.annotation.tailrec + import com.typesafe.tools.mima.core._ private[analyze] object MethodChecker { @@ -47,12 +49,62 @@ private[analyze] object MethodChecker { hasMatchingSignature(oldmeth.signature, newmeth.signature, newmeth.bytecodeName) } - private def hasMatchingSignature(oldsig: String, newsig: String, bytecodeName: String): Boolean = { - oldsig == newsig || // Special case for scala#7975 + 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) } - private[analyze] def hasMatchingCtorSig(oldsig: String, newsig: String): Boolean = + 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 '(') diff --git a/core/src/test/scala/com/typesafe/tools/mima/lib/analyze/method/MethodCheckerSpec.scala b/core/src/test/scala/com/typesafe/tools/mima/lib/analyze/method/MethodCheckerSpec.scala index 0ce5e128..f4d2b9db 100644 --- a/core/src/test/scala/com/typesafe/tools/mima/lib/analyze/method/MethodCheckerSpec.scala +++ b/core/src/test/scala/com/typesafe/tools/mima/lib/analyze/method/MethodCheckerSpec.scala @@ -1,5 +1,7 @@ package com.typesafe.tools.mima.lib.analyze.method +import com.typesafe.tools.mima.core.MemberInfo + import org.scalatest.matchers.should.Matchers import org.scalatest.wordspec.AnyWordSpec @@ -21,11 +23,31 @@ final class MethodCheckerSpec extends AnyWordSpec with Matchers { // 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.hasMatchingCtorSig(`signature_in_2.12.8`, `signature_in_2.12.9`)) + assert(MethodChecker.hasMatchingSignature(`signature_in_2.12.8`, `signature_in_2.12.9`, MemberInfo.ConstructorName)) } "reject adding the first parameter of the Signature attribute of a constructor back" in { - assert(!MethodChecker.hasMatchingCtorSig(`signature_in_2.12.9`, `signature_in_2.12.8`)) + assert(!MethodChecker.hasMatchingSignature(`signature_in_2.12.9`, `signature_in_2.12.8`, MemberInfo.ConstructorName)) + } + + "allow renaming a generic parameter" in { + assert(MethodChecker.hasMatchingSignature( + "(TU;Lscala/collection/immutable/List;)Lscala/Option;", + "(TT;Lscala/collection/immutable/List;)Lscala/Option;", + "foo")) + } + } + + "The signature parser" should { + "parse a signature with generic bounds that themselves have generics" in { + val (types, rest) = MethodChecker.FormalTypeParameter.parseList( + "T:Ljava/lang/Object;U:Lscala/collection/immutable/List;>(TT;Lscala/collection/immutable/List;)Lscala/Option;>") + types.length should be(2) + types(0).identifier should be("T") + types(0).bound should be("Ljava/lang/Object") + types(1).identifier should be("U") + types(1).bound should be("Lscala/collection/immutable/List") + rest should be("(TT;Lscala/collection/immutable/List;)Lscala/Option;>") } } } diff --git a/functional-tests/src/test/class-method-generics-nok/problems.txt b/functional-tests/src/test/class-method-generics-nok/problems.txt index 0085964e..ed9cc5be 100644 --- a/functional-tests/src/test/class-method-generics-nok/problems.txt +++ b/functional-tests/src/test/class-method-generics-nok/problems.txt @@ -1,4 +1,3 @@ -method genericWithChangingName()scala.Option in class A has a different generic signature in new version, where it is ()Lscala/Option; rather than ()Lscala/Option;. See https://github.com/lightbend/mima#incompatiblesignatureproblem method backwardsCompatibleNarrowing()scala.Option in class A has a different generic signature in new version, where it is ()Lscala/Option; rather than ()Lscala/Option;. See https://github.com/lightbend/mima#incompatiblesignatureproblem # method cov1()java.lang.Object in class Api has a different generic signature in new version, where it is ()TT; rather than . See https://github.com/lightbend/mima#incompatiblesignatureproblem diff --git a/functional-tests/src/test/class-method-generics-nok/v2/A.scala b/functional-tests/src/test/class-method-generics-nok/v2/A.scala index b5d732ec..3de34e2b 100644 --- a/functional-tests/src/test/class-method-generics-nok/v2/A.scala +++ b/functional-tests/src/test/class-method-generics-nok/v2/A.scala @@ -1,6 +1,6 @@ class A { - def genericWithChangingName[T]: Option[T] = ??? - def backwardsCompatibleNarrowing: Option[String] = ??? + def genericWithChangingName[T]: Option[T] = ??? // OK + def backwardsCompatibleNarrowing: Option[String] = ??? // KO } final class Api { diff --git a/functional-tests/src/test/type-parameter-names-change-ok/app/App.scala b/functional-tests/src/test/type-parameter-names-change-ok/app/App.scala new file mode 100644 index 00000000..2cc914c1 --- /dev/null +++ b/functional-tests/src/test/type-parameter-names-change-ok/app/App.scala @@ -0,0 +1,6 @@ +object App { + def main(args: Array[String]): Unit = { + val x: Option[String] = new Tree().someMethod("foo", List(42)) + println(x) + } +} diff --git a/functional-tests/src/test/type-parameter-names-change-ok/problems.txt b/functional-tests/src/test/type-parameter-names-change-ok/problems.txt new file mode 100644 index 00000000..e69de29b diff --git a/functional-tests/src/test/type-parameter-names-change-ok/v1/Tree.scala b/functional-tests/src/test/type-parameter-names-change-ok/v1/Tree.scala new file mode 100644 index 00000000..1518ff76 --- /dev/null +++ b/functional-tests/src/test/type-parameter-names-change-ok/v1/Tree.scala @@ -0,0 +1,3 @@ +class Tree { + def someMethod[T,U](e: T, l: List[U]): Option[T] = Some(e) +} diff --git a/functional-tests/src/test/type-parameter-names-change-ok/v2/Tree.scala b/functional-tests/src/test/type-parameter-names-change-ok/v2/Tree.scala new file mode 100644 index 00000000..0b0790b2 --- /dev/null +++ b/functional-tests/src/test/type-parameter-names-change-ok/v2/Tree.scala @@ -0,0 +1,3 @@ +class Tree { + def someMethod[X,Y](e: X, l: List[Y]): Option[X] = Some(e) +} diff --git a/project/MimaSettings.scala b/project/MimaSettings.scala index c30d3c8a..c8f115d7 100644 --- a/project/MimaSettings.scala +++ b/project/MimaSettings.scala @@ -34,6 +34,7 @@ object MimaSettings { exclude[Problem]("*mima.plugin.SbtLogger*"), exclude[Problem]("*mima.plugin.SbtMima.*"), exclude[Problem]("scala.tools.nsc.mima*"), + exclude[DirectMissingMethodProblem]("*mima.lib.analyze.method.MethodChecker.hasMatchingCtorSig"), ), ) }