Skip to content

Commit

Permalink
Merge pull request lightbend-labs#378 from lightbend/ignoreWhenFirstC…
Browse files Browse the repository at this point in the history
…onstructorParam..

Ignore when first constructor parameter is dropped from Signature
  • Loading branch information
dwijnand authored Aug 22, 2019
2 parents fc2b616 + 6eefdc7 commit d726234
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 137 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.typesafe.tools.mima.lib.analyze.Checker

private[analyze] abstract class BaseMethodChecker extends Checker[MethodInfo, ClassInfo] {
import MethodRules._
import BaseMethodChecker._

protected val rules = Seq(AccessModifier, FinalModifier, AbstractModifier, JavaStatic)

Expand All @@ -13,7 +14,7 @@ private[analyze] abstract class BaseMethodChecker extends Checker[MethodInfo, Cl
if (meths.isEmpty)
Some(DirectMissingMethodProblem(method))
else {
meths.find(m => m.descriptor == method.descriptor && methSigCheck(method, m)) match {
meths.find(newMethod => hasMatchingDescriptorAndSignature(method, newMethod)) match {
case Some(found) => checkRules(rules)(method, found)
case None =>
val filtered = meths.filter(method.matchesType(_))
Expand All @@ -34,15 +35,24 @@ private[analyze] abstract class BaseMethodChecker extends Checker[MethodInfo, Cl
}
}

private def methSigCheck(oldmeth: MethodInfo, newMeth: MethodInfo): Boolean = {
oldmeth.signature == newMeth.signature || (
newMeth.bytecodeName == MemberInfo.ConstructorName && newMeth.signature.isEmpty
)
}

private def uniques(methods: List[MethodInfo]): List[MethodInfo] =
methods.groupBy(_.parametersDesc).values.map(_.head).toList
}
private[analyze] object BaseMethodChecker {
def hasMatchingDescriptorAndSignature(oldMethod: MethodInfo, newMethod: MethodInfo): Boolean =
oldMethod.descriptor == newMethod.descriptor && hasMatchingSignature(oldMethod.signature, newMethod.signature, newMethod.bytecodeName)

// Assumes it is already checked that the descriptors match
def hasMatchingSignature(oldSignature: String, newSignature: String, bytecodeName: String): Boolean =
oldSignature == newSignature ||
// Special case for https://github.com/scala/scala/pull/7975:
(bytecodeName == MemberInfo.ConstructorName &&
(newSignature.isEmpty ||
// The dropped character is the leading '('
oldSignature.endsWith(newSignature.tail)
)
)
}

private[analyze] class ClassMethodChecker extends BaseMethodChecker {
def check(method: MethodInfo, inclazz: ClassInfo): Option[Problem] = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.typesafe.tools.mima.lib.analyze.method

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

import org.scalatest.Matchers
import org.scalatest.WordSpec

final class MethodCheckerSpec extends WordSpec with Matchers {
"The method checker" should {
val `signatureOn2.12.8` =
"(Lakka/http/impl/engine/server/GracefulTerminatorStage;Lscala/concurrent/Promise<Lscala/Function1<Lscala/concurrent/duration/FiniteDuration;Lscala/concurrent/Future<Lakka/http/scaladsl/Http$HttpTerminated;>;>;>;)V"
val `signatureOn2.12.9` =
"(Lscala/concurrent/Promise<Lscala/Function1<Lscala/concurrent/duration/FiniteDuration;Lscala/concurrent/Future<Lakka/http/scaladsl/Http$HttpTerminated;>;>;>;)V"

"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
// https://github.com/scala/scala/pull/7975 (2.12.8, 2.13.0) to
// one that does
BaseMethodChecker.hasMatchingSignature(
`signatureOn2.12.8`,
`signatureOn2.12.9`,
MemberInfo.ConstructorName
) should be(true)
}

"reject adding the first parameter of the Signature attribute of a constructor back" in {
BaseMethodChecker.hasMatchingSignature(
`signatureOn2.12.9`,
`signatureOn2.12.8`,
MemberInfo.ConstructorName
) should be(false)
}
}
}
Loading

0 comments on commit d726234

Please sign in to comment.