LF: Structure package loading errors (#9980)

part of #9974.

CHANGELOG_BEGIN
CHANGELOG_END

daml-lf/encoder/src/main/scala/com/digitalasset/daml/lf/archive/testing/DamlLfEncoder.scala

@@ -69,7 +69,8 @@ private[daml] object DamlLfEncoder extends App {
         )
       } else None
 
-    val pkg = Ast.Package(modules, Set.empty[PackageId], parserParameters.languageVersion, metadata)
+    val pkg =
+      Ast.Package(modules, Set.empty[PackageId], parserParameters.languageVersion, metadata)
     val pkgs = Interface(Map(pkgId -> pkg))
 
     Validation.checkPackage(pkgs, pkgId, pkg).left.foreach(e => error(e.pretty))

daml-lf/encoder/src/test/scala/com/digitalasset/daml/lf/archive/testing/EncodeV1Spec.scala

@@ -160,7 +160,8 @@ class EncodeV1Spec extends AnyWordSpec with Matchers with TableDrivenPropertyChe
       """
 
       validate(pkgId, pkg)
-      val archive = Encode.encodeArchive(pkgId -> pkg, defaultParserParameters.languageVersion)
+      val archive =
+        Encode.encodeArchive(pkgId -> pkg, defaultParserParameters.languageVersion)
       val ((hashCode @ _, decodedPackage: Package), _) = Decode.readArchiveAndVersion(archive)
 
       val pkg1 = normalize(decodedPackage, hashCode, pkgId)

daml-lf/engine/BUILD.bazel

@@ -28,6 +28,7 @@ da_scala_library(
         "//daml-lf/language",
         "//daml-lf/transaction",
         "//daml-lf/validation",
+        "//libs-scala/nameof",
     ],
 )
 

daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/ConcurrentCompiledPackages.scala

@@ -11,7 +11,7 @@ import com.daml.lf.engine.ConcurrentCompiledPackages.AddPackageState
 import com.daml.lf.language.Ast.{Package, PackageSignature}
 import com.daml.lf.language.{Interface, Util => AstUtil}
 import com.daml.lf.speedy.Compiler
-import com.daml.lf.speedy.Compiler.CompilationError
+import com.daml.nameof.NameOf
 
 import scala.jdk.CollectionConverters._
 import scala.collection.concurrent.{Map => ConcurrentMap}
@@ -61,7 +61,13 @@ private[lf] final class ConcurrentCompiledPackages(compilerConfig: Compiler.Conf
         if (!signatures.contains(pkgId)) {
 
           val pkg = state.packages.get(pkgId) match {
-            case None => return ResultError(Error.Package.Generic(s"Could not find package $pkgId"))
+            case None =>
+              return ResultError(
+                Error.Package.Internal(
+                  NameOf.qualifiedNameOfCurrentFunc,
+                  s"broken invariant: Could not find package $pkgId",
+                )
+              )
             case Some(pkg_) => pkg_
           }
 
@@ -72,7 +78,7 @@ private[lf] final class ConcurrentCompiledPackages(compilerConfig: Compiler.Conf
                 dependency,
                 {
                   case None =>
-                    ResultError(Error.Package.Generic(s"Could not find package $dependency"))
+                    ResultError(Error.Package.MissingPackage(dependency))
                   case Some(dependencyPkg) =>
                     addPackageInternal(
                       AddPackageState(
@@ -101,10 +107,26 @@ private[lf] final class ConcurrentCompiledPackages(compilerConfig: Compiler.Conf
                 new speedy.Compiler(extendedSignatures, compilerConfig)
                   .unsafeCompilePackage(pkgId, pkg)
               } catch {
-                case CompilationError(msg) =>
-                  return ResultError(Error.Package.Generic(s"Compilation Error: $msg"))
                 case e: validation.ValidationError =>
                   return ResultError(Error.Package.Validation(e))
+                case Compiler.LanguageVersionError(
+                      packageId,
+                      languageVersion,
+                      allowedLanguageVersions,
+                    ) =>
+                  return ResultError(
+                    Error.Package
+                      .AllowedLanguageVersion(packageId, languageVersion, allowedLanguageVersions)
+                  )
+                case Compiler.CompilationError(msg) =>
+                  return ResultError(
+                    // compilation errors are internal since typechecking should
+                    // catch any errors arising during compilation
+                    Error.Package.Internal(
+                      NameOf.qualifiedNameOfCurrentFunc,
+                      s"Compilation Error: $msg",
+                    )
+                  )
               }
             defns.foreach { case (defnId, defn) =>
               definitions.put(defnId, defn)
@@ -142,5 +164,8 @@ object ConcurrentCompiledPackages {
       packages: Map[PackageId, Package], // the packages we're currently compiling
       seenDependencies: Set[PackageId], // the dependencies we've found so far
       toCompile: List[PackageId],
-  )
+  ) {
+    // Invariant
+    // assert(toCompile.forall(packages.contains))
+  }
 }

daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Engine.scala

@@ -228,7 +228,7 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
             case Some(pkg) =>
               compiledPackages.addPackage(pkgId, pkg).flatMap(_ => loadPackages(rest))
             case None =>
-              ResultError(Error.Package.Generic(s"package $pkgId not found"))
+              ResultError(Error.Package.MissingPackage(pkgId))
           },
         )
       case Nil =>
@@ -246,7 +246,7 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
         case speedy.Compiler.PackageNotFound(_) =>
           handleMissingDependencies.flatMap(_ => start)
         case speedy.Compiler.CompilationError(error) =>
-          ResultError(Error.Package.Generic(s"CompilationError: $error"))
+          ResultError(Error.Preprocessing.Generic(s"CompilationError: $error"))
       }
     start
   }
@@ -426,24 +426,16 @@ class Engine(val config: EngineConfig = new EngineConfig(LanguageVersion.StableV
       _ <- pkgs
         .collectFirst {
           case (pkgId, pkg) if !config.allowedLanguageVersions.contains(pkg.languageVersion) =>
-            Error.Package.Generic(
-              s"Disallowed language version in package $pkgId: " +
-                s"Expected version between ${config.allowedLanguageVersions.min.pretty} and ${config.allowedLanguageVersions.max.pretty} but got ${pkg.languageVersion.pretty}"
+            Error.Package.AllowedLanguageVersion(
+              pkgId,
+              pkg.languageVersion,
+              config.allowedLanguageVersions,
             )
         }
         .toLeft(())
-      _ <- {
-        val pkgIds = pkgs.keySet
-        val missingDeps = pkgs.valuesIterator.flatMap(_.directDeps).toSet.filterNot(pkgIds)
-        Either.cond(
-          missingDeps.isEmpty,
-          (),
-          Error.Package.Generic(
-            s"The set of packages ${pkgIds.mkString("{'", "', '", "'}")} is not self consistent, the missing dependencies are ${missingDeps
-              .mkString("{'", "', '", "'}")}."
-          ),
-        )
-      }
+      pkgIds = pkgs.keySet
+      missingDeps = pkgs.valuesIterator.flatMap(_.directDeps).toSet.filterNot(pkgIds)
+      _ <- Either.cond(missingDeps.isEmpty, (), Error.Package.SelfConsistency(pkgIds, missingDeps))
       interface = Interface(pkgs)
       _ <- {
         pkgs.iterator

daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/Error.scala

@@ -25,14 +25,40 @@ object Error {
       def msg: String
     }
 
-    // TODO https://github.com/digital-asset/daml/issues/9974
-    //  get rid of Generic
-    final case class Generic(override val msg: String) extends Error
+    final case class Internal(nameOfFunc: String, override val msg: String, detailMsg: String = "")
+        extends Error
 
     final case class Validation(validationError: validation.ValidationError) extends Error {
       def msg: String = validationError.pretty
     }
 
+    final case class MissingPackages(packageIds: Set[Ref.PackageId]) extends Error {
+      val s = if (packageIds.size <= 1) "" else "s"
+      override def msg: String = s"Couldn't find package$s ${packageIds.mkString(",")}"
+    }
+    private[engine] object MissingPackage {
+      def apply(packageId: Ref.PackageId): MissingPackages = MissingPackages(Set(packageId))
+    }
+
+    final case class AllowedLanguageVersion(
+        packageId: Ref.PackageId,
+        languageVersion: language.LanguageVersion,
+        allowedLanguageVersions: VersionRange[language.LanguageVersion],
+    ) extends Error {
+      def msg: String =
+        s"Disallowed language version in package $packageId: " +
+          s"Expected version between ${allowedLanguageVersions.min.pretty} and ${allowedLanguageVersions.max.pretty} but got ${languageVersion.pretty}"
+    }
+
+    final case class SelfConsistency(
+        packageIds: Set[Ref.PackageId],
+        missingDependencies: Set[Ref.PackageId],
+    ) extends Error {
+      def msg: String =
+        s"The set of packages ${packageIds.mkString("{'", "', '", "'}")} is not self consistent, " +
+          s"the missing dependencies are ${missingDependencies.mkString("{'", "', '", "'}")}."
+    }
+
   }
 
   // Error happening during command/transaction preprocessing

daml-lf/engine/src/main/scala/com/digitalasset/daml/lf/engine/preprocessing/Preprocessor.scala

@@ -54,7 +54,7 @@ private[engine] final class Preprocessor(compiledPackages: MutableCompiledPackag
                 )
               } yield r
             case None =>
-              ResultError(Error.Package.Generic(s"Couldn't find package $pkgId"))
+              ResultError(Error.Package.MissingPackage(pkgId))
           },
         )
 

daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/ConcurrentCompiledPackagesTest.scala

@@ -0,0 +1,66 @@
+// Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.daml.lf
+package engine
+
+import com.daml.lf.language.LanguageVersion
+import com.daml.lf.speedy.Compiler
+import com.daml.lf.testing.parser.Implicits._
+import org.scalatest.Inside
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+class ConcurrentCompiledPackagesTest extends AnyWordSpec with Matchers with Inside {
+
+  "ConcurrentCompiledPackages" should {
+
+    val pkg =
+      p"""
+        module Mod { 
+          val string: Text = "t";
+        }
+      """
+
+    "load valid package" in {
+
+      new ConcurrentCompiledPackages(Compiler.Config.Dev)
+        .addPackage(defaultParserParameters.defaultPackageId, pkg) shouldBe ResultDone(())
+
+    }
+
+    "not load of an invalid package" in {
+
+      val packages = new ConcurrentCompiledPackages(Compiler.Config.Dev)
+
+      val illFormedPackage =
+        p"""
+        module Mod { 
+          val string: Text = 1;
+        }
+      """;
+
+      inside(packages.addPackage(defaultParserParameters.defaultPackageId, illFormedPackage)) {
+        case ResultError(Error.Package(_: Error.Package.Validation)) =>
+      }
+    }
+
+    "not load of a package with disallowed language version" in {
+
+      val packages = new ConcurrentCompiledPackages(
+        Compiler.Config.Default.copy(allowedLanguageVersions = LanguageVersion.LegacyVersions)
+      )
+
+      assert(!LanguageVersion.LegacyVersions.contains(defaultParserParameters.languageVersion))
+
+      inside(packages.addPackage(defaultParserParameters.defaultPackageId, pkg)) {
+        case ResultError(Error.Package(err: Error.Package.AllowedLanguageVersion)) =>
+          err.packageId shouldBe defaultParserParameters.defaultPackageId
+          err.languageVersion shouldBe defaultParserParameters.languageVersion
+          err.allowedLanguageVersions shouldBe LanguageVersion.LegacyVersions
+      }
+    }
+
+  }
+
+}

daml-lf/engine/src/test/scala/com/digitalasset/daml/lf/engine/EngineValidatePackagesTest.scala

@@ -0,0 +1,89 @@
+// Copyright (c) 2021 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.daml.lf
+package engine
+
+import com.daml.lf.data.Ref
+import com.daml.lf.language.LanguageVersion
+import com.daml.lf.testing.parser.Implicits._
+import org.scalatest.Inside
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.wordspec.AnyWordSpec
+
+class EngineValidatePackagesTest extends AnyWordSpec with Matchers with Inside {
+
+  import defaultParserParameters.{defaultPackageId => pkgId, languageVersion => langVersion}
+
+  private def newEngine = new Engine(EngineConfig(LanguageVersion.DevVersions))
+
+  "Engine.validatePackages" should {
+
+    val pkg =
+      p"""
+        module Mod { 
+          val string: Text = "t";
+        }
+      """
+
+    "accept valid package" in {
+
+      newEngine.validatePackages(Map(pkgId -> pkg)) shouldBe Right(())
+
+    }
+
+    "reject ill-typed packages" in {
+
+      val illTypedPackage =
+        p"""
+        module Mod { 
+          val string: Text = 1;
+        }
+      """
+
+      inside(
+        newEngine.validatePackages(Map(pkgId -> illTypedPackage))
+      ) { case Left(_: Error.Package.Validation) =>
+      }
+
+    }
+
+    "reject packages with disallowed language version" in {
+
+      val engine = new Engine(EngineConfig(LanguageVersion.LegacyVersions))
+
+      assert(!LanguageVersion.LegacyVersions.contains(langVersion))
+
+      inside(engine.validatePackages(Map(pkgId -> pkg))) {
+        case Left(err: Error.Package.AllowedLanguageVersion) =>
+          err.packageId shouldBe pkgId
+          err.languageVersion shouldBe langVersion
+          err.allowedLanguageVersions shouldBe LanguageVersion.LegacyVersions
+      }
+
+    }
+
+    "reject non self-consistent sets of packages" in {
+
+      val libraryId = Ref.PackageId.assertFromString("-library-")
+
+      val dependentPackage =
+        p"""
+        module Mod { 
+          val string: Text = '-library-':Mod:Text;
+        }
+      """
+          // TODO: parser should set dependencies properly
+          .copy(directDeps = Set(libraryId))
+
+      inside(newEngine.validatePackages(Map(pkgId -> dependentPackage))) {
+        case Left(Error.Package.SelfConsistency(pkgIds, deps)) =>
+          pkgIds shouldBe Set(pkgId)
+          deps shouldBe Set(libraryId)
+      }
+
+    }
+
+  }
+
+}

daml-lf/interpreter/src/main/scala/com/digitalasset/daml/lf/speedy/Compiler.scala

@@ -7,7 +7,7 @@ package speedy
 import java.util
 
 import com.daml.lf.data.Ref._
-import com.daml.lf.data.{ImmArray, Numeric, Struct, Time}
+import com.daml.lf.data.{ImmArray, Numeric, Ref, Struct, Time}
 import com.daml.lf.language.Ast._
 import com.daml.lf.language.{LanguageVersion, LookupError, Interface}
 import com.daml.lf.speedy.Anf.flattenToAnf
@@ -34,6 +34,11 @@ import scala.reflect.ClassTag
 private[lf] object Compiler {
 
   case class CompilationError(error: String) extends RuntimeException(error, null, true, false)
+  case class LanguageVersionError(
+      packageId: Ref.PackageId,
+      languageVersion: language.LanguageVersion,
+      allowedLanguageVersions: VersionRange[language.LanguageVersion],
+  ) extends RuntimeException(s"Disallowed language version $languageVersion", null, true, false)
   case class PackageNotFound(pkgId: PackageId)
       extends RuntimeException(s"Package not found $pkgId", null, true, false)
 
@@ -339,10 +344,7 @@ private[lf] final class Compiler(
 
     interface.lookupPackage(pkgId) match {
       case Right(pkg) if !config.allowedLanguageVersions.contains(pkg.languageVersion) =>
-        throw CompilationError(
-          s"Disallowed language version in package $pkgId: " +
-            s"Expected version between ${config.allowedLanguageVersions.min.pretty} and ${config.allowedLanguageVersions.max.pretty} but got ${pkg.languageVersion.pretty}"
-        )
+        throw LanguageVersionError(pkgId, pkg.languageVersion, config.allowedLanguageVersions)
       case _ =>
     }