Merge pull request #88 from scalacenter/issue/1518

Fix sbt/sbt#1518: Handle cross-versioned exclusions

librarymanagement/src/main/contraband/librarymanagement.json

@@ -207,18 +207,20 @@
       "target": "Scala",
       "type": "record",
       "doc": [
-        "Rule to either:",
+        "Defines a rule to either:",
         "<ul>",
-        "<li> exclude unwanted dependencies pulled in transitively by a module, or to</li>",
+        "<li>Exclude unwanted dependencies pulled in transitively by a module, or to</li>",
-        "<li> include and merge artifacts coming from the ModuleDescriptor if \"dependencyArtifacts\" are also provided.</li>",
+        "<li>Include and merge artifacts coming from the ModuleDescriptor if \"dependencyArtifacts\" are also provided.</li>",
         "</ul>",
-        "Which one depends on the parameter name which it is passed to, but the filter has the same fields in both cases."
+        "The use case that is applied depends on the parameter name which it is passed to, but the",
+        "filter has the same fields in both cases."
       ],
       "fields": [
         { "name": "organization",   "type": "String",  "default": "\"*\"",        "since": "0.0.1" },
         { "name": "name",           "type": "String",  "default": "\"*\"",        "since": "0.0.1" },
         { "name": "artifact",       "type": "String",  "default": "\"*\"",        "since": "0.0.1" },
-        { "name": "configurations", "type": "String*", "default": "Vector.empty", "since": "0.0.1" }
+        { "name": "configurations", "type": "String*", "default": "Vector.empty", "since": "0.0.1" },
+        { "name": "crossVersion",   "type": "sbt.librarymanagement.CrossVersion", "default": "sbt.librarymanagement.Disabled()", "since": "0.0.1"}
       ],
       "parentsCompanion": "sbt.librarymanagement.InclExclRuleFunctions"
     },
@@ -374,7 +376,7 @@
             { "name": "moduleInfo",           "type": "sbt.librarymanagement.ModuleInfo"                                                                                               },
             { "name": "dependencies",         "type": "sbt.librarymanagement.ModuleID*"                                                                                                },
             { "name": "overrides",            "type": "Set[sbt.librarymanagement.ModuleID]",              "default": "Set.empty",                                     "since": "0.0.1" },
-            { "name": "excludes",             "type": "sbt.internal.librarymanagement.SbtExclusionRule*", "default": "Vector.empty",                                  "since": "0.0.1" },
+            { "name": "excludes",             "type": "sbt.librarymanagement.InclExclRule*", "default": "Vector.empty",                                  "since": "0.0.1" },
             { "name": "ivyXML",               "type": "scala.xml.NodeSeq",                                "default": "scala.xml.NodeSeq.Empty",                       "since": "0.0.1" },
             { "name": "configurations",       "type": "sbt.librarymanagement.Configuration*",             "default": "Vector.empty",                                  "since": "0.0.1" },
             { "name": "defaultConfiguration", "type": "Option[sbt.librarymanagement.Configuration]",      "default": "None",                                          "since": "0.0.1" },
@@ -781,20 +783,6 @@
         { "name": "configurationsToRetrieve", "type": "Option[Set[sbt.librarymanagement.Configuration]]", "default": "None",  "since": "0.0.1" }
       ]
     },
-    {
-      "name": "SbtExclusionRule",
-      "namespace": "sbt.internal.librarymanagement",
-      "target": "Scala",
-      "type": "record",
-      "fields": [
-        { "name": "organization",   "type": "String"                             },
-        { "name": "name",           "type": "String"                             },
-        { "name": "artifact",       "type": "String"                             },
-        { "name": "configurations", "type": "String*"                            },
-        { "name": "crossVersion",   "type": "sbt.librarymanagement.CrossVersion" }
-      ],
-      "parentsCompanion": "sbt.internal.librarymanagement.SbtExclusionRuleFunctions"
-    },
     {
       "name": "UpdateReportLite",
       "namespace": "sbt.internal.librarymanagement",

librarymanagement/src/main/scala/sbt/internal/librarymanagement/Ivy.scala

@@ -567,22 +567,30 @@ private[sbt] object IvySbt {
     )
   }
 
-  private def substituteCross(m: ModuleSettings): ModuleSettings =
+  private def substituteCross(m: ModuleSettings): ModuleSettings = {
     m.ivyScala match {
       case None     => m
       case Some(is) => substituteCross(m, is.scalaFullVersion, is.scalaBinaryVersion)
     }
+  }
+
   private def substituteCross(
       m: ModuleSettings,
       scalaFullVersion: String,
       scalaBinaryVersion: String
   ): ModuleSettings = {
-    val sub = CrossVersion(scalaFullVersion, scalaBinaryVersion)
     m match {
       case ic: InlineConfiguration =>
-        ic.withModule(sub(ic.module))
+        val applyCross = CrossVersion(scalaFullVersion, scalaBinaryVersion)
-          .withDependencies(ic.dependencies map sub)
+        def propagateCrossVersion(moduleID: ModuleID): ModuleID = {
-          .withOverrides(ic.overrides map sub)
+          val crossExclusions: Vector[ExclusionRule] =
+            moduleID.exclusions.map(CrossVersion.substituteCross(_, ic.ivyScala))
+          applyCross(moduleID)
+            .withExclusions(crossExclusions)
+        }
+        ic.withModule(applyCross(ic.module))
+          .withDependencies(ic.dependencies.map(propagateCrossVersion))
+          .withOverrides(ic.overrides map applyCross)
       case _ => m
     }
   }
