objectMatcher v3 (#282)

* Use scala macros to reduce number of methods for objectMatcher replacement

* Add more tests for useMatcher macro

* Should match list by value using object optional property

* Should preserve order even if there are missing or additional entities

* Should prefer identical matches when there are multiple possible choices

* Set full of duplicates according to object matcher should be identical to itself

* Introduce SetEntry box for sets

* Map full of duplicates according to object matcher should be identical to itself

* Simplify code

* Make string implementation details package private

* Fix docs compilation

* Rename TupleInstances -> DiffTupleInstances

* Remove optionObjectMacher

* Fix incorrect test case

* Move problematic test out of js tests

Author: ghostbuster91

cats/src/test/scala/com/softwaremill/diffx/cats/DiffxCatsTest.scala

@@ -19,7 +19,7 @@ class DiffxCatsTest extends AnyFreeSpec with Matchers {
 
   "nonEmptySet" in {
     compare(NonEmptySet.of(1), NonEmptySet.of(2)) shouldBe DiffResultSet(
-      List(DiffResultAdditional(1), DiffResultMissing(2))
+      Set(DiffResultAdditional(1), DiffResultMissing(2))
     )
   }
 

core/src/main/boilerplate/com/softwaremill/diffx/TupleInstances.scala.template renamed to core/src/main/boilerplate/com/softwaremill/diffx/DiffTupleInstances.scala.template

@@ -1,6 +1,6 @@
 package com.softwaremill.diffx
 
-trait TupleInstances {
+trait DiffTupleInstances {
 
   [2..#implicit def dTuple1[[#T1#]](implicit [#d1: Diff[T1]#]): Diff[Tuple1[[#T1#]]] = new Diff[Tuple1[[#T1#]]] {
       override def apply(

core/src/main/scala/com/softwaremill/diffx/Diff.scala

@@ -1,5 +1,5 @@
 package com.softwaremill.diffx
-import com.softwaremill.diffx.ObjectMatcher.{IterableEntry, MapEntry}
+import com.softwaremill.diffx.ObjectMatcher.{IterableEntry, MapEntry, SetEntry}
 import com.softwaremill.diffx.generic.{DiffMagnoliaDerivation, MagnoliaDerivedMacro}
 import com.softwaremill.diffx.instances._
 
@@ -35,7 +35,7 @@ trait Diff[-T] { outer =>
     }
 }
 
-object Diff extends MiddlePriorityDiff with TupleInstances with DiffxPlatformExtensions {
+object Diff extends MiddlePriorityDiff with DiffTupleInstances with DiffxPlatformExtensions {
   def apply[T: Diff]: Diff[T] = implicitly[Diff[T]]
 
   def ignored[T]: Diff[T] = (_: T, _: T, _: DiffContext) => DiffResult.Ignored
@@ -64,7 +64,7 @@ object Diff extends MiddlePriorityDiff with TupleInstances with DiffxPlatformExt
   implicit def diffForOptional[T](implicit ddt: Diff[T]): Diff[Option[T]] = new DiffForOption[T](ddt)
   implicit def diffForSet[T, C[W] <: scala.collection.Set[W]](implicit
       dt: Diff[T],
-      matcher: ObjectMatcher[T]
+      matcher: ObjectMatcher[SetEntry[T]]
   ): Diff[C[T]] = new DiffForSet[T, C](dt, matcher)
   implicit def diffForEither[L, R](implicit ld: Diff[L], rd: Diff[R]): Diff[Either[L, R]] =
     new DiffForEither[L, R](ld, rd)
@@ -112,12 +112,7 @@ case class DiffLens[T, U](outer: Diff[T], path: List[String]) {
 
   def ignore(implicit config: DiffConfiguration): Diff[T] = outer.modifyUnsafe(path: _*)(config.makeIgnored)
 
-  def withMapMatcher[K, V](m: ObjectMatcher[MapEntry[K, V]])(implicit ev1: U <:< scala.collection.Map[K, V]): Diff[T] =
-    outer.modifyMatcherUnsafe(path: _*)(m)
-  def withSetMatcher[V](m: ObjectMatcher[V])(implicit ev2: U <:< scala.collection.Set[V]): Diff[T] =
-    outer.modifyMatcherUnsafe(path: _*)(m)
-  def withListMatcher[V](m: ObjectMatcher[IterableEntry[V]])(implicit ev3: U <:< Iterable[V]): Diff[T] =
-    outer.modifyMatcherUnsafe(path: _*)(m)
+  def useMatcher[M](matcher: ObjectMatcher[M]): Diff[T] = macro ModifyMacro.withObjectMatcher[T, U, M]
 }
 case class DerivedDiffLens[T, U](outer: Diff[T], path: List[String]) {
   def setTo(d: Diff[U]): Derived[Diff[T]] = using(_ => d)
@@ -130,12 +125,5 @@ case class DerivedDiffLens[T, U](outer: Diff[T], path: List[String]) {
     outer.modifyUnsafe(path: _*)(config.makeIgnored)
   )
 
-  def withMapMatcher[K, V](m: ObjectMatcher[MapEntry[K, V]])(implicit
-      ev1: U <:< scala.collection.Map[K, V]
-  ): Derived[Diff[T]] =
-    Derived(outer.modifyMatcherUnsafe(path: _*)(m))
-  def withSetMatcher[V](m: ObjectMatcher[V])(implicit ev2: U <:< scala.collection.Set[V]): Derived[Diff[T]] =
-    Derived(outer.modifyMatcherUnsafe(path: _*)(m))
-  def withListMatcher[V](m: ObjectMatcher[IterableEntry[V]])(implicit ev3: U <:< Iterable[V]): Derived[Diff[T]] =
-    Derived(outer.modifyMatcherUnsafe(path: _*)(m))
+  def useMatcher[M](matcher: ObjectMatcher[M]): Derived[Diff[T]] = macro ModifyMacro.withObjectMatcherDerived[T, U, M]
 }

core/src/main/scala/com/softwaremill/diffx/DiffResult.scala

@@ -94,7 +94,7 @@ case class DiffResultMap(entries: Map[DiffResult, DiffResult]) extends DiffResul
   override def isIdentical: Boolean = entries.forall { case (k, v) => k.isIdentical && v.isIdentical }
 }
 
-case class DiffResultSet(diffs: List[DiffResult]) extends DiffResult {
+case class DiffResultSet(diffs: Set[DiffResult]) extends DiffResult {
   override private[diffx] def showIndented(indent: Int, renderIdentical: Boolean)(implicit
       c: ConsoleColorConfig
   ): String = {

core/src/main/scala/com/softwaremill/diffx/TupleInstances.scala renamed to core/src/main/scala/com/softwaremill/diffx/DiffTupleInstances.scala

@@ -1,6 +1,6 @@
 package com.softwaremill.diffx
 
-trait TupleInstances {
+trait DiffTupleInstances {
 
   implicit def dTuple2[T1, T2](implicit d1: Diff[T1], d2: Diff[T2]): Diff[Tuple2[T1, T2]] = new Diff[Tuple2[T1, T2]] {
     override def apply(

core/src/main/scala/com/softwaremill/diffx/ModifyMacro.scala

@@ -1,5 +1,7 @@
 package com.softwaremill.diffx
 
+import com.softwaremill.diffx.ObjectMatcher.{IterableEntry, MapEntry, SetEntry}
+
 import scala.annotation.tailrec
 import scala.reflect.macros.blackbox
 
@@ -112,4 +114,36 @@ object ModifyMacro {
 
   private[diffx] def modifiedFromPath[T, U](path: T => U): List[String] =
     macro modifiedFromPathMacro[T, U]
+
+  def withObjectMatcherDerived[T: c.WeakTypeTag, U: c.WeakTypeTag, M: c.WeakTypeTag](
+      c: blackbox.Context
+  )(matcher: c.Expr[ObjectMatcher[M]]): c.Tree = {
+    import c.universe._
+    val diff = withObjectMatcher[T, U, M](c)(matcher)
+    q"com.softwaremill.diffx.Derived($diff)"
+  }
+
+  def withObjectMatcher[T: c.WeakTypeTag, U: c.WeakTypeTag, M: c.WeakTypeTag](
+      c: blackbox.Context
+  )(matcher: c.Expr[ObjectMatcher[M]]): c.Tree = {
+    import c.universe._
+    val t = weakTypeOf[T]
+    val u = weakTypeOf[U]
+    val m = weakTypeOf[M]
+
+    val baseIsIterable = u <:< typeOf[Iterable[_]]
+    val baseIsSet = u <:< typeOf[Set[_]]
+    val baseIsMap = u <:< typeOf[Map[_, _]]
+    val typeArgsTheSame = u.typeArgs == m.typeArgs
+    val setRequirements = baseIsSet && typeArgsTheSame && m <:< typeOf[SetEntry[_]]
+    val iterableRequirements = !baseIsSet && baseIsIterable && typeArgsTheSame && m <:< typeOf[IterableEntry[_]]
+    val mapRequirements = baseIsMap && typeArgsTheSame && m <:< typeOf[MapEntry[_, _]]
+    if (!setRequirements && !iterableRequirements && !mapRequirements) { //  weakTypeOf[U] <:< tq"Iterable[${u.typeArgs.head.termSymbol}]"
+      c.abort(c.enclosingPosition, s"Invalid objectMather type $u for given lens($t,$m)")
+    }
+    q""" 
+       val lens = ${c.prefix}
+       lens.outer.modifyMatcherUnsafe(lens.path: _*)($matcher)
+    """
+  }
 }

core/src/main/scala/com/softwaremill/diffx/ObjectMatcher.scala

@@ -10,29 +10,45 @@ trait ObjectMatcher[T] {
 object ObjectMatcher extends LowPriorityObjectMatcher {
   def apply[T: ObjectMatcher]: ObjectMatcher[T] = implicitly[ObjectMatcher[T]]
 
-  /** Given product of type T and its property U, match that products using U's objectMatcher */
-  def by[T, U: ObjectMatcher](f: T => U): ObjectMatcher[T] = (left: T, right: T) =>
+  private def by[T, U: ObjectMatcher](f: T => U): ObjectMatcher[T] = (left: T, right: T) =>
     ObjectMatcher[U].isSameObject(f(left), f(right))
 
-  /** Given MapEntry[K,V] match them using V's objectMatcher */
-  def byValue[K, V: ObjectMatcher]: ObjectMatcher[MapEntry[K, V]] = ObjectMatcher.by[MapEntry[K, V], V](_.value)
+  /** Given MapEntry[K,V], match them using K's objectMatcher */
+  implicit def mapEntryByKey[K: ObjectMatcher, V]: ObjectMatcher[MapEntry[K, V]] =
+    ObjectMatcher.by[MapEntry[K, V], K](_.key)
+
+  implicit def setEntryByValue[T: ObjectMatcher]: ObjectMatcher[SetEntry[T]] = ObjectMatcher.by[SetEntry[T], T](_.t)
 
-  /** Given MapEntry[K,V], where V is a type of product and U is a property of V, match them using U's objectMatcher */
-  def byValue[K, V, U: ObjectMatcher](f: V => U): ObjectMatcher[MapEntry[K, V]] =
-    ObjectMatcher.byValue[K, V](ObjectMatcher.by[V, U](f))
+  type IterableEntry[T] = MapEntry[Int, T]
+  case class SetEntry[T](t: T)
+  case class MapEntry[K, V](key: K, value: V)
 
-  implicit def optionMatcher[T: ObjectMatcher]: ObjectMatcher[Option[T]] = (left: Option[T], right: Option[T]) => {
-    (left, right) match {
-      case (Some(l), Some(r)) => ObjectMatcher[T].isSameObject(l, r)
-      case _                  => false
-    }
+  def list[T] = new ObjectMatcherListHelper[T]
+
+  class ObjectMatcherListHelper[V] {
+    def byValue[U: ObjectMatcher](f: V => U): ObjectMatcher[IterableEntry[V]] = byValue(ObjectMatcher.by[V, U](f))
+    def byValue(implicit ev: ObjectMatcher[V]): ObjectMatcher[IterableEntry[V]] =
+      ObjectMatcher.by[IterableEntry[V], V](_.value)
+
+    def byKey[U: ObjectMatcher](f: Int => U): ObjectMatcher[IterableEntry[V]] = byKey(ObjectMatcher.by(f))
+    def byKey(implicit ko: ObjectMatcher[Int]): ObjectMatcher[IterableEntry[V]] = ObjectMatcher.by(_.key)
   }
 
-  /** Given MapEntry[K,V], match them using K's objectMatcher */
-  implicit def byKey[K: ObjectMatcher, V]: ObjectMatcher[MapEntry[K, V]] = ObjectMatcher.by[MapEntry[K, V], K](_.key)
+  def set[T] = new ObjectMatcherSetHelper[T]
 
-  type IterableEntry[T] = MapEntry[Int, T]
-  case class MapEntry[K, V](key: K, value: V)
+  class ObjectMatcherSetHelper[T] {
+    def by[U: ObjectMatcher](f: T => U): ObjectMatcher[SetEntry[T]] = setEntryByValue(ObjectMatcher.by(f))
+  }
+
+  def map[K, V] = new ObjectMatcherMapHelper[K, V]
+
+  class ObjectMatcherMapHelper[K, V] {
+    def byValue[U: ObjectMatcher](f: V => U): ObjectMatcher[MapEntry[K, V]] = byValue(ObjectMatcher.by(f))
+    def byValue(implicit ev: ObjectMatcher[V]): ObjectMatcher[MapEntry[K, V]] = ObjectMatcher.by(_.value)
+
+    def byKey[U: ObjectMatcher](f: K => U): ObjectMatcher[MapEntry[K, V]] = byKey(ObjectMatcher.by(f))
+    def byKey(implicit ko: ObjectMatcher[K]): ObjectMatcher[MapEntry[K, V]] = ObjectMatcher.by(_.key)
+  }
 }
 
 trait LowPriorityObjectMatcher {

core/src/main/scala/com/softwaremill/diffx/instances/DiffForIterable.scala

@@ -1,67 +1,89 @@
 package com.softwaremill.diffx.instances
 
-import com.softwaremill.diffx.Matching.MatchingResults
 import com.softwaremill.diffx.ObjectMatcher.{IterableEntry, MapEntry}
 import com.softwaremill.diffx._
+import com.softwaremill.diffx.instances.DiffForIterable._
+import com.softwaremill.diffx.instances.internal.MatchResult
 
 import scala.annotation.tailrec
-import scala.collection.immutable.{ListMap, ListSet}
+import scala.collection.immutable.ListMap
 
 private[diffx] class DiffForIterable[T, C[W] <: Iterable[W]](
     dt: Diff[T],
     matcher: ObjectMatcher[IterableEntry[T]]
 ) extends Diff[C[T]] {
   override def apply(left: C[T], right: C[T], context: DiffContext): DiffResult = nullGuard(left, right) {
     (left, right) =>
-      val keys = Range(0, Math.max(left.size, right.size))
-
-      val leftAsMap = left.toList.lift
-      val rightAsMap = right.toList.lift
-      val lEntries = keys.map(i => i -> leftAsMap(i)).collect { case (k, Some(v)) => MapEntry(k, v) }.toList
-      val rEntries = keys.map(i => i -> rightAsMap(i)).collect { case (k, Some(v)) => MapEntry(k, v) }.toList
-
       val adjustedMatcher = context.getMatcherOverride[IterableEntry[T]].getOrElse(matcher)
-      val MatchingResults(unMatchedLeftInstances, unMatchedRightInstances, matchedInstances) =
-        matchPairs(lEntries, rEntries, adjustedMatcher)
-      val leftDiffs = unMatchedLeftInstances
-        .diff(unMatchedRightInstances)
-        .collectFirst { case MapEntry(k, v) => k -> DiffResultAdditional(v) }
-        .toList
-      val rightDiffs = unMatchedRightInstances
-        .diff(unMatchedLeftInstances)
-        .collectFirst { case MapEntry(k, v) => k -> DiffResultMissing(v) }
-        .toList
-      val matchedDiffs = matchedInstances.map { case (l, r) => l.key -> dt(l.value, r.value, context) }.toList
+      val leftWithIndex = left.zipWithIndex.map { case (lv, i) => MapEntry(i, lv) }.toList
+      val rightWithIndex = right.zipWithIndex.map { case (rv, i) => MapEntry(i, rv) }.toList
 
-      val diffs = ListMap((matchedDiffs ++ leftDiffs ++ rightDiffs).map { case (k, v) => k.toString -> v }: _*)
+      val matches = matchPairs(leftWithIndex, rightWithIndex, adjustedMatcher, List.empty, context)
+      val sortedDiffs = matches.sorted.map {
+        case MatchResult.UnmatchedLeft(v)  => DiffResultAdditional(v.value)
+        case MatchResult.UnmatchedRight(v) => DiffResultMissing(v.value)
+        case MatchResult.Matched(l, r)     => dt.apply(l.value, r.value, context)
+      }
+      val reindexed = sortedDiffs.zipWithIndex.map(_.swap)
+      val diffs = ListMap(reindexed.map { case (k, v) => k.toString -> v }: _*)
       DiffResultObject("List", diffs)
   }
 
+  @tailrec
   private def matchPairs(
       left: List[IterableEntry[T]],
       right: List[IterableEntry[T]],
-      matcher: ObjectMatcher[IterableEntry[T]]
-  ): MatchingResults[IterableEntry[T]] = {
-    @tailrec
-    def loop(
-        left: List[IterableEntry[T]],
-        right: List[IterableEntry[T]],
-        matches: List[(IterableEntry[T], IterableEntry[T])],
-        leftUnmatched: List[IterableEntry[T]]
-    ): MatchingResults[IterableEntry[T]] = {
-      left match {
-        case lHead :: tail =>
-          val maybeMatch = right.collectFirst {
-            case r if matcher.isSameObject(lHead, r) => lHead -> r
-          }
-          maybeMatch match {
-            case Some(m @ (_, rm)) =>
-              loop(tail, right.filterNot(r => r.key == rm.key), matches :+ m, leftUnmatched)
-            case None => loop(tail, right, matches, leftUnmatched :+ lHead)
-          }
-        case Nil => MatchingResults(ListSet(leftUnmatched: _*), ListSet(right: _*), ListSet(matches: _*))
-      }
+      matcher: ObjectMatcher[IterableEntry[T]],
+      matched: List[MatchResult[IterableEntry[T]]],
+      context: DiffContext
+  ): List[MatchResult[IterableEntry[T]]] = {
+    right match {
+      case ::(rHead, tailRight) =>
+        val maybeMatched = left
+          .collect { case l if matcher.isSameObject(rHead, l) => l -> rHead }
+          .sortBy { case (l, r) => !dt.apply(l.value, r.value, context).isIdentical }
+          .headOption
+        maybeMatched match {
+          case Some((lm, rm)) =>
+            matchPairs(
+              left.filterNot(l => l.key == lm.key),
+              tailRight,
+              matcher,
+              matched :+ MatchResult.Matched(lm, rm),
+              context
+            )
+          case None => matchPairs(left, tailRight, matcher, matched :+ MatchResult.UnmatchedRight(rHead), context)
+        }
+      case Nil => matched ++ left.map(l => MatchResult.UnmatchedLeft(l))
     }
-    loop(left, right, List.empty, List.empty)
   }
 }
+
+object DiffForIterable {
+  implicit def iterableEntryOrdering[T]: Ordering[IterableEntry[T]] = Ordering.by(_.key)
+  implicit def diffResultOrdering[T]: Ordering[MatchResult[IterableEntry[T]]] =
+    new Ordering[MatchResult[IterableEntry[T]]] {
+      override def compare(x: MatchResult[IterableEntry[T]], y: MatchResult[IterableEntry[T]]): Int = {
+        (x, y) match {
+          case (ur: MatchResult.UnmatchedRight[IterableEntry[T]], m: MatchResult.Matched[IterableEntry[T]]) =>
+            iterableEntryOrdering.compare(ur.v, m.r)
+          case (_: MatchResult.UnmatchedRight[_], _: MatchResult.UnmatchedLeft[_]) => 1
+          case (ur: MatchResult.UnmatchedRight[IterableEntry[T]], ur2: MatchResult.UnmatchedRight[IterableEntry[T]]) =>
+            iterableEntryOrdering.compare(ur.v, ur2.v)
+          case (ur: MatchResult.UnmatchedLeft[IterableEntry[T]], m: MatchResult.Matched[IterableEntry[T]]) =>
+            iterableEntryOrdering.compare(ur.v, m.l)
+          case (_: MatchResult.UnmatchedLeft[_], _: MatchResult.UnmatchedRight[_]) => -1
+          case (ur: MatchResult.UnmatchedLeft[IterableEntry[T]], ur2: MatchResult.UnmatchedLeft[IterableEntry[T]]) =>
+            iterableEntryOrdering.compare(ur.v, ur2.v)
+          case (m1: MatchResult.Matched[IterableEntry[T]], m2: MatchResult.Matched[IterableEntry[T]]) =>
+            Ordering
+              .by[MatchResult.Matched[IterableEntry[T]], (IterableEntry[T], IterableEntry[T])](m => (m.r, m.l))
+              .compare(m1, m2)
+          case (m: MatchResult.Matched[IterableEntry[T]], ur: MatchResult.UnmatchedRight[IterableEntry[T]]) =>
+            iterableEntryOrdering.compare(m.r, ur.v)
+          case (m: MatchResult.Matched[IterableEntry[T]], ul: MatchResult.UnmatchedLeft[IterableEntry[T]]) =>
+            iterableEntryOrdering.compare(m.l, ul.v)
+        }
+      }
+    }
+}