Merge pull request #78 from alexarchambault/allow-duplicate-user-params

Allow duplicate user params
coursier · Nov 12, 2024 · c9ff1f5 · c9ff1f5
2 parents 69d29f5 + 22831d4
commit c9ff1f5
Show file tree

Hide file tree

Showing 12 changed files with 68 additions and 38 deletions.
diff --git a/README.md b/README.md
@@ -142,7 +142,7 @@ Custom parameters can be passed in dependencies:
 ```scala mdoc
 val depWithParams = dep"io.get-coursier::coursier:2.0.6,url=https://dl.cs/cs.jar,intransitive"
 
-assert(depWithParams.userParams == Map("url" -> Some("https://dl.cs/cs.jar"), "intransitive" -> None))
+assert(depWithParams.userParams == Seq("url" -> Some("https://dl.cs/cs.jar"), "intransitive" -> None))
 ```
 
 ### Scala binary version

diff --git a/build.sc b/build.sc
@@ -71,10 +71,20 @@ trait DependencyMima extends Mima {
       .map(_.stripPrefix("v"))
       .filter(!_.startsWith("0.0."))
       .filter(!_.startsWith("0.1."))
+      .filter(!_.startsWith("0.2."))
       .map(coursier.core.Version(_))
       .sorted
       .map(_.repr)
   }
+  // Remove once 0.3.0 is out
+  def mimaPreviousArtifacts = T {
+    val versions     = mimaPreviousVersions()
+    val organization = pomSettings().organization
+    val artifactId0  = artifactId()
+    Agg.from(
+      versions.map(version => ivy"$organization:$artifactId0:$version")
+    )
+  }
 }
 
 private def scalaDirNames(sv: String): Seq[String] = {

diff --git a/dependency-interface/src/main/scala/dependency/api/ApiConverter.scala b/dependency-interface/src/main/scala/dependency/api/ApiConverter.scala
@@ -22,13 +22,13 @@ object ApiConverter {
           }
           .asJava
       }
-    val cl = dep.userParams.get("classifier").flatten
+    val cl = dep.userParamsMap.get("classifier").flatMap(_.flatMap(_.toSeq).headOption)
       .orElse(Option(dep0.getPublication).map(_.getClassifier))
       .getOrElse("")
-    val tpe = dep.userParams.get("type").flatten
+    val tpe = dep.userParamsMap.get("type").flatMap(_.flatMap(_.toSeq).headOption)
       .orElse(Option(dep0.getPublication).map(_.getType))
       .getOrElse("")
-    val ext = dep.userParams.get("ext").flatten
+    val ext = dep.userParamsMap.get("ext").flatMap(_.flatMap(_.toSeq).headOption)
       .orElse(Option(dep0.getPublication).map(_.getExtension))
       .getOrElse("")
     val pubName = Option(dep0.getPublication).map(_.getName)
@@ -39,7 +39,7 @@ object ApiConverter {
       else
         new coursierapi.Publication(pubName, tpe, ext, cl)
     )
-    for (_ <- dep.userParams.get("intransitive"))
+    for (_ <- dep.userParamsMap.get("intransitive"))
       dep0 = dep0.withTransitive(false)
     dep0
   }

diff --git a/dependency/shared/src/main/scala-2/dependency/literal/DependencyLiteralMacros.scala b/dependency/shared/src/main/scala-2/dependency/literal/DependencyLiteralMacros.scala
@@ -15,13 +15,13 @@ class DependencyLiteralMacros(override val c: whitebox.Context) extends ModuleLi
     c.Expr(expr)
   }
 
