Misc cleanups (#360)

Misc cleanups
lightbend-labs · Jul 28, 2019 · e3148f3 · e3148f3
2 parents 72ec02f + bfcd2e5
commit e3148f3
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 97 deletions.
diff --git 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:[email protected]:lightbend/mima.git")),
   git.gitTagToVersionNumber := (tag => if (tag matches "[0.9]+\\..*") Some(tag) else None),
@@ -26,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,
 )
 
@@ -47,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,
   )
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
@@ -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,41 +51,48 @@ 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
 
     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/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`.
    *

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
@@ -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/core/src/test/scala/com/typesafe/tools/mima/core/ProblemFiltersSpec.scala b/core/src/test/scala/com/typesafe/tools/mima/core/ProblemFiltersSpec.scala
@@ -1,24 +1,23 @@
 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),
     (ProblemFilters.exclude[Problem]("impl.Http"), problem("impl.Http2"), true),
     (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
@@ -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 {

diff --git 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*"),

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
@@ -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