Remove TODOs relevant to #14112 (#14663)

* Remove outdated TODO on ValueEnricher

* Rename collectNewPackagesFromTemplateIds for interface use, remove TODO

* Remove outdated TODO on interfaceInstanceBody parser

* Check body view has interface viewtype as type

* Amend type iterator to go over Interface view method, remove TODO

* empty changelog commit

CHANGELOG_BEGIN
CHANGELOG_END

* Fail in Preprocessing when viewing nonimplementing interfaces

* Fix interface instance check in Typing

* Fix lint

* Typecheck view expression with tmplParam in scope

* Remove done TODO in checkInterfaceInstance

* Move AmbiguousInterfaceInstance error into LookupError as a case variant

* lint

* Add unapply to LookupError to default to NotFound

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

@@ -63,8 +63,6 @@ final class ValueEnricher(
       interfaceId: Identifier,
       viewValue: Value,
   ): Result[Value] = for {
-    // TODO: https://github.com/digital-asset/daml/issues/14112
-    // Switch to builtin views once those are implemented.
     iface <- handleLookup(
       compiledPackages.pkgInterface.lookupInterface(interfaceId)
     )

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

@@ -257,6 +257,8 @@ private[lf] final class CommandPreprocessor(
   ): speedy.InterfaceView = {
     discard(handleLookup(pkgInterface.lookupTemplate(templateId)))
     discard(handleLookup(pkgInterface.lookupInterface(interfaceId)))
+    discard(handleLookup(pkgInterface.lookupInterfaceInstance(interfaceId, templateId)))
+
     val version =
       TransactionVersion.assignNodeVersion(
         pkgInterface.packageLanguageVersion(interfaceId.packageId)

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

@@ -85,16 +85,16 @@ private[engine] final class Preprocessor(
         ResultDone(acc.toList)
     }
 
-  private[this] def collectNewPackagesFromTemplateIds(
-      templateIds: Iterable[Ref.TypeConName]
+  private[this] def collectNewPackagesFromTemplatesOrInterfaces(
+      tycons: Iterable[Ref.TypeConName]
   ): List[(Ref.PackageId, language.Reference)] =
-    templateIds
-      .foldLeft(Map.empty[Ref.PackageId, language.Reference]) { (acc, tmplId) =>
-        val pkgId = tmplId.packageId
+    tycons
+      .foldLeft(Map.empty[Ref.PackageId, language.Reference]) { (acc, tycon) =>
+        val pkgId = tycon.packageId
         if (compiledPackages.packageIds(pkgId) || acc.contains(pkgId))
           acc
         else
-          acc.updated(pkgId, language.Reference.TemplateOrInterface(tmplId))
+          acc.updated(pkgId, language.Reference.TemplateOrInterface(tycon))
       }
       .toList
 
@@ -120,12 +120,10 @@ private[engine] final class Preprocessor(
     collectNewPackagesFromTypes(List(typ)).flatMap(pullPackages)
 
   private[this] def pullTemplatePackage(tyCons: Iterable[Ref.TypeConName]): Result[Unit] =
-    pullPackages(collectNewPackagesFromTemplateIds(tyCons))
+    pullPackages(collectNewPackagesFromTemplatesOrInterfaces(tyCons))
 
   private[this] def pullInterfacePackage(tyCons: Iterable[Ref.TypeConName]): Result[Unit] =
-    // TODO https://github.com/digital-asset/daml/issues/14112
-    // Double check that this makes sense
-    pullTemplatePackage(tyCons)
+    pullPackages(collectNewPackagesFromTemplatesOrInterfaces(tyCons))
 
   /** Translates the LF value `v0` of type `ty0` to a speedy value.
     * Fails if the nesting is too deep or if v0 does not match the type `ty0`.

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

@@ -58,6 +58,7 @@ class InterfaceViewSpec extends AnyWordSpec with Matchers with EitherValues with
     engine
       .computeInterfaceView(templateId, argument, interfaceId)
       .consume(_ => None, lookupPackage, _ => None)
+
   private val t1 = id("T1")
   private val t2 = id("T2")
   private val t3 = id("T3")
@@ -97,9 +98,7 @@ class InterfaceViewSpec extends AnyWordSpec with Matchers with EitherValues with
         err shouldBe a[Error.Interpretation]
       }
     }
-    // TODO https://github.com/digital-asset/daml/issues/14112
-    // Catch during preprocessing
-    "fail with Error.Interpretation if template does not implement interface" in {
+    "fail with Error.Preprocessing if template does not implement interface" in {
       inside(
         computeView(
           t4,
@@ -107,7 +106,7 @@ class InterfaceViewSpec extends AnyWordSpec with Matchers with EitherValues with
           i,
         )
       ) { case Left(err) =>
-        err shouldBe a[Error.Interpretation]
+        err shouldBe a[Error.Preprocessing]
       }
     }
     "fail with Error.Preprocessing if argument has invalid type" in {

daml-lf/language/src/main/scala/com/digitalasset/daml/lf/language/LookupError.scala

@@ -7,10 +7,8 @@ import com.daml.lf.data.Ref
 import com.daml.lf.data.Ref._
 import com.daml.lf.language.{TemplateOrInterface => TorI}
 
-final case class LookupError(notFound: Reference, context: Reference) {
-  val pretty: String = "unknown " + notFound.pretty + (
-    if (context == notFound) "" else LookupError.contextDetails(context)
-  )
+sealed abstract class LookupError {
+  def pretty: String
 }
 
 object LookupError {
@@ -21,8 +19,20 @@ object LookupError {
       case otherwise => " while looking for " + otherwise.pretty
     }
 
+  final case class NotFound(notFound: Reference, context: Reference) extends LookupError {
+    def pretty: String = "unknown " + notFound.pretty + (
+      if (context == notFound) "" else LookupError.contextDetails(context)
+    )
+  }
+
+  final case class AmbiguousInterfaceInstance(instance: Reference.InterfaceInstance)
+      extends LookupError {
+    def pretty: String =
+      s"Ambiguous interface instance: two instances for ${instance.pretty}"
+  }
+
   object MissingPackage {
-    def unapply(err: LookupError): Option[(PackageId, Reference)] =
+    def unapply(err: LookupError.NotFound): Option[(PackageId, Reference)] =
       err.notFound match {
         case Reference.Package(packageId) => Some(packageId -> err.context)
         case _ => None
@@ -32,6 +42,12 @@ object LookupError {
       s"Couldn't find package $pkgId" + contextDetails(context)
   }
 
+  def apply(notFound: Reference, context: Reference) =
+    NotFound(notFound, context)
+
+  def unapply(err: LookupError.NotFound): Option[(Reference, Reference)] =
+    Some(err.notFound, err.context)
+
 }
 
 sealed abstract class Reference extends Product with Serializable {

daml-lf/language/src/main/scala/com/digitalasset/daml/lf/language/PackageInterface.scala

@@ -219,24 +219,24 @@ private[lf] class PackageInterface(signatures: PartialFunction[PackageId, Packag
       templateName: TypeConName,
       context: => Reference,
   ): Either[
-    Either[LookupError, AmbiguousInterfaceInstanceError],
+    LookupError,
     PackageInterface.InterfaceInstanceInfo,
   ] = {
     val ref = Reference.InterfaceInstance(interfaceName, templateName)
     for {
-      interface <- lookupInterface(interfaceName, context).left.map(Left(_))
-      template <- lookupTemplate(templateName, context).left.map(Left(_))
+      interface <- lookupInterface(interfaceName, context)
+      template <- lookupTemplate(templateName, context)
       onInterface = interface.coImplements.get(templateName)
       onTemplate = template.implements.get(interfaceName)
       ok = { tOrI: TemplateOrInterface[Unit, Unit] =>
         PackageInterface.InterfaceInstanceInfo(tOrI, interfaceName, templateName, interface)
       }
       r <- (onInterface, onTemplate) match {
-        case (None, None) => Left(Left(LookupError(ref, context)))
+        case (None, None) => Left(LookupError.NotFound(ref, context))
         case (Some(_), None) => Right(ok(TemplateOrInterface.Interface(())))
         case (None, Some(_)) => Right(ok(TemplateOrInterface.Template(())))
         case (Some(_), Some(_)) =>
-          Left(Right(PackageInterface.AmbiguousInterfaceInstanceError(ref, context)))
+          Left(LookupError.AmbiguousInterfaceInstance(ref))
       }
     } yield r
   }
@@ -245,7 +245,7 @@ private[lf] class PackageInterface(signatures: PartialFunction[PackageId, Packag
       interfaceName: TypeConName,
       templateName: TypeConName,
   ): Either[
-    Either[LookupError, AmbiguousInterfaceInstanceError],
+    LookupError,
     PackageInterface.InterfaceInstanceInfo,
   ] =
     lookupInterfaceInstance(
@@ -446,11 +446,6 @@ object PackageInterface {
 
   }
 
-  final case class AmbiguousInterfaceInstanceError(
-      notFound: Reference.InterfaceInstance,
-      context: Reference,
-  )
-
   final case class InterfaceInstanceInfo(
       val parentTemplateOrInterface: TemplateOrInterface[Unit, Unit],
       val interfaceId: TypeConName,

daml-lf/parser/src/main/scala/com/digitalasset/daml/lf/testing/parser/ModParser.scala

@@ -129,7 +129,6 @@ private[parser] class ModParser[P](parameters: ParserParameters[P]) {
 
   private lazy val interfaceInstanceBody: Parser[InterfaceInstanceBody] =
     `{` ~> (implementsView <~ `;`) ~ rep(method <~ `;`) <~ `}` ^^ { case view ~ methods =>
-      // TODO: Represent a view method and parse it. Currently hardcoding views to unit
       InterfaceInstanceBody.build(
         methods,
         view,

daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Typing.scala

@@ -7,6 +7,7 @@ import com.daml.lf.data.{ImmArray, Numeric, Struct}
 import com.daml.lf.data.Ref._
 import com.daml.lf.language.Ast._
 import com.daml.lf.language.Util._
+import com.daml.lf.language.LookupError
 import com.daml.lf.language.{LanguageVersion, PackageInterface, Reference}
 import com.daml.lf.validation.Util._
 import com.daml.lf.validation.iterable.TypeIterable
@@ -572,15 +573,19 @@ private[validation] object Typing {
       pkgInterface.lookupInterfaceInstance(interfaceId, templateId) match {
         case Left(err) =>
           err match {
-            case Left(lookupErr) =>
+            case lookupErr: LookupError.NotFound =>
               lookupErr.notFound match {
                 case _: Reference.InterfaceInstance =>
                   throw EMissingInterfaceInstance(ctx, interfaceId, templateId)
                 case _ =>
                   throw EUnknownDefinition(ctx, lookupErr)
               }
-            case Right(_: PackageInterface.AmbiguousInterfaceInstanceError) =>
-              throw EAmbiguousInterfaceInstance(ctx, interfaceId, templateId)
+            case ambiIfaceErr: LookupError.AmbiguousInterfaceInstance =>
+              throw EAmbiguousInterfaceInstance(
+                ctx,
+                ambiIfaceErr.instance.interfaceName,
+                ambiIfaceErr.instance.templateName,
+              )
           }
         case Right(iiInfo) => iiInfo
       }
@@ -604,8 +609,7 @@ private[validation] object Typing {
       val env = Env(languageVersion, pkgInterface, ctx)
         .introExprVar(tmplParam, TTyCon(templateId))
 
-      val DefInterfaceSignature(requires, _, _, methods, _, _) =
-        // TODO https://github.com/digital-asset/daml/issues/14112
+      val DefInterfaceSignature(requires, _, _, methods, _, view) =
         iiInfo.interfaceSignature
 
       requires
@@ -629,6 +633,8 @@ private[validation] object Typing {
           case Some(method) => env.checkTopExpr(value, method.returnType)
         }
       }
+
+      env.checkTopExpr(iiBody.view, view)
     }
 
     private[Typing] def checkDefException(

daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/iterable/TypeIterable.scala

@@ -253,9 +253,8 @@ private[validation] object TypeIterable {
 
   private[validation] def iterator(iiBody: InterfaceInstanceBody): Iterator[Type] =
     iiBody match {
-      // TODO https://github.com/digital-asset/daml/issues/14112
-      case InterfaceInstanceBody(methods, view @ _) =>
-        methods.values.iterator.flatMap(iterator)
+      case InterfaceInstanceBody(methods, view) =>
+        methods.values.iterator.flatMap(iterator) ++ iterator(view)
     }
 
   private[validation] def iterator(method: InterfaceInstanceMethod): Iterator[Type] =