-  private def stringOptionStringMap(map: Map[String, Option[String]], mappings: Mappings): c.Expr[Map[String, Option[String]]] = {
-    val entries = map.toVector.sorted.map {
+  private def stringOptionStringMap(params: Seq[(String, Option[String])], mappings: Mappings): c.Expr[Seq[(String, Option[String])]] = {
+    val entries = params.map {
       case (k, v) =>
         val value = optionString(v, mappings)
         c.Expr(q"_root_.scala.Tuple2(${applyMappings(k, mappings)}, $value)")
     }
-    c.Expr(q"_root_.scala.collection.immutable.Map[_root_.java.lang.String, _root_.scala.Option[_root_.java.lang.String]](..$entries)")
+    c.Expr(q"_root_.scala.collection.immutable.Seq[_root_.scala.Tuple2[_root_.java.lang.String, _root_.scala.Option[_root_.java.lang.String]]](..$entries)")
   }
 
   private def dependencyExpr(dep: AnyDependency, mappings: Mappings): c.Tree = {

diff --git a/dependency/shared/src/main/scala-3/dependency/literal/Extensions.scala b/dependency/shared/src/main/scala-3/dependency/literal/Extensions.scala
@@ -97,7 +97,7 @@ object Extensions {
             $module0,
             ${mappings.Expr(dep.version)},
             CovariantSet(${Varargs(excludes)}: _*),
-            ${mappings.mapStringStringOption(dep.userParams)}
+            ${mappings.seqStringStringOption(dep.userParams)}
           )
         }
       case (false, true, false, true) =>
@@ -108,7 +108,7 @@ object Extensions {
             $module0,
             ${mappings.Expr(dep.version)},
             CovariantSet(${Varargs(excludes)}: _*),
-            ${mappings.mapStringStringOption(dep.userParams)}
+            ${mappings.seqStringStringOption(dep.userParams)}
           )
         }
       case (false, _, _, _) =>
@@ -119,7 +119,7 @@ object Extensions {
             $module0,
             ${mappings.Expr(dep.version)},
             CovariantSet(${Varargs(excludes)}: _*),
-            ${mappings.mapStringStringOption(dep.userParams)}
+            ${mappings.seqStringStringOption(dep.userParams)}
           )
         }
       case (true, false, _, _) | (true, true, true, _) =>
@@ -130,7 +130,7 @@ object Extensions {
             $module0,
             ${mappings.Expr(dep.version)},
             CovariantSet(${Varargs(excludes)}: _*),
-            ${mappings.mapStringStringOption(dep.userParams)}
+            ${mappings.seqStringStringOption(dep.userParams)}
           )
         }
       case (true, true, false, true) =>
@@ -141,7 +141,7 @@ object Extensions {
             $module0,
             ${mappings.Expr(dep.version)},
             CovariantSet(${Varargs(excludes)}: _*),
-            ${mappings.mapStringStringOption(dep.userParams)}
+            ${mappings.seqStringStringOption(dep.userParams)}
           )
         }
       case (true, _, _, _) =>
@@ -152,7 +152,7 @@ object Extensions {
             $module0,
             ${mappings.Expr(dep.version)},
             CovariantSet(${Varargs(excludes)}: _*),
-            ${mappings.mapStringStringOption(dep.userParams)}
+            ${mappings.seqStringStringOption(dep.userParams)}
           )
         }
     }

