Fix parsing of java codegen module prefixes (digital-asset#12977)

* Fix parsing of java codegen module prefixes I admit I have no idea what I thought when I wrote that split but it was clearly nothing sensible. changelog_begin - [Java Codegen] Fix parsing of `module-prefixes` to support package names containing dashes. changelog_end * reveiw comments * Inline nameVersion changelog_begin changelog_end * . changelog_begin changelog_end
kgeonw · Feb 17, 2022 · ca6167e · ca6167e
1 parent 3730f02
commit ca6167e
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 11 deletions.
diff --git a/language-support/codegen-common/BUILD.bazel b/language-support/codegen-common/BUILD.bazel
@@ -32,6 +32,7 @@ da_scala_test(
     deps = [
         ":codegen-common",
         "//daml-assistant/scala-daml-project-config",
+        "//daml-lf/data",
         "@maven//:ch_qos_logback_logback_classic",
     ],
 )
diff --git a/...gen-common/src/main/scala/com/digitalasset/daml/lf/codegen/conf/CodegenConfigReader.scala b/...gen-common/src/main/scala/com/digitalasset/daml/lf/codegen/conf/CodegenConfigReader.scala
@@ -158,17 +158,18 @@ object CodegenConfigReader {
   implicit val decodePackageReference: KeyDecoder[PackageReference] =
     new KeyDecoder[PackageReference] {
       // TODO (MK) https://github.com/digital-asset/daml/issues/9934
-      // For now we only allow name-vesion pairs to match the compiler. Once the compiler
+      // For now we only allow name-version pairs to match the compiler. Once the compiler
       // accepts package ids we can allow for those here as well.
-      final def apply(key: String): Option[PackageReference] = nameVersion(key)
-
-      private def nameVersion(key: String): Option[PackageReference] = key.split("-") match {
-        case Array(name, version) =>
-          for {
-            name <- PackageName.fromString(name).toOption
-            version <- PackageVersion.fromString(version).toOption
-          } yield PackageReference.NameVersion(name, version)
-        case _ => None
-      }
+      final def apply(key: String): Option[PackageReference] =
+        key.splitAt(key.lastIndexOf('-')) match {
+          case (rawPackageName, rawPackageVersion)
+              if rawPackageName.nonEmpty && rawPackageVersion.nonEmpty =>
+            for {
+              name <- PackageName.fromString(rawPackageName).toOption
+              version <- PackageVersion.fromString(rawPackageVersion.tail).toOption
+            } yield PackageReference.NameVersion(name, version)
+          case _ =>
+            None
+        }
     }
 }
diff --git a/...common/src/test/scala/com/digitalasset/daml/lf/codegen/conf/CodegenConfigReaderSpec.scala b/...common/src/test/scala/com/digitalasset/daml/lf/codegen/conf/CodegenConfigReaderSpec.scala
@@ -8,6 +8,7 @@ import java.nio.file.{Path, Paths}
 import ch.qos.logback.classic.Level
 import com.daml.assistant.config.{ConfigMissing, ConfigParseError, ProjectConfig}
 import com.daml.lf.codegen.conf.CodegenConfigReader.{CodegenDest, Java, Result}
+import com.daml.lf.data.Ref.{PackageName, PackageVersion}
 import org.scalatest.matchers.should.Matchers
 import org.scalatest.flatspec.AnyFlatSpec
 
@@ -114,6 +115,91 @@ class CodegenConfigReaderSpec extends AnyFlatSpec with Matchers {
     )
   }
 
+  it should "parse package references with >= 1 dash" in {
+    val badConfigStr = """|
+      |name: quickstart
+      |version: 1.2.3
+      |codegen:
+      |  java:
+      |    package-prefix: my.company.java.package
+      |    output-directory: path/to/output/java/directory
+      |    decoderClass: my.company.java.DecoderClass
+      |module-prefixes:
+      |  a-0.0.0: a
+      |  a-a-0.0.0: a
+      |  a-a-a-0.0.0: a
+      |  a-a-a-a-0.0.0: a
+      |  a-a-a-a-a-0.0.0: a""".stripMargin
+
+    val version = PackageVersion.assertFromString("0.0.0")
+    val prefix = "a"
+
+    val expected = Conf(
+      darFiles = Map(
+        projectRoot.resolve(".daml/dist/quickstart-1.2.3.dar") -> Some("my.company.java.package")
+      ),
+      outputDirectory = path("path/to/output/java/directory"),
+      decoderPkgAndClass = Some(("my.company.java", "DecoderClass")),
+      modulePrefixes = Seq("a", "a-a", "a-a-a", "a-a-a-a", "a-a-a-a-a").view
+        .map(x => PackageReference.NameVersion(PackageName.assertFromString(x), version) -> prefix)
+        .toMap,
+    )
+
+    codegenConf(badConfigStr, Java) shouldBe Right(
+      expected
+    )
+  }
+
+  it should "parse package references with at least one number" in {
+    val badConfigStr = """|
+      |name: quickstart
+      |version: 1.2.3
+      |codegen:
+      |  java:
+      |    package-prefix: my.company.java.package
+      |    output-directory: path/to/output/java/directory
+      |    decoderClass: my.company.java.DecoderClass
+      |module-prefixes:
+      |  a-0: a
+      |  a-0.0: a
+      |  a-0.0.0: a""".stripMargin
+
+    val name = PackageName.assertFromString("a")
+    val prefix = "a"
+
+    val expected = Conf(
+      darFiles = Map(
+        projectRoot.resolve(".daml/dist/quickstart-1.2.3.dar") -> Some("my.company.java.package")
+      ),
+      outputDirectory = path("path/to/output/java/directory"),
+      decoderPkgAndClass = Some(("my.company.java", "DecoderClass")),
+      modulePrefixes = Seq("0", "0.0", "0.0.0").view
+        .map(x => PackageReference.NameVersion(name, PackageVersion.assertFromString(x)) -> prefix)
+        .toMap,
+    )
+
+    codegenConf(badConfigStr, Java) shouldBe Right(
+      expected
+    )
+  }
+
+  it should "reject package references with no dash" in {
+    val badConfigStr = """|
+      |name: quickstart
+      |version: 1.2.3
+      |codegen:
+      |  java:
+      |    package-prefix: my.company.java.package
+      |    output-directory: path/to/output/java/directory
+      |    decoderClass: my.company.java.DecoderClass
+      |module-prefixes:
+      |  a: a""".stripMargin
+
+    codegenConf(badConfigStr, Java) shouldBe Left(
+      ConfigParseError("[K, V]Map[K, V]: DownField(a),DownField(module-prefixes)")
+    )
+  }
+
   private def path(s: String): Path = Paths.get(s)
 
   private val projectRoot = Paths.get("/project/root")