From 00f72b435d0035d3f1c6372bba95dfc7f9ed27b6 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 26 Jul 2019 07:04:53 +0100 Subject: [PATCH 1/6] Type: cleanup --- .../tools/mima/core/Definitions.scala | 58 +++++++++++-------- .../com/typesafe/tools/mima/core/Type.scala | 52 ++++++----------- project/MimaSettings.scala | 3 +- 3 files changed, 54 insertions(+), 59 deletions(-) diff --git a/core/src/main/scala/com/typesafe/tools/mima/core/Definitions.scala b/core/src/main/scala/com/typesafe/tools/mima/core/Definitions.scala index 7fee3efb..d8a0e9ea 100644 --- a/core/src/main/scala/com/typesafe/tools/mima/core/Definitions.scala +++ b/core/src/main/scala/com/typesafe/tools/mima/core/Definitions.scala @@ -2,6 +2,7 @@ package com.typesafe.tools.mima.core import scala.tools.nsc.classpath.AggregateClassPath import scala.tools.nsc.util.ClassPath +import scala.tools.nsc.symtab.classfile.ClassfileConstants._ /** This class holds together a root package and a classpath. It * also offers definitions of commonly used classes, including @@ -50,10 +51,8 @@ class Definitions(val lib: Option[ClassPath], val classPath: ClassPath) { pkg.classes getOrElse (part, new SyntheticClassInfo(pkg, part)) } - import Type._ - - /** Return the type corresponding to this descriptor. Class names are resolved - * relative to the current classpath. + /** Return the type corresponding to this descriptor. + * Class names are resolved relative to the current classpath. */ def fromDescriptor(descriptor: String): Type = { var in = 0 @@ -61,30 +60,39 @@ class Definitions(val lib: Option[ClassPath], val classPath: ClassPath) { def getType(): Type = { val ch = descriptor(in) in += 1 - abbrevToValueType get ch match { - case Some(tp) => - tp - case None => - if (ch == '[') { - ArrayType(getType()) - } else if (ch == 'L') { - val end = descriptor indexOf (';', in) - val fullname = descriptor.substring(in, end) replace ('/', '.') - in = end + 1 - ClassType(fromName(fullname)) - } else if (ch == '(') { - val params = getParamTypes() - in += 1 - MethodType(params, getType()) - } else { - throw new MatchError("unknown signature: "+descriptor.substring(in)) - } + ch match { + case VOID_TAG => Type.unitType + case BOOL_TAG => Type.booleanType + case BYTE_TAG => Type.byteType + case SHORT_TAG => Type.shortType + case CHAR_TAG => Type.charType + case INT_TAG => Type.intType + case LONG_TAG => Type.longType + case FLOAT_TAG => Type.floatType + case DOUBLE_TAG => Type.doubleType + case OBJECT_TAG => newClassType + case ARRAY_TAG => ArrayType(getType()) + case '(' => newMethodType + case _ => throw new MatchError(s"unknown signature: ${descriptor.substring(in)}") } } - def getParamTypes(): List[Type] = - if (descriptor(in) == ')') List() - else getType() :: getParamTypes() + def newClassType: ClassType = { + val end = descriptor.indexOf(';', in) + val fullName = descriptor.substring(in, end).replace('/', '.') + in = end + 1 + ClassType(fromName(fullName)) + } + + def newMethodType: MethodType = { + def getParamTypes(): List[Type] = { + if (descriptor(in) == ')') Nil + else getType() :: getParamTypes() + } + val params = getParamTypes() + in += 1 + MethodType(params, getType()) + } getType() } diff --git a/core/src/main/scala/com/typesafe/tools/mima/core/Type.scala b/core/src/main/scala/com/typesafe/tools/mima/core/Type.scala index 652d64d0..b82d44ca 100644 --- a/core/src/main/scala/com/typesafe/tools/mima/core/Type.scala +++ b/core/src/main/scala/com/typesafe/tools/mima/core/Type.scala @@ -1,43 +1,29 @@ package com.typesafe.tools.mima.core object Type { - - val byteType = ValueType("Byte") - val shortType = ValueType("Short") - val charType = ValueType("Char") - val intType = ValueType("Int") - val longType = ValueType("Long") - val floatType = ValueType("Float") - val doubleType = ValueType("Double") + val byteType = ValueType("Byte") + val shortType = ValueType("Short") + val charType = ValueType("Char") + val intType = ValueType("Int") + val longType = ValueType("Long") + val floatType = ValueType("Float") + val doubleType = ValueType("Double") val booleanType = ValueType("Boolean") - val unitType = ValueType("Unit") - - val abbrevToValueType = Map( - 'B' -> byteType, - 'S' -> shortType, - 'C' -> charType, - 'I' -> intType, - 'J' -> longType, - 'F' -> floatType, - 'D' -> doubleType, - 'Z' -> booleanType, - 'V' -> unitType) + val unitType = ValueType("Unit") } -abstract class Type { +sealed abstract class Type { def resultType: Type = throw new UnsupportedOperationException -} -case class ValueType(name: String) extends Type { - override def toString = name -} -case class ClassType(private val clazz: ClassInfo) extends Type { - override def toString = ClassInfo.formatClassName(clazz.fullName) -} -case class ArrayType(elemType: Type) extends Type { - override def toString = "Array["+elemType+"]" -} -case class MethodType(paramTypes: List[Type], override val resultType: Type) extends Type { - override def toString = paramTypes.mkString("(", ",", ")"+resultType) + override def toString = this match { + case ValueType(name) => name + case ClassType(clazz) => ClassInfo.formatClassName(clazz.fullName) // formattedFullName? + case ArrayType(elemType) => s"Array[$elemType]" + case MethodType(paramTypes, resTpe) => paramTypes.mkString("(", ",", s")$resTpe") + } } +final case class ValueType(name: String) extends Type +final case class ClassType(private val clazz: ClassInfo) extends Type +final case class ArrayType(elemType: Type) extends Type +final case class MethodType(paramTypes: List[Type], override val resultType: Type) extends Type diff --git a/project/MimaSettings.scala b/project/MimaSettings.scala index 23700156..f5eef838 100644 --- a/project/MimaSettings.scala +++ b/project/MimaSettings.scala @@ -64,8 +64,9 @@ object MimaSettings { ProblemFilters.exclude[Problem]("*mima.core.UTF8Codec*"), // Dropped dead code internal to types ProblemFilters.exclude[Problem]("*mima.core.Type*"), - ProblemFilters.exclude[Problem]("*mima.core.ClassType*"), ProblemFilters.exclude[Problem]("*mima.core.ArrayType*"), + ProblemFilters.exclude[Problem]("*mima.core.ClassType*"), + ProblemFilters.exclude[Problem]("*mima.core.MethodType*"), ProblemFilters.exclude[Problem]("*mima.core.ValueType*"), // Dropped dead code internal to the info classes ProblemFilters.exclude[Problem]("*mima.core.Definitions*"), From 2949a4c72314148b74ea5d45eae847e581265dd6 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 26 Jul 2019 06:42:45 +0100 Subject: [PATCH 2/6] MimaPlugin: cleanup --- .../tools/mima/plugin/MimaPlugin.scala | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/sbtplugin/src/main/scala/com/typesafe/tools/mima/plugin/MimaPlugin.scala b/sbtplugin/src/main/scala/com/typesafe/tools/mima/plugin/MimaPlugin.scala index 2a85d795..aed619e0 100644 --- a/sbtplugin/src/main/scala/com/typesafe/tools/mima/plugin/MimaPlugin.scala +++ b/sbtplugin/src/main/scala/com/typesafe/tools/mima/plugin/MimaPlugin.scala @@ -60,32 +60,32 @@ object MimaPlugin extends AutoPlugin { mimaFiltersDirectory := (sourceDirectory in Compile).value / "mima-filters", ) - /** Setup mima with default settings, applicable for most projects. */ + /** Setup MiMa with default settings, applicable for most projects. */ def mimaDefaultSettings: Seq[Setting[_]] = globalSettings ++ buildSettings ++ projectSettings // Allows reuse between mimaFindBinaryIssues and mimaReportBinaryIssues // without blowing up the Akka build's heap private def binaryIssuesIterator = Def.task { val s = streams.value - val projectName = name.value - mimaPreviousClassfiles.value match { - case _: NoPreviousClassfiles.type => - val msg = s"$projectName: mimaPreviousArtifacts not set, not analyzing binary compatibility." - if (mimaFailOnNoPrevious.value) sys.error(msg) - else s.log.info(msg) - Iterator.empty - case previousClassfiles if previousClassfiles.isEmpty => - s.log.info(s"$projectName: mimaPreviousArtifacts is empty, not analyzing binary compatibility.") - Iterator.empty - case previousClassfiles => - val currentClassfiles = mimaCurrentClassfiles.value - val cp = (fullClasspath in mimaFindBinaryIssues).value - val checkDirection = mimaCheckDirection.value - val log = new SbtLogger(s) - previousClassfiles.iterator.map { case (moduleId, file) => - val problems = SbtMima.runMima(file, currentClassfiles, cp, checkDirection, log) - (moduleId, (problems._1, problems._2)) - } + val previousClassfiles = mimaPreviousClassfiles.value + + if (previousClassfiles.isEmpty) { + val projectName = name.value + var msg = s"$projectName: mimaPreviousArtifacts is empty, not analyzing binary compatibility." + if (previousClassfiles eq NoPreviousClassfiles) { + msg = s"$projectName: mimaPreviousArtifacts not set, not analyzing binary compatibility." + if (mimaFailOnNoPrevious.value) + sys.error(msg) + } + s.log.info(msg) + } + + val currentClassfiles = mimaCurrentClassfiles.value + val cp = (fullClasspath in mimaFindBinaryIssues).value + val log = new SbtLogger(s) + + previousClassfiles.iterator.map { case (moduleId, prevClassfiles) => + moduleId -> SbtMima.runMima(prevClassfiles, currentClassfiles, cp, mimaCheckDirection.value, log) } } @@ -110,7 +110,6 @@ object MimaPlugin extends AutoPlugin { def + [V1 >: V](kv: (K, V1)) = updated(kv._1, kv._2) def - (key: K) = this - override def size = 0 override def contains(key: K) = false override def getOrElse[V1 >: V](key: K, default: => V1) = default From 58bed7a3f25da11e60b4f21305217f973d1d5be2 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 24 Jul 2019 07:50:54 +0100 Subject: [PATCH 3/6] ProblemFilters: cleanup --- .../typesafe/tools/mima/core/ProblemFilters.scala | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/core/src/main/scala/com/typesafe/tools/mima/core/ProblemFilters.scala b/core/src/main/scala/com/typesafe/tools/mima/core/ProblemFilters.scala index b1b1bc3c..528b31c4 100644 --- a/core/src/main/scala/com/typesafe/tools/mima/core/ProblemFilters.scala +++ b/core/src/main/scala/com/typesafe/tools/mima/core/ProblemFilters.scala @@ -7,19 +7,22 @@ object ProblemFilters { private case class ExcludeByName[P <: ProblemRef: ClassTag](name: String) extends ProblemFilter { private[this] val pattern = Pattern.compile(name.split("\\*", -1).map(Pattern.quote).mkString(".*")) + private[this] val cls = classTag[P].runtimeClass override def apply(problem: Problem): Boolean = { - !(classTag[P].runtimeClass.isAssignableFrom(problem.getClass) && - pattern.matcher(problem.matchName.getOrElse("")).matches) + !(cls.isAssignableFrom(problem.getClass) && pattern.matcher(problem.matchName.getOrElse("")).matches) } - override def toString() = s"""ExcludeByName[${classTag[P].runtimeClass.getSimpleName}]("$name")""" + override def toString() = s"""ExcludeByName[${cls.getSimpleName}]("$name")""" } + /** Creates an exclude filter by taking the type of the problem and the name of a match + * (such as the class or method name). + */ def exclude[P <: ProblemRef: ClassTag](name: String): ProblemFilter = ExcludeByName[P](name) - /** - * Creates exclude filter by taking name of a problem and name of a match (e.g. class/method name). + /** Creates an exclude filter by taking the name of the problem and the name of a match + * (such as the class or method name). * * The problemName is name of a class corresponding to a problem like `AbstractMethodProblem`. * From 013e535b9b681b8242be65001b15f693dfb1d836 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 23 Jul 2019 22:32:01 +0100 Subject: [PATCH 4/6] build.sbt add myself as a developer --- build.sbt | 1 + 1 file changed, 1 insertion(+) diff --git a/build.sbt b/build.sbt index a258100d..ecef02a9 100644 --- a/build.sbt +++ b/build.sbt @@ -7,6 +7,7 @@ inThisBuild(Seq( developers := List( Developer("mdotta", "Mirco Dotta", "@dotta", url("https://github.com/dotta")), Developer("jsuereth", "Josh Suereth", "@jsuereth", url("https://github.com/jsuereth")), + Developer("dwijnand", "Dale Wijnand", "@dwijnand", url("https://github.com/dwijnand")), ), scmInfo := Some(ScmInfo(url("https://github.com/lightbend/mima"), "scm:git:git@github.com:lightbend/mima.git")), git.gitTagToVersionNumber := (tag => if (tag matches "[0.9]+\\..*") Some(tag) else None), From f40add890206eb16de65f9ad7f79fec9fab24246 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 23 Jul 2019 22:33:11 +0100 Subject: [PATCH 5/6] build: Move hocon dep to functionalTests --- build.sbt | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/build.sbt b/build.sbt index ecef02a9..405f3777 100644 --- a/build.sbt +++ b/build.sbt @@ -27,9 +27,10 @@ aggregateProjects(core, sbtplugin, functionalTests) val core = project.disablePlugins(BintrayPlugin).settings( name := "mima-core", - libraryDependencies += "com.typesafe" % "config" % "1.3.4", - libraryDependencies += "org.scala-lang" % "scala-compiler" % scalaVersion.value, - libraryDependencies += "org.scalatest" %% "scalatest" % "3.1.0-SNAP13" % Test, + libraryDependencies ++= Seq( + "org.scala-lang" % "scala-compiler" % scalaVersion.value, + "org.scalatest" %% "scalatest" % "3.1.0-SNAP13" % Test, + ), MimaSettings.mimaSettings, ) @@ -48,7 +49,10 @@ val functionalTests = Project("functional-tests", file("functional-tests")) .enablePlugins(TestsPlugin) .disablePlugins(BintrayPlugin) .settings( - libraryDependencies += "org.scala-lang.modules" %% "scala-collection-compat" % "2.1.1", + libraryDependencies ++= Seq( + "com.typesafe" % "config" % "1.3.4", + "org.scala-lang.modules" %% "scala-collection-compat" % "2.1.1", + ), mimaFailOnNoPrevious := false, skip in publish := true, ) From bfcd2e5318a12a2aba17316354796d851164cfa3 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 24 Jul 2019 06:39:25 +0100 Subject: [PATCH 6/6] ProblemReporting/ProblemFiltersSpec: use AnyWordSpec --- .../typesafe/tools/mima/core/ProblemFiltersSpec.scala | 11 +++++------ .../tools/mima/core/ProblemReportingSpec.scala | 6 ++++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/core/src/test/scala/com/typesafe/tools/mima/core/ProblemFiltersSpec.scala b/core/src/test/scala/com/typesafe/tools/mima/core/ProblemFiltersSpec.scala index b562cfa8..c478696b 100644 --- a/core/src/test/scala/com/typesafe/tools/mima/core/ProblemFiltersSpec.scala +++ b/core/src/test/scala/com/typesafe/tools/mima/core/ProblemFiltersSpec.scala @@ -1,9 +1,10 @@ package com.typesafe.tools.mima.core -import org.scalatest._ +import org.scalatest.Matchers import org.scalatest.prop.TableDrivenPropertyChecks +import org.scalatest.wordspec.AnyWordSpec -class ProblemFiltersSpec extends WordSpec with TableDrivenPropertyChecks with Matchers { +final class ProblemFiltersSpec extends AnyWordSpec with TableDrivenPropertyChecks with Matchers { val filters = Table( ("filter", "problem", "realProblem"), (ProblemFilters.exclude[Problem]("impl.Http"), problem("impl.Http"), false), @@ -11,14 +12,12 @@ class ProblemFiltersSpec extends WordSpec with TableDrivenPropertyChecks with Ma (ProblemFilters.exclude[Problem]("impl.*"), problem("impl.Http"), false), (ProblemFilters.exclude[Problem]("impl.*"), problem("impl2.Http"), true), (ProblemFilters.exclude[Problem]("a$Impl*"), problem("a$Impl$B"), false), - (ProblemFilters.exclude[Problem]("a$Impl*"), problem("a2$Impl$B"), true) + (ProblemFilters.exclude[Problem]("a$Impl*"), problem("a2$Impl$B"), true), ) "problem filters" should { "filter problems" in { - forAll (filters) { (filter, problem, realProblem) => - filter(problem) shouldBe realProblem - } + forAll(filters) { (filter, problem, realProblem) => filter(problem) shouldBe realProblem } } } diff --git a/core/src/test/scala/com/typesafe/tools/mima/core/ProblemReportingSpec.scala b/core/src/test/scala/com/typesafe/tools/mima/core/ProblemReportingSpec.scala index 38d0f877..40c1bdc8 100644 --- a/core/src/test/scala/com/typesafe/tools/mima/core/ProblemReportingSpec.scala +++ b/core/src/test/scala/com/typesafe/tools/mima/core/ProblemReportingSpec.scala @@ -1,9 +1,11 @@ package com.typesafe.tools.mima.core import com.typesafe.tools.mima.core.util.log.Logging -import org.scalatest.{ Matchers, WordSpec } -class ProblemReportingSpec extends WordSpec with Matchers { +import org.scalatest.Matchers +import org.scalatest.wordspec.AnyWordSpec + +final class ProblemReportingSpec extends AnyWordSpec with Matchers { import ProblemReportingSpec._ "problem" should {