From 961cd5c9f20a03eb46a63b96177052999963eef9 Mon Sep 17 00:00:00 2001 From: Jort Date: Fri, 15 Nov 2024 15:38:39 -0800 Subject: [PATCH] Upgrade Errors for struct enum functions (#20151) ## Description Add struct, enum and function errors to upgrade errors, leaves out type params to give it, it's own PR. ## Test plan snapshots --- ## Release notes no effect until enabled Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API: --- .../Move.toml | 0 .../sources/enum.move | 0 .../sources/func.move | 0 .../sources/struct.move | 0 .../Move.toml | 0 .../sources/enum.move | 0 .../sources/func.move | 0 .../sources/struct.move | 0 .../upgrade_errors/enum_errors_v1/Move.toml | 6 + .../enum_errors_v1/sources/UpgradeErrors.move | 77 ++ .../upgrade_errors/enum_errors_v2/Move.toml | 6 + .../enum_errors_v2/sources/UpgradeErrors.move | 79 ++ .../function_errors_v1/Move.toml | 6 + .../sources/UpgradeErrors.move | 28 + .../function_errors_v2/Move.toml | 6 + .../sources/UpgradeErrors.move | 33 + .../upgrade_errors/struct_errors_v1/Move.toml | 6 + .../sources/UpgradeErrors.move | 37 + .../upgrade_errors/struct_errors_v2/Move.toml | 6 + .../sources/UpgradeErrors.move | 39 + ...atibility_tests__declarations_missing.snap | 12 +- ...ty__upgrade_compatibility_tests__enum.snap | 210 ++++ ...upgrade_compatibility_tests__function.snap | 60 ++ ...__upgrade_compatibility_tests__struct.snap | 93 ++ .../unit_tests/upgrade_compatibility_tests.rs | 32 +- crates/sui/src/upgrade_compatibility.rs | 903 +++++++++++++++++- 26 files changed, 1583 insertions(+), 56 deletions(-) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v1 => declaration_errors_v1}/Move.toml (100%) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v1 => declaration_errors_v1}/sources/enum.move (100%) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v1 => declaration_errors_v1}/sources/func.move (100%) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v1 => declaration_errors_v1}/sources/struct.move (100%) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v2 => declaration_errors_v2}/Move.toml (100%) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v2 => declaration_errors_v2}/sources/enum.move (100%) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v2 => declaration_errors_v2}/sources/func.move (100%) rename crates/sui/src/unit_tests/fixtures/upgrade_errors/{declarations_missing_v2 => declaration_errors_v2}/sources/struct.move (100%) create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/Move.toml create mode 100644 crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move create mode 100644 crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__enum.snap create mode 100644 crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__function.snap create mode 100644 crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct.snap diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/Move.toml similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/Move.toml rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/Move.toml diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/sources/enum.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/sources/enum.move similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/sources/enum.move rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/sources/enum.move diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/sources/func.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/sources/func.move similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/sources/func.move rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/sources/func.move diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/sources/struct.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/sources/struct.move similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v1/sources/struct.move rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v1/sources/struct.move diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/Move.toml similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/Move.toml rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/Move.toml diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/enum.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/sources/enum.move similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/enum.move rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/sources/enum.move diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/func.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/sources/func.move similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/func.move rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/sources/func.move diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/struct.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/sources/struct.move similarity index 100% rename from crates/sui/src/unit_tests/fixtures/upgrade_errors/declarations_missing_v2/sources/struct.move rename to crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/sources/struct.move diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..6c0c47b58b78b --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v1/sources/UpgradeErrors.move @@ -0,0 +1,77 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + + public enum EnumAddAbility has copy { // add drop + A, + } + + public enum EnumRemoveAbility has copy, drop { + A, + } + + public enum EnumAddAndRemoveAbility has copy, drop { + A, + } + + public enum EnumAddVariant { + A, + // B, to be added + } + + public enum EnumRemoveVariant { + A, + B, // to be removed + } + + public enum EnumChangeVariant { + A, + B, // to be changed to C + } + + public enum EnumChangeAndAddVariant { + A, + B, // to be changed to C + // D, to be added + } + + public enum EnumChangeAndRemoveVariant { + A, + B, // to be changed to C + C, // to be removed + } + + public enum EnumAddAbilityWithTypes has copy { // add drop + A { a: u8 }, + } + + public enum EnumRemoveAbilityWithTypes has copy, drop { + A { a: u8 }, + } + + public enum EnumAddVariantWithTypes { + A { a: u8 }, + // B { b: u8 }, to be added + } + + public enum EnumRemoveVariantWithTypes { + A { a: u8 }, + B { b: u8 }, // to be removed + } + + public enum EnumChangeVariantWithTypes { + A { a: u8 }, + B { b: u8 }, // to be changed to C + } + + public enum EnumChangeAndAddVariantWithTypes { + A { a: u8 }, + B { b: u8 }, // to be changed to C + // D { d: u8 }, to be added + } +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..a6a9d2ff8bbd8 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move @@ -0,0 +1,79 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + + public enum EnumAddAbility has copy, drop { // add drop + A, + } + + public enum EnumRemoveAbility has copy { // drop removed + A, + } + + public enum EnumAddAndRemoveAbility has copy, store { + A, + } + + public enum EnumAddVariant { + A, + B, // added + } + + public enum EnumRemoveVariant { + A, + // B, removed + } + + public enum EnumChangeVariant { + A, + C, // changed from B + } + + public enum EnumChangeAndAddVariant { + A, + C, // to be changed to C + D // added + } + + public enum EnumChangeAndRemoveVariant { + A, + C, // changed to C + // removed C, + } + + // with types + public enum EnumAddAbilityWithTypes has copy, drop { // drop added + A { a: u8 }, + } + + public enum EnumRemoveAbilityWithTypes has copy { // drop removed + A { a: u8 }, + } + + public enum EnumAddVariantWithTypes { + A { a: u8 }, + B { b: u8 }, // added + } + + public enum EnumRemoveVariantWithTypes { + A { a: u8 }, + // B { b: u8 }, removed + } + + public enum EnumChangeVariantWithTypes { + A { a: u8 }, + C { b: u8 }, // changed to C + } + + public enum EnumChangeAndAddVariantWithTypes { + A { a: u8 }, + C { b: u8 }, // to be changed to C + D { d: u8 }, // added + } + +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..0e88bf16d0196 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/sources/UpgradeErrors.move @@ -0,0 +1,28 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + public fun func_with_wrong_param(a: u64): u64 { + 0 + } + + public fun func_with_wrong_return(): u64 { + 0 + } + + public fun func_with_wrong_param_and_return(a: u64): u64 { + 0 + } + + public fun func_with_wrong_param_length(a: u64, b: u64): u64 { + 0 + } + + public fun func_with_wrong_return_length(): (u64, u64) { + (0,0) + } +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..ab70bca89e9a1 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move @@ -0,0 +1,33 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + // changed argument from u64 to u32 + public fun func_with_wrong_param(a: u32): u64 { + 0 + } + + // changed return type from u64 to u32 + public fun func_with_wrong_return(): u32 { + 0 + } + + // changed argument from u64 to u32 and return type from u64 to u32 + public fun func_with_wrong_param_and_return(a: u32): u32 { + 0 + } + + // removed second argument + public fun func_with_wrong_param_length(a: u64): u64 { + 0 + } + + // changed return type from (u64, u64) to u64 + public fun func_with_wrong_return_length(): u64 { + 0 + } +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..eecbceeacfa24 --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v1/sources/UpgradeErrors.move @@ -0,0 +1,37 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + + // ability mismatch + public struct AddExtraAbility {} + public struct RemoveAbility has copy, drop {} + public struct AddAndRemoveAbility has copy, drop {} + public struct RemoveMultipleAbilities has copy, drop, store {} + public struct AddMultipleAbilities {} + + + // field mismatch + public struct AddField {} + // remove field + public struct RemoveField { + a: u64, + b: u64, // remove this field + } + + // change field name + public struct ChangeFieldName { + a: u64, + b: u64, // change this field name to c + } + + // change field type + public struct ChangeFieldType { + a: u64, + b: u64, // change this field type to u32 + } +} + diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/Move.toml b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/Move.toml new file mode 100644 index 0000000000000..39cc2752ab46d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/Move.toml @@ -0,0 +1,6 @@ +[package] +name = "upgrades" +edition = "2024.beta" # edition = "legacy" to use legacy (pre-2024) Move + +[addresses] +upgrades = "0x0" diff --git a/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move new file mode 100644 index 0000000000000..084bc0997bd2d --- /dev/null +++ b/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move @@ -0,0 +1,39 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +/// Module: UpgradeErrors + +#[allow(unused_field)] +module upgrades::upgrades { + + // ability mismatch + public struct AddExtraAbility has copy {} // added copy + public struct RemoveAbility has drop {} // removed copy + public struct AddAndRemoveAbility has drop, store {} // remove copy, add store + public struct RemoveMultipleAbilities has drop {} // remove copy, store + public struct AddMultipleAbilities has drop, copy {} + + // field mismatch + public struct AddField { + a: u64, + b: u64, + } + // remove field + public struct RemoveField { + a: u64, + // b removed here + } + + // change field name + public struct ChangeFieldName { + a: u64, + c: u64, // changed from b to c + } + + // change field type + public struct ChangeFieldType { + a: u64, + b: u32, // changed to u32 + } +} + diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__declarations_missing.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__declarations_missing.snap index e47d7459c9250..16ea839f042b5 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__declarations_missing.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__declarations_missing.snap @@ -3,33 +3,33 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: normalize_path(err.to_string()) --- error[Compatibility E01001]: missing public declaration - ┌─ /fixtures/upgrade_errors/declarations_missing_v2/sources/enum.move:4:18 + ┌─ /fixtures/upgrade_errors/declaration_errors_v2/sources/enum.move:4:18 │ 4 │ module upgrades::enum_ { │ ^^^^^ enum 'EnumToBeRemoved' is missing │ = enum is missing expected enum 'EnumToBeRemoved', but found none - = enums are part of a module's public interface and cannot be removed or changed during an upgrade + = enums are part of a module's public interface and cannot be removed or changed during an upgrade. = add missing enum 'EnumToBeRemoved' back to the module 'enum_'. error[Compatibility E01001]: missing public declaration - ┌─ /fixtures/upgrade_errors/declarations_missing_v2/sources/func.move:4:18 + ┌─ /fixtures/upgrade_errors/declaration_errors_v2/sources/func.move:4:18 │ 4 │ module upgrades::func_ { │ ^^^^^ public function 'fun_to_be_removed' is missing │ = public function is missing expected public function 'fun_to_be_removed', but found none - = public functions are part of a module's public interface and cannot be removed or changed during an upgrade + = public functions are part of a module's public interface and cannot be removed or changed during an upgrade. = add missing public function 'fun_to_be_removed' back to the module 'func_'. error[Compatibility E01001]: missing public declaration - ┌─ /fixtures/upgrade_errors/declarations_missing_v2/sources/struct.move:4:18 + ┌─ /fixtures/upgrade_errors/declaration_errors_v2/sources/struct.move:4:18 │ 4 │ module upgrades::struct_ { │ ^^^^^^^ struct 'StructToBeRemoved' is missing │ = struct is missing expected struct 'StructToBeRemoved', but found none - = structs are part of a module's public interface and cannot be removed or changed during an upgrade + = structs are part of a module's public interface and cannot be removed or changed during an upgrade. = add missing struct 'StructToBeRemoved' back to the module 'struct_'. diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__enum.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__enum.snap new file mode 100644 index 0000000000000..b312c82d9af71 --- /dev/null +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__enum.snap @@ -0,0 +1,210 @@ +--- +source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs +expression: normalize_path(err.to_string()) +--- +error[Compatibility E01003]: ability mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:9:17 + │ +9 │ public enum EnumAddAbility has copy, drop { // add drop + │ ^^^^^^^^^^^^^^ Unexpected abilities 'drop' + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's abilities for enum 'EnumAddAbility' including the ordering. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:13:17 + │ +13 │ public enum EnumRemoveAbility has copy { // drop removed + │ ^^^^^^^^^^^^^^^^^ Missing abilities 'drop' + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's abilities for enum 'EnumRemoveAbility' including the ordering. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:17:17 + │ +17 │ public enum EnumAddAndRemoveAbility has copy, store { + │ ^^^^^^^^^^^^^^^^^^^^^^^ Mismatched abilities: missing 'drop', unexpected 'store' + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's abilities for enum 'EnumAddAndRemoveAbility' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:23:9 + │ +21 │ public enum EnumAddVariant { + │ -------------- Enum definition +22 │ A, +23 │ B, // added + │ ^ New unexpected variant 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumAddVariant' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:26:17 + │ +26 │ public enum EnumRemoveVariant { + │ ^^^^^^^^^^^^^^^^^ Missing variant 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variant 'B' for enum 'EnumRemoveVariant' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:33:9 + │ +31 │ public enum EnumChangeVariant { + │ ----------------- Enum definition +32 │ A, +33 │ C, // changed from B + │ ^ Mismatched variant name 'C', expected 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeVariant' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:38:9 + │ +36 │ public enum EnumChangeAndAddVariant { + │ ----------------------- Enum definition +37 │ A, +38 │ C, // to be changed to C + │ ^ Mismatched variant name 'C', expected 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeAndAddVariant' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:38:9 + │ +36 │ public enum EnumChangeAndAddVariant { + │ ----------------------- Enum definition +37 │ A, +38 │ C, // to be changed to C + │ ^ New unexpected variant 'C'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeAndAddVariant' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:39:9 + │ +36 │ public enum EnumChangeAndAddVariant { + │ ----------------------- Enum definition + · +39 │ D // added + │ ^ New unexpected variant 'D'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeAndAddVariant' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:42:17 + │ +42 │ public enum EnumChangeAndRemoveVariant { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing variant 'C'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variant 'C' for enum 'EnumChangeAndRemoveVariant' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:44:9 + │ +42 │ public enum EnumChangeAndRemoveVariant { + │ -------------------------- Enum definition +43 │ A, +44 │ C, // changed to C + │ ^ Mismatched variant name 'C', expected 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeAndRemoveVariant' including the ordering. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:49:17 + │ +49 │ public enum EnumAddAbilityWithTypes has copy, drop { // drop added + │ ^^^^^^^^^^^^^^^^^^^^^^^ Unexpected abilities 'drop' + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's abilities for enum 'EnumAddAbilityWithTypes' including the ordering. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:53:17 + │ +53 │ public enum EnumRemoveAbilityWithTypes has copy { // drop removed + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing abilities 'drop' + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's abilities for enum 'EnumRemoveAbilityWithTypes' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:59:9 + │ +57 │ public enum EnumAddVariantWithTypes { + │ ----------------------- Enum definition +58 │ A { a: u8 }, +59 │ B { b: u8 }, // added + │ ^^^^^^^^^^^ New unexpected variant 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumAddVariantWithTypes' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:62:17 + │ +62 │ public enum EnumRemoveVariantWithTypes { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^ Missing variant 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variant 'B' for enum 'EnumRemoveVariantWithTypes' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:69:9 + │ +67 │ public enum EnumChangeVariantWithTypes { + │ -------------------------- Enum definition +68 │ A { a: u8 }, +69 │ C { b: u8 }, // changed to C + │ ^^^^^^^^^^^ Mismatched variant name 'C', expected 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeVariantWithTypes' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:74:9 + │ +72 │ public enum EnumChangeAndAddVariantWithTypes { + │ -------------------------------- Enum definition +73 │ A { a: u8 }, +74 │ C { b: u8 }, // to be changed to C + │ ^^^^^^^^^^^ Mismatched variant name 'C', expected 'B'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeAndAddVariantWithTypes' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:74:9 + │ +72 │ public enum EnumChangeAndAddVariantWithTypes { + │ -------------------------------- Enum definition +73 │ A { a: u8 }, +74 │ C { b: u8 }, // to be changed to C + │ ^^^^^^^^^^^ New unexpected variant 'C'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeAndAddVariantWithTypes' including the ordering. + +error[Compatibility E03001]: variant mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:75:9 + │ +72 │ public enum EnumChangeAndAddVariantWithTypes { + │ -------------------------------- Enum definition + · +75 │ D { d: u8 }, // added + │ ^^^^^^^^^^^ New unexpected variant 'D'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangeAndAddVariantWithTypes' including the ordering. + + +Upgrade failed, this package requires changes to be compatible with the existing package. Its upgrade policy is set to 'Compatible'. diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__function.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__function.snap new file mode 100644 index 0000000000000..89ec2e2832a4f --- /dev/null +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__function.snap @@ -0,0 +1,60 @@ +--- +source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs +expression: normalize_path(err.to_string()) +--- +error[Compatibility E04001]: function signature mismatch + ┌─ /fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:9:38 + │ +9 │ public fun func_with_wrong_param(a: u32): u64 { + │ ^ Unexpected parameter u32, expected u64 + │ + = Functions are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original function's parameters for function 'func_with_wrong_param'. + +error[Compatibility E04001]: function signature mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:14:42 + │ +14 │ public fun func_with_wrong_return(): u32 { + │ ^^^ Unexpected return type u32, expected u64 + │ + = Functions are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original function's return types for function 'func_with_wrong_return'. + +error[Compatibility E04001]: function signature mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:19:49 + │ +19 │ public fun func_with_wrong_param_and_return(a: u32): u32 { + │ ^ Unexpected parameter u32, expected u64 + │ + = Functions are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original function's parameters for function 'func_with_wrong_param_and_return'. + +error[Compatibility E04001]: function signature mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:19:58 + │ +19 │ public fun func_with_wrong_param_and_return(a: u32): u32 { + │ ^^^ Unexpected return type u32, expected u64 + │ + = Functions are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original function's return types for function 'func_with_wrong_param_and_return'. + +error[Compatibility E04001]: function signature mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:24:16 + │ +24 │ public fun func_with_wrong_param_length(a: u64): u64 { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected 2 parameters, have 1 + │ + = Functions are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original function's parameters for function 'func_with_wrong_param_length', expected 2 parameters. + +error[Compatibility E04001]: function signature mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:29:16 + │ +29 │ public fun func_with_wrong_return_length(): u64 { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected to have 2 return type(s), have 1 + │ + = Functions are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original function's return types for function 'func_with_wrong_return_length'. + + +Upgrade failed, this package requires changes to be compatible with the existing package. Its upgrade policy is set to 'Compatible'. diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct.snap new file mode 100644 index 0000000000000..222c473df1075 --- /dev/null +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct.snap @@ -0,0 +1,93 @@ +--- +source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs +expression: normalize_path(err.to_string()) +--- +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:10:19 + │ +10 │ public struct AddExtraAbility has copy {} // added copy + │ ^^^^^^^^^^^^^^^ Unexpected abilities 'copy' + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's abilities for struct 'AddExtraAbility'. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:11:19 + │ +11 │ public struct RemoveAbility has drop {} // removed copy + │ ^^^^^^^^^^^^^ Missing abilities 'copy' + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's abilities for struct 'RemoveAbility'. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:12:19 + │ +12 │ public struct AddAndRemoveAbility has drop, store {} // remove copy, add store + │ ^^^^^^^^^^^^^^^^^^^ Mismatched abilities: missing 'copy', unexpected 'store' + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's abilities for struct 'AddAndRemoveAbility'. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:13:19 + │ +13 │ public struct RemoveMultipleAbilities has drop {} // remove copy, store + │ ^^^^^^^^^^^^^^^^^^^^^^^ Missing abilities 'copy' and 'store' + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's abilities for struct 'RemoveMultipleAbilities'. + +error[Compatibility E01003]: ability mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:14:19 + │ +14 │ public struct AddMultipleAbilities has drop, copy {} + │ ^^^^^^^^^^^^^^^^^^^^ Unexpected abilities 'copy' and 'drop' + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's abilities for struct 'AddMultipleAbilities'. + +error[Compatibility E01002]: type mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:17:19 + │ +17 │ public struct AddField { + │ ^^^^^^^^ Incorrect number of fields: expected 1, found 2 + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's fields for struct 'AddField' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:22:19 + │ +22 │ public struct RemoveField { + │ ^^^^^^^^^^^ Incorrect number of fields: expected 2, found 1 + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's fields for struct 'RemoveField' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:30:9 + │ +28 │ public struct ChangeFieldName { + │ --------------- Struct definition +29 │ a: u64, +30 │ c: u64, // changed from b to c + │ ^ Mismatched field name 'c', expected 'b'. + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's fields for struct 'ChangeFieldName' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:36:9 + │ +34 │ public struct ChangeFieldType { + │ --------------- Struct definition +35 │ a: u64, +36 │ b: u32, // changed to u32 + │ ^ Mismatched field type 'u32', expected 'u64'. + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's fields for struct 'ChangeFieldType' including the ordering. + + +Upgrade failed, this package requires changes to be compatible with the existing package. Its upgrade policy is set to 'Compatible'. diff --git a/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs b/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs index 8e6d002c712e0..513ec9eda87c4 100644 --- a/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs +++ b/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs @@ -19,7 +19,37 @@ fn test_all_fail() { #[test] fn test_declarations_missing() { - let (pkg_v1, pkg_v2) = get_packages("declarations_missing"); + let (pkg_v1, pkg_v2) = get_packages("declaration_errors"); + let result = compare_packages(pkg_v1, pkg_v2); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_snapshot!(normalize_path(err.to_string())); +} + +#[test] +fn test_function() { + let (pkg_v1, pkg_v2) = get_packages("function_errors"); + let result = compare_packages(pkg_v1, pkg_v2); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_snapshot!(normalize_path(err.to_string())); +} + +#[test] +fn test_struct() { + let (pkg_v1, pkg_v2) = get_packages("struct_errors"); + let result = compare_packages(pkg_v1, pkg_v2); + + assert!(result.is_err()); + let err = result.unwrap_err(); + assert_snapshot!(normalize_path(err.to_string())); +} + +#[test] +fn test_enum() { + let (pkg_v1, pkg_v2) = get_packages("enum_errors"); let result = compare_packages(pkg_v1, pkg_v2); assert!(result.is_err()); diff --git a/crates/sui/src/upgrade_compatibility.rs b/crates/sui/src/upgrade_compatibility.rs index 96d5a64d01947..f5d3db5670ab6 100644 --- a/crates/sui/src/upgrade_compatibility.rs +++ b/crates/sui/src/upgrade_compatibility.rs @@ -5,13 +5,14 @@ #[cfg(test)] mod upgrade_compatibility_tests; -use std::collections::{HashMap, HashSet}; +use anyhow::{anyhow, Context, Error}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::fs; -use std::io::{stdout, IsTerminal}; use std::sync::Arc; -use anyhow::{anyhow, Context, Error}; - +use move_binary_format::file_format::{ + AbilitySet, EnumDefinitionIndex, FunctionDefinitionIndex, StructDefinitionIndex, TableIndex, +}; use move_binary_format::{ compatibility::Compatibility, compatibility_mode::CompatibilityMode, @@ -22,7 +23,6 @@ use move_binary_format::{ use move_command_line_common::files::FileHash; use move_compiler::diagnostics::codes::DiagnosticInfo; use move_compiler::{ - diag, diagnostics::{ codes::{custom, Severity}, report_diagnostics_to_buffer, Diagnostic, Diagnostics, @@ -360,6 +360,55 @@ impl CompatibilityMode for CliCompatibilityMode { } } +struct IdentifierTableLookup { + struct_identifier_to_index: BTreeMap, + enum_identifier_to_index: BTreeMap, + function_identifier_to_index: BTreeMap, +} + +fn table_index(compiled_module: &CompiledModule) -> IdentifierTableLookup { + // for each in compiled module + let struct_identifier_to_index: BTreeMap = compiled_module + .struct_defs() + .iter() + .enumerate() + .map(|(i, d)| { + // get the identifier of the struct + let s_id = compiled_module + .identifier_at(compiled_module.datatype_handle_at(d.struct_handle).name); + (s_id.to_owned(), i as TableIndex) + }) + .collect(); + + let enum_identifier_to_index: BTreeMap = compiled_module + .enum_defs() + .iter() + .enumerate() + .map(|(i, d)| { + let e_id = compiled_module + .identifier_at(compiled_module.datatype_handle_at(d.enum_handle).name); + (e_id.to_owned(), i as TableIndex) + }) + .collect(); + + let function_identifier_to_index: BTreeMap = compiled_module + .function_defs() + .iter() + .enumerate() + .map(|(i, d)| { + let f_id = + compiled_module.identifier_at(compiled_module.function_handle_at(d.function).name); + (f_id.to_owned(), i as TableIndex) + }) + .collect(); + + IdentifierTableLookup { + struct_identifier_to_index, + enum_identifier_to_index, + function_identifier_to_index, + } +} + const COMPATIBILITY_PREFIX: &str = "Compatibility "; /// Generates an enum Category along with individual enum for each individual category /// and impls into diagnostic info for each category. @@ -384,7 +433,7 @@ macro_rules! upgrade_codes { $($code,)* } - // impl into diagnostic info + #[allow(clippy::from_over_into)] impl Into for $cat { fn into(self) -> DiagnosticInfo { match self { @@ -405,9 +454,22 @@ macro_rules! upgrade_codes { } // Used to generate diagnostics primary labels for upgrade compatibility errors. +// WARNING: you should add new codes to the END of each category to avoid breaking the existing codes. +// adding into the middle of a list will change the error code numbers "error[Compatibility EXXXXX]" +// similarly new categories should be added to the end of the list. upgrade_codes!( Declarations: [ PublicMissing: { msg: "missing public declaration" }, + TypeMismatch: { msg: "type mismatch" }, + AbilityMismatch: { msg: "ability mismatch" }, + FieldMismatch: { msg: "field mismatch" }, + ], + Structs: [], + Enums: [ + VariantMismatch: { msg: "variant mismatch" }, + ], + Function_: [ + SignatureMismatch: { msg: "function signature mismatch" }, ], ); @@ -456,6 +518,11 @@ fn compare_packages( .map(|m| (m.self_id().name().to_owned(), m.clone())) .collect(); + let lookup: HashMap = existing_modules + .iter() + .map(|m| (m.self_id().name().to_owned(), table_index(m))) + .collect(); + let errors: Vec<(Identifier, UpgradeCompatibilityModeError)> = existing_modules .iter() .flat_map(|existing_module| { @@ -513,15 +580,26 @@ fn compare_packages( ), ); - file_set.insert(compiled_unit_with_source.source_path.clone()); + file_set.insert(&compiled_unit_with_source.source_path); } - diags.add(diag_from_error(&err, compiled_unit_with_source)) + diags.extend(diag_from_error( + &err, + compiled_unit_with_source, + &lookup[&name], + )?); } + // use colors but inline Err(anyhow!( - "{}\nUpgrade failed, this package requires changes to be compatible with the existing package. Its upgrade policy is set to 'Compatible'.", - String::from_utf8(report_diagnostics_to_buffer(&files.into(), diags, stdout().is_terminal())).context("Unable to convert buffer to string")? + "{}\nUpgrade failed, this package requires changes to be compatible with the existing package. \ + Its upgrade policy is set to 'Compatible'.", + String::from_utf8(report_diagnostics_to_buffer( + &files.into(), + diags, + use_colors() + )) + .context("Unable to convert buffer to string")? )) } @@ -529,69 +607,796 @@ fn compare_packages( fn diag_from_error( error: &UpgradeCompatibilityModeError, compiled_unit_with_source: &CompiledUnitWithSource, -) -> Diagnostic { + lookup: &IdentifierTableLookup, +) -> Result { match error { - UpgradeCompatibilityModeError::StructMissing { name, .. } => missing_definition_diag( - Declarations::PublicMissing, - "struct", - &name, + UpgradeCompatibilityModeError::StructMissing { name, .. } => { + missing_definition_diag("struct", name, compiled_unit_with_source) + } + UpgradeCompatibilityModeError::StructAbilityMismatch { + name, + old_struct, + new_struct, + } => struct_ability_mismatch_diag( + name, + old_struct, + new_struct, compiled_unit_with_source, + lookup, ), - UpgradeCompatibilityModeError::EnumMissing { name, .. } => missing_definition_diag( - Declarations::PublicMissing, - "enum", - &name, + UpgradeCompatibilityModeError::StructFieldMismatch { + name, + old_struct, + new_struct, + } => struct_field_mismatch_diag( + name, + old_struct, + new_struct, compiled_unit_with_source, + lookup, ), + UpgradeCompatibilityModeError::EnumMissing { name, .. } => { + missing_definition_diag("enum", name, compiled_unit_with_source) + } + UpgradeCompatibilityModeError::EnumAbilityMismatch { + name, + old_enum, + new_enum, + } => { + enum_ability_mismatch_diag(name, old_enum, new_enum, compiled_unit_with_source, lookup) + } + UpgradeCompatibilityModeError::EnumNewVariant { + name, + old_enum, + new_enum, + } => enum_new_variant_diag( + name, + old_enum, + new_enum, + // *tag, + compiled_unit_with_source, + lookup, + ), + UpgradeCompatibilityModeError::EnumVariantMissing { + name, + tag, + old_enum, + } => enum_variant_missing_diag(name, old_enum, *tag, compiled_unit_with_source, lookup), + UpgradeCompatibilityModeError::EnumVariantMismatch { + name, + old_enum, + new_enum, + .. + } => { + enum_variant_mismatch_diag(name, old_enum, new_enum, compiled_unit_with_source, lookup) + } UpgradeCompatibilityModeError::FunctionMissingPublic { name, .. } => { - missing_definition_diag( - Declarations::PublicMissing, - "public function", - &name, - compiled_unit_with_source, - ) + missing_definition_diag("public function", name, compiled_unit_with_source) } UpgradeCompatibilityModeError::FunctionMissingEntry { name, .. } => { - missing_definition_diag( - Declarations::PublicMissing, - "entry function", - &name, - compiled_unit_with_source, - ) + missing_definition_diag("entry function", name, compiled_unit_with_source) } + UpgradeCompatibilityModeError::FunctionSignatureMismatch { + name, + old_function, + new_function, + } => function_signature_mismatch_diag( + name, + old_function, + new_function, + compiled_unit_with_source, + lookup, + ), _ => todo!("Implement diag_from_error for {:?}", error), } } /// Return a diagnostic for a missing definition. fn missing_definition_diag( - error: impl Into, declaration_kind: &str, identifier_name: &Identifier, compiled_unit_with_source: &CompiledUnitWithSource, -) -> Diagnostic { +) -> Result { + let mut diags = Diagnostics::new(); + let module_name = compiled_unit_with_source.unit.name; let loc = compiled_unit_with_source .unit .source_map .definition_location; - Diagnostic::new( - error, - (loc, format!( - "{declaration_kind} '{identifier_name}' is missing", - declaration_kind = declaration_kind, - identifier_name = identifier_name, - )), + diags.add(Diagnostic::new( + Declarations::PublicMissing, + ( + loc, + format!( + "{declaration_kind} '{identifier_name}' is missing", + declaration_kind = declaration_kind, + identifier_name = identifier_name, + ), + ), std::iter::empty::<(Loc, String)>(), - vec![format!( - "{declaration_kind} is missing expected {declaration_kind} '{identifier_name}', but found none", + vec![ + format!( + "{declaration_kind} is missing expected {declaration_kind} '{identifier_name}', \ + but found none", + ), + format!( + "{declaration_kind}s are part of a module's public interface \ + and cannot be removed or changed during an upgrade.", + ), + format!( + "add missing {declaration_kind} '{identifier_name}' \ + back to the module '{module_name}'.", + ), + ], + )); + + Ok(diags) +} + +/// Return a diagnostic for a function signature mismatch. +/// start by checking the lengths of the parameters and returns and return a diagnostic if they are different +/// if the lengths are the same check each parameter piece wise and return a diagnostic for each mismatch +fn function_signature_mismatch_diag( + function_name: &Identifier, + old_function: &Function, + new_function: &Function, + compiled_unit_with_source: &CompiledUnitWithSource, + lookup: &IdentifierTableLookup, +) -> Result { + let mut diags = Diagnostics::new(); + + let old_func_index = lookup + .function_identifier_to_index + .get(function_name) + .context("Unable to get function index")?; + + let new_func_sourcemap = compiled_unit_with_source + .unit + .source_map + .get_function_source_map(FunctionDefinitionIndex::new(*old_func_index)) + .context("Unable to get function source map")?; + + let def_loc = new_func_sourcemap.definition_location; + + // handle function arguments + if old_function.parameters.len() != new_function.parameters.len() { + diags.add(Diagnostic::new( + Function_::SignatureMismatch, + ( + def_loc, + format!( + "Expected {} parameters, have {}", + old_function.parameters.len(), + new_function.parameters.len() + ), + ), + Vec::<(Loc, String)>::new(), + vec![ + "Functions are part of a module's public interface and cannot be \ + changed during an upgrade." + .to_string(), + format!( + "Restore the original function's parameters for \ + function '{function_name}', expected {} parameters.", + old_function.parameters.len() + ), + ], + )); + } else if old_function.parameters != new_function.parameters { + for ((i, old_param), new_param) in old_function + .parameters + .iter() + .enumerate() + .zip(new_function.parameters.iter()) + { + if old_param != new_param { + let param_loc = new_func_sourcemap + .parameters + .get(i) + .context("Unable to get parameter location")? + .1; + + diags.add(Diagnostic::new( + Function_::SignatureMismatch, + ( + param_loc, + format!("Unexpected parameter {new_param}, expected {old_param}"), + ), + Vec::<(Loc, String)>::new(), + vec![ + "Functions are part of a module's public interface \ + and cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original function's parameters \ + for function '{function_name}'." + ), + ], + )); + } + } + } + + // handle return + if old_function.return_.len() != new_function.return_.len() { + diags.add(Diagnostic::new( + Function_::SignatureMismatch, + ( + def_loc, + format!( + "Expected to have {} return type(s), have {}", + old_function.return_.len(), + new_function.return_.len() + ), + ), + Vec::<(Loc, String)>::new(), + vec![ + "Functions are part of a module's public interface \ + and cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original function's return types \ + for function '{function_name}'." + ), + ], + )); + } else if old_function.return_ != new_function.return_ { + for ((i, old_return), new_return) in old_function + .return_ + .iter() + .enumerate() + .zip(new_function.return_.iter()) + { + let return_ = new_func_sourcemap + .returns + .get(i) + .context("Unable to get return location")?; + + if old_return != new_return { + diags.add(Diagnostic::new( + Function_::SignatureMismatch, + ( + *return_, + if new_function.return_.len() == 1 { + format!( + "Unexpected return type {new_return}, \ + expected {old_return}" + ) + } else { + format!( + "Unexpected return type {new_return} at \ + position {i}, expected {old_return}" + ) + }, + ), + Vec::<(Loc, String)>::new(), + vec![ + "Functions are part of a module's public interface \ + and cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original function's return \ + types for function '{function_name}'." + ), + ], + )); + } + } + } + + Ok(diags) +} + +fn struct_ability_mismatch_diag( + struct_name: &Identifier, + old_struct: &Struct, + new_struct: &Struct, + compiled_unit_with_source: &CompiledUnitWithSource, + lookup: &IdentifierTableLookup, +) -> Result { + let mut diags = Diagnostics::new(); + + let old_struct_index = lookup + .struct_identifier_to_index + .get(struct_name) + .context("Unable to get struct index")?; + + let struct_sourcemap = compiled_unit_with_source + .unit + .source_map + .get_struct_source_map(StructDefinitionIndex::new(*old_struct_index)) + .context("Unable to get struct source map")?; + + let def_loc = struct_sourcemap.definition_location; + + if old_struct.abilities != new_struct.abilities { + let missing_abilities = + AbilitySet::from_u8(old_struct.abilities.into_u8() & !new_struct.abilities.into_u8()) + .context("Unable to get missing abilities")?; + let extra_abilities = + AbilitySet::from_u8(new_struct.abilities.into_u8() & !old_struct.abilities.into_u8()) + .context("Unable to get extra abilities")?; + + let label = match ( + missing_abilities != AbilitySet::EMPTY, + extra_abilities != AbilitySet::EMPTY, + ) { + (true, true) => format!( + "Mismatched abilities: missing {}, unexpected {}", + format_list( + missing_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ), + format_list( + extra_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ), + ), + (true, false) => format!( + "Missing abilities {}", + format_list( + missing_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ) + ), + (false, true) => format!( + "Unexpected abilities {}", + format_list( + extra_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ) + ), + (false, false) => unreachable!("Abilities should not be the same"), + }; + + diags.add(Diagnostic::new( + Declarations::AbilityMismatch, + (def_loc, label), + Vec::<(Loc, String)>::new(), + vec![ + "Structs are part of a module's public interface and \ + cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original struct's abilities \ + for struct '{struct_name}'." + ), + ], + )); + } + + Ok(diags) +} + +fn struct_field_mismatch_diag( + struct_name: &Identifier, + old_struct: &Struct, + new_struct: &Struct, + compiled_unit_with_source: &CompiledUnitWithSource, + lookup: &IdentifierTableLookup, +) -> Result { + let mut diags = Diagnostics::new(); + + let old_struct_index = lookup + .struct_identifier_to_index + .get(struct_name) + .context("Unable to get struct index")?; + + let struct_sourcemap = compiled_unit_with_source + .unit + .source_map + .get_struct_source_map(StructDefinitionIndex::new(*old_struct_index)) + .context("Unable to get struct source map")?; + + let def_loc = struct_sourcemap.definition_location; + + if old_struct.fields.len() != new_struct.fields.len() { + diags.add(Diagnostic::new( + Declarations::TypeMismatch, + ( + def_loc, + format!( + "Incorrect number of fields: expected {}, found {}", + old_struct.fields.len(), + new_struct.fields.len() + ), + ), + Vec::<(Loc, String)>::new(), + vec![ + "Structs are part of a module's public interface and \ + cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original struct's fields \ + for struct '{struct_name}' including the ordering." + ), + ], + )); + } else if old_struct.fields != new_struct.fields { + for (i, (old_field, new_field)) in old_struct + .fields + .iter() + .zip(new_struct.fields.iter()) + .enumerate() + { + if old_field != new_field { + let field_loc = struct_sourcemap + .fields + .get(i) + .context("Unable to get field location")?; + + let (code, label) = match ( + old_field.name != new_field.name, + old_field.type_ != new_field.type_, + ) { + (true, true) => ( + Declarations::FieldMismatch, + format!( + "Mismatched field '{}: {}' expected '{}: {}'.", + new_field.name, new_field.type_, old_field.name, old_field.type_ + ), + ), + (true, false) => ( + Declarations::FieldMismatch, + format!( + "Mismatched field name '{}', expected '{}'.", + new_field.name, old_field.name + ), + ), + (false, true) => ( + Declarations::TypeMismatch, + format!( + "Mismatched field type '{}', expected '{}'.", + new_field.type_, old_field.type_ + ), + ), + (false, false) => unreachable!("Fields should no be the same"), + }; + + diags.add(Diagnostic::new( + code, + (*field_loc, label), + vec![(def_loc, "Struct definition".to_string())], + vec![ + "Structs are part of a module's public interface \ + and cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original struct's fields for \ + struct '{struct_name}' including the ordering." + ), + ], + )); + } + } + } + + Ok(diags) +} + +fn enum_ability_mismatch_diag( + enum_name: &Identifier, + old_enum: &Enum, + new_enum: &Enum, + compiled_unit_with_source: &CompiledUnitWithSource, + lookup: &IdentifierTableLookup, +) -> Result { + let mut diags = Diagnostics::new(); + + let old_enum_index = lookup + .enum_identifier_to_index + .get(enum_name) + .context("Unable to get enum index")?; + + let enum_sourcemap = compiled_unit_with_source + .unit + .source_map + .get_enum_source_map(EnumDefinitionIndex::new(*old_enum_index)) + .context("Unable to get enum source map")?; + + let def_loc = enum_sourcemap.definition_location; + + if old_enum.abilities != new_enum.abilities { + let missing_abilities = + AbilitySet::from_u8(old_enum.abilities.into_u8() & !new_enum.abilities.into_u8()) + .context("Unable to get missing abilities")?; + let extra_abilities = + AbilitySet::from_u8(new_enum.abilities.into_u8() & !old_enum.abilities.into_u8()) + .context("Unable to get extra abilities")?; + + let label = match ( + missing_abilities != AbilitySet::EMPTY, + extra_abilities != AbilitySet::EMPTY, + ) { + (true, true) => format!( + "Mismatched abilities: missing {}, unexpected {}", + format_list( + missing_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ), + format_list( + extra_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ), + ), + (true, false) => format!( + "Missing abilities {}", + format_list( + missing_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ), + ), + (false, true) => format!( + "Unexpected abilities {}", + format_list( + extra_abilities + .into_iter() + .map(|a| format!("'{:?}'", a).to_lowercase()) + ), + ), + (false, false) => unreachable!("Abilities should not be the same"), + }; + + diags.add(Diagnostic::new( + Declarations::AbilityMismatch, + (def_loc, label), + Vec::<(Loc, String)>::new(), + vec![ + "Enums are part of a module's public interface \ + and cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original enum's abilities \ + for enum '{enum_name}' including the ordering." + ), + ], + )); + } + Ok(diags) +} + +fn enum_variant_mismatch_diag( + enum_name: &Identifier, + old_enum: &Enum, + new_enum: &Enum, + compiled_unit_with_source: &CompiledUnitWithSource, + lookup: &IdentifierTableLookup, +) -> Result { + let mut diags = Diagnostics::new(); + + let enum_index = lookup + .enum_identifier_to_index + .get(enum_name) + .context("Unable to get enum index")?; + + let enum_sourcemap = compiled_unit_with_source + .unit + .source_map + .get_enum_source_map(EnumDefinitionIndex::new(*enum_index)) + .context("Unable to get enum source map")?; + + let def_loc = enum_sourcemap.definition_location; + + for (i, (old_variant, new_variant)) in old_enum + .variants + .iter() + .zip(new_enum.variants.iter()) + .enumerate() + { + if old_variant != new_variant { + let variant_loc = enum_sourcemap + .variants + .get(i) + .context("Unable to get variant location")? + .0 + .1; + + let (code, label): (DiagnosticInfo, String) = match ( + old_variant.name != new_variant.name, + old_variant.fields != new_variant.fields, + ) { + (true, true) => ( + Enums::VariantMismatch.into(), + format!( + "Mismatched variant '{}', expected '{}'.", + new_variant.name, old_variant.name + ), + ), + (true, false) => ( + Enums::VariantMismatch.into(), + format!( + "Mismatched variant name '{}', expected '{}'.", + new_variant.name, old_variant.name + ), + ), + (false, true) => { + let new_variant_fields = new_variant + .fields + .iter() + .map(|f| format!("{:?}", f)) + .collect::>() + .join(", "); + + let old_variant_fields = old_variant + .fields + .iter() + .map(|f| format!("{:?}", f)) + .collect::>() + .join(", "); + + ( + Declarations::FieldMismatch.into(), + format!( + "Mismatched variant field '{}', expected '{}'.", + new_variant_fields, old_variant_fields + ), + ) + } + (false, false) => unreachable!("Variants should not be the same"), + }; + + diags.add(Diagnostic::new( + code, + (variant_loc, label), + vec![(def_loc, "Enum definition".to_string())], + vec![ + "Enums are part of a module's public interface \ + and cannot be changed during an upgrade." + .to_string(), + format!( + "Restore the original enum's variants for \ + enum '{enum_name}' including the ordering." + ), + ], + )); + } + } + + Ok(diags) +} + +fn enum_new_variant_diag( + enum_name: &Identifier, + old_enum: &Enum, + new_enum: &Enum, + compiled_unit_with_source: &CompiledUnitWithSource, + lookup: &IdentifierTableLookup, +) -> Result { + let mut diags = Diagnostics::new(); + + let enum_index = lookup + .enum_identifier_to_index + .get(enum_name) + .context("Unable to get enum index")?; + + let enum_sourcemap = compiled_unit_with_source + .unit + .source_map + .get_enum_source_map(EnumDefinitionIndex::new(*enum_index)) + .context("Unable to get enum source map")?; + + let old_enum_map = old_enum + .variants + .iter() + .map(|v| &v.name) + .collect::>(); + + let def_loc = enum_sourcemap.definition_location; + + for (i, new_variant) in new_enum.variants.iter().enumerate() { + if !old_enum_map.contains(&new_variant.name) { + let variant_loc = enum_sourcemap + .variants + .get(i) + .context("Unable to get variant location")? + .0 + .1; + + diags.add(Diagnostic::new( + Enums::VariantMismatch, + ( + variant_loc, + format!("New unexpected variant '{}'.", new_variant.name), + ), + vec![(def_loc, "Enum definition".to_string())], + vec![ + "Enums are part of a module's public interface and cannot be \ + changed during an upgrade." + .to_string(), + format!( + "Restore the original enum's variants for enum \ + '{enum_name}' including the ordering." + ), + ], + )) + } + } + + Ok(diags) +} + +fn enum_variant_missing_diag( + enum_name: &Identifier, + old_enum: &Enum, + tag: usize, + compiled_unit_with_source: &CompiledUnitWithSource, + lookup: &IdentifierTableLookup, +) -> Result { + let mut diags = Diagnostics::new(); + + let enum_index = lookup + .enum_identifier_to_index + .get(enum_name) + .context("Unable to get enum index")?; + + let enum_sourcemap = compiled_unit_with_source + .unit + .source_map + .get_enum_source_map(EnumDefinitionIndex::new(*enum_index)) + .context("Unable to get enum source map")?; + + let variant_name = &old_enum + .variants + .get(tag) + .context("Unable to get variant")? + .name; + + diags.add(Diagnostic::new( + Enums::VariantMismatch, + ( + enum_sourcemap.definition_location, + format!("Missing variant '{variant_name}'.",), ), - format!( - "{declaration_kind}s are part of a module's public interface and cannot be removed or changed during an upgrade", - ), - format!( - "add missing {declaration_kind} '{identifier_name}' back to the module '{module_name}'.", - )] - ) + Vec::<(Loc, String)>::new(), + vec![ + "Enums are part of a module's public interface and cannot \ + be changed during an upgrade." + .to_string(), + format!( + "Restore the original enum's variant '{variant_name}' for enum \ + '{enum_name}' including the ordering." + ), + ], + )); + + Ok(diags) +} + +// TODO does this exist somewhere? +fn format_list(items: impl IntoIterator) -> String { + let items: Vec<_> = items.into_iter().map(|i| i.to_string()).collect(); + match items.len() { + 0 => String::new(), + 1 => items[0].to_string(), + 2 => format!("{} and {}", items[0], items[1]), + _ => { + let all_but_last = &items[..items.len() - 1].join(", "); + let last = items.last().unwrap(); + format!("{}, and {}", all_but_last, last) + } + } +} + +/// Helper function to determine if colors should be used in the output. +/// disables colors in tests +fn use_colors() -> bool { + #[cfg(test)] + { + false + } + + #[cfg(not(test))] + { + use std::io::{stdout, IsTerminal}; + stdout().is_terminal() + } }