From 18c292a3d0a2cd53ac2275690a3a072dd9e861ff Mon Sep 17 00:00:00 2001 From: Dylan Thinnes Date: Wed, 25 Sep 2024 02:31:46 +0100 Subject: [PATCH 1/7] Disallow upgrading exceptions in 2.x --- .../src/DA/Daml/LF/TypeChecker/Error.hs | 10 ++++ .../src/DA/Daml/LF/TypeChecker/Upgrade.hs | 54 +++++++++++++------ sdk/compiler/damlc/tests/BUILD.bazel | 2 + .../damlc/tests/src/DA/Test/DamlcUpgrades.hs | 14 +++++ sdk/daml-lf/validation/BUILD.bazel | 5 ++ .../daml/lf/validation/Upgrading.scala | 45 +++++++++++++--- .../validation/upgrade/UpgradesSpecBase.scala | 22 ++++++++ sdk/test-common/BUILD.bazel | 2 + .../v1/Main.daml | 11 ++++ .../v2/Main.daml | 11 ++++ .../v1/Main.daml | 11 ++++ .../v2/Main.daml | 6 +++ 12 files changed, 169 insertions(+), 24 deletions(-) create mode 100644 sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v1/Main.daml create mode 100644 sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v2/Main.daml create mode 100644 sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v1/Main.daml create mode 100644 sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v2/Main.daml diff --git a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs index 0530890d648f..67f436b8340c 100644 --- a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs +++ b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs @@ -221,6 +221,7 @@ data UnwarnableError | EForbiddenNewImplementation !TypeConName !TypeConName | EUpgradeDependenciesFormACycle ![(PackageId, Maybe PackageMetadata)] | EUpgradeMultiplePackagesWithSameNameAndVersion !PackageName !RawPackageVersion ![PackageId] + | EUpgradeTriedToUpgradeException !TypeConName deriving (Show) data WarnableError @@ -229,6 +230,7 @@ data WarnableError | WEUpgradeShouldDefineTplInSeparatePackage !TypeConName !TypeConName | WEDependencyHasUnparseableVersion !PackageName !PackageVersion !PackageUpgradeOrigin | WEDependencyHasNoMetadataDespiteUpgradeability !PackageId !PackageUpgradeOrigin + | WEUpgradeShouldDefineExceptionsAndTemplatesSeparately deriving (Eq, Show) instance Pretty WarnableError where @@ -257,6 +259,12 @@ instance Pretty WarnableError where "Dependency " <> pPrint pkgName <> " of " <> pPrint packageOrigin <> " has a version which cannot be parsed: '" <> pPrint version <> "'" WEDependencyHasNoMetadataDespiteUpgradeability pkgId packageOrigin -> "Dependency with package ID " <> pPrint pkgId <> " of " <> pPrint packageOrigin <> " has no metadata, despite being compiled with an SDK version that supports metadata." + WEUpgradeShouldDefineExceptionsAndTemplatesSeparately -> + vsep + [ "This package defines both exceptions and templates. This may make this package and its dependents not upgradeable." + , "It is recommended that exceptions are defined in their own package separate from their implementations." + --, "Ignore this error message with the --warn-bad-exceptions=yes flag." + ] data PackageUpgradeOrigin = UpgradingPackage | UpgradedPackage deriving (Eq, Ord, Show) @@ -694,6 +702,8 @@ instance Pretty UnwarnableError where pprintDep (pkgId, Just meta) = pPrint pkgId <> "(" <> pPrint (packageName meta) <> ", " <> pPrint (packageVersion meta) <> ")" pprintDep (pkgId, Nothing) = pPrint pkgId EUpgradeMultiplePackagesWithSameNameAndVersion name version ids -> "Multiple packages with name " <> pPrint name <> " and version " <> pPrint (show version) <> ": " <> hcat (L.intersperse ", " (map pPrint ids)) + EUpgradeTriedToUpgradeException exception -> + "Tried to upgrade exception " <> pPrint exception <> ", but exceptions cannot be upgraded. They should be removed in any upgrading package." instance Pretty UpgradedRecordOrigin where diff --git a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs index 01fc29dcff83..a5f93bdeea15 100644 --- a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs +++ b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs @@ -130,7 +130,7 @@ checkPackageSingle mbContext pkg = withReaderT (\(version, upgradeInfo) -> mkGamma version upgradeInfo presentWorld) $ withMbContext $ do checkNewInterfacesAreUnused pkg - checkNewInterfacesHaveNoTemplates + checkInterfacesAndExceptionsHaveNoTemplates type UpgradedPkgWithNameAndVersion = (LF.PackageId, LF.Package, LF.PackageName, Maybe LF.PackageVersion) @@ -145,7 +145,7 @@ checkModule world0 module_ deps version upgradeInfo mbUpgradedPkg = let world = extendWorldSelf module_ world0 withReaderT (\(version, upgradeInfo) -> mkGamma version upgradeInfo world) $ do checkNewInterfacesAreUnused module_ - checkNewInterfacesHaveNoTemplates + checkInterfacesAndExceptionsHaveNoTemplates case mbUpgradedPkg of Nothing -> pure () Just (upgradedPkgWithId@(upgradedPkgIdRaw, upgradedPkg, _, _), upgradingDeps) -> do @@ -410,42 +410,50 @@ checkModuleM upgradedPackageId module_ = do let ifaceDts :: Upgrading (HMS.HashMap LF.TypeConName (DefDataType, DefInterface)) unownedDts :: Upgrading (HMS.HashMap LF.TypeConName DefDataType) - (ifaceDts, unownedDts) = + exceptionDts :: Upgrading (HMS.HashMap LF.TypeConName (DefDataType, DefException)) + (ifaceDts, exceptionDts, unownedDts) = let Upgrading - { _past = (pastIfaceDts, pastUnownedDts) - , _present = (presentIfaceDts, presentUnownedDts) + { _past = (pastIfaceDts, pastExceptionDts, pastUnownedDts) + , _present = (presentIfaceDts, presentExceptionDts, presentUnownedDts) } = fmap splitModuleDts module_ in ( Upgrading pastIfaceDts presentIfaceDts + , Upgrading pastExceptionDts presentExceptionDts , Upgrading pastUnownedDts presentUnownedDts ) splitModuleDts :: Module -> ( HMS.HashMap LF.TypeConName (DefDataType, DefInterface) + , HMS.HashMap LF.TypeConName (DefDataType, DefException) , HMS.HashMap LF.TypeConName DefDataType) splitModuleDts module_ = - let (ifaceDtsList, unownedDtsList) = - partitionEithers - $ map (\(tcon, def) -> lookupInterface module_ tcon def) + let (ifaceDtsList, (exceptionDtsList, unownedDtsList)) = + fmap partitionEithers $ partitionEithers + $ map (\(tcon, def) -> lookupInterfaceOrException module_ tcon def) $ HMS.toList $ NM.toHashMap $ moduleDataTypes module_ in - (HMS.fromList ifaceDtsList, HMS.fromList unownedDtsList) + (HMS.fromList ifaceDtsList, HMS.fromList exceptionDtsList, HMS.fromList unownedDtsList) - lookupInterface + lookupInterfaceOrException :: Module -> LF.TypeConName -> DefDataType - -> Either (LF.TypeConName, (DefDataType, DefInterface)) (LF.TypeConName, DefDataType) - lookupInterface module_ tcon datatype = + -> Either (LF.TypeConName, (DefDataType, DefInterface)) (Either (LF.TypeConName, (DefDataType, DefException)) (LF.TypeConName, DefDataType)) + lookupInterfaceOrException module_ tcon datatype = case NM.name datatype `NM.lookup` moduleInterfaces module_ of - Nothing -> Right (tcon, datatype) + Nothing -> Right $ + case NM.name datatype `NM.lookup` moduleExceptions module_ of + Nothing -> Right (tcon, datatype) + Just exception -> Left (tcon, (datatype, exception)) Just iface -> Left (tcon, (datatype, iface)) -- Check that no interfaces have been deleted, nor propagated - -- New interface checks are handled by `checkNewInterfacesHaveNoTemplates`, + -- New interface checks are handled by `checkInterfacesAndExceptionsHaveNoTemplates`, -- invoked in `singlePkgDiagnostics` above -- Interface deletion is the correct behaviour so we ignore that let (_ifaceDel, ifaceExisting, _ifaceNew) = extractDelExistNew ifaceDts checkContinuedIfaces module_ ifaceExisting + let (_exceptionDel, exceptionExisting, _exceptionNew) = extractDelExistNew exceptionDts + checkContinuedExceptions module_ exceptionExisting let flattenInstances :: Module @@ -515,6 +523,17 @@ checkContinuedIfaces module_ ifaces = withContextF present' (ContextDefInterface (_present module_) iface IPWhole) $ throwWithContextF present' $ EUpgradeTriedToUpgradeIface (NM.name iface) +checkContinuedExceptions + :: Upgrading Module + -> HMS.HashMap LF.TypeConName (Upgrading (DefDataType, DefException)) + -> TcUpgradeM () +checkContinuedExceptions module_ exceptions = + forM_ exceptions $ \upgradedDtException -> + let (_dt, exception) = _present upgradedDtException + in + withContextF present' (ContextDefException (_present module_) exception) $ + throwWithContextF present' $ EUpgradeTriedToUpgradeException (NM.name exception) + class HasModules a where getModules :: a -> NM.NameMap LF.Module @@ -530,13 +549,16 @@ instance HasModules [LF.Module] where -- Check that a module or package does not define both interfaces and templates. -- This warning should trigger even when no previous version DAR is specified in -- the `upgrades:` field. -checkNewInterfacesHaveNoTemplates :: TcM () -checkNewInterfacesHaveNoTemplates = do +checkInterfacesAndExceptionsHaveNoTemplates :: TcM () +checkInterfacesAndExceptionsHaveNoTemplates = do modules <- NM.toList . packageModules . getWorldSelf <$> getWorld let templateDefined = filter (not . NM.null . moduleTemplates) modules interfaceDefined = filter (not . NM.null . moduleInterfaces) modules + exceptionDefined = filter (not . NM.null . moduleExceptions) modules when (not (null templateDefined) && not (null interfaceDefined)) $ diagnosticWithContext WEUpgradeShouldDefineIfacesAndTemplatesSeparately + when (not (null templateDefined) && not (null exceptionDefined)) $ + diagnosticWithContext WEUpgradeShouldDefineExceptionsAndTemplatesSeparately -- Check that any interfaces defined in this package or module do not also have -- an instance. Interfaces defined in other packages are allowed to have diff --git a/sdk/compiler/damlc/tests/BUILD.bazel b/sdk/compiler/damlc/tests/BUILD.bazel index 775dd72bc7cf..cd5f223da3ec 100644 --- a/sdk/compiler/damlc/tests/BUILD.bazel +++ b/sdk/compiler/damlc/tests/BUILD.bazel @@ -459,6 +459,7 @@ da_haskell_test( "//test-common:upgrades-FailsWhenAnInstanceIsAddedUpgradedPackage-files", "//test-common:upgrades-FailsWhenAnInstanceIsDropped-files", "//test-common:upgrades-FailsWhenAnInterfaceIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-files", + "//test-common:upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-files", "//test-common:upgrades-FailsWhenDatatypeChangesVariety-files", "//test-common:upgrades-FailsWhenDependencyIsNotAValidUpgrade-files", "//test-common:upgrades-FailsWhenDepsDowngradeVersionsWhileUsingDatatypes-files", @@ -495,6 +496,7 @@ da_haskell_test( "//test-common:upgrades-SucceedsWhenAnInstanceIsAddedToNewTemplateSeparateDep-files", "//test-common:upgrades-SucceedsWhenAnInstanceIsAddedToNewTemplateUpgradedPackage-files", "//test-common:upgrades-SucceedsWhenAnInterfaceIsOnlyDefinedInTheInitialPackage-files", + "//test-common:upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-files", "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-files", "//test-common:upgrades-SucceedsWhenNewFieldWithOptionalTypeIsAddedToTemplate-files", "//test-common:upgrades-SucceedsWhenNewFieldWithOptionalTypeIsAddedToTemplateChoice-files", diff --git a/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs b/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs index 0fc462a9c88d..0a2a03b20dd0 100644 --- a/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs +++ b/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs @@ -516,6 +516,20 @@ tests damlc = -- (SeparateDeps False) -- False -- setUpgradeField + , test + "SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage" + Succeed + versionDefault + NoDependencies + False + setUpgradeField + , test + "FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage" + (FailWithError "\ESC\\[0;91merror type checking exception Main.E:\n Tried to upgrade exception E, but exceptions cannot be upgraded. They should be removed in any upgrading package.") + versionDefault + NoDependencies + False + setUpgradeField ] | setUpgradeField <- [True, False] ] ++ diff --git a/sdk/daml-lf/validation/BUILD.bazel b/sdk/daml-lf/validation/BUILD.bazel index 02c34b42b796..f98b6b4fadaf 100644 --- a/sdk/daml-lf/validation/BUILD.bazel +++ b/sdk/daml-lf/validation/BUILD.bazel @@ -284,6 +284,11 @@ da_scala_test_suite( "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-dep-v2.dar", "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-v1.dar", "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-v2.dar", + + "//test-common:upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-v1.dar", + "//test-common:upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-v2.dar", + "//test-common:upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-v1.dar", + "//test-common:upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-v2.dar", ], flaky = False, scala_deps = [ diff --git a/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala b/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala index d3a5dfcc3954..637a55a1bce8 100644 --- a/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala +++ b/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala @@ -201,6 +201,11 @@ object UpgradeError { override def message: String = s"Implementation of interface $iface by template $tpl appears in this package, but does not appear in package that is being upgraded." } + + final case class TriedToUpgradeException(exception: Ref.DottedName) extends Error { + override def message: String = + s"Tried to upgrade exception $exception, but exceptions cannot be upgraded. They should be removed in any upgrading package." + } } sealed abstract class UpgradedRecordOrigin @@ -406,27 +411,35 @@ case class TypecheckUpgrades( module: Ast.Module ): ( Map[Ref.DottedName, (Ast.DDataType, Ast.DefInterface)], + Map[Ref.DottedName, (Ast.DDataType, Ast.DefException)], Map[Ref.DottedName, Ast.DDataType], ) = { val datatypes: Map[Ref.DottedName, Ast.DDataType] = module.definitions.collect({ case (name, dt: Ast.DDataType) => (name, dt) }) - val (ifaces, other) = datatypes.partitionMap({ case (tcon, dt) => - lookupInterface(module, tcon, dt) + val (ifaces, other1) = datatypes.partitionMap({ case (tcon, dt) => + lookupInterfaceOrException(module, tcon, dt) }) - (ifaces.toMap, other.filter(_._2.serializable).toMap) + val (exceptions, other) = other1.partitionMap(identity) + (ifaces.toMap, exceptions.toMap, other.filter(_._2.serializable).toMap) } - private def lookupInterface( + private def lookupInterfaceOrException( module: Ast.Module, tcon: Ref.DottedName, dt: Ast.DDataType, ): Either[ (Ref.DottedName, (Ast.DDataType, Ast.DefInterface)), - (Ref.DottedName, Ast.DDataType), + Either[ + (Ref.DottedName, (Ast.DDataType, Ast.DefException)), + (Ref.DottedName, Ast.DDataType), + ] ] = { module.interfaces.get(tcon) match { - case None => Right((tcon, dt)) + case None => Right(module.exceptions.get(tcon) match { + case None => Right((tcon, dt)) + case Some(exception) => Left((tcon, (dt, exception))) + }) case Some(iface) => Left((tcon, (dt, iface))) } } @@ -441,9 +454,10 @@ case class TypecheckUpgrades( } private def checkModule(module: Upgrading[Ast.Module]): Try[Unit] = { - val (pastIfaceDts, pastUnownedDts) = splitModuleDts(module.past) - val (presentIfaceDts, presentUnownedDts) = splitModuleDts(module.present) + val (pastIfaceDts, pastExceptionDts, pastUnownedDts) = splitModuleDts(module.past) + val (presentIfaceDts, presentExceptionDts, presentUnownedDts) = splitModuleDts(module.present) val ifaceDts = Upgrading(past = pastIfaceDts, present = presentIfaceDts) + val exceptionDts = Upgrading(past = pastExceptionDts, present = presentExceptionDts) val unownedDts = Upgrading(past = pastUnownedDts, present = presentUnownedDts) val moduleWithMetadata = module.map(ModuleWithMetadata) @@ -457,6 +471,9 @@ case class TypecheckUpgrades( (_ifaceDel, ifaceExisting, _ifaceNew) = extractDelExistNew(ifaceDts) _ <- checkContinuedIfaces(ifaceExisting) + (_exceptionDel, exceptionExisting, _exceptionNew) = extractDelExistNew(exceptionDts) + _ <- checkContinuedExceptions(exceptionExisting) + (instanceDel, _instanceExisting, instanceNew) = extractDelExistNew( module.map(flattenInstances) ) @@ -485,6 +502,18 @@ case class TypecheckUpgrades( ).map(_ => ()) } + private def checkContinuedExceptions( + exceptions: Map[Ref.DottedName, Upgrading[(Ast.DDataType, Ast.DefException)]] + ): Try[Unit] = { + tryAll( + exceptions, + (arg: (Ref.DottedName, Upgrading[(Ast.DDataType, Ast.DefException)])) => { + val (name, _) = arg + fail(UpgradeError.TriedToUpgradeException(name)) + }, + ).map(_ => ()) + } + private def checkDeletedInstances( deletedInstances: Map[(Ref.DottedName, TypeConName), (Ast.Template, Ast.TemplateImplements)] ): Try[Unit] = diff --git a/sdk/daml-lf/validation/src/test/scala/com/digitalasset/daml/lf/validation/upgrade/UpgradesSpecBase.scala b/sdk/daml-lf/validation/src/test/scala/com/digitalasset/daml/lf/validation/upgrade/UpgradesSpecBase.scala index fa9e9bf31257..24f0499463f2 100644 --- a/sdk/daml-lf/validation/src/test/scala/com/digitalasset/daml/lf/validation/upgrade/UpgradesSpecBase.scala +++ b/sdk/daml-lf/validation/src/test/scala/com/digitalasset/daml/lf/validation/upgrade/UpgradesSpecBase.scala @@ -772,6 +772,28 @@ trait LongTests { this: UpgradesSpec => ), ) } + + "Succeeds when an exception is only defined in the initial package." in { + testPackagePair( + "test-common/upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-v1.dar", + "test-common/upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-v2.dar", + assertPackageUpgradeCheck( + None + ), + ) + } + + "Fails when an exception is defined in an upgrading package when it was already in the prior package." in { + testPackagePair( + "test-common/upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-v1.dar", + "test-common/upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-v2.dar", + assertPackageUpgradeCheck( + Some( + "Tried to upgrade exception E, but exceptions cannot be upgraded. They should be removed in any upgrading package." + ) + ), + ) + } } } diff --git a/sdk/test-common/BUILD.bazel b/sdk/test-common/BUILD.bazel index 6e5cfeac666c..d5b1a701f124 100644 --- a/sdk/test-common/BUILD.bazel +++ b/sdk/test-common/BUILD.bazel @@ -451,6 +451,8 @@ da_scala_dar_resources_library( # More more more tests ported from DamlcUpgrades.hs ("FailsWhenAnInterfaceIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage", {}, {}), ("SucceedsWhenAnInterfaceIsOnlyDefinedInTheInitialPackage", {}, {}), + ("FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage", {}, {}), + ("SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage", {}, {}), ( "FailsWhenAnInstanceIsAddedUpgradedPackage", {}, diff --git a/sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v1/Main.daml b/sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v1/Main.daml new file mode 100644 index 000000000000..65facf6ad9a2 --- /dev/null +++ b/sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v1/Main.daml @@ -0,0 +1,11 @@ +-- Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 + +module Main where + +exception E with + field1 : Text + where + message field1 + +data IsSchemaPackage = IsSchemaPackage { isSchemaPackage : Text } diff --git a/sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v2/Main.daml b/sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v2/Main.daml new file mode 100644 index 000000000000..65facf6ad9a2 --- /dev/null +++ b/sdk/test-common/src/main/daml/upgrades/FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage/v2/Main.daml @@ -0,0 +1,11 @@ +-- Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 + +module Main where + +exception E with + field1 : Text + where + message field1 + +data IsSchemaPackage = IsSchemaPackage { isSchemaPackage : Text } diff --git a/sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v1/Main.daml b/sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v1/Main.daml new file mode 100644 index 000000000000..65facf6ad9a2 --- /dev/null +++ b/sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v1/Main.daml @@ -0,0 +1,11 @@ +-- Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 + +module Main where + +exception E with + field1 : Text + where + message field1 + +data IsSchemaPackage = IsSchemaPackage { isSchemaPackage : Text } diff --git a/sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v2/Main.daml b/sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v2/Main.daml new file mode 100644 index 000000000000..c431dc708459 --- /dev/null +++ b/sdk/test-common/src/main/daml/upgrades/SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage/v2/Main.daml @@ -0,0 +1,6 @@ +-- Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 + +module Main where + +data IsSchemaPackage = IsSchemaPackage { isSchemaPackage : Text } From c322918ef9cfb2775c0795f9c6552539a3785bd4 Mon Sep 17 00:00:00 2001 From: Dylan Thinnes Date: Wed, 25 Sep 2024 02:34:59 +0100 Subject: [PATCH 2/7] lint --- sdk/compiler/damlc/tests/BUILD.bazel | 4 ++-- sdk/daml-lf/validation/BUILD.bazel | 1 - .../digitalasset/daml/lf/validation/Upgrading.scala | 11 ++++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/sdk/compiler/damlc/tests/BUILD.bazel b/sdk/compiler/damlc/tests/BUILD.bazel index cd5f223da3ec..b9fc87f76fd1 100644 --- a/sdk/compiler/damlc/tests/BUILD.bazel +++ b/sdk/compiler/damlc/tests/BUILD.bazel @@ -455,11 +455,11 @@ da_haskell_test( "//test-common:upgrades-FailsWhenATopLevelVariantAddsAFieldToAVariantsType-files", "//test-common:upgrades-FailsWhenATopLevelVariantAddsAVariant-files", "//test-common:upgrades-FailsWhenATopLevelVariantRemovesAVariant-files", + "//test-common:upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-files", "//test-common:upgrades-FailsWhenAnInstanceIsAddedSeparateDep-files", "//test-common:upgrades-FailsWhenAnInstanceIsAddedUpgradedPackage-files", "//test-common:upgrades-FailsWhenAnInstanceIsDropped-files", "//test-common:upgrades-FailsWhenAnInterfaceIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-files", - "//test-common:upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-files", "//test-common:upgrades-FailsWhenDatatypeChangesVariety-files", "//test-common:upgrades-FailsWhenDependencyIsNotAValidUpgrade-files", "//test-common:upgrades-FailsWhenDepsDowngradeVersionsWhileUsingDatatypes-files", @@ -493,10 +493,10 @@ da_haskell_test( "//test-common:upgrades-SucceedsWhenATopLevelTypeSynonymChanges-files", "//test-common:upgrades-SucceedsWhenATopLevelVariantAddsAVariant-files", "//test-common:upgrades-SucceedsWhenATopLevelVariantAddsAnOptionalFieldToAVariantsType-files", + "//test-common:upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-files", "//test-common:upgrades-SucceedsWhenAnInstanceIsAddedToNewTemplateSeparateDep-files", "//test-common:upgrades-SucceedsWhenAnInstanceIsAddedToNewTemplateUpgradedPackage-files", "//test-common:upgrades-SucceedsWhenAnInterfaceIsOnlyDefinedInTheInitialPackage-files", - "//test-common:upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-files", "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-files", "//test-common:upgrades-SucceedsWhenNewFieldWithOptionalTypeIsAddedToTemplate-files", "//test-common:upgrades-SucceedsWhenNewFieldWithOptionalTypeIsAddedToTemplateChoice-files", diff --git a/sdk/daml-lf/validation/BUILD.bazel b/sdk/daml-lf/validation/BUILD.bazel index f98b6b4fadaf..a476977d284e 100644 --- a/sdk/daml-lf/validation/BUILD.bazel +++ b/sdk/daml-lf/validation/BUILD.bazel @@ -284,7 +284,6 @@ da_scala_test_suite( "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-dep-v2.dar", "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-v1.dar", "//test-common:upgrades-SucceedsWhenDepsDowngradeVersionsWithoutUsingDatatypes-v2.dar", - "//test-common:upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-v1.dar", "//test-common:upgrades-SucceedsWhenAnExceptionIsOnlyDefinedInTheInitialPackage-v2.dar", "//test-common:upgrades-FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage-v1.dar", diff --git a/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala b/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala index 637a55a1bce8..c5e8fc69eb99 100644 --- a/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala +++ b/sdk/daml-lf/validation/src/main/scala/com/digitalasset/daml/lf/validation/Upgrading.scala @@ -433,13 +433,14 @@ case class TypecheckUpgrades( Either[ (Ref.DottedName, (Ast.DDataType, Ast.DefException)), (Ref.DottedName, Ast.DDataType), - ] + ], ] = { module.interfaces.get(tcon) match { - case None => Right(module.exceptions.get(tcon) match { - case None => Right((tcon, dt)) - case Some(exception) => Left((tcon, (dt, exception))) - }) + case None => + Right(module.exceptions.get(tcon) match { + case None => Right((tcon, dt)) + case Some(exception) => Left((tcon, (dt, exception))) + }) case Some(iface) => Left((tcon, (dt, iface))) } } From 81c11a4b23f419304e2968a367d011939a668914 Mon Sep 17 00:00:00 2001 From: Dylan Thinnes Date: Thu, 26 Sep 2024 11:04:43 +0100 Subject: [PATCH 3/7] Replace setUpgradeField with True --- sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs b/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs index c7cda5ef0704..66723767ca21 100644 --- a/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs +++ b/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs @@ -624,14 +624,14 @@ tests damlc = versionDefault NoDependencies False - setUpgradeField + True , test "FailsWhenAnExceptionIsDefinedInAnUpgradingPackageWhenItWasAlreadyInThePriorPackage" (FailWithError "\ESC\\[0;91merror type checking exception Main.E:\n Tried to upgrade exception E, but exceptions cannot be upgraded. They should be removed in any upgrading package.") versionDefault NoDependencies False - setUpgradeField + True ] ) where From d1dc74b9098dc31ada917fc68f7080571821b1ad Mon Sep 17 00:00:00 2001 From: Dylan Thinnes Date: Thu, 26 Sep 2024 18:46:01 +0100 Subject: [PATCH 4/7] Add flag for downgrading exception errors to warnings Update tests to look for these warnings --- .../src/DA/Daml/LF/TypeChecker/Error.hs | 2 +- .../src/DA/Daml/LF/TypeChecker/Upgrade.hs | 11 +++++++++-- .../daml-opts-types/DA/Daml/Options/Types.hs | 6 +++++- sdk/compiler/damlc/lib/DA/Cli/Options.hs | 9 +++++++++ .../Daml3ScriptTrySubmitConcurrently.daml | 3 ++- .../tests/daml-test-files/ExceptionSemantics.daml | 9 +++++---- .../tests/daml-test-files/ExceptionSyntax.daml | 1 + .../tests/daml-test-files/InterfaceGuarded.daml | 11 ++++++----- .../daml-test-files/RestrictedNameWarnings.daml | 9 +++++---- .../damlc/tests/src/DA/Test/DamlcIntegration.hs | 2 +- .../src/DA/Daml/Assistant/IntegrationTests.hs | 2 +- sdk/docs/BUILD.bazel | 13 +++++++------ 12 files changed, 52 insertions(+), 26 deletions(-) diff --git a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs index 67f436b8340c..9850f14ae9f3 100644 --- a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs +++ b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs @@ -263,7 +263,7 @@ instance Pretty WarnableError where vsep [ "This package defines both exceptions and templates. This may make this package and its dependents not upgradeable." , "It is recommended that exceptions are defined in their own package separate from their implementations." - --, "Ignore this error message with the --warn-bad-exceptions=yes flag." + , "Ignore this error message with the --warn-bad-exceptions=yes flag." ] data PackageUpgradeOrigin = UpgradingPackage | UpgradedPackage diff --git a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs index a5f93bdeea15..e95edee955b3 100644 --- a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs +++ b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Upgrade.hs @@ -65,7 +65,7 @@ shouldTypecheckM = asks (uncurry shouldTypecheck) mkGamma :: Version -> UpgradeInfo -> World -> Gamma mkGamma version upgradeInfo world = - let addBadIfaceSwapIndicator :: Gamma -> Gamma + let addBadIfaceSwapIndicator, addBadExceptionSwapIndicator :: Gamma -> Gamma addBadIfaceSwapIndicator = if uiWarnBadInterfaceInstances upgradeInfo then @@ -75,8 +75,15 @@ mkGamma version upgradeInfo world = Left WEUpgradeShouldDefineIfacesAndTemplatesSeparately {} -> Just True _ -> Nothing) else id + addBadExceptionSwapIndicator = + if uiWarnBadExceptions upgradeInfo + then + addDiagnosticSwapIndicator (\case + Left WEUpgradeShouldDefineExceptionsAndTemplatesSeparately {} -> Just True + _ -> Nothing) + else id in - addBadIfaceSwapIndicator $ emptyGamma world version + addBadExceptionSwapIndicator $ addBadIfaceSwapIndicator $ emptyGamma world version gammaM :: World -> TcPreUpgradeM Gamma gammaM world = asks (flip (uncurry mkGamma) world) diff --git a/sdk/compiler/damlc/daml-opts/daml-opts-types/DA/Daml/Options/Types.hs b/sdk/compiler/damlc/daml-opts/daml-opts-types/DA/Daml/Options/Types.hs index 64c23ec8d336..5a7d8383f1ef 100644 --- a/sdk/compiler/damlc/daml-opts/daml-opts-types/DA/Daml/Options/Types.hs +++ b/sdk/compiler/damlc/daml-opts/daml-opts-types/DA/Daml/Options/Types.hs @@ -37,6 +37,7 @@ module DA.Daml.Options.Types , UpgradeInfo (..) , defaultUiTypecheckUpgrades , defaultUiWarnBadInterfaceInstances + , defaultUiWarnBadExceptions , defaultUpgradeInfo ) where @@ -141,6 +142,7 @@ data UpgradeInfo = UpgradeInfo { uiUpgradedPackagePath :: Maybe FilePath , uiTypecheckUpgrades :: Bool , uiWarnBadInterfaceInstances :: Bool + , uiWarnBadExceptions :: Bool } newtype IncrementalBuild = IncrementalBuild { getIncrementalBuild :: Bool } @@ -291,11 +293,13 @@ defaultUpgradeInfo = UpgradeInfo { uiUpgradedPackagePath = Nothing , uiTypecheckUpgrades = defaultUiTypecheckUpgrades , uiWarnBadInterfaceInstances = defaultUiWarnBadInterfaceInstances + , uiWarnBadExceptions = defaultUiWarnBadExceptions } -defaultUiTypecheckUpgrades, defaultUiWarnBadInterfaceInstances :: Bool +defaultUiTypecheckUpgrades, defaultUiWarnBadInterfaceInstances, defaultUiWarnBadExceptions :: Bool defaultUiTypecheckUpgrades = True defaultUiWarnBadInterfaceInstances = False +defaultUiWarnBadExceptions = False pkgNameVersion :: LF.PackageName -> Maybe LF.PackageVersion -> UnitId pkgNameVersion (LF.PackageName n) mbV = diff --git a/sdk/compiler/damlc/lib/DA/Cli/Options.hs b/sdk/compiler/damlc/lib/DA/Cli/Options.hs index b29c8f515e82..9bded69eb6a9 100644 --- a/sdk/compiler/damlc/lib/DA/Cli/Options.hs +++ b/sdk/compiler/damlc/lib/DA/Cli/Options.hs @@ -587,11 +587,20 @@ optionsParser numProcessors enableScenarioService parsePkgName parseDlintUsage = "Convert errors about bad, non-upgradeable interface instances into warnings." idm + optWarnBadExceptions :: Parser Bool + optWarnBadExceptions = + flagYesNoAuto + "warn-bad-exceptions" + defaultUiWarnBadExceptions + "Convert errors about bad, non-upgradeable exceptions into warnings." + idm + optUpgradeInfo :: Parser UpgradeInfo optUpgradeInfo = do uiTypecheckUpgrades <- optTypecheckUpgrades uiUpgradedPackagePath <- optUpgradeDar uiWarnBadInterfaceInstances <- optWarnBadInterfaceInstances + uiWarnBadExceptions <- optWarnBadExceptions pure UpgradeInfo {..} optGhcCustomOptions :: Parser [String] diff --git a/sdk/compiler/damlc/tests/daml-test-files/Daml3ScriptTrySubmitConcurrently.daml b/sdk/compiler/damlc/tests/daml-test-files/Daml3ScriptTrySubmitConcurrently.daml index 817e3c6a2186..1501496ee67a 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/Daml3ScriptTrySubmitConcurrently.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/Daml3ScriptTrySubmitConcurrently.daml @@ -3,7 +3,8 @@ -- @ SCRIPT-V2 --- @ WARN range=19:1-19:52; Import of internal module Daml.Script.Internal of package daml3-script is discouraged, as this module will change without warning. +-- @ WARN range=20:1-20:52; Import of internal module Daml.Script.Internal of package daml3-script is discouraged, as this module will change without warning. +-- @ WARN warn-bad-exceptions {-# LANGUAGE ApplicativeDo #-} diff --git a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml index a7669e9678cf..aa10e311cec6 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml @@ -2,10 +2,11 @@ -- SPDX-License-Identifier: Apache-2.0 -- @SUPPORTS-LF-FEATURE DAML_EXCEPTIONS --- @ERROR range=132:1-132:23; Unhandled exception: ExceptionSemantics:E --- @ERROR range=147:1-147:25; Unhandled exception: DA.Exception.ArithmeticError:ArithmeticError@XXXXXX with message = "ArithmeticError while evaluating (DIV_INT64 1 0)." --- @WARN range=181:3-181:12; Use of divulged contracts is deprecated --- @ERROR range=184:1-184:11; Attempt to exercise a consumed contract +-- @ERROR range=133:1-133:23; Unhandled exception: ExceptionSemantics:E +-- @ERROR range=148:1-148:25; Unhandled exception: DA.Exception.ArithmeticError:ArithmeticError@XXXXXX with message = "ArithmeticError while evaluating (DIV_INT64 1 0)." +-- @WARN range=182:3-182:12; Use of divulged contracts is deprecated +-- @ERROR range=185:1-185:11; Attempt to exercise a consumed contract +-- @WARN warn-bad-exceptions module ExceptionSemantics where import Daml.Script diff --git a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml index 34a6d7f755b7..bdd854c8824d 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml @@ -2,6 +2,7 @@ -- SPDX-License-Identifier: Apache-2.0 -- @SUPPORTS-LF-FEATURE DAML_EXCEPTIONS +-- @WARN warn-bad-exceptions -- @QUERY-LF [ $pkg.modules[].exceptions[] ] | length == 1 -- | Test that exception syntax is correctly handled. diff --git a/sdk/compiler/damlc/tests/daml-test-files/InterfaceGuarded.daml b/sdk/compiler/damlc/tests/daml-test-files/InterfaceGuarded.daml index cd1306a4911f..56a315670f07 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/InterfaceGuarded.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/InterfaceGuarded.daml @@ -6,6 +6,7 @@ -- @WARN since-lf=1.16 2.0; warn-bad-interface-instances -- @WARN since-lf=1.16 2.0; warn-bad-interface-instances -- @WARN since-lf=1.16 2.0; warn-bad-interface-instances +-- @WARN since-lf=1.16 2.0; warn-bad-exceptions module InterfaceGuarded where @@ -146,17 +147,17 @@ exerciseTest c = script do trueGuard = exerciseTest TrueGuard --- @ERROR range=150:1-150:11; failing guard +-- @ERROR range=151:1-151:11; failing guard falseGuard = exerciseTest FalseGuard --- @ERROR range=153:1-153:11; failing guard +-- @ERROR range=154:1-154:11; failing guard errorGuard = exerciseTest ErrorGuard --- @ERROR range=156:1-156:17; failing guard +-- @ERROR range=157:1-157:17; failing guard customErrorGuard = exerciseTest CustomErrorGuard --- @ERROR range=159:1-159:14; failing guard +-- @ERROR range=160:1-160:14; failing guard tryErrorGuard = exerciseTest TryErrorGuard --- @ERROR range=162:1-162:20; failing guard +-- @ERROR range=163:1-163:20; failing guard tryCustomErrorGuard = exerciseTest TryCustomErrorGuard diff --git a/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml b/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml index 18c4a831e708..30b9face6229 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml @@ -1,7 +1,8 @@ --- @WARN range=9:5-9:9; `self' is an unsupported field name, and may break without warning in future versions. Please use something else. --- @WARN range=14:5-14:8; `arg' is an unsupported field name, and may break without warning in future versions. Please use something else. --- @WARN range=19:5-19:9; `self' is an unsupported field name, and may break without warning in future versions. Please use something else. --- @WARN range=24:5-24:8; `arg' is an unsupported field name, and may break without warning in future versions. Please use something else. +-- @WARN range=10:5-10:9; `self' is an unsupported field name, and may break without warning in future versions. Please use something else. +-- @WARN range=15:5-15:8; `arg' is an unsupported field name, and may break without warning in future versions. Please use something else. +-- @WARN range=20:5-20:9; `self' is an unsupported field name, and may break without warning in future versions. Please use something else. +-- @WARN range=25:5-25:8; `arg' is an unsupported field name, and may break without warning in future versions. Please use something else. +-- @WARN warn-bad-exceptions module RestrictedNameWarnings where diff --git a/sdk/compiler/damlc/tests/src/DA/Test/DamlcIntegration.hs b/sdk/compiler/damlc/tests/src/DA/Test/DamlcIntegration.hs index 9ffc322665fe..17a5a51b52f9 100644 --- a/sdk/compiler/damlc/tests/src/DA/Test/DamlcIntegration.hs +++ b/sdk/compiler/damlc/tests/src/DA/Test/DamlcIntegration.hs @@ -329,7 +329,7 @@ getIntegrationTests registerTODO scenarioService (packageDbPath, packageFlags) = , dlintHintFiles = NoDlintHintFiles } , optPackageImports = packageFlags - , optUpgradeInfo = (optUpgradeInfo opts0) { uiWarnBadInterfaceInstances = True } + , optUpgradeInfo = (optUpgradeInfo opts0) { uiWarnBadInterfaceInstances = True, uiWarnBadExceptions = True } } mkIde options = do diff --git a/sdk/daml-assistant/integration-tests/src/DA/Daml/Assistant/IntegrationTests.hs b/sdk/daml-assistant/integration-tests/src/DA/Daml/Assistant/IntegrationTests.hs index 36ffecf870d8..d25e751b428c 100644 --- a/sdk/daml-assistant/integration-tests/src/DA/Daml/Assistant/IntegrationTests.hs +++ b/sdk/daml-assistant/integration-tests/src/DA/Daml/Assistant/IntegrationTests.hs @@ -123,7 +123,7 @@ damlStart tmpDir disableUpgradeValidation = do , " npm-scope: daml.js" , " java:" , " output-directory: ui/java" - ] ++ [ "build-options:\n- --warn-bad-interface-instances=yes" | disableUpgradeValidation ] + ] ++ [ "build-options:\n- --warn-bad-interface-instances=yes\n- --warn-bad-exceptions=yes" | disableUpgradeValidation ] writeFileUTF8 (projDir "daml/Main.daml") $ unlines [ "module Main where" diff --git a/sdk/docs/BUILD.bazel b/sdk/docs/BUILD.bazel index 8d2e5096f6fc..f43f94e37f8e 100644 --- a/sdk/docs/BUILD.bazel +++ b/sdk/docs/BUILD.bazel @@ -422,14 +422,14 @@ java_binary( daml_test( name = "ledger-api-daml-test", srcs = glob(["source/app-dev/code-snippets/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes"], + additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], deps = ["//daml-script/daml:daml-script.dar"], ) daml_test( name = "bindings-java-daml-test", srcs = glob(["source/app-dev/bindings-java/code-snippets/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes"], + additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], # FIXME: https://github.com/digital-asset/daml/issues/12051 # remove target, once interfaces are stable. target = lf_version_configuration.get("latest"), @@ -458,7 +458,7 @@ daml_test( name = "daml-ref-daml-test", timeout = "long", srcs = glob(["source/daml/code-snippets/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes"], + additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], deps = ["//daml-script/daml:daml-script.dar"], ) @@ -467,7 +467,7 @@ daml_test( name = "daml-ref-daml-test-{}".format(version), timeout = "long", srcs = glob(["source/daml/code-snippets-dev/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes"], + additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], target = version, ) for version in LF_DEV_VERSIONS @@ -527,7 +527,7 @@ daml_test( srcs = glob( ["source/daml/intro/daml/daml-intro-12-part1/**/*.daml"], ), - additional_compiler_flags = ["--warn-bad-interface-instances=yes"], + additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], target = "1.15", deps = ["//daml-script/daml:daml-script-1.15.dar"], ) @@ -546,7 +546,7 @@ daml_test( srcs = glob( ["source/daml/intro/daml/daml-intro-12-part2/**/*.daml"], ), - additional_compiler_flags = ["--warn-bad-interface-instances=yes"], + additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], data_deps = ["daml-intro12-part1.dar"], target = "1.15", deps = ["//daml-script/daml:daml-script-1.15.dar"], @@ -574,6 +574,7 @@ daml_test( srcs = glob(["source/daml/intro/daml/daml-intro-8/**/*.daml"]), target = version, deps = ["//daml-script/daml:daml-script-{}.dar".format(version)], + additional_compiler_flags = ["--warn-bad-exceptions=yes"], ) for version in LF_DEV_VERSIONS ] From 95caad371f28ec32a01990ae9f0906459df4e9c2 Mon Sep 17 00:00:00 2001 From: Dylan Thinnes Date: Fri, 27 Sep 2024 11:49:03 +0100 Subject: [PATCH 5/7] Add two tests for Phantom type, fix error messages --- .../daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs | 4 ++-- sdk/compiler/damlc/tests/BUILD.bazel | 1 + sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs | 11 +++++++++-- sdk/test-common/BUILD.bazel | 1 + .../upgrades/SucceedWhenParamNameChanges/v1/Main.daml | 4 ++-- .../upgrades/SucceedWhenParamNameChanges/v2/Main.daml | 3 ++- .../SucceedWhenPhantomParamBecomesUsed/v1/Main.daml | 8 ++++++++ .../SucceedWhenPhantomParamBecomesUsed/v2/Main.daml | 7 +++++++ 8 files changed, 32 insertions(+), 7 deletions(-) create mode 100644 sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v1/Main.daml create mode 100644 sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v2/Main.daml diff --git a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs index 121efe93d79d..0d9a1ee0a381 100644 --- a/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs +++ b/sdk/compiler/daml-lf-tools/src/DA/Daml/LF/TypeChecker/Error.hs @@ -696,8 +696,8 @@ instance Pretty UnwarnableError where pprintDep (pkgId, Just meta) = pPrint pkgId <> "(" <> pPrint (packageName meta) <> ", " <> pPrint (packageVersion meta) <> ")" pprintDep (pkgId, Nothing) = pPrint pkgId EUpgradeMultiplePackagesWithSameNameAndVersion name version ids -> "Multiple packages with name " <> pPrint name <> " and version " <> pPrint (show version) <> ": " <> hcat (L.intersperse ", " (map pPrint ids)) - EUpgradeDifferentParamsCount origin -> "EUpgradeDifferentParamsCount " <> pPrint origin - EUpgradeDifferentParamsKinds origin -> "EUpgradeDifferentParamsKinds " <> pPrint origin + EUpgradeDifferentParamsCount origin -> "The upgraded " <> pPrint origin <> " has changed the number of type variables it has." + EUpgradeDifferentParamsKinds origin -> "The upgraded " <> pPrint origin <> " has changed the kind of one of its type variables." instance Pretty UpgradedRecordOrigin where diff --git a/sdk/compiler/damlc/tests/BUILD.bazel b/sdk/compiler/damlc/tests/BUILD.bazel index 6cd64fc315e8..2c3a6c5cde18 100644 --- a/sdk/compiler/damlc/tests/BUILD.bazel +++ b/sdk/compiler/damlc/tests/BUILD.bazel @@ -526,6 +526,7 @@ da_haskell_test( "//test-common:upgrades-WarnsWhenTemplateChangesKeyMaintainers-files", "//test-common:upgrades-WarnsWhenTemplateChangesObservers-files", "//test-common:upgrades-WarnsWhenTemplateChangesSignatories-files", + "//test-common:upgrades-SucceedWhenPhantomParamBecomesUsed-files", ], hackage_deps = [ "base", diff --git a/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs b/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs index 329c9e586314..c43a96bcc859 100644 --- a/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs +++ b/sdk/compiler/damlc/tests/src/DA/Test/DamlcUpgrades.hs @@ -620,14 +620,14 @@ tests damlc = version1_dev , test "FailWhenParamCountChanges" - (FailWithError "\ESC\\[0;91merror type checking data type Main.MyStruct:\n EUpgradeDifferentParamsCount") + (FailWithError "\ESC\\[0;91merror type checking data type Main.MyStruct:\n The upgraded data type MyStruct has changed the number of type variables it has.") versionDefault NoDependencies False True , test "FailWhenParamKindChanges" - (FailWithError "\ESC\\[0;91merror type checking data type Main.MyStruct:\n EUpgradeDifferentParamsKinds") + (FailWithError "\ESC\\[0;91merror type checking data type Main.MyStruct:\n The upgraded data type MyStruct has changed the kind of one of its type variables.") versionDefault NoDependencies False @@ -639,6 +639,13 @@ tests damlc = NoDependencies False True + , test + "SucceedWhenPhantomParamBecomesUsed" + Succeed + versionDefault + NoDependencies + False + True ] ) where diff --git a/sdk/test-common/BUILD.bazel b/sdk/test-common/BUILD.bazel index 9254ca56cea2..3cdea57310d8 100644 --- a/sdk/test-common/BUILD.bazel +++ b/sdk/test-common/BUILD.bazel @@ -506,6 +506,7 @@ da_scala_dar_resources_library( ("FailWhenParamCountChanges", {}, {}), ("FailWhenParamKindChanges", {}, {}), ("SucceedWhenParamNameChanges", {}, {}), + ("SucceedWhenPhantomParamBecomesUsed", {}, {}), ] ] diff --git a/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v1/Main.daml b/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v1/Main.daml index 7dce9f05778a..43465e5464f9 100644 --- a/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v1/Main.daml +++ b/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v1/Main.daml @@ -3,5 +3,5 @@ module Main where -data MyStruct a = MyStruct { field1 : a } - +data MyStruct1 a = MyStruct1 { field1 : a } +data MyStruct2 a b = MyStruct2 { field1 : a } diff --git a/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v2/Main.daml b/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v2/Main.daml index 5aae2347219a..0cc0dfbedd0a 100644 --- a/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v2/Main.daml +++ b/sdk/test-common/src/main/daml/upgrades/SucceedWhenParamNameChanges/v2/Main.daml @@ -3,5 +3,6 @@ module Main where -data MyStruct b = MyStruct { field1 : b } +data MyStruct1 b = MyStruct1 { field1 : b } +data MyStruct2 b a = MyStruct2 { field1 : b, field2 : Optional a, field3: Optional b } diff --git a/sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v1/Main.daml b/sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v1/Main.daml new file mode 100644 index 000000000000..6da04c01e80f --- /dev/null +++ b/sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v1/Main.daml @@ -0,0 +1,8 @@ +-- Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 + +module Main where + +data MyStruct a b = MyStruct { field1 : a } + + diff --git a/sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v2/Main.daml b/sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v2/Main.daml new file mode 100644 index 000000000000..1641788af413 --- /dev/null +++ b/sdk/test-common/src/main/daml/upgrades/SucceedWhenPhantomParamBecomesUsed/v2/Main.daml @@ -0,0 +1,7 @@ +-- Copyright (c) 2024 Digital Asset (Switzerland) GmbH and/or its affiliates. All rights reserved. +-- SPDX-License-Identifier: Apache-2.0 + +module Main where + +data MyStruct a b = MyStruct { field1 : a, field2 : Optional b } + From c0ba26747b464bd7423332ddc3b57a1db33cc65f Mon Sep 17 00:00:00 2001 From: Dylan Thinnes Date: Fri, 27 Sep 2024 12:03:44 +0100 Subject: [PATCH 6/7] Predicate warn-bad-exceptions on LF 1.17 for integration tests --- .../damlc/tests/daml-test-files/ExceptionSemantics.daml | 2 +- sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml | 2 +- .../damlc/tests/daml-test-files/RestrictedNameWarnings.daml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml index aa10e311cec6..3da8a57f73bc 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSemantics.daml @@ -6,7 +6,7 @@ -- @ERROR range=148:1-148:25; Unhandled exception: DA.Exception.ArithmeticError:ArithmeticError@XXXXXX with message = "ArithmeticError while evaluating (DIV_INT64 1 0)." -- @WARN range=182:3-182:12; Use of divulged contracts is deprecated -- @ERROR range=185:1-185:11; Attempt to exercise a consumed contract --- @WARN warn-bad-exceptions +-- @WARN since-lf=1.17 2.0; warn-bad-exceptions module ExceptionSemantics where import Daml.Script diff --git a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml index bdd854c8824d..6fbbc2ca4b05 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/ExceptionSyntax.daml @@ -2,7 +2,7 @@ -- SPDX-License-Identifier: Apache-2.0 -- @SUPPORTS-LF-FEATURE DAML_EXCEPTIONS --- @WARN warn-bad-exceptions +-- @WARN since-lf=1.17 2.0; warn-bad-exceptions -- @QUERY-LF [ $pkg.modules[].exceptions[] ] | length == 1 -- | Test that exception syntax is correctly handled. diff --git a/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml b/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml index 30b9face6229..36cf15641407 100644 --- a/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml +++ b/sdk/compiler/damlc/tests/daml-test-files/RestrictedNameWarnings.daml @@ -2,7 +2,7 @@ -- @WARN range=15:5-15:8; `arg' is an unsupported field name, and may break without warning in future versions. Please use something else. -- @WARN range=20:5-20:9; `self' is an unsupported field name, and may break without warning in future versions. Please use something else. -- @WARN range=25:5-25:8; `arg' is an unsupported field name, and may break without warning in future versions. Please use something else. --- @WARN warn-bad-exceptions +-- @WARN since-lf=1.17 2.0; warn-bad-exceptions module RestrictedNameWarnings where From f0c592b2f54a0bd975cc91ddbeffca5e30c6545a Mon Sep 17 00:00:00 2001 From: Dylan Thinnes Date: Fri, 27 Sep 2024 12:23:48 +0100 Subject: [PATCH 7/7] lint --- sdk/compiler/damlc/tests/BUILD.bazel | 2 +- sdk/docs/BUILD.bazel | 32 ++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/sdk/compiler/damlc/tests/BUILD.bazel b/sdk/compiler/damlc/tests/BUILD.bazel index ee7e52b0d796..634c00171146 100644 --- a/sdk/compiler/damlc/tests/BUILD.bazel +++ b/sdk/compiler/damlc/tests/BUILD.bazel @@ -489,6 +489,7 @@ da_haskell_test( "//test-common:upgrades-RecordFieldsNewNonOptional-files", "//test-common:upgrades-SucceedWhenATopLevelEnumAddsAField-files", "//test-common:upgrades-SucceedWhenParamNameChanges-files", + "//test-common:upgrades-SucceedWhenPhantomParamBecomesUsed-files", "//test-common:upgrades-SucceedsWhenATopLevelEnumChanges-files", "//test-common:upgrades-SucceedsWhenATopLevelRecordAddsAnOptionalFieldAtTheEnd-files", "//test-common:upgrades-SucceedsWhenATopLevelRecordAddsAnOptionalFieldAtTheEnd-v1.dar", @@ -528,7 +529,6 @@ da_haskell_test( "//test-common:upgrades-WarnsWhenTemplateChangesKeyMaintainers-files", "//test-common:upgrades-WarnsWhenTemplateChangesObservers-files", "//test-common:upgrades-WarnsWhenTemplateChangesSignatories-files", - "//test-common:upgrades-SucceedWhenPhantomParamBecomesUsed-files", ], hackage_deps = [ "base", diff --git a/sdk/docs/BUILD.bazel b/sdk/docs/BUILD.bazel index f43f94e37f8e..a29a8d8373ba 100644 --- a/sdk/docs/BUILD.bazel +++ b/sdk/docs/BUILD.bazel @@ -422,14 +422,20 @@ java_binary( daml_test( name = "ledger-api-daml-test", srcs = glob(["source/app-dev/code-snippets/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], + additional_compiler_flags = [ + "--warn-bad-interface-instances=yes", + "--warn-bad-exceptions=yes", + ], deps = ["//daml-script/daml:daml-script.dar"], ) daml_test( name = "bindings-java-daml-test", srcs = glob(["source/app-dev/bindings-java/code-snippets/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], + additional_compiler_flags = [ + "--warn-bad-interface-instances=yes", + "--warn-bad-exceptions=yes", + ], # FIXME: https://github.com/digital-asset/daml/issues/12051 # remove target, once interfaces are stable. target = lf_version_configuration.get("latest"), @@ -458,7 +464,10 @@ daml_test( name = "daml-ref-daml-test", timeout = "long", srcs = glob(["source/daml/code-snippets/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], + additional_compiler_flags = [ + "--warn-bad-interface-instances=yes", + "--warn-bad-exceptions=yes", + ], deps = ["//daml-script/daml:daml-script.dar"], ) @@ -467,7 +476,10 @@ daml_test( name = "daml-ref-daml-test-{}".format(version), timeout = "long", srcs = glob(["source/daml/code-snippets-dev/**/*.daml"]), - additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], + additional_compiler_flags = [ + "--warn-bad-interface-instances=yes", + "--warn-bad-exceptions=yes", + ], target = version, ) for version in LF_DEV_VERSIONS @@ -527,7 +539,10 @@ daml_test( srcs = glob( ["source/daml/intro/daml/daml-intro-12-part1/**/*.daml"], ), - additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], + additional_compiler_flags = [ + "--warn-bad-interface-instances=yes", + "--warn-bad-exceptions=yes", + ], target = "1.15", deps = ["//daml-script/daml:daml-script-1.15.dar"], ) @@ -546,7 +561,10 @@ daml_test( srcs = glob( ["source/daml/intro/daml/daml-intro-12-part2/**/*.daml"], ), - additional_compiler_flags = ["--warn-bad-interface-instances=yes", "--warn-bad-exceptions=yes"], + additional_compiler_flags = [ + "--warn-bad-interface-instances=yes", + "--warn-bad-exceptions=yes", + ], data_deps = ["daml-intro12-part1.dar"], target = "1.15", deps = ["//daml-script/daml:daml-script-1.15.dar"], @@ -572,9 +590,9 @@ daml_test( daml_test( name = "daml-intro-8-daml-test-{}".format(version), srcs = glob(["source/daml/intro/daml/daml-intro-8/**/*.daml"]), + additional_compiler_flags = ["--warn-bad-exceptions=yes"], target = version, deps = ["//daml-script/daml:daml-script-{}.dar".format(version)], - additional_compiler_flags = ["--warn-bad-exceptions=yes"], ) for version in LF_DEV_VERSIONS ]