Merge pull request #34 from ckipp01/rangeFix

fix: ensure ranges don't end up in manifest

.gitignore

@@ -2,3 +2,4 @@
 .bsp/
 .metals/
 out/
+manifest.json

build.sc

@@ -97,6 +97,9 @@ object itest extends MillIntegrationTestModule {
         ),
         PathRef(testBase / "eviction") -> Seq(
           TestInvocation.Targets(Seq("verify"), noServer = true)
+        ),
+        PathRef(testBase / "range") -> Seq(
+          TestInvocation.Targets(Seq("verify"), noServer = true)
         )
       )
     }

itest/src/minimal/manifests.json

@@ -12,7 +12,7 @@
         "dependencies": [
           "com.lihaoyi:fansi_3:0.3.1",
           "com.lihaoyi:sourcecode_3:0.2.8",
-          "org.scala-lang:scala3-library_3:3.0.2"
+          "org.scala-lang:scala3-library_3:3.1.3"
         ],
         "relationship": "direct",
         "package_url": "pkg:maven/com.lihaoyi/[email protected]"
@@ -23,7 +23,7 @@
         },
         "dependencies": [
           "com.lihaoyi:sourcecode_3:0.2.8",
-          "org.scala-lang:scala3-library_3:3.0.2"
+          "org.scala-lang:scala3-library_3:3.1.3"
         ],
         "relationship": "indirect",
         "package_url": "pkg:maven/com.lihaoyi/[email protected]"
@@ -33,7 +33,7 @@
 
         },
         "dependencies": [
-          "org.scala-lang:scala3-library_3:3.0.0"
+          "org.scala-lang:scala3-library_3:3.1.3"
         ],
         "relationship": "indirect",
         "package_url": "pkg:maven/com.lihaoyi/[email protected]"
@@ -76,7 +76,7 @@
         "dependencies": [
           "com.lihaoyi:fansi_3:0.3.1",
           "com.lihaoyi:sourcecode_3:0.2.8",
-          "org.scala-lang:scala3-library_3:3.0.2"
+          "org.scala-lang:scala3-library_3:3.1.3"
         ],
         "relationship": "direct",
         "package_url": "pkg:maven/com.lihaoyi/[email protected]"
@@ -98,7 +98,7 @@
         },
         "dependencies": [
           "com.lihaoyi:sourcecode_3:0.2.8",
-          "org.scala-lang:scala3-library_3:3.0.2"
+          "org.scala-lang:scala3-library_3:3.1.3"
         ],
         "relationship": "indirect",
         "package_url": "pkg:maven/com.lihaoyi/[email protected]"
@@ -118,8 +118,8 @@
 
         },
         "dependencies": [
-          "junit:junit:4.13.1",
-          "org.scala-lang:scala3-library_3:3.0.1",
+          "junit:junit:4.13.2",
+          "org.scala-lang:scala3-library_3:3.1.3",
           "org.scalameta:junit-interface:0.7.29"
         ],
         "relationship": "direct",
@@ -140,7 +140,7 @@
 
         },
         "dependencies": [
-          "org.scala-lang:scala3-library_3:3.0.0"
+          "org.scala-lang:scala3-library_3:3.1.3"
         ],
         "relationship": "indirect",
         "package_url": "pkg:maven/com.lihaoyi/[email protected]"

itest/src/range/build.sc

@@ -0,0 +1,32 @@
+import mill._, scalalib._
+import $exec.plugins
+import io.kipp.mill.github.dependency.graph.Graph
+import mill.eval.Evaluator
+import $ivy.`org.scalameta::munit:0.7.29`
+import munit.Assertions._
+
+object minimal extends ScalaModule {
+  def scalaVersion = "3.1.3"
+
+  def ivyDeps = Agg(
+    ivy"org.eclipse.lsp4j:org.eclipse.lsp4j.jsonrpc:0.12.0"
+  )
+}
+
+def verify(ev: Evaluator) = T.command {
+  val manifestMapping = Graph.generate(ev)()
+  assert(manifestMapping.size == 1)
+
+  // We want to ensure that the transitive dependency of the above, which has a
+  // range dependency doesn't end up in the actualy manifest as a range, but
+  // the reconciled version. So we want to ensure `2.8.9` and not
+  // `[2.8.6,2.0)`.
+  val expected = Set(
+    "org.scala-lang:scala-library:2.13.8",
+    "org.scala-lang:scala3-library_3:3.1.3",
+    "org.eclipse.lsp4j:org.eclipse.lsp4j.jsonrpc:0.12.0",
+    "com.google.code.gson:gson:2.8.9"
+  )
+
+  assertEquals(manifestMapping.head._2.resolved.keys, expected)
+}

