interfaces: Do some TODOs (#11231)

* interfaces: Do some TODOs

- Add uniqueness check between fixed choices and virtual choices in
  haskell decoder.
- Encode interface methods and fixed choices in scala encoder.
- ExprIterable for interfaces.

changelog_begin
changelog_end

* scalafmt

compiler/daml-lf-proto/src/DA/Daml/LF/Proto3/DecodeV1.hs

@@ -24,6 +24,7 @@ import           Data.List
 import           DA.Daml.LF.Mangling
 import qualified Com.Daml.DamlLfDev.DamlLf1 as LF1
 import qualified Data.NameMap as NM
+import qualified Data.HashSet as HS
 import qualified Data.Text as T
 import qualified Data.Text.Lazy as TL
 import qualified Data.Vector.Extended as V
@@ -229,16 +230,18 @@ decodeModule (LF1.Module name flags synonyms dataTypes values templates exceptio
     <*> decodeNM DuplicateInterface decodeDefInterface interfaces
 
 decodeDefInterface :: LF1.DefInterface -> Decode DefInterface
-decodeDefInterface LF1.DefInterface {..} =
-  DefInterface
-    <$> traverse decodeLocation defInterfaceLocation
-    <*> decodeDottedNameId TypeConName defInterfaceTyconInternedDname
-    <*> decodeNameId ExprVarName defInterfaceParamInternedStr
-    <*> decodeNM DuplicateChoice decodeInterfaceChoice defInterfaceChoices
-    <*> decodeNM DuplicateChoice decodeChoice defInterfaceFixedChoices
-    <*> decodeNM DuplicateMethod decodeInterfaceMethod defInterfaceMethods
-    -- TODO https://github.com/digital-asset/daml/issues/11137
-    --   maybe also check that choice names are unique between virtual choices & fixed choices
+decodeDefInterface LF1.DefInterface {..} = do
+  intLocation <- traverse decodeLocation defInterfaceLocation
+  intName <- decodeDottedNameId TypeConName defInterfaceTyconInternedDname
+  intParam <- decodeNameId ExprVarName defInterfaceParamInternedStr
+  intVirtualChoices <- decodeNM DuplicateChoice decodeInterfaceChoice defInterfaceChoices
+  intFixedChoices <- decodeNM DuplicateChoice decodeChoice defInterfaceFixedChoices
+  intMethods <- decodeNM DuplicateMethod decodeInterfaceMethod defInterfaceMethods
+  unless (HS.null (NM.namesSet intFixedChoices `HS.intersection` NM.namesSet intVirtualChoices)) $
+    throwError $ ParseError $ unwords
+      [ "Interface", T.unpack (T.intercalate "." (unTypeConName intName))
+      , "has collision between fixed choice and virtual choice." ]
+  pure DefInterface {..}
 
 decodeInterfaceChoice :: LF1.InterfaceChoice -> Decode InterfaceChoice
 decodeInterfaceChoice LF1.InterfaceChoice {..} =

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

@@ -724,12 +724,35 @@ private[daml] class EncodeV1(minor: LV.Minor) {
       val (dottedName, interface) = nameWithDef
       val builder = PLF.DefInterface.newBuilder()
       builder.setTyconInternedDname(dottedNameTable.insert(dottedName))
+      builder.setParamInternedStr(stringsTable.insert(interface.param))
       builder.accumulateLeft(interface.virtualChoices.sortByKey)(_ addChoices _)
-      // TODO https://github.com/digital-asset/daml/issues/11006
-      //   encode interface methods as well
+      builder.accumulateLeft(interface.fixedChoices.sortByKey)(_ addFixedChoices _)
+      builder.accumulateLeft(interface.methods.sortByKey)(_ addMethods _)
       builder.build()
     }
 
+    private implicit def encodeInterfaceChoice(
+        nameWithChoice: (ChoiceName, InterfaceChoice)
+    ): PLF.InterfaceChoice = {
+      val (name, choice) = nameWithChoice
+      val b = PLF.InterfaceChoice.newBuilder()
+      b.setNameInternedString(stringsTable.insert(name))
+      b.setConsuming(choice.consuming)
+      b.setArgType(choice.argType)
+      b.setRetType(choice.returnType)
+      b.build()
+    }
+
+    private implicit def encodeInterfaceMethod(
+        nameWithMethod: (MethodName, InterfaceMethod)
+    ): PLF.InterfaceMethod = {
+      val (name, method) = nameWithMethod
+      val b = PLF.InterfaceMethod.newBuilder()
+      b.setMethodInternedName(stringsTable.insert(name))
+      b.setType(method.returnType)
+      b.build()
+    }
+
     private implicit def encodeSynonymDef(nameWithDef: (DottedName, DTypeSyn)): PLF.DefTypeSyn = {
       val (dottedName, typeSyn) = nameWithDef
       val builder = PLF.DefTypeSyn.newBuilder()
@@ -783,18 +806,6 @@ private[daml] class EncodeV1(minor: LV.Minor) {
       b.build()
     }
 
-    private implicit def encodeInterfaceChoice(
-        nameWithChoice: (ChoiceName, InterfaceChoice)
-    ): PLF.InterfaceChoice = {
-      val (name, choice) = nameWithChoice
-      val b = PLF.InterfaceChoice.newBuilder()
-      b.setNameInternedString(stringsTable.insert(name))
-      b.setConsuming(choice.consuming)
-      b.setArgType(choice.argType)
-      b.setRetType(choice.returnType)
-      b.build()
-    }
-
     private implicit def encodeTemplateKey(key: TemplateKey): PLF.DefTemplate.DefKey =
       PLF.DefTemplate.DefKey
         .newBuilder()
@@ -816,17 +827,27 @@ private[daml] class EncodeV1(minor: LV.Minor) {
       b.accumulateLeft(template.choices.sortByKey)(_ addChoices _)
       b.setObservers(template.observers)
       template.key.foreach(b.setKey(_))
-      b.accumulateLeft(template.implements.values)(_ addImplements encodeTemplateImplements(_))
+      b.accumulateLeft(template.implements.sortByKey)(_ addImplements _)
       b.build()
     }
 
-    // TODO https://github.com/digital-asset/daml/issues/11006
-    //  encode rest of TemplateImplements
     private implicit def encodeTemplateImplements(
-        impl: TemplateImplements
+        interfaceWithImplements: (TypeConName, TemplateImplements)
     ): PLF.DefTemplate.Implements = {
+      val (interface, implements) = interfaceWithImplements
       val b = PLF.DefTemplate.Implements.newBuilder()
-      b.setInterface(impl.interface)
+      b.setInterface(interface)
+      b.accumulateLeft(implements.methods.sortByKey)(_ addMethods _)
+      b.build()
+    }
+
+    private implicit def encodeTemplateImplementsMethod(
+        nameWithMethod: (MethodName, TemplateImplementsMethod)
+    ): PLF.DefTemplate.ImplementsMethod = {
+      val (name, method) = nameWithMethod
+      val b = PLF.DefTemplate.ImplementsMethod.newBuilder()
+      b.setMethodInternedName(stringsTable.insert(name))
+      b.setValue(method.value)
       b.build()
     }
 

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

@@ -141,12 +141,13 @@ private[validation] object ExprIterable {
             choices,
             observers,
             key,
-            implements @ _, // TODO https://github.com/digital-asset/daml/issues/11006
+            implements,
           ) =>
         Iterator(precond, signatories, agreementText) ++
           choices.values.iterator.flatMap(iterator(_)) ++
           Iterator(observers) ++
-          key.iterator.flatMap(iterator(_))
+          key.iterator.flatMap(iterator(_)) ++
+          implements.values.iterator.flatMap(iterator(_))
     }
 
   private[iterable] def iterator(x: TemplateChoice): Iterator[Expr] =
@@ -172,6 +173,43 @@ private[validation] object ExprIterable {
         Iterator(body, maintainers)
     }
 
+  private[iterable] def iterator(x: TemplateImplements): Iterator[Expr] =
+    x match {
+      case TemplateImplements(
+            interface @ _,
+            methods,
+          ) =>
+        methods.values.iterator.flatMap(iterator(_))
+    }
+
+  private[iterable] def iterator(x: TemplateImplementsMethod): Iterator[Expr] =
+    x match {
+      case TemplateImplementsMethod(
+            name @ _,
+            value,
+          ) =>
+        Iterator(value)
+    }
+
+  private[iterable] def iterator(x: DefException): Iterator[Expr] =
+    x match {
+      case DefException(
+            message
+          ) =>
+        Iterator(message)
+    }
+
+  private[iterable] def iterator(x: DefInterface): Iterator[Expr] =
+    x match {
+      case DefInterface(
+            param @ _,
+            virtualChoices @ _,
+            fixedChoices,
+            methods @ _,
+          ) =>
+        fixedChoices.values.iterator.flatMap(iterator(_))
+    }
+
   def apply(expr: Expr): Iterable[Expr] =
     new Iterable[Expr] {
       override def iterator: Iterator[Expr] = ExprIterable.iterator(expr)
@@ -186,6 +224,8 @@ private[validation] object ExprIterable {
     new Iterable[Expr] {
       override def iterator: Iterator[Expr] =
         module.definitions.values.iterator.flatMap(ExprIterable.iterator(_)) ++
+          module.exceptions.values.iterator.flatMap(ExprIterable.iterator(_)) ++
+          module.interfaces.values.iterator.flatMap(ExprIterable.iterator(_)) ++
           module.templates.values.iterator.flatMap(ExprIterable.iterator(_))
     }
 }