Unify Scala and Java Codegen Inputs (#585)

* Extract codegen-common module, #166

* Scala Codegen Main using the same option parser as Java Codegen, #166

There is one important difference, Scala Codegen does not allow mapping
dars to different package names, all dars have to be mapped to the same
package name.

Replace Scala Codegen println's with scala logging, respecting the
configured codegen verbosity

* Fix bazel formatting

* Update the release dry run script

* Releasing codegen-common

* Improving Scala Codegen error reporting (code review)

* Addressing codereview comments

* Make it explicit that we skip not supported option

language-support/codegen-common/BUILD.bazel

@@ -0,0 +1,30 @@
+# Copyright (c) 2019 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+
+load(
+    "//bazel_tools:scala.bzl",
+    "da_scala_library",
+    "da_scala_test",
+)
+
+da_scala_library(
+    name = "codegen-common",
+    srcs = glob(["src/main/**/*.scala"]),
+    tags = ["maven_coordinates=com.daml:codegen-common:__VERSION__"],
+    visibility = ["//visibility:public"],
+    deps = [
+        "//3rdparty/jvm/ch/qos/logback:logback_classic",
+        "//3rdparty/jvm/com/github/scopt",
+    ],
+)
+
+da_scala_test(
+    name = "test",
+    srcs = glob(["src/test/**/*.scala"]),
+    resources = glob(["src/test/resources/**/*"]),
+    deps = [
+        ":codegen-common",
+        "//3rdparty/jvm/com/github/scopt",
+        "//3rdparty/jvm/org/scalatest",
+    ],
+)

language-support/codegen-common/src/main/scala/com/digitalasset/daml/lf/codegen/conf/Conf.scala

@@ -6,9 +6,7 @@ package com.digitalasset.daml.lf.codegen.conf
 import java.nio.file.{Path, Paths}
 
 import ch.qos.logback.classic.Level
-import com.digitalasset.daml.lf.codegen.backend.Backend
-import com.digitalasset.daml.lf.codegen.backend.java.JavaBackend
-import scopt.Read
+import scopt.{OptionParser, Read}
 
 import scala.io.Source
 import scala.util.Try
@@ -18,13 +16,11 @@ import scala.util.Try
   * @param darFiles The [[Set]] of DAML-LF [[Path]]s to convert into code. It MUST contain
   *                        all the DAML-LF packages dependencies.
   * @param outputDirectory The directory where the code will be generated
-  * @param backend The backend that will be used to generate code (currently not exposed)
   * @param decoderPkgAndClass the fully qualified name of the generated decoder class (optional)
   */
 final case class Conf(
     darFiles: Map[Path, Option[String]] = Map(),
     outputDirectory: Path,
-    backend: Backend = JavaBackend,
     decoderPkgAndClass: Option[(String, String)] = None,
     verbosity: Level = Level.ERROR
 )
@@ -35,9 +31,9 @@ object Conf {
     """(?:(\p{javaJavaIdentifierStart}\p{javaJavaIdentifierPart}+(?:\.\p{javaJavaIdentifierStart}\p{javaJavaIdentifierPart}+)*)\.)(\p{javaJavaIdentifierStart}\p{javaJavaIdentifierPart}+)""".r
 
   def parse(args: Array[String]): Option[Conf] =
-    parser.parse(args, Conf(Map.empty, Paths.get("."), JavaBackend))
+    parser.parse(args, Conf(Map.empty, Paths.get(".")))
 
-  def parser = new scopt.OptionParser[Conf]("codegen") {
+  def parser: OptionParser[Conf] = new scopt.OptionParser[Conf]("codegen") {
     head("codegen", Version)
     note("Code generator for the DAML ledger bindings.\n")
 
@@ -101,7 +97,7 @@ object Conf {
       }
     }
 
-  lazy val Version =
+  lazy val Version: String =
     Try(Source.fromResource("COMPONENT-VERSION").getLines.reduce((t, u) => t + u).trim)
       .getOrElse("{component version not found on classpath}")
 

language-support/java/codegen/BUILD.bazel

@@ -49,6 +49,7 @@ da_scala_library(
         "//daml-lf/archive:daml_lf_java_proto",
         "//daml-lf/data",
         "//daml-lf/interface",
+        "//language-support/codegen-common",
         "//language-support/java/bindings:bindings-java",
     ],
 )
@@ -67,6 +68,7 @@ test_deps = [
     "//daml-lf/data",
     "//daml-lf/interface",
     "//language-support/java/bindings:bindings-java",
+    "//language-support/codegen-common",
 ]
 
 ########################################################

language-support/java/codegen/src/main/scala/com/digitalasset/daml/lf/codegen/CodeGenRunner.scala

@@ -10,6 +10,8 @@ import java.util.zip.ZipFile
 
 import com.digitalasset.daml.lf.DarManifestReader
 import com.digitalasset.daml.lf.archive.DarReader
+import com.digitalasset.daml.lf.codegen.backend.Backend
+import com.digitalasset.daml.lf.codegen.backend.java.JavaBackend
 import com.digitalasset.daml.lf.codegen.conf.Conf
 import com.digitalasset.daml.lf.data.ImmArray
 import com.digitalasset.daml.lf.iface.reader.{Interface, InterfaceReader}
@@ -111,7 +113,7 @@ private[codegen] object CodeGenRunner extends StrictLogging {
 
     // TODO (mp): pre-processing and escaping
     val preprocessingFuture: Future[InterfaceTrees] =
-      conf.backend.preprocess(interfaces, conf, pkgPrefixes)
+      backend.preprocess(interfaces, conf, pkgPrefixes)
 
     val future: Future[Unit] = {
       for {
@@ -127,6 +129,9 @@ private[codegen] object CodeGenRunner extends StrictLogging {
       s"Finish processing packageIds ''${interfaces.map(_.packageId.underlyingString).mkString(", ")}''")
   }
 
+  // TODO (#584): Make Java Codegen Backend configurable
+  private[codegen] val backend: Backend = JavaBackend
+
   private[CodeGenRunner] def processInterfaceTree(
       interfaceTree: InterfaceTree,
       conf: Conf,
@@ -134,7 +139,7 @@ private[codegen] object CodeGenRunner extends StrictLogging {
     logger.info(
       s"Start processing packageId '${interfaceTree.interface.packageId.underlyingString}'")
     for {
-      _ <- interfaceTree.process(conf.backend.process(_, conf, packagePrefixes))
+      _ <- interfaceTree.process(backend.process(_, conf, packagePrefixes))
     } yield {
       logger.info(
         s"Stop processing packageId '${interfaceTree.interface.packageId.underlyingString}'")

language-support/java/codegen/src/test/scala/com/digitalasset/daml/lf/codegen/CodeGenRunnerTests.scala

@@ -8,10 +8,10 @@ import java.nio.file.Files
 
 import com.digitalasset.daml.lf.codegen.backend.java.JavaBackend
 import com.digitalasset.daml.lf.codegen.conf.Conf
-import org.scalatest.FlatSpec
+import org.scalatest.{FlatSpec, Matchers}
 
 @SuppressWarnings(Array("org.wartremover.warts.Any"))
-class CodeGenRunnerTests extends FlatSpec {
+class CodeGenRunnerTests extends FlatSpec with Matchers {
 
   behavior of "collectDamlLfInterfaces"
 
@@ -21,12 +21,15 @@ class CodeGenRunnerTests extends FlatSpec {
 
   val dummyOutputDir = Files.createTempDirectory("codegen")
 
+  it should "always use JavaBackend, which is currently hardcoded" in {
+    CodeGenRunner.backend should be theSameInstanceAs JavaBackend
+  }
+
   it should "read interfaces from a single DAR file without a prefix" in {
 
     val conf = Conf(
       Map(testDar -> None),
       dummyOutputDir,
-      JavaBackend
     )
 
     val (interfaces, pkgPrefixes) = CodeGenRunner.collectDamlLfInterfaces(conf)
@@ -40,7 +43,6 @@ class CodeGenRunnerTests extends FlatSpec {
     val conf = Conf(
       Map(testDar -> Some("PREFIX")),
       dummyOutputDir,
-      JavaBackend
     )
 
     val (interfaces, pkgPrefixes) = CodeGenRunner.collectDamlLfInterfaces(conf)

language-support/scala/codegen-sample-app/BUILD.bazel

@@ -32,7 +32,7 @@ genrule(
         ":MySecondMain",
     ],
     outs = ["MyMain-codegen-out"],
-    cmd = "$(execpath //language-support/scala/codegen:codegen-main) --input-files $(location :MyMain.dar),$(location :MySecondMain.dar) --package-name com.digitalasset.sample --output-dir $@",
+    cmd = "$(execpath //language-support/scala/codegen:codegen-main) $(location :MyMain.dar)=com.digitalasset.sample $(location :MySecondMain.dar)=com.digitalasset.sample --output-directory=$@ --verbosity=2",
     tools = [
         ":MyMain.dar",
         ":MySecondMain.dar",

language-support/scala/codegen/BUILD.bazel

@@ -66,7 +66,11 @@ da_scala_binary(
     ],
     deps = [
         ":codegen",
+        "//3rdparty/jvm/ch/qos/logback:logback_classic",
         "//3rdparty/jvm/com/github/scopt",
+        "//3rdparty/jvm/com/typesafe/scala_logging",
+        "//3rdparty/jvm/org/scalaz:scalaz_core",
+        "//language-support/codegen-common",
     ],
 )
 

language-support/scala/codegen/src/main/scala/com/digitalasset/codegen/CodeGen.scala

@@ -18,6 +18,7 @@ import lf.{DefTemplateWithRecord, EnvironmentInterface, LFUtil, ScopedDataType}
 import com.digitalasset.daml.lf.data.Ref._
 import com.digitalasset.daml.lf.iface.reader.Errors.ErrorLoc
 import com.digitalasset.daml_lf.DamlLf
+import com.typesafe.scalalogging.Logger
 import scalaz._
 import scalaz.std.tuple._
 import scalaz.std.list._
@@ -32,6 +33,8 @@ import scala.util.{Failure, Success}
 
 object CodeGen {
 
+  private val logger: Logger = Logger(getClass)
+
   type Payload = (PackageId, DamlLf.ArchivePayload)
 
   sealed abstract class Mode extends Serializable with Product
@@ -97,7 +100,7 @@ object CodeGen {
     reader.readFile(f) match {
       case Success(p) => \/.right(p)
       case Failure(e) =>
-        e.printStackTrace()
+        logger.error("Scala Codegen error", e)
         \/.left(e.getLocalizedMessage)
     }
 
@@ -113,8 +116,7 @@ object CodeGen {
   private def decodeInterface(p: Payload): String \/ Interface =
     \/.fromTryCatchNonFatal {
       val packageId: PackageId = p._1
-      println(
-        s"Scala Codegen - decoding archive with Package ID: ${packageId.underlyingString: String}")
+      logger.info(s"decoding archive with Package ID: ${packageId.underlyingString: String}")
       val (errors, out) = Interface.read(p)
       if (!errors.empty) {
         \/.left(formatDecodeErrors(packageId, errors))
@@ -163,11 +165,13 @@ object CodeGen {
     // Each record/variant has Scala code generated for it individually, unless their names are related
     writeTemplatesAndTypes(util)(WriteParams(supportedTemplateIds, typeDeclsToGenerate))
 
-    println("Scala Codegen result:")
-    println(s"Number of generated templates: ${supportedTemplateIds.size}")
-    println(
-      s"Number of not generated templates: ${util.templateCount(interface) - supportedTemplateIds.size}")
-    println(s"Details: ${orderedDependencies.errors.map(_.msg).mkString("\n")}")
+    logger.info(
+      s"""Scala Codegen result:
+          |Number of generated templates: ${supportedTemplateIds.size}
+          |Number of not generated templates: ${util
+           .templateCount(interface) - supportedTemplateIds.size}
+          |Details: ${orderedDependencies.errors.map(_.msg).mkString("\n")}""".stripMargin
+    )
   }
 
   private[codegen] def produceTemplateAndTypeFilesLF(
@@ -265,9 +269,9 @@ object CodeGen {
   private[this] def writeTemplatesAndTypes(util: Util)(
       wp: WriteParams[util.TemplateInterface]): Unit = {
     util.templateAndTypeFiles(wp) foreach {
-      case -\/(msg) => println(msg)
+      case -\/(msg) => logger.debug(msg)
       case \/-((msg, filePath, trees)) =>
-        msg foreach (println(_))
+        msg foreach (m => logger.debug(m))
         writeCode(filePath, trees)
     }
   }
@@ -283,6 +287,6 @@ object CodeGen {
         writer.close()
       }
     } else {
-      println(s"WARNING: nothing to generate, empty trees passed, file: $filePath")
+      logger.warn(s"WARNING: nothing to generate, empty trees passed, file: $filePath")
     }
 }

language-support/scala/codegen/src/main/scala/com/digitalasset/codegen/Main.scala

@@ -4,47 +4,66 @@
 package com.digitalasset.codegen
 
 import java.io.File
+import java.nio.file.Path
 
-import scopt.OptionParser
+import ch.qos.logback.classic.Level
+import com.digitalasset.daml.lf.codegen.conf.Conf
+import com.typesafe.scalalogging.StrictLogging
+import org.slf4j.{Logger, LoggerFactory}
+import scalaz.Cord
 
-object Main {
+import scala.collection.breakOut
 
-  case class Config(
-      inputFiles: Seq[File] = Seq(),
-      packageName: String = "",
-      outputDir: File = new File("."),
-      codeGenMode: CodeGen.Mode = CodeGen.Novel)
+object Main extends StrictLogging {
 
-  private val parser = new OptionParser[Config]("codegen") {
-    help("help").text("prints this usage text")
+  private val codegenId = "Scala Codegen"
 
-    opt[Seq[File]]("input-files").required
-      .abbr("i")
-      .action((d, c) => c.copy(inputFiles = d))
-      .text("input DAR or DALF files")
+  def main(args: Array[String]): Unit =
+    Conf.parse(args) match {
+      case Some(Conf(darMap, outputDir, decoderPkgAndClass, verbosity)) =>
+        setGlobalLogLevel(verbosity)
+        logUnsupportedEventDecoderOverride(decoderPkgAndClass)
+        val (dars, packageName) = darsAndOnePackageName(darMap)
+        CodeGen.generateCode(dars, packageName, outputDir.toFile, CodeGen.Novel)
+      case None =>
+        throw new IllegalArgumentException(
+          s"Invalid ${codegenId: String} command line arguments: ${args.mkString(" "): String}")
+    }
 
-    opt[String]("package-name")
-      .required()
-      .abbr("p")
-      .action((d, c) => c.copy(packageName = d))
-      .text("package name e.g. com.digitalasset.mypackage")
-
-    opt[File]("output-dir")
-      .required()
-      .abbr("o")
-      .action((d, c) => c.copy(outputDir = d))
-      .text("output directory for Scala files")
-  }
-
-  def main(args: Array[String]): Unit = {
-    parser.parse(args, Config()) match {
-      case Some(config) =>
-        CodeGen.generateCode(
-          config.inputFiles.toList,
-          config.packageName,
-          config.outputDir,
-          config.codeGenMode)
-      case None => // arguments are bad, error message will have been displayed
+  private def setGlobalLogLevel(verbosity: Level): Unit = {
+    LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME) match {
+      case a: ch.qos.logback.classic.Logger =>
+        a.setLevel(verbosity)
+        logger.info(s"${codegenId: String} verbosity: $verbosity")
+      case _ =>
+        logger.warn(s"${codegenId: String} cannot set requested verbosity: $verbosity")
     }
   }
+
+  private def logUnsupportedEventDecoderOverride(mapping: Option[(String, String)]): Unit =
+    mapping.foreach {
+      case (a, b) =>
+        logger.warn(
+          s"${codegenId: String} does not allow overriding Event Decoder, skipping: ${a: String} -> ${b: String}")
+    }
+
+  private def darsAndOnePackageName(darMap: Map[Path, Option[String]]): (List[File], String) = {
+    val dars: List[File] = darMap.keys.map(_.toFile)(breakOut)
+    val uniquePackageNames: Set[String] = darMap.values.collect { case Some(x) => x }(breakOut)
+    uniquePackageNames.toSeq match {
+      case Seq(packageName) =>
+        (dars, packageName)
+      case _ =>
+        throw new IllegalStateException(
+          s"${codegenId: String} expects all dars mapped to the same package name, " +
+            s"requested: ${format(darMap): String}")
+    }
+  }
+
+  private def format(map: Map[Path, Option[String]]): String = {
+    val cord = map.foldLeft(Cord("{")) { (str, kv) =>
+      str ++ kv._1.toFile.getAbsolutePath ++ "->" ++ kv._2.toString ++ ","
+    }
+    (cord ++ "}").toString
+  }
 }

language-support/scala/codegen/src/main/scala/com/digitalasset/codegen/lf/DamlContractTemplateGen.scala

@@ -8,6 +8,7 @@ import java.io.File
 import com.digitalasset.codegen.Util
 import com.digitalasset.daml.lf.data.ImmArray.ImmArraySeq
 import com.digitalasset.daml.lf.data.Ref.{Identifier, QualifiedName}
+import com.typesafe.scalalogging.Logger
 
 import scala.reflect.runtime.universe._
 
@@ -21,6 +22,8 @@ import scala.reflect.runtime.universe._
 object DamlContractTemplateGen {
   import LFUtil.{domainApiAlias, rpcValueAlias}
 
+  private val logger: Logger = Logger(getClass)
+
   def generate(
       util: LFUtil,
       templateId: Identifier,
@@ -30,7 +33,7 @@ object DamlContractTemplateGen {
     val templateName = util.mkDamlScalaName(Util.Template, templateId)
     val contractName = util.mkDamlScalaName(Util.Contract, templateId)
 
-    LFUtil.lfprintln(s"generate templateDecl: $templateName, $templateInterface")
+    logger.debug(s"generate templateDecl: $templateName, $templateInterface")
 
     val templateChoiceMethods = templateInterface.template.choices.flatMap {
       case (id, interface) => util.genTemplateChoiceMethods(id, interface)

language-support/scala/codegen/src/main/scala/com/digitalasset/codegen/lf/DamlRecordOrVariantTypeGen.scala

@@ -9,6 +9,7 @@ import com.digitalasset.codegen.Util
 import com.digitalasset.codegen.lf.LFUtil.{TupleNesting, escapeIfReservedName}
 import com.digitalasset.daml.lf.iface, iface.{Type => _, _}
 import com.digitalasset.daml.lf.data.Ref.{Identifier, QualifiedName}
+import com.typesafe.scalalogging.Logger
 import scalaz.{-\/, \/, \/-}
 
 import scala.collection.breakOut
@@ -28,6 +29,8 @@ object DamlRecordOrVariantTypeGen {
 
   import runUni._
 
+  private val logger: Logger = Logger(getClass)
+
   type VariantField = (String, List[FieldWithType] \/ iface.Type)
   type RecordOrVariant = ScopedDataType.DT[FieldWithType, VariantField]
 
@@ -54,7 +57,7 @@ object DamlRecordOrVariantTypeGen {
       rootClassChildren: Seq[Tree],
       companionChildren: Iterable[Tree]): (File, Iterable[Tree]) = {
 
-    LFUtil.lfprintln(s"generate typeDecl: $typeDecl")
+    logger.debug(s"generate typeDecl: $typeDecl")
 
     import typeDecl.name
     val damlScalaName = util.mkDamlScalaName(Util.UserDefinedType, name)

language-support/scala/codegen/src/main/scala/com/digitalasset/codegen/lf/LFUtil.scala

@@ -361,11 +361,6 @@ object LFUtil {
   private[this] def unfoldIList[S, A](init: S)(step: S => Option[(A, S)]): IList[A] =
     step(init).fold(IList.empty[A]) { case (a, next) => a :: unfoldIList(next)(step) }
 
-  /** Debugging message for LF codegen */
-  @annotation.elidable(annotation.elidable.FINE)
-  private[codegen] def lfprintln(s: String): Unit =
-    println(s)
-
   val domainApiAlias = q"` lfdomainapi`"
   val rpcValueAlias = q"` rpcvalue`"
   val rpcEventAlias = q"` rpcevent`"

release-dry-run.sh

@@ -9,4 +9,4 @@ set -eux
 
 release_dir=/var/tmp/daml-bintray-release
 
-rm -rf "$release_dir" && bazel build //release:release && ./bazel-out/k8-fastbuild/bin/release/release bintray --release-dir "$release_dir"
+rm -rf "$release_dir" && bazel build //release:release && ./bazel-out/k8-fastbuild/bin/release/release --artifacts release/artifacts.yaml --release-dir "$release_dir"

release/artifacts.yaml

@@ -125,3 +125,5 @@
   type: jar
 - target: //navigator/backend:navigator-binary
   type: jar-deploy
+- target: //language-support/codegen-common:codegen-common
+  type: jar