plugin/src/io/kipp/mill/github/dependency/graph/ModuleTrees.scala

@@ -33,20 +33,22 @@ final case class ModuleTrees(
     def toNode(tree: DependencyTree, root: Boolean): Unit = {
       val dep = tree.dependency
       val moduleOrgName = dep.module.orgName
-      val name = s"${moduleOrgName}:${dep.version}"
+      val reconciledVersion = tree.reconciledVersion
+      val name = s"${moduleOrgName}:${reconciledVersion}"
 
       def putTogether: DependencyNode = {
         // TODO consider classifiers
         val packageUrl =
-          s"pkg:maven/${dep.module.organization.value}/${dep.module.name.value}@${dep.version}"
+          s"pkg:maven/${dep.module.organization.value}/${dep.module.name.value}@${reconciledVersion}"
         val relationShip: DependencyRelationship =
           if (root) DependencyRelationship.direct
           else DependencyRelationship.indirect
         val dependencies = tree.children.map { child =>
-          s"${child.dependency.module.orgName}:${child.dependency.version}"
+          s"${child.dependency.module.orgName}:${child.reconciledVersion}"
         }
         DependencyNode(
           Some(packageUrl),
+          // TODO we can check if original == reconciled here and add metadata that it is a reconciled version
           Map.empty,
           Some(relationShip),
           None,
@@ -57,37 +59,21 @@ final case class ModuleTrees(
       def verifyRelationship(node: DependencyNode) =
         (root && node.isDirectDependency) || (!root && !node.isDirectDependency)
 
-      val getsEvicted = dep.version != tree.retainedVersion
-      // So the idea here is that if the retained version doesn't match the dep
-      // version, we know that the dep ends up getting evicted, so we don't
-      // actually need it as an entry since it won't end up on the classpath.
-      // However if it's a root node, we need to ensure we later go back and
-      // mark it as direct. We don't do it while iterating because we may have
-      // already seen the dep that evicted it. See the evicted test for an
-      // example.
-      if (getsEvicted && root) {
-        reconciledDirects += s"${moduleOrgName}:${tree.retainedVersion}"
-      } else if (getsEvicted) {
-        // If we know it's evicted here, but we're not at the root level, then
-        // it doesn't matter because it's still not a direct dep.
-        ()
-      } else {
-        allDependencies.get(name) match {
-          // If the node is found and the relationship is correct just do nothing
-          case Some(node) if verifyRelationship(node) => ()
-          // If the node is found and the relationship is incorrect, but it's a
-          // root node, then make sure to mark it as direct
-          case Some(node) if root =>
-            val updated =
-              node.copy(relationship = Some(DependencyRelationship.direct))
-            allDependencies += ((name, updated))
-          // Should never really happen, but it it does do nothing
-          case Some(_) => ()
-          // Unseen dependency, create a node for it
-          case None =>
-            val node = putTogether
-            allDependencies += ((name, node))
-        }
+      allDependencies.get(name) match {
+        // If the node is found and the relationship is correct just do nothing
+        case Some(node) if verifyRelationship(node) => ()
+        // If the node is found and the relationship is incorrect, but it's a
+        // root node, then make sure to mark it as direct
+        case Some(node) if root =>
+          val updated =
+            node.copy(relationship = Some(DependencyRelationship.direct))
+          allDependencies += ((name, updated))
+        // Should never really happen, but it it does do nothing
+        case Some(_) => ()
+        // Unseen dependency, create a node for it
+        case None =>
+          val node = putTogether
+          allDependencies += ((name, node))
       }
 
       tree.children.foreach(toNode(_, root = false))