Fix UpdateReport to be compatible with dependency-graph (#156)

Fixes coursier/coursier#1375
Ref sbt/sbt#4706 / sbt/sbt#4688
Ref sbt/sbt-dependency-graph#178

Currently the UpdateReport returned by Coursier is missing callers from the direct dependencies. This is evident from the fact that `thisModule`'s information is not passed. Another missing information in the UpdateReport is ModuleReport that originates from subproject dependencies (aka inter-project dependencies). These two missing info result in broken rendering for sbt-dependency-graph.

This commit attemps to fix them by passing them through to SbtUpdateReport. See the scripted test for confirmation.

modules/lm-coursier/src/main/scala/lmcoursier/CoursierDependencyResolution.scala

@@ -65,10 +65,8 @@ class CoursierDependencyResolution(conf: CoursierConfiguration) extends Dependen
     val sbv = module0.scalaModuleInfo.map(_.scalaBinaryVersion).getOrElse {
       sv.split('.').take(2).mkString(".")
     }
-
+    val (mod, ver) = FromSbt.moduleVersion(module0.module, sv, sbv, optionalCrossVer = true)
     val interProjectDependencies = {
-      val (mod, ver) = FromSbt.moduleVersion(module0.module, sv, sbv, optionalCrossVer = true)
-
       val needed = conf.interProjectDependencies.exists { p =>
         p.module == mod && p.version == ver
       }
@@ -192,11 +190,13 @@ class CoursierDependencyResolution(conf: CoursierConfiguration) extends Dependen
       artifacts: Map[Artifact, File]
     ) =
       UpdateParams(
+        thisModule = (ToCoursier.module(mod), ver),
         shadedConfigOpt = None,
         artifacts = artifacts,
         classifiers = classifiers,
         configs = configs,
         dependencies = dependencies,
+        interProjectDependencies = interProjectDependencies,
         res = resolutions,
         includeSignatures = false,
         sbtBootJarOverrides = sbtBootJarOverrides

modules/lm-coursier/src/main/scala/lmcoursier/internal/SbtUpdateReport.scala

@@ -74,7 +74,7 @@ private[internal] object SbtUpdateReport {
         .withExtraAttributes(module.attributes ++ extraProperties)
   }
 
-  private val moduleReport = caching[(Dependency, Seq[(Dependency, Project)], Project, Seq[(Publication, Artifact, Option[File])]), ModuleReport] {
+  private val moduleReport = caching[(Dependency, Seq[(Dependency, ProjectInfo)], Project, Seq[(Publication, Artifact, Option[File])]), ModuleReport] {
     case (dependency, dependees, project, artifacts) =>
 
     val sbtArtifacts = artifacts.collect {
@@ -95,7 +95,7 @@ private[internal] object SbtUpdateReport {
         Caller(
           moduleId((dependee, dependeeProj.version, Map.empty)),
           // FIXME Shouldn't we only keep the configurations pulling dependency?
-          dependeeProj.configurations.keys.toVector.map(c => ConfigRef(c.value)),
+          dependeeProj.configs,
           dependee.module.attributes ++ dependeeProj.properties,
           // FIXME Set better values here
           isForceDependency = false,
@@ -131,7 +131,10 @@ private[internal] object SbtUpdateReport {
   }
 
   private def moduleReports(
+    thisModule: (Module, String),
+    config: Configuration,
     res: Resolution,
+    interProjectDependencies: Seq[Project],
     classifiersOpt: Option[Seq[Classifier]],
     artifactFileOpt: (Module, String, Attributes, Artifact) => Option[File],
     log: Logger,
@@ -175,18 +178,38 @@ private[internal] object SbtUpdateReport {
       .groupBy(_._1)
       .mapValues(_.map { case (_, attr, a) => (attr, a) })
       .iterator
-      .toMap
+      .toMap ++
+      Map(interProjectDependencies
+        .filter(p => p.module != thisModule._1)
+        .map(p => Dependency(p.module, p.version) -> Nil): _*)
 
-    val versions = res.dependencies.toVector.map { dep =>
-      dep.module -> dep.version
-    }.toMap
+    val versions = (Vector(Dependency(thisModule._1, thisModule._2)) ++ res.dependencies.toVector ++ res.rootDependencies.toVector)
+      .map { dep =>
+        dep.module -> dep.version
+      }.toMap
 
     def clean(dep: Dependency): Dependency =
       dep
         .withConfiguration(Configuration.empty)
         .withExclusions(Set.empty)
         .withOptional(false)
 
+    def lookupProject(mv: coursier.core.Resolution.ModuleVersion): Option[Project] =
+      res.projectCache.get(mv) match {
+        case Some((_, p)) => Some(p)
+        case _ =>
+          interProjectDependencies.find( p =>
+            mv == (p.module, p.version)
+          )
+      }
+
+    val m = Dependency(thisModule._1, "")
+    val directReverseDependencies = res.rootDependencies.toSet.map(clean).map(_.withVersion(""))
+      .map(
+        dep => dep -> Vector(m)
+      )
+      .toMap
+
     val reverseDependencies = res.reverseDependencies
       .toVector
       .map { case (k, v) =>
@@ -195,22 +218,28 @@ private[internal] object SbtUpdateReport {
       .groupBy(_._1)
       .mapValues(_.flatMap(_._2))
       .toVector
-      .toMap
+      .toMap ++ directReverseDependencies
 
     groupedDepArtifacts.map {
       case (dep, artifacts) =>
-        val (_, proj) = res.projectCache(dep.moduleVersion)
+        val proj = lookupProject(dep.moduleVersion).get
 
         // FIXME Likely flaky...
         val dependees = reverseDependencies
           .getOrElse(clean(dep.withVersion("")), Vector.empty)
-          .map { dependee0 =>
+          .flatMap { dependee0 =>
             val version = versions(dependee0.module)
             val dependee = dependee0.withVersion(version)
-            val (_, dependeeProj) = res.projectCache(dependee.moduleVersion)
-            (dependee, dependeeProj)
+            lookupProject(dependee.moduleVersion) match {
+              case Some(dependeeProj) =>
+                Vector((dependee, ProjectInfo(
+                  dependeeProj.version,
+                  dependeeProj.configurations.keys.toVector.map(c => ConfigRef(c.value)),
+                  dependeeProj.properties)))
+              case _ =>
+                Vector.empty
+            }
           }
-
         moduleReport((
           dep,
           dependees,
@@ -221,8 +250,10 @@ private[internal] object SbtUpdateReport {
   }
 
   def apply(
+    thisModule: (Module, String),
     configDependencies: Map[Configuration, Seq[Dependency]],
     resolutions: Map[Configuration, Resolution],
+    interProjectDependencies: Vector[Project],
     configs: Map[Configuration, Set[Configuration]],
     classifiersOpt: Option[Seq[Classifier]],
     artifactFileOpt: (Module, String, Attributes, Artifact) => Option[File],
@@ -241,7 +272,10 @@ private[internal] object SbtUpdateReport {
         val subRes = resolutions(config).subset(configDeps)
 
         val reports = moduleReports(
+          thisModule,
+          config,
           subRes,
+          interProjectDependencies,
           classifiersOpt,
           artifactFileOpt,
           log,
@@ -279,11 +313,12 @@ private[internal] object SbtUpdateReport {
               val dep = Dependency(c.module, c.wantedVersion)
               val dependee = Dependency(c.dependeeModule, c.dependeeVersion)
               val dependeeProj = subRes.projectCache
-                .get((c.dependeeModule, c.dependeeVersion))
-                .map(_._2)
-                .getOrElse {
-                  // should not happen
-                  Project(c.dependeeModule, c.dependeeVersion, Nil, Map(), None, Nil, Nil, Nil, None, None, None, false, None, Nil, coursier.core.Info.empty)
+                .get((c.dependeeModule, c.dependeeVersion)) match {
+                  case Some((_, p)) =>
+                    ProjectInfo(p.version, p.configurations.keys.toVector.map(c => ConfigRef(c.value)), p.properties)
+                  case _ =>
+                    // should not happen
+                    ProjectInfo(c.dependeeVersion, Vector.empty, Vector.empty)
                 }
               val rep = moduleReport((dep, Seq((dependee, dependeeProj)), proj.withVersion(c.wantedVersion), Nil))
                 .withEvicted(true)
@@ -316,4 +351,5 @@ private[internal] object SbtUpdateReport {
     )
   }
 
+  private case class ProjectInfo(version: String, configs: Vector[ConfigRef], properties: Seq[(String, String)])
 }

modules/lm-coursier/src/main/scala/lmcoursier/internal/UpdateParams.scala

@@ -7,11 +7,13 @@ import coursier.util.Artifact
 
 // private[coursier]
 final case class UpdateParams(
+  thisModule: (Module, String),
   shadedConfigOpt: Option[(String, Configuration)],
   artifacts: Map[Artifact, File],
   classifiers: Option[Seq[Classifier]],
   configs: Map[Configuration, Set[Configuration]],
   dependencies: Seq[(Configuration, Dependency)],
+  interProjectDependencies: Seq[Project],
   res: Map[Set[Configuration], Resolution],
   includeSignatures: Boolean,
   sbtBootJarOverrides: Map[(Module, String), File]

modules/lm-coursier/src/main/scala/lmcoursier/internal/UpdateRun.scala

@@ -85,8 +85,10 @@ object UpdateRun {
     }
 
     SbtUpdateReport(
+      params.thisModule,
       depsByConfig,
       configResolutions,
+      params.interProjectDependencies.toVector,
       params.configs,
       params.classifiers,
       params.artifactFileOpt,

modules/sbt-coursier/src/main/scala/coursier/sbtcoursier/UpdateTasks.scala

@@ -102,8 +102,8 @@ object UpdateTasks {
       val log = streams.value.log
 
       val verbosityLevel = coursierVerbosity.value
-
-      val dependencies = ToCoursier.project(currentProjectTask.value).dependencies
+      val p = ToCoursier.project(currentProjectTask.value)
+      val dependencies = p.dependencies
       val res = resTask.value
 
       val key = SbtCoursierCache.ReportKey(
@@ -114,6 +114,8 @@ object UpdateTasks {
         includeSignatures
       )
 
+      val interProjectDependencies = coursierInterProjectDependencies.value.map(ToCoursier.project)
+
       SbtCoursierCache.default.reportOpt(key) match {
         case Some(report) =>
           Def.task(report)
@@ -125,11 +127,13 @@ object UpdateTasks {
             val configs = configsTask.value
 
             val params = UpdateParams(
+              (p.module, p.version),
               shadedConfigOpt,
               artifactFilesOrErrors0,
               classifiers,
               configs,
               dependencies,
+              interProjectDependencies,
               res,
               includeSignatures,
               sbtBootJarOverrides

modules/sbt-coursier/src/sbt-test/shared-1/caller/build.sbt

@@ -0,0 +1,39 @@
+lazy val check = taskKey[Unit]("")
+
+scalaVersion in ThisBuild := "2.12.8"
+organization in ThisBuild := "com.example"
+
+lazy val app = (project in file("app"))
+  .dependsOn(util)
+  .settings(
+    name := "app",
+    libraryDependencies += "com.chuusai" %% "shapeless" % "2.3.3",
+    check := {
+      val ur = update.value
+      val cr = ur.configuration(Compile).get
+      // configuration report must include a module report for subproject dependency
+      val coreReport = cr.modules.find(m =>
+        m.module.name == "core_2.12"
+      ).getOrElse(sys.error("report for core is missing"))
+      assert(coreReport.callers.exists(c => c.caller.name == "util_2.12"),
+        s"caller on core is missing util: ${coreReport.callers}")
+
+      // configuration report must include a module report for library dependency
+      val shapelessReport = cr.modules.find(m =>
+        m.module.name == "shapeless_2.12"
+      ).getOrElse(sys.error("report for shapeless is missing"))
+      assert(shapelessReport.callers.exists(c => c.caller.name == "app_2.12"),
+        s"caller on shapeless is missing self module (app): ${shapelessReport.callers}")
+    }
+  )
+
+lazy val util = (project in file("util"))
+  .dependsOn(core)
+  .settings(
+    name := "util"
+  )
+
+lazy val core = (project in file("core"))
+  .settings(
+    name := "core"
+  )

modules/sbt-coursier/src/sbt-test/shared-1/caller/project/plugins.sbt

@@ -0,0 +1,13 @@
+addSbtPlugin {
+
+  val name = sys.props.getOrElse(
+    "plugin.name",
+    sys.error("plugin.name Java property not set")
+  )
+  val version = sys.props.getOrElse(
+    "plugin.version",
+    sys.error("plugin.version Java property not set")
+  )
+
+  "io.get-coursier" % name % version
+}

modules/sbt-coursier/src/sbt-test/shared-1/caller/test

@@ -0,0 +1 @@
+> check