Switch on NonUnitStatements warning in daml-lf/transaction (#11048)

* Add NonUnitStatements warning in daml-lf/transaction & fix using discard or similar.

CHANGELOG_BEGIN
CHANGELOG_END

* address comment

* prefer parens to braces for single statement map/foreach bodies

daml-lf/transaction/BUILD.bazel

@@ -10,6 +10,10 @@ load(
     "silencer_plugin",
 )
 
+lf_scalacopts_stricter = lf_scalacopts + [
+    "-P:wartremover:traverser:org.wartremover.warts.NonUnitStatements",
+]
+
 #
 # Transaction and value protocol buffers
 #
@@ -53,7 +57,7 @@ da_scala_library(
         "@maven//:org_scalaz_scalaz_core",
         "@maven//:org_scala_lang_modules_scala_collection_compat",
     ],
-    scalacopts = lf_scalacopts,
+    scalacopts = lf_scalacopts_stricter,
     tags = ["maven_coordinates=com.daml:daml-lf-transaction:__VERSION__"],
     visibility = ["//visibility:public"],
     deps = [
@@ -62,6 +66,7 @@ da_scala_library(
         "//daml-lf/data",
         "//daml-lf/language",
         "//libs-scala/nameof",
+        "//libs-scala/scala-utils",
         "@maven//:com_google_protobuf_protobuf_java",
     ],
 )

daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/crypto/Hash.scala

@@ -10,6 +10,7 @@ import java.util.concurrent.atomic.AtomicLong
 
 import com.daml.lf.data.{Bytes, ImmArray, Ref, Time, Utf8}
 import com.daml.lf.value.Value
+import com.daml.scalautil.Statement.discard
 import scalaz.Order
 import javax.crypto.Mac
 import javax.crypto.spec.SecretKeySpec
@@ -133,16 +134,16 @@ object Hash {
     private val intBuffer = ByteBuffer.allocate(java.lang.Integer.BYTES)
 
     final def add(a: Int): this.type = {
-      intBuffer.rewind()
-      intBuffer.putInt(a).position(0)
+      discard(intBuffer.rewind())
+      discard(intBuffer.putInt(a).position(0))
       add(intBuffer)
     }
 
     private val longBuffer = ByteBuffer.allocate(java.lang.Long.BYTES)
 
     final def add(a: Long): this.type = {
-      longBuffer.rewind()
-      longBuffer.putLong(a).position(0)
+      discard(longBuffer.rewind())
+      discard(longBuffer.putLong(a).position(0))
       add(longBuffer)
     }
 
@@ -172,8 +173,7 @@ object Hash {
     @throws[HashingError]
     final def addCid(cid: Value.ContractId): this.type = {
       val bytes = cid2Bytes(cid)
-      add(bytes.length)
-      add(bytes)
+      add(bytes.length).add(bytes)
     }
 
     // In order to avoid hash collision, this should be used together

daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/ledger/EventId.scala

@@ -15,11 +15,11 @@ case class EventId(
     nodeId: NodeId,
 ) {
   lazy val toLedgerString: LedgerString = {
-    val builder = new StringBuilder()
-    builder += '#'
-    builder ++= transactionId
-    builder += ':'
-    builder ++= nodeId.index.toString
+    val builder = (new StringBuilder()
+      += '#'
+      ++= transactionId
+      += ':'
+      ++= nodeId.index.toString)
     LedgerString.assertFromString(builder.result())
   }
 }

daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/transaction/TransactionCoder.scala

@@ -11,6 +11,7 @@ import com.daml.lf.data.Ref.{Name, Party}
 import com.daml.lf.transaction.Node._
 import com.daml.lf.value.{Value, ValueCoder, ValueOuterClass}
 import com.daml.lf.value.ValueCoder.{DecodeError, EncodeError}
+import com.daml.scalautil.Statement.discard
 import com.google.protobuf.{ByteString, GeneratedMessageV3, ProtocolStringList}
 
 import scala.Ordering.Implicits.infixOrderingOps
@@ -203,7 +204,7 @@ object TransactionCoder {
   ) = {
     key match {
       case Some(key) =>
-        encodeKeyWithMaintainers(encodeCid, version, key).map { k => setKey(k); () }
+        encodeKeyWithMaintainers(encodeCid, version, key).map(k => discard(setKey(k)))
       case None =>
         Right(())
     }
@@ -217,15 +218,9 @@ object TransactionCoder {
       setUnversioned: ByteString => GeneratedMessageV3.Builder[_],
   ): Either[EncodeError, Unit] = {
     if (version < TransactionVersion.minNoVersionValue) {
-      encodeVersionedValue(encodeCid, version, value).map { v =>
-        setVersioned(v);
-        {}
-      }
+      encodeVersionedValue(encodeCid, version, value).map(v => discard(setVersioned(v)))
     } else {
-      encodeValue(encodeCid, version, value).map { v =>
-        setUnversioned(v)
-        ()
-      }
+      encodeValue(encodeCid, version, value).map(v => discard(setUnversioned(v)))
     }
   }
 
@@ -255,7 +250,7 @@ object TransactionCoder {
     node match {
       case NodeRollback(children) =>
         val builder = TransactionOuterClass.NodeRollback.newBuilder()
-        children.foreach { id => builder.addChildren(encodeNid.asString(id)); () }
+        children.foreach(id => discard(builder.addChildren(encodeNid.asString(id))))
         for {
           _ <- Either.cond(
             test = enclosingVersion >= TransactionVersion.minExceptions || disableVersionCheck,
@@ -274,15 +269,15 @@ object TransactionCoder {
           )
         else {
           if (enclosingVersion >= TransactionVersion.minNodeVersion)
-            nodeBuilder.setVersion(nodeVersion.protoValue)
+            discard(nodeBuilder.setVersion(nodeVersion.protoValue))
 
           node match {
 
             case nc @ NodeCreate(_, _, _, _, _, _, _, _) =>
               val builder = TransactionOuterClass.NodeCreate.newBuilder()
               nc.stakeholders.foreach(builder.addStakeholders)
               nc.signatories.foreach(builder.addSignatories)
-              builder.setContractIdStruct(encodeCid.encode(nc.coid))
+              discard(builder.setContractIdStruct(encodeCid.encode(nc.coid)))
               for {
                 _ <-
                   if (nodeVersion < TransactionVersion.minNoVersionValue) {
@@ -295,11 +290,12 @@ object TransactionCoder {
                     )
                       .map(builder.setContractInstance)
                   } else {
-                    encodeValue(encodeCid, nodeVersion, nc.arg).map { arg =>
-                      builder.setTemplateId(ValueCoder.encodeIdentifier(nc.templateId))
-                      builder.setArgUnversioned(arg)
-                      builder.setAgreement(nc.agreementText)
-                    }
+                    encodeValue(encodeCid, nodeVersion, nc.arg).map(arg =>
+                      builder
+                        .setTemplateId(ValueCoder.encodeIdentifier(nc.templateId))
+                        .setArgUnversioned(arg)
+                        .setAgreement(nc.agreementText)
+                    )
                   }
                 _ <- encodeAndSetContractKey(
                   encodeCid,
@@ -311,12 +307,12 @@ object TransactionCoder {
 
             case nf @ NodeFetch(_, _, _, _, _, _, _, _) =>
               val builder = TransactionOuterClass.NodeFetch.newBuilder()
-              builder.setTemplateId(ValueCoder.encodeIdentifier(nf.templateId))
+              discard(builder.setTemplateId(ValueCoder.encodeIdentifier(nf.templateId)))
               nf.stakeholders.foreach(builder.addStakeholders)
               nf.signatories.foreach(builder.addSignatories)
-              builder.setContractIdStruct(encodeCid.encode(nf.coid))
+              discard(builder.setContractIdStruct(encodeCid.encode(nf.coid)))
               if (nodeVersion >= TransactionVersion.minByKey) {
-                builder.setByKey(nf.byKey)
+                discard(builder.setByKey(nf.byKey))
               }
               nf.actingParties.foreach(builder.addActors)
               for {
@@ -330,17 +326,20 @@ object TransactionCoder {
 
             case ne @ NodeExercises(_, _, _, _, _, _, _, _, _, _, _, _, _, _) =>
               val builder = TransactionOuterClass.NodeExercise.newBuilder()
-              builder.setContractIdStruct(encodeCid.encode(ne.targetCoid))
-              builder.setChoice(ne.choiceId)
-              builder.setTemplateId(ValueCoder.encodeIdentifier(ne.templateId))
-              builder.setConsuming(ne.consuming)
+              discard(
+                builder
+                  .setContractIdStruct(encodeCid.encode(ne.targetCoid))
+                  .setChoice(ne.choiceId)
+                  .setTemplateId(ValueCoder.encodeIdentifier(ne.templateId))
+                  .setConsuming(ne.consuming)
+              )
               ne.actingParties.foreach(builder.addActors)
-              ne.children.foreach { id => builder.addChildren(encodeNid.asString(id)); () }
+              ne.children.foreach(id => discard(builder.addChildren(encodeNid.asString(id))))
               ne.signatories.foreach(builder.addSignatories)
               ne.stakeholders.foreach(builder.addStakeholders)
               ne.choiceObservers.foreach(builder.addObservers)
               if (nodeVersion >= TransactionVersion.minByKey) {
-                builder.setByKey(ne.byKey)
+                discard(builder.setByKey(ne.byKey))
               }
               for {
                 _ <- Either.cond(
@@ -382,12 +381,14 @@ object TransactionCoder {
 
             case nlbk @ NodeLookupByKey(_, _, _, _) =>
               val builder = TransactionOuterClass.NodeLookupByKey.newBuilder()
-              builder.setTemplateId(ValueCoder.encodeIdentifier(nlbk.templateId))
-              nlbk.result.foreach(cid => builder.setContractIdStruct(encodeCid.encode(cid)))
+              discard(builder.setTemplateId(ValueCoder.encodeIdentifier(nlbk.templateId)))
+              nlbk.result.foreach(cid =>
+                discard(builder.setContractIdStruct(encodeCid.encode(cid)))
+              )
               for {
                 encodedKey <- encodeKeyWithMaintainers(encodeCid, nlbk.version, nlbk.key)
               } yield {
-                builder.setKeyWithMaintainers(encodedKey)
+                discard(builder.setKeyWithMaintainers(encodedKey))
                 nodeBuilder.setLookupByKey(builder).build()
               }
           }
@@ -567,7 +568,7 @@ object TransactionCoder {
                 nodeVersion,
                 protoExe.getResultVersioned,
                 protoExe.getResultUnversioned,
-              ).map { v => Some(v) }
+              ).map(v => Some(v))
             }
           keyWithMaintainers <-
             decodeOptionalKeyWithMaintainers(decodeCid, nodeVersion, protoExe.getKeyWithMaintainers)
@@ -673,10 +674,7 @@ object TransactionCoder {
     val builder = TransactionOuterClass.Transaction
       .newBuilder()
       .setVersion(transaction.version.protoValue)
-    transaction.roots.foreach { nid =>
-      builder.addRoots(encodeNid.asString(nid))
-      ()
-    }
+    transaction.roots.foreach(nid => discard(builder.addRoots(encodeNid.asString(nid))))
 
     transaction
       .fold[Either[EncodeError, TransactionOuterClass.Transaction.Builder]](

daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/value/CidContainer.scala

@@ -5,6 +5,7 @@ package com.daml.lf
 package value
 
 import com.daml.lf.data.Bytes
+import com.daml.scalautil.Statement.discard
 import Value.ContractId
 
 import scala.util.control.NoStackTrace
@@ -16,11 +17,10 @@ trait CidContainer[+A] {
   def mapCid(f: ContractId => ContractId): A
 
   def foreachCid(f: ContractId => Unit) = {
-    mapCid(cid => {
+    discard(mapCid(cid => {
       f(cid)
       cid
-    })
-    ()
+    }))
   }
 
   // We cheat using exceptions, to get a cheap implementation of traverse using the `map` function above.

daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/value/Value.scala

@@ -9,6 +9,7 @@ import com.daml.lf.data.Ref.{Identifier, Name}
 import com.daml.lf.data._
 import com.daml.lf.language.Ast
 import com.daml.lf.transaction.TransactionVersion
+import com.daml.scalautil.Statement.discard
 import data.ScalazEqual._
 
 import scalaz.{@@, Equal, Order, Tag}
@@ -76,7 +77,7 @@ sealed abstract class Value extends CidContainer[Value] with Product with Serial
 
   def cids[Cid2 >: ContractId] = {
     val cids = Set.newBuilder[Cid2]
-    foreach1(cids += _)(this)
+    foreach1(x => discard(cids += x))(this)
     cids.result()
   }
 

daml-lf/transaction/src/main/scala/com/digitalasset/daml/lf/value/ValueCoder.scala

@@ -9,6 +9,7 @@ import com.daml.lf.data._
 import com.daml.lf.transaction.TransactionVersion
 import com.daml.lf.value.Value._
 import com.daml.lf.value.{ValueOuterClass => proto}
+import com.daml.scalautil.Statement.discard
 import com.google.protobuf
 import com.google.protobuf.{ByteString, CodedInputStream}
 
@@ -93,10 +94,12 @@ object ValueCoder {
     * @return wire format identifier
     */
   def encodeIdentifier(id: Identifier): proto.Identifier = {
-    val builder = proto.Identifier.newBuilder().setPackageId(id.packageId)
-    builder.addAllModuleName((id.qualifiedName.module.segments.toSeq: Seq[String]).asJava)
-    builder.addAllName((id.qualifiedName.name.segments.toSeq: Seq[String]).asJava)
-    builder.build()
+    proto.Identifier
+      .newBuilder()
+      .setPackageId(id.packageId)
+      .addAllModuleName((id.qualifiedName.module.segments.toSeq: Seq[String]).asJava)
+      .addAllName((id.qualifiedName.name.segments.toSeq: Seq[String]).asJava)
+      .build()
   }
 
   /** Decode identifier from wire format
@@ -173,7 +176,7 @@ object ValueCoder {
   private[this] def parseValue(bytes: ByteString): Either[DecodeError, proto.Value] =
     Try {
       val cis = CodedInputStream.newInstance(bytes.asReadOnlyByteBuffer())
-      cis.setRecursionLimit(MAXIMUM_PROTO_RECURSION_LIMIT)
+      discard(cis.setRecursionLimit(MAXIMUM_PROTO_RECURSION_LIMIT))
       proto.Value.parseFrom(cis)
     } match {
       case Failure(exception: Error) =>
@@ -438,28 +441,28 @@ object ValueCoder {
           case ValueList(elems) =>
             val listBuilder = proto.List.newBuilder()
             elems.foreach(elem => {
-              listBuilder.addElements(go(newNesting, elem))
-              ()
+              discard(listBuilder.addElements(go(newNesting, elem)))
             })
             builder.setList(listBuilder).build()
 
           case ValueRecord(id, fields) =>
             val recordBuilder = proto.Record.newBuilder()
             fields.foreach { case (fieldName, field) =>
               val b = proto.RecordField.newBuilder()
-              if (valueVersion < TransactionVersion.minTypeErasure) fieldName.map(b.setLabel)
-              b.setValue(go(newNesting, field))
-              recordBuilder.addFields(b)
-              ()
+              if (valueVersion < TransactionVersion.minTypeErasure)
+                discard(fieldName.map(b.setLabel))
+              discard(b.setValue(go(newNesting, field)))
+              discard(recordBuilder.addFields(b))
             }
             if (valueVersion < TransactionVersion.minTypeErasure)
               id.foreach(i => recordBuilder.setRecordId(encodeIdentifier(i)))
             builder.setRecord(recordBuilder).build()
 
           case ValueVariant(id, con, arg) =>
-            val protoVar = proto.Variant.newBuilder()
-            protoVar.setConstructor(con)
-            protoVar.setValue(go(newNesting, arg))
+            val protoVar = proto.Variant
+              .newBuilder()
+              .setConstructor(con)
+              .setValue(go(newNesting, arg))
             if (valueVersion < TransactionVersion.minTypeErasure)
               id.foreach(i => protoVar.setVariantId(encodeIdentifier(i)))
             builder.setVariant(protoVar).build()
@@ -480,27 +483,29 @@ object ValueCoder {
           case ValueTextMap(map) =>
             val protoMap = proto.Map.newBuilder()
             map.toImmArray.foreach { case (key, value) =>
-              protoMap.addEntries(
-                proto.Map.Entry
-                  .newBuilder()
-                  .setKey(key)
-                  .setValue(go(newNesting, value))
+              discard(
+                protoMap.addEntries(
+                  proto.Map.Entry
+                    .newBuilder()
+                    .setKey(key)
+                    .setValue(go(newNesting, value))
+                )
               )
-              ()
             }
             builder.setMap(protoMap).build()
 
           case ValueGenMap(entries) =>
             assertSince(TransactionVersion.minGenMap, "Value.SumCase.MAP")
             val protoMap = proto.GenMap.newBuilder()
             entries.foreach { case (key, value) =>
-              protoMap.addEntries(
-                proto.GenMap.Entry
-                  .newBuilder()
-                  .setKey(go(newNesting, key))
-                  .setValue(go(newNesting, value))
+              discard(
+                protoMap.addEntries(
+                  proto.GenMap.Entry
+                    .newBuilder()
+                    .setKey(go(newNesting, key))
+                    .setValue(go(newNesting, value))
+                )
               )
-              ()
             }
             builder.setGenMap(protoMap).build()