Support for backward/forward checks

This commit cleans up the Problem case classes, removing
mutable state and making the error messages parametric on
the affected version. The commit also includes new keys for
the sbt plugin, allowing the user to specify the comparison
direction, and separate filters for backward and forward
checks.

core/src/main/scala/com/typesafe/tools/mima/core/Problems.scala

@@ -1,12 +1,5 @@
 package com.typesafe.tools.mima.core
 
-object Problem {
-  object ClassVersion extends Enumeration {
-    val New = Value("new")
-    val Old = Value("old")
-  }
-}
-
 trait ProblemRef {
   type Ref
   def ref: Ref
@@ -35,8 +28,12 @@ trait MemberRef extends ProblemRef {
 }
 
 sealed abstract class Problem extends ProblemRef {
-  var affectedVersion = Problem.ClassVersion.New
-  def description: String
+  // each description accepts a name for the affected files,
+  // and generates the corresponding diagnostic message.
+  // For backward checking, the affected version is "current",
+  // while for forward checking it could be "other" or "previous",
+  // for example.
+  def description: String => String
 }
 
 abstract class TemplateProblem(override val ref: ClassInfo) extends Problem with TemplateRef {
@@ -48,87 +45,97 @@ abstract class MemberProblem(override val ref: MemberInfo) extends Problem with
 }
 
 case class MissingFieldProblem(oldfld: MemberInfo) extends MemberProblem(oldfld) {
-  def description = oldfld.fieldString + " does not have a correspondent in " + affectedVersion + " version"
+  def description = affectedVersion => oldfld.fieldString + " does not have a correspondent in " + affectedVersion + " version"
 }
 
 case class MissingMethodProblem(meth: MemberInfo) extends MemberProblem(meth) {
-  def description = (if (meth.isDeferred && !meth.owner.isTrait) "abstract " else "") + meth.methodString + " does not have a correspondent in " + affectedVersion + " version"
+  def description = affectedVersion => (if (meth.isDeferred && !meth.owner.isTrait) "abstract " else "") + meth.methodString + " does not have a correspondent in " + affectedVersion + " version"
+}
+
+case class ReversedMissingMethodProblem(meth: MemberInfo) extends MemberProblem(meth) {
+  def description = affectedVersion => (if (meth.isDeferred && !meth.owner.isTrait) "abstract " else "") + meth.methodString + " is present only in " + affectedVersion + " version"
 }
 
 case class UpdateForwarderBodyProblem(meth: MemberInfo) extends MemberProblem(meth) {
   assert(meth.owner.isTrait)
   assert(meth.owner.hasStaticImpl(meth))
 
-  def description = "classes mixing " + meth.owner.fullName + " needs to update body of " + meth.shortMethodString
+  def description = affectedVersion => "in " + affectedVersion + " version, classes mixing " + meth.owner.fullName + " needs to update body of " + meth.shortMethodString
 }
 
 case class MissingClassProblem(oldclazz: ClassInfo) extends TemplateProblem(oldclazz) {
-  def description = oldclazz.classString + " does not have a correspondent in " + affectedVersion + " version"
+  def description = affectedVersion => oldclazz.classString + " does not have a correspondent in " + affectedVersion + " version"
 }
 
 case class AbstractClassProblem(oldclazz: ClassInfo) extends TemplateProblem(oldclazz) {
-  def description = oldclazz.classString + " was concrete; is declared abstract in " + affectedVersion + " version"
+  def description = affectedVersion => oldclazz.classString + " was concrete; is declared abstract in " + affectedVersion + " version"
 }
 
 case class FinalClassProblem(oldclazz: ClassInfo) extends TemplateProblem(oldclazz) {
-  def description = oldclazz.classString + " is declared final in " + affectedVersion + " version"
+  def description = affectedVersion => oldclazz.classString + " is declared final in " + affectedVersion + " version"
 }
 
 case class FinalMethodProblem(newmemb: MemberInfo) extends MemberProblem(newmemb) {
-  def description = newmemb.methodString + " is declared final in " + affectedVersion + " version"
+  def description = affectedVersion => newmemb.methodString + " is declared final in " + affectedVersion + " version"
 }
 
 case class IncompatibleFieldTypeProblem(oldfld: MemberInfo, newfld: MemberInfo) extends MemberProblem(oldfld) {
-  def description = newfld.fieldString + "'s type has changed; was: " + oldfld.tpe + ", is now: " + newfld.tpe
+  def description = affectedVersion => newfld.fieldString + "'s type is different in " + affectedVersion + " version, where it is: " + newfld.tpe + " rather than: " + oldfld.tpe
 }
 
 case class IncompatibleMethTypeProblem(oldmeth: MemberInfo, newmeths: List[MemberInfo]) extends MemberProblem(oldmeth) {
-  def description = {
+  def description = affectedVersion => {
     oldmeth.methodString + (if (newmeths.tail.isEmpty)
-      "'s type has changed; was " + oldmeth.tpe + ", is now: " + newmeths.head.tpe
+      "'s type is different in " + affectedVersion + " version, where it is " + newmeths.head.tpe + " instead of " + oldmeth.tpe
     else
-      " does not have a correspondent with same parameter signature among " +
+      " in " + affectedVersion + " version does not have a correspondent with same parameter signature among " +
         (newmeths map (_.tpe) mkString ", "))
   }
 }
 
 case class IncompatibleResultTypeProblem(oldmeth: MemberInfo, newmeth: MemberInfo) extends MemberProblem(oldmeth) {
-  def description = {
-    oldmeth.methodString + " has now a different result type; was: " +
-      oldmeth.tpe.resultType + ", is now: " + newmeth.tpe.resultType
+  def description = affectedVersion => {
+    oldmeth.methodString + " has a different result type in " + affectedVersion + " version, where it is " + newmeth.tpe.resultType +
+       " rather than " + oldmeth.tpe.resultType
   }
 }
 
+// In some older code within Mima, the affectedVersion could be reversed. We split AbstractMethodProblem and MissingMethodProblem
+// into two, in case the affected version is the other one, rather than the current one. (reversed if forward check).
 case class AbstractMethodProblem(newmeth: MemberInfo) extends MemberProblem(newmeth) {
-  def description = "abstract " + newmeth.methodString + " does not have a correspondent in " + affectedVersion + " version"
+  def description = affectedVersion => "abstract " + newmeth.methodString + " does not have a correspondent in " + affectedVersion + " version"
+}
+
+case class ReversedAbstractMethodProblem(newmeth: MemberInfo) extends MemberProblem(newmeth) {
+  def description = affectedVersion => "in " + affectedVersion + " version there is abstract " + newmeth.methodString + ", which does not have a correspondent"
 }
 
 case class IncompatibleTemplateDefProblem(oldclazz: ClassInfo, newclazz: ClassInfo) extends TemplateProblem(oldclazz) {
-  def description = {
-    "declaration of " + oldclazz.description + " has changed to " + newclazz.description +
-      " in new version; changing " + oldclazz.declarationPrefix + " to " + newclazz.declarationPrefix + " breaks client code"
+  def description = affectedVersion => {
+    "declaration of " + oldclazz.description + " is " + newclazz.description +
+      " in " + affectedVersion + " version; changing " + oldclazz.declarationPrefix + " to " + newclazz.declarationPrefix + " breaks client code"
   }
 }
 
 case class MissingTypesProblem(newclazz: ClassInfo, missing: Iterable[ClassInfo]) extends TemplateProblem(newclazz) {
-  def description = "the type hierarchy of " + newclazz.description + " has changed in new version. " +
+  def description = affectedVersion => "the type hierarchy of " + newclazz.description + " is different in " + affectedVersion + " version. " +
     "Missing types " + missing.map(_.fullName).mkString("{", ",", "}")
 }
 
 case class CyclicTypeReferenceProblem(clz: ClassInfo) extends TemplateProblem(clz) {
-  def description = {
-    "the type hierarchy of " + clz.description + " has changed in new version. Type " + clz.bytecodeName + " appears to be a subtype of itself"
+  def description = affectedVersion => {
+    "the type hierarchy of " + clz.description + " is different in " + affectedVersion + " version. Type " + clz.bytecodeName + " appears to be a subtype of itself"
   }
 }
 
 case class InaccessibleFieldProblem(newfld: MemberInfo) extends MemberProblem(newfld) {
-  def description = newfld.fieldString + " was public; is inaccessible in " + affectedVersion + " version"
+  def description = affectedVersion => newfld.fieldString + " is inaccessible in " + affectedVersion + " version, it must be public."
 }
 
 case class InaccessibleMethodProblem(newmeth: MemberInfo) extends MemberProblem(newmeth) {
-  def description = newmeth.methodString + " was public; is inaccessible in " + affectedVersion + " version"
+  def description = affectedVersion => newmeth.methodString + " is inaccessible in " + affectedVersion + " version, it must be public."
 }
 
 case class InaccessibleClassProblem(newclazz: ClassInfo) extends TemplateProblem(newclazz) {
-  def description = newclazz.classString + " was public; is inaccessible in " + affectedVersion + " version"
+  def description = affectedVersion => newclazz.classString + " is inaccessible in " + affectedVersion + " version, it must be public."
 }

reporter/functional-tests/src/main/scala/com/typesafe/tools/mima/lib/CollectProblemsTest.scala

@@ -18,7 +18,7 @@ class CollectProblemsTest {
     val mima = new MiMaLib(Config.baseClassPath)
 
     // SUT
-    val problems = mima.collectProblems(oldJarPath, newJarPath).map(_.description)
+    val problems = mima.collectProblems(oldJarPath, newJarPath).map(_.description("new"))
 
     // load oracle
     var expectedProblems = Source.fromFile(oraclePath).getLines.toList

reporter/functional-tests/src/test/case-class-concrete-becomes-abstract-nok/problems.txt

@@ -1,3 +1,3 @@
 class A was concrete; is declared abstract in new version
 method apply()A in object A does not have a correspondent in new version
-the type hierarchy of object A has changed in new version. Missing types {scala.runtime.AbstractFunction0}
+the type hierarchy of object A is different in new version. Missing types {scala.runtime.AbstractFunction0}

reporter/functional-tests/src/test/class-becomes-trait-nok/problems.txt

@@ -1 +1 @@
-declaration of class A has changed to trait A in new version; changing class to trait breaks client code
+declaration of class A is trait A in new version; changing class to trait breaks client code

reporter/functional-tests/src/test/class-changed-val-type-in-new-version-nok/problems.txt

@@ -1 +1 @@
-method foo()Int in class A has now a different result type; was: Int, is now: java.lang.Object
+method foo()Int in class A has a different result type in new version, where it is java.lang.Object rather than Int

reporter/functional-tests/src/test/class-changed-var-type-in-new-version-nok/problems.txt

@@ -1,2 +1,2 @@
-method foo()Int in class A has now a different result type; was: Int, is now: java.lang.Object
-method foo_=(Int)Unit in class A's type has changed; was (Int)Unit, is now: (java.lang.Object)Unit
+method foo()Int in class A has a different result type in new version, where it is java.lang.Object rather than Int
+method foo_=(Int)Unit in class A's type is different in new version, where it is (java.lang.Object)Unit instead of (Int)Unit

reporter/functional-tests/src/test/class-method-abstract-override-of-concrete-superclass-method-nok/problems.txt

@@ -1 +1 @@
-abstract method foo()Int in class B does not have a correspondent in old version
+in new version there is abstract method foo()Int in class B, which does not have a correspondent

reporter/functional-tests/src/test/class-method-abstract-override-of-concrete-supertrait-method-nok/problems.txt

@@ -1,2 +1,2 @@
 abstract method foo()Int in class B does not have a correspondent in new version
-abstract method foo()Int in class B does not have a correspondent in old version
+in new version there is abstract method foo()Int in class B, which does not have a correspondent

reporter/functional-tests/src/test/class-method-changed-parameter-type-nok/problems.txt

@@ -1 +1 @@
-method foo(Int)Int in class A's type has changed; was (Int)Int, is now: (java.lang.Object)java.lang.Object
+method foo(Int)Int in class A's type is different in new version, where it is (java.lang.Object)java.lang.Object instead of (Int)Int

reporter/functional-tests/src/test/class-method-changed-parameters2-nok/problems.txt

@@ -1 +1 @@
-method foo(java.lang.Object,Int)Int in class A's type has changed; was (java.lang.Object,Int)Int, is now: (Int,java.lang.Object)Int
+method foo(java.lang.Object,Int)Int in class A's type is different in new version, where it is (Int,java.lang.Object)Int instead of (java.lang.Object,Int)Int

reporter/functional-tests/src/test/class-narrowing-method-type-in-new-version-ok/problems.txt

@@ -1 +1 @@
-method foo()Foo in class A has now a different result type; was: Foo, is now: Bar
+method foo()Foo in class A has a different result type in new version, where it is Bar rather than Foo

reporter/functional-tests/src/test/class-widening-method-type-in-new-version-nok/problems.txt

@@ -1 +1 @@
-method foo()Int in class A has now a different result type; was: Int, is now: java.lang.Object
+method foo()Int in class A has a different result type in new version, where it is java.lang.Object rather than Int

reporter/functional-tests/src/test/trait-abstract-method-becomes-concrete2-ok/problems.txt

@@ -1 +1 @@
-method foo()Int in trait A1 does not have a correspondent in old version
+method foo()Int in trait A1 is present only in new version

reporter/functional-tests/src/test/trait-abstract-val-become-concrete-ok/problems.txt

@@ -1 +1 @@
-synthetic method A$_setter_$foo_=(Int)Unit in trait A does not have a correspondent in old version
+synthetic method A$_setter_$foo_=(Int)Unit in trait A is present only in new version

reporter/functional-tests/src/test/trait-added-method-in-new-version-nok/problems.txt

@@ -1 +1 @@
-method bar()Int in trait A does not have a correspondent in old version
+method bar()Int in trait A is present only in new version

reporter/functional-tests/src/test/trait-added-val-in-new-version-nok/problems.txt

@@ -1,2 +1,2 @@
-synthetic method A$_setter_|_=(Int)Unit in trait A does not have a correspondent in old version
-method bar()Int in trait A does not have a correspondent in old version
+synthetic method A$_setter_|_=(Int)Unit in trait A is present only in new version
+method bar()Int in trait A is present only in new version

reporter/functional-tests/src/test/trait-added-var-in-new-version-nok/problems.txt

@@ -1,2 +1,2 @@
-method bar_=(Int)Unit in trait A does not have a correspondent in old version
-method bar()Int in trait A does not have a correspondent in old version
+method bar_=(Int)Unit in trait A is present only in new version
+method bar()Int in trait A is present only in new version

reporter/functional-tests/src/test/trait-deleting-concrete-methods-is-nok/problems.txt

@@ -1 +1 @@
-classes mixing B needs to update body of method foo()Int
+in new version, classes mixing B needs to update body of method foo()Int

reporter/functional-tests/src/test/trait-method-overloading-is-ok/problems.txt

@@ -1 +1 @@
-method foo(java.lang.Object)Int in trait A does not have a correspondent in old version
+method foo(java.lang.Object)Int in trait A is present only in new version

reporter/functional-tests/src/test/trait-moving-methods-nok/problems.txt

@@ -1,2 +1,2 @@
 method foo()Int in trait A does not have a correspondent in new version
-method foo()Int in trait B does not have a correspondent in old version
+method foo()Int in trait B is present only in new version

reporter/functional-tests/src/test/trait-pushing-up-abstract-methods-is-nok/problems.txt

@@ -1 +1 @@
-abstract method foo()Int in interface A does not have a correspondent in old version
+abstract method foo()Int in interface A is present only in new version

reporter/functional-tests/src/test/trait-pushing-up-concrete-methods-is-nok/problems.txt

@@ -1,2 +1,2 @@
-method foo()Int in trait A does not have a correspondent in old version
-classes mixing B needs to update body of method foo()Int
+method foo()Int in trait A is present only in new version
+in new version, classes mixing B needs to update body of method foo()Int

reporter/functional-tests/src/test/type-parameters-change-breaks-method-signature-nok/problems.txt

@@ -1 +1 @@
-method contains(NodeImpl)Boolean in class Tree's type has changed; was (NodeImpl)Boolean, is now: (Node)Boolean
+method contains(NodeImpl)Boolean in class Tree's type is different in new version, where it is (Node)Boolean instead of (NodeImpl)Boolean

reporter/src/main/scala/com/typesafe/tools/mima/cli/Main.scala

@@ -20,8 +20,11 @@ trait MimaSpec extends Spec with Meta.StdOpts with Interpolation {
   val currentfile = "curr" / "Current classpath/jar for binary compatibility testing." defaultTo ""
   heading("optional settings:")
   val classpath    = "classpath"    / "an optional classpath setting"        defaultTo System.getProperty("java.class.path")
-  val problemFilters = "filters" / "an optional problem filters configuration file" --|
+  val problemFilters = "filters" / "an optional problem filters configuration file, applied to both backward and forward checking" --|
+  val backwardFilters = "backward-filters" / "an optional problem filters configuration file, only applied to backward checking" --|
+  val forwardFilters = "forward-filters" / "an optional problem filters configuration file, only applied to forward checking" --|
   val generateFilters = "generate-filters" / "generate filters definition for displayed problems" --?
+  val direction = "direction" / "check direction, default is \"backward\", but can also be \"forward\" or \"both\"" --|
 }
 
 object MimaSpec extends MimaSpec with Property {
@@ -63,7 +66,7 @@ class Main(args: List[String]) extends {
   }
 
   /** Converts a problem to a human-readable mapped string. */
-  private def printProblem(p: core.Problem): String = {
+  private def printProblem(p: core.Problem, affected: String): String = {
     def wrap(words: Seq[String], result: List[String] = Nil): Seq[String] =
       if(words.isEmpty) result.reverse
       else {
@@ -78,7 +81,7 @@ class Main(args: List[String]) extends {
         wrap(rest, line :: result)
       }
     def wrapString(s: String) = wrap(s split "\\s")
-    wrapString(" * " + p.description) mkString "\n   "
+    wrapString(" * " + p.description(affected)) mkString "\n   "
   }
 
   private def loadFilters(configFile: File): Seq[ProblemFilter] = {
@@ -98,32 +101,48 @@ class Main(args: List[String]) extends {
     }
   }
 
-  private def printGeneratedFilters(errors: Seq[core.Problem]): Unit = {
+  private def printGeneratedFilters(errors: Seq[core.Problem], direction: String): Unit = {
     val errorsFilterConfig = ProblemFiltersConfig.problemsToProblemFilterConfig(errors)
-    val header = "Generated filter config definition"
+    val header = "\nGenerated " + direction + " filter config definition"
     println(header)
     println(Seq.fill(header.length)("=") mkString "")
     import com.typesafe.config._
     val renderOptions = ConfigRenderOptions.defaults().setOriginComments(false).setJson(false)
     println(errorsFilterConfig.root.render(renderOptions))
   }
 
+  def parseFilters(os:Option[String]) = os.toSeq.map(filePath => loadFilters(new File(filePath))).flatten
+
   def run(): Int = {
     val mima = makeMima
-    val foundProblems = mima.collectProblems(prevfile, currentfile)
-    val filters = problemFilters.toSeq.map(filePath => loadFilters(new File(filePath))).flatten
-    def isReported(problem: core.Problem) = filters.forall(filter => filter(problem))
-    val errors = foundProblems.filter(isReported)
-    val header = "Found " + errors.size + " binary incompatibilities" + {
-      val filteredOutSize = foundProblems.size - errors.size
+    val backwardProblems = (direction getOrElse Nil) match {
+      case "backward" | "backwards" | "both" => mima.collectProblems(prevfile, currentfile)
+      case _ => Nil
+    }
+    val forwardProblems = (direction getOrElse Nil) match {
+      case "forward" | "forwards" | "both" => mima.collectProblems(currentfile, prevfile)
+      case _ => Nil
+    }
+    val bothFilters = parseFilters(problemFilters)
+    val backFilters = bothFilters ++ parseFilters(backwardFilters)
+    val forwFilters = bothFilters ++ parseFilters(forwardFilters)
+    def isReported(problem: core.Problem, filters: Seq[core.Problem => Boolean]) = filters.forall(filter => filter(problem))
+    val backErrors = backwardProblems.filter(isReported(_,backFilters))
+    val forwErrors = forwardProblems.filter(isReported(_,forwFilters))
+    val errorsSize = backErrors.size + forwErrors.size
+    val header = "Found " + errorsSize + " binary incompatibilities" + {
+      val filteredOutSize = backwardProblems.size + forwardProblems.size - errorsSize
       if (filteredOutSize > 0) " (" + filteredOutSize + " were filtered out)" else ""
     }
     println(header)
     println(Seq.fill(header.length)("=") mkString "")
-    errors map printProblem foreach println
-    if (generateFilters)
-      printGeneratedFilters(errors)
-    errors.size
+    backErrors map {p:core.Problem => printProblem(p,"current")} foreach println
+    forwErrors map {p:core.Problem => printProblem(p,"other")} foreach println
+    if (generateFilters) {
+      printGeneratedFilters(backErrors, "backward")
+      printGeneratedFilters(forwErrors, "forward")
+    }
+    errorsSize
   }
 }