diff --git a/dependency/shared/src/main/scala-3/dependency/literal/Mappings.scala b/dependency/shared/src/main/scala-3/dependency/literal/Mappings.scala
@@ -54,11 +54,11 @@ private[literal] final case class Mappings(mappings: List[(String, QExpr[String]
     '{Map(${Varargs(entries)}: _*)}
   }
 
-  def mapStringStringOption(map: Map[String, Option[String]])(using Quotes): QExpr[Map[String, Option[String]]] = {
-    val entries = map.toVector.sorted.map {
+  def seqStringStringOption(seq: Seq[(String, Option[String])])(using Quotes): QExpr[Seq[(String, Option[String])]] = {
+    val entries = seq.map {
       case (k, v) => '{(${Expr(k)}, ${stringOption(v)})}
     }
-    '{Map(${Varargs(entries)}: _*)}
+    '{Seq(${Varargs(entries)}: _*)}
   }
 
 }

diff --git a/dependency/shared/src/main/scala/dependency/DependencyLike.scala b/dependency/shared/src/main/scala/dependency/DependencyLike.scala
@@ -4,7 +4,7 @@ final case class DependencyLike[+A <: NameAttributes, +E <: NameAttributes](
   module: ModuleLike[A],
   version: String,
   exclude: CovariantSet[ModuleLike[E]],
-  userParams: Map[String, Option[String]]
+  userParams: Seq[(String, Option[String])]
 ) {
   def applyParams(params: ScalaParameters): Dependency =
     DependencyLike(
@@ -19,11 +19,15 @@ final case class DependencyLike[+A <: NameAttributes, +E <: NameAttributes](
   def nameAttributes: A = module.nameAttributes
   def attributes: Map[String, String] = module.attributes
 
+  lazy val userParamsMap: Map[String, Seq[Option[String]]] = userParams
+    .groupBy(_._1)
+    .mapValues(_.map(_._2))
+    .toMap
+
   private def excludeString: String =
     exclude.toVector.map(",exclude=" + _.render("%")).sorted.mkString
   private def userParamsString: String =
     userParams
-      .toVector
       .map {
         case (k, None) => k
         case (k, Some(v)) => s"$k=$v"
@@ -49,7 +53,7 @@ object DependencyLike {
       module,
       version,
       exclude,
-      Map.empty[String, Option[String]]
+      Nil
     )
 
   def apply[A <: NameAttributes](
@@ -60,6 +64,6 @@ object DependencyLike {
       module,
       version,
       CovariantSet.empty[AnyModule],
-      Map.empty[String, Option[String]]
+      Nil
     )
 }
diff --git a/dependency/shared/src/main/scala/dependency/package.scala b/dependency/shared/src/main/scala/dependency/package.scala
@@ -40,7 +40,7 @@ package object dependency extends dependency.literal.Extensions {
       module: Module,
       version: String,
       exclude: CovariantSet[Module],
-      userParams: Map[String, Option[String]]
+      userParams: Seq[(String, Option[String])]
     ): Dependency =
       DependencyLike(
         module,
@@ -58,7 +58,7 @@ package object dependency extends dependency.literal.Extensions {
         module,
         version,
         exclude,
-        Map()
+        Nil
       )
 
     def apply(
@@ -69,7 +69,7 @@ package object dependency extends dependency.literal.Extensions {
         module,
         version,
         CovariantSet(),
-        Map()
+        Nil
       )
 
     def apply(
@@ -81,7 +81,7 @@ package object dependency extends dependency.literal.Extensions {
         Module(organization, name),
         version,
         CovariantSet(),
-        Map()
+        Nil
       )
   }
 
@@ -154,7 +154,7 @@ package object dependency extends dependency.literal.Extensions {
       module: ScalaModule,
       version: String,
       exclude: CovariantSet[AnyModule],
-      userParams: Map[String, Option[String]]
+      userParams: Seq[(String, Option[String])]
     ): ScalaDependency =
       DependencyLike(
         module,
@@ -172,7 +172,7 @@ package object dependency extends dependency.literal.Extensions {
         module,
         version,
         exclude,
-        Map()
+        Nil
       )
 
     def apply(
@@ -183,7 +183,7 @@ package object dependency extends dependency.literal.Extensions {
         module,
         version,
         CovariantSet(),
-        Map()
+        Nil
       )
 
     def apply(
@@ -195,7 +195,7 @@ package object dependency extends dependency.literal.Extensions {
         ScalaModule(organization, name),
         version,
         CovariantSet(),
-        Map()
+        Nil
       )
   }
 }
diff --git a/dependency/shared/src/main/scala/dependency/parser/DependencyParser.scala b/dependency/shared/src/main/scala/dependency/parser/DependencyParser.scala
@@ -40,7 +40,7 @@ object DependencyParser {
         for {
           exclusions <- maybeExclusions
         } yield {
-          val userParams = remainingParams.iterator.map(parseParam).toMap ++ configOpt.toSeq.map("$inlineConfiguration" -> Some(_))
+          val userParams = remainingParams.iterator.map(parseParam).toSeq ++ configOpt.toSeq.map("$inlineConfiguration" -> Some(_))
           DependencyLike(module, version, exclusions, userParams)
         }
     }

diff --git a/dependency/shared/src/test/scala/dependency/LiteralTests.scala b/dependency/shared/src/test/scala/dependency/LiteralTests.scala
@@ -99,4 +99,8 @@ class LiteralTests extends munit.FunSuite {
     expect(dep.version == expected.version)
   }
 
+  test("params order") {
+    val depWithParams = dep"io.get-coursier::coursier:2.0.6,url=https://dl.cs/cs.jar,intransitive"
+    assert(depWithParams.userParams == Seq("url" -> Some("https://dl.cs/cs.jar"), "intransitive" -> None))
+  }
 }
diff --git a/dependency/shared/src/test/scala/dependency/ToStringTests.scala b/dependency/shared/src/test/scala/dependency/ToStringTests.scala
@@ -57,7 +57,7 @@ class ToStringTests extends munit.FunSuite {
 
   test("dependency with params") {
     val dep = Dependency("org", "name", "1.2").copy(
-      userParams = Map(
+      userParams = Seq(
         "intransitive" -> None,
         "aa" -> Some("bb")
       )

diff --git a/dependency/shared/src/test/scala/dependency/parser/DependencyParserTests.scala b/dependency/shared/src/test/scala/dependency/parser/DependencyParserTests.scala
@@ -67,13 +67,25 @@ class DependencyParserTests extends munit.FunSuite {
 
   test("param") {
     val res = DependencyParser.parse("org:name:1.2,something=ba")
-    val expected = Right(Dependency("org", "name", "1.2").copy(userParams = Map("something" -> Some("ba"))))
+    val expected = Right(Dependency("org", "name", "1.2").copy(userParams = Seq("something" -> Some("ba"))))
     expect(res == expected)
   }
 
   test("no-value param") {
     val res = DependencyParser.parse("org:name:1.2,something")
-    val expected = Right(Dependency("org", "name", "1.2").copy(userParams = Map("something" -> None)))
+    val expected = Right(Dependency("org", "name", "1.2").copy(userParams = Seq("something" -> None)))
+    expect(res == expected)
+  }
+
+  test("multiple same key params") {
+    val res = DependencyParser.parse("org:name:1.2,something=a,something,something=b")
+    val expected = Right(Dependency("org", "name", "1.2").copy(
+      userParams = Seq(
+        "something" -> Some("a"),
+        "something" -> None,
+        "something" -> Some("b")
+      )
+    ))
     expect(res == expected)
   }
 
@@ -86,7 +98,7 @@ class DependencyParserTests extends munit.FunSuite {
             Module("foo", "*"),
             ScalaModule("comp", "*")
           ),
-          userParams = Map(
+          userParams = Seq(
             "intransitive" -> None,
             "url" -> Some("aaaa")
           )
@@ -99,7 +111,7 @@ class DependencyParserTests extends munit.FunSuite {
     val res = DependencyParser.parse("org:name:1.2:runtime")
     val expected = Right(
       Dependency("org", "name", "1.2").copy(
-        userParams = Map(
+        userParams = Seq(
           "$inlineConfiguration" -> Some("runtime")
         )
       )
@@ -110,9 +122,9 @@ class DependencyParserTests extends munit.FunSuite {
     val res = DependencyParser.parse("org:name:1.2:runtime,something=ba")
     val expected = Right(
       Dependency("org", "name", "1.2").copy(
-        userParams = Map(
-          "$inlineConfiguration" -> Some("runtime"),
-          "something" -> Some("ba")
+        userParams = Seq(
+          "something" -> Some("ba"),
+          "$inlineConfiguration" -> Some("runtime")
         )
       )
     )