@@ -852,13 +860,12 @@ private[sbt] object IvySbt {
 
   def addExcludes(
       moduleID: DefaultModuleDescriptor,
-      excludes: Seq[SbtExclusionRule],
+      excludes: Seq[ExclusionRule],
       ivyScala: Option[IvyScala]
-  ): Unit =
+  ): Unit = excludes.foreach(exclude => addExclude(moduleID, ivyScala)(exclude))
-    excludes foreach addExclude(moduleID, ivyScala)
+
   def addExclude(moduleID: DefaultModuleDescriptor, ivyScala: Option[IvyScala])(
-      exclude0: SbtExclusionRule
+      exclude0: ExclusionRule): Unit = {
-  ): Unit = {
     // this adds _2.11 postfix
     val exclude = CrossVersion.substituteCross(exclude0, ivyScala)
     val confs =

librarymanagement/src/main/scala/sbt/internal/librarymanagement/SbtExclusionRuleExtra.scala

@@ -1,18 +0,0 @@
-package sbt.internal.librarymanagement
-
-import sbt.internal.librarymanagement.impl._
-import sbt.librarymanagement._
-
-abstract class SbtExclusionRuleFunctions {
-  def apply(organization: String, name: String): SbtExclusionRule =
-    SbtExclusionRule(organization, name, "*", Vector.empty, Disabled())
-
-  def apply(organization: String): SbtExclusionRule = apply(organization, "*")
-
-  implicit def groupIdToExclusionRule(organization: GroupID): SbtExclusionRule =
-    apply(organization.groupID)
-  implicit def stringToExclusionRule(organization: String): SbtExclusionRule = apply(organization)
-
-  implicit def groupArtifactIDToExclusionRule(gaid: GroupArtifactID): SbtExclusionRule =
-    SbtExclusionRule(gaid.groupID, gaid.artifactID, "*", Vector.empty, gaid.crossVersion)
-}

librarymanagement/src/main/scala/sbt/librarymanagement/CrossVersionExtra.scala

@@ -1,6 +1,5 @@
 package sbt.librarymanagement
 
-import sbt.internal.librarymanagement.SbtExclusionRule
 import sbt.internal.librarymanagement.cross.CrossVersionUtil
 
 final case class ScalaVersion(full: String, binary: String)
@@ -69,9 +68,7 @@ abstract class CrossVersionFunctions {
 
   /** Constructs the cross-version function defined by `module` and `is`, if one is configured. */
   def apply(module: ModuleID, is: Option[IvyScala]): Option[String => String] =
-    is flatMap { i =>
+    is.flatMap(i => apply(module, i))
-      apply(module, i)
-    }
 
   /** Cross-version each `Artifact` in `artifacts` according to cross-version function `cross`. */
   def substituteCross(
@@ -94,9 +91,9 @@ abstract class CrossVersionFunctions {
 
   /** Cross-versions `exclude` according to its `crossVersion`. */
   private[sbt] def substituteCross(
-      exclude: SbtExclusionRule,
+      exclude: ExclusionRule,
       is: Option[IvyScala]
-  ): SbtExclusionRule = {
+  ): ExclusionRule = {
     val fopt: Option[String => String] =
       is flatMap { i =>
         CrossVersion(exclude.crossVersion, i.scalaFullVersion, i.scalaBinaryVersion)
@@ -111,8 +108,7 @@ abstract class CrossVersionFunctions {
   private[sbt] def substituteCrossA(
       as: Vector[Artifact],
       cross: Option[String => String]
-  ): Vector[Artifact] =
+  ): Vector[Artifact] = as.map(art => substituteCross(art, cross))
-    as.map(art => substituteCross(art, cross))
 
   /**
    * Constructs a function that will cross-version a ModuleID

librarymanagement/src/main/scala/sbt/librarymanagement/IvyInterface.scala

@@ -5,9 +5,30 @@ package sbt.librarymanagement
 
 import org.apache.ivy.core.module.descriptor
 import org.apache.ivy.util.filter.{ Filter => IvyFilter }
+import sbt.internal.librarymanagement.impl.{ GroupArtifactID, GroupID }
 
 abstract class InclExclRuleFunctions {
-  def everything = InclExclRule("*", "*", "*", Vector.empty)
+  def everything = InclExclRule("*", "*", "*", Vector.empty, Disabled())
+
+  def apply(organization: String, name: String): InclExclRule =
+    InclExclRule(organization, name, "*", Vector.empty, Disabled())
+
+  def apply(organization: String): InclExclRule = apply(organization, "*")
+
+  implicit def groupIdToExclusionRule(organization: GroupID): InclExclRule =
+    apply(organization.groupID)
+  implicit def stringToExclusionRule(organization: String): InclExclRule = apply(organization)
+
+  implicit def groupArtifactIDToExclusionRule(gaid: GroupArtifactID): InclExclRule =
+    InclExclRule(gaid.groupID, gaid.artifactID, "*", Vector.empty, gaid.crossVersion)
+
+  implicit def moduleIDToExclusionRule(moduleID: ModuleID): InclExclRule = {
+    val org = moduleID.organization
+    val name = moduleID.name
+    val version = moduleID.revision
+    val crossVersion = moduleID.crossVersion
+    InclExclRule(org, name, version, Vector.empty, crossVersion)
+  }
 }
 
 abstract class ArtifactTypeFilterExtra {

librarymanagement/src/main/scala/sbt/librarymanagement/ModuleIDExtra.scala

@@ -121,11 +121,11 @@ abstract class ModuleIDExtra {
    * Applies the provided exclusions to dependencies of this module.  Note that only exclusions that specify
    * both the exact organization and name and nothing else will be included in a pom.xml.
    */
-  def excludeAll(rules: InclExclRule*) = copy(exclusions = this.exclusions ++ rules)
+  def excludeAll(rules: ExclusionRule*) = copy(exclusions = this.exclusions ++ rules)
 
   /** Excludes the dependency with organization `org` and `name` from being introduced by this dependency during resolution. */
   def exclude(org: String, name: String) =
-    excludeAll(InclExclRule().withOrganization(org).withName(name))
+    excludeAll(ExclusionRule().withOrganization(org).withName(name))
 
   /**
    * Adds extra attributes for this module.  All keys are prefixed with `e:` if they are not already so prefixed.

librarymanagement/src/test/scala/InclExclSpec.scala

@@ -0,0 +1,67 @@
+package sbt.librarymanagement
+
+import org.scalatest.Assertion
+import sbt.internal.librarymanagement.BaseIvySpecification
+import sbt.internal.librarymanagement.impl.{ DependencyBuilders, GroupArtifactID }
+
+class InclExclSpec extends BaseIvySpecification with DependencyBuilders {
+  def createLiftDep(toExclude: ExclusionRule): ModuleID =
+    ("net.liftweb" %% "lift-mapper" % "2.6-M4" % "compile").excludeAll(toExclude)
+
+  def createMetaDep(toExclude: ExclusionRule): ModuleID =
+    ("org.scalameta" %% "paradise" % "3.0.0-M8" % "compile")
+      .cross(CrossVersion.full)
+      .excludeAll(toExclude)
+
+  def getIvyReport(dep: ModuleID, scalaVersion: Option[String]): UpdateReport = {
+    cleanIvyCache()
+    val ivyModule = module(defaultModuleId, Vector(dep), scalaVersion)
+    ivyUpdate(ivyModule)
+  }
+
+  def testLiftJsonIsMissing(report: UpdateReport): Assertion = {
+    assert(
+      !report.allModules.exists(_.name.contains("lift-json")),
+      "lift-json has not been excluded."
+    )
+  }
+
+  def testScalahostIsMissing(report: UpdateReport): Assertion = {
+    assert(
+      !report.allModules.exists(_.name.contains("scalahost")),
+      "scalahost has not been excluded."
+    )
+  }
+
+  val scala210 = Some("2.10.4")
+  it should "exclude any version of lift-json via a new exclusion rule" in {
+    val toExclude = ExclusionRule("net.liftweb", "lift-json_2.10")
+    val report = getIvyReport(createLiftDep(toExclude), scala210)
+    testLiftJsonIsMissing(report)
+  }
+
+  it should "exclude any version of lift-json with explicit Scala version" in {
+    val excluded: GroupArtifactID = "net.liftweb" % "lift-json_2.10"
+    val report = getIvyReport(createLiftDep(excluded), scala210)
+    testLiftJsonIsMissing(report)
+  }
+
+  it should "exclude any version of cross-built lift-json" in {
+    val excluded: GroupArtifactID = "net.liftweb" %% "lift-json"
+    val report = getIvyReport(createLiftDep(excluded), scala210)
+    testLiftJsonIsMissing(report)
+  }
+
+  val scala2122 = Some("2.12.2")
+  it should "exclude a concrete version of lift-json when it's full cross version" in {
+    val excluded: ModuleID = ("org.scalameta" % "scalahost" % "1.7.0").cross(CrossVersion.full)
+    val report = getIvyReport(createMetaDep(excluded), scala2122)
+    testScalahostIsMissing(report)
+  }
+
+  it should "exclude any version of lift-json when it's full cross version" in {
+    val excluded = new GroupArtifactID("net.liftweb", "lift-json", CrossVersion.full)
+    val report = getIvyReport(createMetaDep(excluded), scala2122)
+    testScalahostIsMissing(report)
+  }
+}