diff --git a/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala b/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala index 37756ed..8aaa49e 100644 --- a/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala +++ b/src/scala/com/github/johnynek/bazel_deps/GradleLockFile.scala @@ -3,6 +3,7 @@ package com.github.johnynek.bazel_deps import io.circe.Decoder.Result import io.circe.{Decoder, Error, HCursor, Json, KeyDecoder, Parser} import io.circe.generic.auto +import scala.collection.immutable.SortedMap import scala.util.{Failure, Success, Try} @@ -69,10 +70,10 @@ object GradleLockFile { } case class GradleLockFile( - annotationProcessor: Option[Map[String, GradleLockDependency]], - compileClasspath: Option[Map[String, GradleLockDependency]], - resolutionRules: Option[Map[String, GradleLockDependency]], - runtimeClasspath: Option[Map[String, GradleLockDependency]], - testCompileClasspath: Option[Map[String, GradleLockDependency]], - testRuntimeClasspath: Option[Map[String, GradleLockDependency]] + annotationProcessor: Option[SortedMap[String, GradleLockDependency]], + compileClasspath: Option[SortedMap[String, GradleLockDependency]], + resolutionRules: Option[SortedMap[String, GradleLockDependency]], + runtimeClasspath: Option[SortedMap[String, GradleLockDependency]], + testCompileClasspath: Option[SortedMap[String, GradleLockDependency]], + testRuntimeClasspath: Option[SortedMap[String, GradleLockDependency]] ) diff --git a/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala b/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala index c269fa9..384069d 100644 --- a/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala +++ b/src/scala/com/github/johnynek/bazel_deps/GradleResolver.scala @@ -42,7 +42,7 @@ class GradleResolver( // we just want one, so we merge them all private def collapseDepTypes( lockFile: GradleLockFile - ): Try[Map[String, GradleLockDependency]] = + ): Try[SortedMap[String, GradleLockDependency]] = TryMerge.tryMergeAll( None, NonEmptyList.of( @@ -54,7 +54,7 @@ class GradleResolver( lockFile.testRuntimeClasspath ) ) - .map(_.getOrElse(Map.empty)) + .map(_.getOrElse(SortedMap.empty[String, GradleLockDependency])) private val unit = Validated.valid(()) @@ -238,7 +238,7 @@ private def cleanUpMap( } def buildGraphFromDepMap( - depMap: Map[String, GradleLockDependency] + depMap: SortedMap[String, GradleLockDependency] ): Try[Graph[MavenCoordinate, Unit]] = assertConnectedDependencyMap(depMap) .map(_ => cleanUpMap(depMap)) @@ -274,7 +274,6 @@ private def cleanUpMap( depMap .toList - .sortBy(_._1) .traverse[V, (MavenCoordinate, List[MavenCoordinate])] { case (n, deps) => toCoord(n) .product( diff --git a/src/scala/com/github/johnynek/bazel_deps/TryMerge.scala b/src/scala/com/github/johnynek/bazel_deps/TryMerge.scala index 39b4438..86b702b 100644 --- a/src/scala/com/github/johnynek/bazel_deps/TryMerge.scala +++ b/src/scala/com/github/johnynek/bazel_deps/TryMerge.scala @@ -3,6 +3,7 @@ package com.github.johnynek.bazel_deps import cats.{Foldable, Reducible} import cats.data.NonEmptyList import scala.util.{Failure, Success, Try} +import scala.collection.immutable.SortedMap import cats.syntax.all._ @@ -40,33 +41,29 @@ object TryMerge { } } - implicit def tryStringMapMerge[T: TryMerge]: TryMerge[Map[String, T]] = - new TryMerge[Map[String, T]] { - private val rev = reverse + implicit def tryStringMapMerge[T: TryMerge]: TryMerge[SortedMap[String, T]] = + new TryMerge[SortedMap[String, T]] { def tryMerge( debugName: Option[String], - left: Map[String, T], - right: Map[String, T] - ): Try[Map[String, T]] = { - if (right.size > left.size) rev.tryMerge(debugName, right, left) - else { - // right <= left in size - val overlaps = right.exists { case (k, _) => left.contains(k) } - if (!overlaps) Success(left ++ right) - else { - right.toList.sortBy(_._1) - .foldM(left) { case (acc, (k, rv)) => - acc.get(k) match { - case Some(lv) => - TryMerge.tryMerge(Some(debugName.fold(k) {p => s"$p:$k"}), lv, rv) - .map(acc.updated(k, _)) - case None => - Success(acc.updated(k, rv)) - } - } + left: SortedMap[String, T], + right: SortedMap[String, T] + ): Try[SortedMap[String, T]] = { + val tmt = implicitly[TryMerge[T]] + val (big, small, tm) = + if (right.size > left.size) (right, left, tmt.reverse) + else (left, right, tmt) + + small.toStream + .foldM(big) { case (acc, (k, rv)) => + acc.get(k) match { + case Some(lv) => + val nextDebug = Some(debugName.fold(k) {p => s"$p:$k"}) + tm.tryMerge(nextDebug, lv, rv).map(acc.updated(k, _)) + case None => + Success(acc.updated(k, rv)) + } } } } - } } diff --git a/test/scala/com/github/johnynek/bazel_deps/BUILD.bazel b/test/scala/com/github/johnynek/bazel_deps/BUILD.bazel index 902fce1..e88015f 100644 --- a/test/scala/com/github/johnynek/bazel_deps/BUILD.bazel +++ b/test/scala/com/github/johnynek/bazel_deps/BUILD.bazel @@ -203,4 +203,16 @@ scala_test( ] ) +scala_test( + name = "trymergetest", + srcs = ["TryMergeTest.scala"], + deps = [ + "//src/scala/com/github/johnynek/bazel_deps:trymerge", + "//3rdparty/jvm/org/scalacheck", + "//3rdparty/jvm/org/scalatest:scalatest_propspec", + "//3rdparty/jvm/org/scalatestplus:scalacheck_1_17", + "//3rdparty/jvm/org/typelevel:cats_core", + ] +) + test_suite(name = "all_tests") diff --git a/test/scala/com/github/johnynek/bazel_deps/GradleResolverTest.scala b/test/scala/com/github/johnynek/bazel_deps/GradleResolverTest.scala index eda165c..53dcc7a 100644 --- a/test/scala/com/github/johnynek/bazel_deps/GradleResolverTest.scala +++ b/test/scala/com/github/johnynek/bazel_deps/GradleResolverTest.scala @@ -4,6 +4,7 @@ import io.circe.jawn.JawnParser import java.nio.file.Paths import org.scalatest.funsuite.AnyFunSuite import scala.util.{Success, Failure} +import scala.collection.immutable.SortedMap import Decoders._ @@ -18,8 +19,8 @@ class GradleResolverTest extends AnyFunSuite { case Left(error) => sys.error(s"Failed to decode $str: ${error}") } - def deps(str: String): Map[String, GradleLockDependency] = - (new JawnParser).decode[Map[String, GradleLockDependency]](str) match { + def deps(str: String): SortedMap[String, GradleLockDependency] = + (new JawnParser).decode[SortedMap[String, GradleLockDependency]](str) match { case Right(t) => t case Left(error) => sys.error(s"Failed to decode $str: ${error}") } diff --git a/test/scala/com/github/johnynek/bazel_deps/GraphPropTest.scala b/test/scala/com/github/johnynek/bazel_deps/GraphPropTest.scala index 1871d9f..cfe8fa5 100644 --- a/test/scala/com/github/johnynek/bazel_deps/GraphPropTest.scala +++ b/test/scala/com/github/johnynek/bazel_deps/GraphPropTest.scala @@ -1,10 +1,10 @@ package com.github.johnynek.bazel_deps import org.scalatest.propspec.AnyPropSpec -import org.scalacheck.Prop.forAll +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks import org.scalacheck.{Gen, Arbitrary} -class GraphPropTest extends AnyPropSpec { +class GraphPropTest extends AnyPropSpec with ScalaCheckPropertyChecks { def graphGen[N, E](g: Gen[(N, Option[(N, E)])]): Gen[Graph[N, E]] = Gen.sized { s => @@ -89,9 +89,9 @@ class GraphPropTest extends AnyPropSpec { property("Sanity checks on generated graphs (non-dag) 4") { forAll(graphGen(genIntNode), nodeGen) { (g, n) => val newG = g.removeNode(n) - (!newG.nodes(n)) && (!newG.edgeIterator.exists { case Edge(s, d, _) => + assert((!newG.nodes(n)) && (!newG.edgeIterator.exists { case Edge(s, d, _) => (s == n) || (d == n) - }) + })) } } @@ -99,11 +99,12 @@ class GraphPropTest extends AnyPropSpec { forAll(graphGen(genIntNode), nodeGen, nodeGen) { (g, s, d) => val newEdge = Edge(s, d, ()) val newG = g.addEdge(newEdge) - newG.nodes(s) && - newG.nodes(d) && - newG.hasSource(s)(newEdge) && - newG.hasDestination(d)(newEdge) && - newG.edgeIterator.toSet(newEdge) + + assert(newG.nodes(s) && + newG.nodes(d) && + newG.hasSource(s)(newEdge) && + newG.hasDestination(d)(newEdge) && + newG.edgeIterator.toSet(newEdge)) } } @@ -114,8 +115,8 @@ class GraphPropTest extends AnyPropSpec { }) { case (g, Some(e)) => val newG = g.removeEdge(e) - !(newG.edgeIterator.exists(_ == e)) - case (g, None) => true + assert(!(newG.edgeIterator.exists(_ == e))) + case (g, None) => () } } @@ -126,9 +127,12 @@ class GraphPropTest extends AnyPropSpec { }) { case (g, Some(n)) => val newG = g.removeNode(n) - newG.hasDestination(n).isEmpty && - newG.hasSource(n).isEmpty && - (!newG.nodes(n)) + + assert( + newG.hasDestination(n).isEmpty && + newG.hasSource(n).isEmpty && + (!newG.nodes(n)) + ) case (g, None) => true } } @@ -142,7 +146,7 @@ class GraphPropTest extends AnyPropSpec { } yield (g, optPair) forAll(genEnds) { - case (g, Some((s, e))) => g.reflexiveTransitiveClosure(List(s))(e) + case (g, Some((s, e))) => assert(g.reflexiveTransitiveClosure(List(s))(e)) case (g, None) => true } } @@ -152,6 +156,7 @@ class GraphPropTest extends AnyPropSpec { g <- graphGen(genIntNode) e <- edgeFrom(g) } yield (g, e) + forAll(gen, nodeGen, nodeGen) { case ((g, e), n1, n2) => val maybePresent = Edge(n1, n2, ()) val maybeCheck = @@ -164,9 +169,9 @@ class GraphPropTest extends AnyPropSpec { g.hasDestination(n2).contains(maybePresent)) } - g.edgeIterator.forall(g.hasEdge) && + assert(g.edgeIterator.forall(g.hasEdge) && e.forall(g.hasEdge(_)) && - maybeCheck + maybeCheck) } } -} +} \ No newline at end of file diff --git a/test/scala/com/github/johnynek/bazel_deps/TryMergeTest.scala b/test/scala/com/github/johnynek/bazel_deps/TryMergeTest.scala new file mode 100644 index 0000000..1807c77 --- /dev/null +++ b/test/scala/com/github/johnynek/bazel_deps/TryMergeTest.scala @@ -0,0 +1,87 @@ +package com.github.johnynek.bazel_deps + +import org.scalatest.propspec.AnyPropSpec +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks +import org.scalacheck.{Prop, Gen, Arbitrary} +import cats.{Semigroup, Eq} +import scala.util.{Success, Failure} +import scala.collection.immutable.SortedMap + +import TryMerge.tryMerge +import java.util.concurrent.Semaphore + +class TryMergeTest extends AnyPropSpec with ScalaCheckPropertyChecks { + def associative[A: TryMerge: Eq](a: A, b: A, c: A) = { + val right = tryMerge(None, b, c).flatMap(tryMerge(None, a, _)) + val left = tryMerge(None, a, b).flatMap(tryMerge(None, _, c)) + + (left, right) match { + case (Success(l), Success(r)) => assert(Eq.eqv(l, r)) + case (Failure(_), Failure(_)) => () + case mismatch => fail(s"associative mismatch: $a, $b, $c => $mismatch") + } + } + + implicit val tryMergeI: TryMerge[Int] = + new TryMerge[Int] { + def tryMerge(debug: Option[String], a: Int, b: Int) = + // take the high 16 bits of a and low 16 bits of b + // this is associative but not commutative + Success((a & 0xFFFF0000) | (b & 0x0000FFFF)) + } + + def revTry[A: TryMerge]: TryMerge[A] = implicitly[TryMerge[A]].reverse + + property("try merge is associative Int") { + forAll { (a: Int, b: Int, c: Int) => + associative(a, b, c) + associative(a, b, c)(revTry, implicitly) + } + } + + + property("try merge is associative Option[Int]") { + forAll { (a: Option[Int], b: Option[Int], c: Option[Int]) => + associative(a, b, c) + associative(a, b, c)(revTry, implicitly) + } + } + + property("try merge is associative SortedMap[String, Int]") { + forAll { (a: SortedMap[String, Int], b: SortedMap[String, Int], c: SortedMap[String, Int]) => + associative(a, b, c) + associative(a, b, c)(revTry, implicitly) + } + } + + property("try merge with taking this rhs is the same as ++ on SortedMap") { + forAll { (a: SortedMap[String, Int], b: SortedMap[String, Int]) => + implicit val rhs = new TryMerge[Int] { + def tryMerge(d: Option[String], a: Int, b: Int) = Success(b) + } + assert(tryMerge(None, a, b) == Success(a ++ b)) + } + } + + property("if we always return it's the same as a Semigroup") { + forAll { (a: SortedMap[String, Int], b: SortedMap[String, Int]) => + implicit val sgBits = new Semigroup[Int] { + def combine(a: Int, b: Int) = (a & 0xFFFF0000) | (b & 0x0000FFFF) + } + val sm = new Semigroup[SortedMap[String, Int]] { + def combine(a: SortedMap[String, Int], b: SortedMap[String, Int]) = { + val allKeys = a.keySet | b.keySet + SortedMap(allKeys.iterator.map { k => + (a.get(k), b.get(k)) match { + case (Some(l), Some(r)) => (k, sgBits.combine(l, r)) + case (Some(l), None) => (k, l) + case (None, Some(r)) => (k, r) + case (None, None) => sys.error(s"unreachable: non-sense key: $k") + } + }.toList: _*) + } + } + assert(tryMerge(None, a, b) == Success(sm.combine(a, b))) + } + } +}