From a8b3ad3f794fd5ef1ec08e098a4c7da162a3cfe7 Mon Sep 17 00:00:00 2001 From: Jort Date: Fri, 20 Dec 2024 14:11:37 -0800 Subject: [PATCH] upgrade errors formatting positional fields, nested fields, named fields on enums (#20653) ## Description Adds tests and formatting fixes for: - positional fields on structs and enums - more cases for named fields on enums - positional struct nested in struct - positional struct nested in enum ## Test plan How did you test the new or updated feature? snapshot testing --- ## Release notes 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): - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --- .../enum_errors_v1/sources/UpgradeErrors.move | 38 ++- .../enum_errors_v2/sources/UpgradeErrors.move | 35 ++- .../sources/UpgradeErrors.move | 64 ++++- .../sources/UpgradeErrors.move | 64 ++++- ...ty__upgrade_compatibility_tests__enum.snap | 96 ++++++- ...__upgrade_compatibility_tests__struct.snap | 145 +++++++++- crates/sui/src/upgrade_compatibility.rs | 257 +++++++++++------- 7 files changed, 576 insertions(+), 123 deletions(-) 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 index 6c0c47b58b78b..f0e18832d1e33 100644 --- 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 @@ -10,11 +10,11 @@ module upgrades::upgrades { A, } - public enum EnumRemoveAbility has copy, drop { + public enum EnumRemoveAbility has copy, drop { // remove drop A, } - public enum EnumAddAndRemoveAbility has copy, drop { + public enum EnumAddAndRemoveAbility has copy, drop { // change drop to store A, } @@ -45,6 +45,7 @@ module upgrades::upgrades { C, // to be removed } + // with types public enum EnumAddAbilityWithTypes has copy { // add drop A { a: u8 }, } @@ -73,5 +74,38 @@ module upgrades::upgrades { B { b: u8 }, // to be changed to C // D { d: u8 }, to be added } + + public enum EnumChangeAndRemoveVariantWithPositionalTypes { + A(u8), + B(u8), // to be changed to C + C(u8), // to be removed + } + + public enum EnumChangePositionalType { + A, // add u8 + B(u8), // to be changed to u16 + C(u8, u8), // remove u8 + D(u8) // remove last u8 + } + + public struct ChangeFieldA { + a: u32, + } + + public struct ChangeFieldB { + b: u32, + } + + public enum EnumWithPositionalChanged { + A(ChangeFieldA), // change to ChangeFieldB + } + + public enum EnumWithNamedChanged { + A { + x: ChangeFieldA, + y: ChangeFieldA, + z: ChangeFieldA, // change to ChangeFieldB + }, + } } 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 index a6a9d2ff8bbd8..887ecdf66d2e6 100644 --- 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 @@ -14,7 +14,7 @@ module upgrades::upgrades { A, } - public enum EnumAddAndRemoveAbility has copy, store { + public enum EnumAddAndRemoveAbility has copy, store { // change drop to store A, } @@ -75,5 +75,38 @@ module upgrades::upgrades { D { d: u8 }, // added } + public enum EnumChangeAndRemoveVariantWithPositionalTypes { + A(u8), + C(u8), // changed to C + // C(u8) removed + } + + public enum EnumChangePositionalType { + A(u8), // add u8 + B(u16), // to be changed to u16 + C(u8), // removed u8 + D, // removed last u8 + } + + public struct ChangeFieldA { + a: u32, + } + + public struct ChangeFieldB { + b: u32, + } + + public enum EnumWithPositionalChanged { + A(ChangeFieldB), // changed to ChangeFieldB + } + + public enum EnumWithNamedChanged { + A { + x: ChangeFieldA, + y: ChangeFieldA, + z: ChangeFieldB, // changed to ChangeFieldB + }, + } + } 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 index eecbceeacfa24..59862adc7b9c1 100644 --- 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 @@ -14,9 +14,23 @@ module upgrades::upgrades { public struct AddMultipleAbilities {} - // field mismatch - public struct AddField {} - // remove field + // add field to empty struct + public struct AddFieldToEmpty { + // add a, + } + + // add fields + public struct AddField { + a: u32 + // b + } + + // remove field from struct with one field + public struct RemoveOnlyField { + a: u64, + } + + // remove field from struct with multiple fields public struct RemoveField { a: u64, b: u64, // remove this field @@ -33,5 +47,47 @@ module upgrades::upgrades { a: u64, b: u64, // change this field type to u32 } -} + // change field name and type + public struct ChangeFieldNameAndType { + a: u64, + b: u64, // change field name to c and type to u32 + } + + // add positional to empty positional struct + public struct EmptyPositionalAdd() // add u64 + + // struct new positional + public struct PositionalAdd(u64, u64) // add u64 + + // struct field missing + public struct PositionalRemove(u64, u64, u64) // remove u64 + + // struct field mismatch + public struct PositionalChange(u32, u32) // change second u32 to u64 + + // add named to empty positional struct + public struct PositionalAddNamed() // change to named { a: u64 } + + // change positional to named + public struct PositionalToNamed(u64) // change to named { a: u64 } + + // change positional to named and change type + public struct PositionalToNamedAndChangeType(u32) // change to named { a: u64 } + + public struct ChangeFieldA { + a: u32, + } + + public struct ChangeFieldB { + b: u32, + } + + // change positional nested struct + public struct ChangePositionalStruct (ChangeFieldA) // change to ChangeFieldB + + // change named nested struct + public struct ChangeNameNestedStruct { + a: ChangeFieldA, // change to ChangeFieldB + } +} 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 index 084bc0997bd2d..9995e75d06701 100644 --- 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 @@ -13,15 +13,27 @@ module upgrades::upgrades { public struct RemoveMultipleAbilities has drop {} // remove copy, store public struct AddMultipleAbilities has drop, copy {} - // field mismatch + + // add field to empty struct + public struct AddFieldToEmpty { + a: u64, + } + + // add field public struct AddField { a: u64, - b: u64, + b: u64, // added b } - // remove field + + // remove field from struct with one field + public struct RemoveOnlyField { + // removed a: u64, + } + + // remove field from struct with multiple fields public struct RemoveField { a: u64, - // b removed here + // removed b: u64, } // change field name @@ -35,5 +47,47 @@ module upgrades::upgrades { a: u64, b: u32, // changed to u32 } -} + // change field name and type + public struct ChangeFieldNameAndType { + a: u64, + c: u32, // changed from b to c and u64 to u32 + } + + // add positional to empty positional struct + public struct EmptyPositionalAdd(u64) // removed the u64 + + // struct new positional + public struct PositionalAdd(u64, u64, u64) // added a u64 + + // struct field missing + public struct PositionalRemove(u64, u64) // removed a u64 + + // struct field mismatch + public struct PositionalChange(u32, u64) // changed second u32 to u64 + + // add named to empty positional struct + public struct PositionalAddNamed{ a: u64 } // changed to named from empty positional + + // positional to named + public struct PositionalToNamed{ a: u64 } // changed to named from positional + + // change positional to named and change type + public struct PositionalToNamedAndChangeType{ a: u64 } // changed to named from positional and changed type to u64 + + public struct ChangeFieldA { + a: u32, + } + + public struct ChangeFieldB { + b: u32, + } + + // change positional nested struct + public struct ChangePositionalStruct (ChangeFieldB) // changed to ChangeFieldB + + // change named nested struct + public struct ChangeNameNestedStruct { + a: ChangeFieldB, // changed to ChangeFieldB + } +} 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 index 305e7a44f555e..b4db9c2859672 100644 --- 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 @@ -23,7 +23,7 @@ error[Compatibility E01003]: ability mismatch error[Compatibility E01003]: ability mismatch ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:17:17 │ -17 │ public enum EnumAddAndRemoveAbility has copy, store { +17 │ public enum EnumAddAndRemoveAbility has copy, store { // change drop to store │ ^^^^^^^^^^^^^^^^^^^^^^^ Mismatched abilities: missing 'drop', unexpected 'store' │ = Enums are part of a module's public interface and cannot be changed during an upgrade. @@ -206,5 +206,99 @@ error[Compatibility E02001]: variant mismatch = 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 E02001]: variant mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:78:17 + │ +78 │ public enum EnumChangeAndRemoveVariantWithPositionalTypes { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 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 'EnumChangeAndRemoveVariantWithPositionalTypes' including the ordering. + +error[Compatibility E02001]: variant mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:80:9 + │ +78 │ public enum EnumChangeAndRemoveVariantWithPositionalTypes { + │ --------------------------------------------- Enum definition +79 │ A(u8), +80 │ C(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 'EnumChangeAndRemoveVariantWithPositionalTypes' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:85:9 + │ +84 │ public enum EnumChangePositionalType { + │ ------------------------ Enum definition +85 │ A(u8), // add u8 + │ ^^^^^ Mismatched variant field count, expected 0, found 1. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:86:9 + │ +84 │ public enum EnumChangePositionalType { + │ ------------------------ Enum definition +85 │ A(u8), // add u8 +86 │ B(u16), // to be changed to u16 + │ ^^^^^^ Mismatched field 'u8' at position 0, expected 'u16'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:87:9 + │ +84 │ public enum EnumChangePositionalType { + │ ------------------------ Enum definition + · +87 │ C(u8), // removed u8 + │ ^^^^^ Mismatched variant field count, expected 2, found 1. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:88:9 + │ +84 │ public enum EnumChangePositionalType { + │ ------------------------ Enum definition + · +88 │ D, // removed last u8 + │ ^ Mismatched variant field count, expected 1, found 0. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:100:9 + │ + 99 │ public enum EnumWithPositionalChanged { + │ ------------------------- Enum definition +100 │ A(ChangeFieldB), // changed to ChangeFieldB + │ ^^^^^^^^^^^^^^^ Mismatched field '0x0::upgrades::ChangeFieldA' at position 0, expected '0x0::upgrades::ChangeFieldB'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variant for enum 'EnumWithPositionalChanged' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:104:9 + │ +103 │ public enum EnumWithNamedChanged { + │ -------------------- Enum definition +104 │ ╭ A { +105 │ │ x: ChangeFieldA, +106 │ │ y: ChangeFieldA, +107 │ │ z: ChangeFieldB, // changed to ChangeFieldB +108 │ │ }, + │ ╰─────────^ Mismatched field 'z: 0x0::upgrades::ChangeFieldA', expected '0x0::upgrades::ChangeFieldB'. + │ + = Enums are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original enum's variant for enum 'EnumWithNamedChanged' 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__struct.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__struct.snap index 5e42dceed67e8..e8fd48dee6562 100644 --- 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 @@ -48,46 +48,167 @@ error[Compatibility E01003]: ability mismatch = Restore the original abilities of struct 'AddMultipleAbilities': none. error[Compatibility E01002]: type mismatch - ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:17:19 + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:18:19 │ -17 │ public struct AddField { +18 │ public struct AddFieldToEmpty { + │ ^^^^^^^^^^^^^^^ Incorrect number of fields: expected 0, 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 'AddFieldToEmpty' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:23:19 + │ +23 │ 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 field for struct 'AddField' including the ordering. error[Compatibility E01002]: type mismatch - ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:22:19 + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:29:19 + │ +29 │ public struct RemoveOnlyField { + │ ^^^^^^^^^^^^^^^ Incorrect number of fields: expected 1, found 0 │ -22 │ public struct RemoveField { + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's field for struct 'RemoveOnlyField' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:34:19 + │ +34 │ 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 - ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:30:9 + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:42:9 │ -28 │ public struct ChangeFieldName { +40 │ public struct ChangeFieldName { │ --------------- Struct definition -29 │ a: u64, -30 │ c: u64, // changed from b to c +41 │ a: u64, +42 │ 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 - ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:36:9 + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:48:9 │ -34 │ public struct ChangeFieldType { +46 │ public struct ChangeFieldType { │ --------------- Struct definition -35 │ a: u64, -36 │ b: u32, // changed to u32 +47 │ a: u64, +48 │ 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. +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:54:9 + │ +52 │ public struct ChangeFieldNameAndType { + │ ---------------------- Struct definition +53 │ a: u64, +54 │ c: u32, // changed from b to c and u64 to u32 + │ ^ Mismatched field 'c: u32', expected 'b: 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 'ChangeFieldNameAndType' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:58:19 + │ +58 │ public struct EmptyPositionalAdd(u64) // removed the u64 + │ ^^^^^^^^^^^^^^^^^^ Incorrect number of fields: expected 0, 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 'EmptyPositionalAdd' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:61:19 + │ +61 │ public struct PositionalAdd(u64, u64, u64) // added a u64 + │ ^^^^^^^^^^^^^ Incorrect number of fields: expected 2, found 3 + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's fields for struct 'PositionalAdd' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:64:19 + │ +64 │ public struct PositionalRemove(u64, u64) // removed a u64 + │ ^^^^^^^^^^^^^^^^ Incorrect number of fields: expected 3, 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 'PositionalRemove' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:67:41 + │ +67 │ public struct PositionalChange(u32, u64) // changed second u32 to u64 + │ ---------------- ^^^ Mismatched field type 'u64', expected 'u32'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's fields for struct 'PositionalChange' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:70:19 + │ +70 │ public struct PositionalAddNamed{ a: u64 } // changed to named from empty positional + │ ^^^^^^^^^^^^^^^^^^ Incorrect number of fields: expected 0, 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 'PositionalAddNamed' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:73:38 + │ +73 │ public struct PositionalToNamed{ a: u64 } // changed to named from positional + │ ----------------- ^ Mismatched field name 'a', expected at position 0. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's field for struct 'PositionalToNamed' including the ordering. + +error[Compatibility E01004]: field mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:76:51 + │ +76 │ public struct PositionalToNamedAndChangeType{ a: u64 } // changed to named from positional and changed type to u64 + │ ------------------------------ ^ Mismatched field 'a: u64', expected 'u32' at position 0. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's field for struct 'PositionalToNamedAndChangeType' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:87:43 + │ +87 │ public struct ChangePositionalStruct (ChangeFieldB) // changed to ChangeFieldB + │ ---------------------- ^^^^^^^^^^^^ Mismatched field type '0x0::upgrades::ChangeFieldB', expected '0x0::upgrades::ChangeFieldA'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's field for struct 'ChangePositionalStruct' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:91:9 + │ +90 │ public struct ChangeNameNestedStruct { + │ ---------------------- Struct definition +91 │ a: ChangeFieldB, // changed to ChangeFieldB + │ ^ Mismatched field type '0x0::upgrades::ChangeFieldB', expected '0x0::upgrades::ChangeFieldA'. + │ + = Structs are part of a module's public interface and cannot be changed during an upgrade. + = Restore the original struct's field for struct 'ChangeNameNestedStruct' 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/upgrade_compatibility.rs b/crates/sui/src/upgrade_compatibility.rs index 9484188c49e1d..d275ab65cdd60 100644 --- a/crates/sui/src/upgrade_compatibility.rs +++ b/crates/sui/src/upgrade_compatibility.rs @@ -6,6 +6,7 @@ mod upgrade_compatibility_tests; use anyhow::{anyhow, Context, Error}; +use regex::Regex; use std::collections::{BTreeMap, HashMap, HashSet}; use std::fs; use std::sync::Arc; @@ -20,7 +21,7 @@ use move_binary_format::{ compatibility::Compatibility, compatibility_mode::CompatibilityMode, file_format::Visibility, - normalized::{Enum, Function, Module, Struct}, + normalized::{Enum, Field, Function, Module, Struct, Type, Variant}, CompiledModule, }; use move_bytecode_source_map::source_map::SourceName; @@ -1247,6 +1248,7 @@ fn function_entry_mismatch( Ok(diags) } +/// Return a label string for an ability mismatch. fn ability_mismatch_label( old_abilities: AbilitySet, new_abilities: AbilitySet, @@ -1352,9 +1354,62 @@ fn struct_ability_mismatch_diag( Ok(diags) } +/// Return a diagnostic for an ability mismatch. returns (full version, name, type) +fn field_to_string(field: &Field) -> (String, String, String) { + let mut field_full = format!("'{}: {}'", field.name, field.type_); + let mut field_name = format!("'{}'", field.name); + let field_type = format!("'{}'", field.type_); + + if let Some(pos_num) = Regex::new(r"^pos(\d)+$") + .ok() + .and_then(|r| r.captures(field.name.as_str())) + .and_then(|c| c.get(1)) + .and_then(|m| m.as_str().parse::().ok()) + { + field_name = format!("at position {}", pos_num); + field_full = format!("{} {}", field_type, field_name); + } + + (field_full, field_name, field_type) +} + +/// returns a message for the given field +fn field_mismatch_message(old_field: &Field, new_field: &Field) -> (Declarations, String) { + let (old_field_full, old_field_name, old_field_type) = field_to_string(old_field); + let (new_field_full, new_field_name, new_field_type) = field_to_string(new_field); + + match ( + old_field.name != new_field.name, + old_field.type_ != new_field.type_, + ) { + (true, true) => ( + Declarations::FieldMismatch, + format!( + "Mismatched field {}, expected {}.", + new_field_full, old_field_full + ), + ), + (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 not be the same"), + } +} + /// Return a diagnostic for a field mismatch -/// start by checking the lengths of the fields and return a diagnostic if they are different -/// if the lengths are the same check each field piece wise and return a diagnostic for each mismatch +/// Start by checking the lengths of the fields and return a diagnostic if they are different. +/// If the lengths are the same check each field piece wise and return a diagnostic for each mismatch. fn struct_field_mismatch_diag( struct_name: &Identifier, old_struct: &Struct, @@ -1377,15 +1432,27 @@ fn struct_field_mismatch_diag( let def_loc = struct_sourcemap.definition_location; - if old_struct.fields.len() != new_struct.fields.len() { + let old_fields: Vec<&Field> = old_struct + .fields + .iter() + .filter(|f| f.name != Identifier::new("dummy_field").unwrap() && f.type_ != Type::Bool) + .collect(); + + let new_fields: Vec<&Field> = new_struct + .fields + .iter() + .filter(|f| f.name != Identifier::new("dummy_field").unwrap() && f.type_ != Type::Bool) + .collect(); + + if old_fields.len() != new_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() + old_fields.len(), + new_fields.len() ), ), Vec::<(Loc, String)>::new(), @@ -1396,50 +1463,19 @@ fn struct_field_mismatch_diag( format!( "Restore the original struct's {} \ for struct '{struct_name}' including the ordering.", - singular_or_plural(old_struct.fields.len(), "field", "fields") + singular_or_plural(old_fields.len(), "field", "fields") ), ], )); - } 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() - { + } else if old_fields != new_fields { + for (i, (old_field, new_field)) in old_fields.iter().zip(new_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 not be the same"), - }; + let (code, label) = field_mismatch_message(old_field, new_field); diags.add(Diagnostic::new( code, @@ -1452,7 +1488,7 @@ fn struct_field_mismatch_diag( format!( "Restore the original struct's {} for \ struct '{struct_name}' including the ordering.", - singular_or_plural(old_struct.fields.len(), "field", "fields") + singular_or_plural(old_fields.len(), "field", "fields") ), ], )); @@ -1560,6 +1596,66 @@ fn enum_ability_mismatch_diag( Ok(diags) } +/// Returns the error code and label for mismatched, missing, or unexpected variant +fn enum_variant_field_error( + old_variant: &Variant, + new_variant: &Variant, + variant_loc: Loc, + def_loc: Loc, +) -> (DiagnosticInfo, Vec) { + if old_variant.fields.len() != new_variant.fields.len() { + return ( + Declarations::FieldMismatch.into(), + vec![format!( + "Mismatched variant field count, expected {}, found {}.", + old_variant.fields.len(), + new_variant.fields.len() + )], + ); + } + + match ( + old_variant.name != new_variant.name, + old_variant.fields != new_variant.fields, + ) { + (true, true) => ( + Enums::VariantMismatch.into(), + vec![format!( + "Mismatched variant '{}', expected '{}'.", + new_variant.name, old_variant.name + )], + ), + (true, false) => ( + Enums::VariantMismatch.into(), + vec![format!( + "Mismatched variant name '{}', expected '{}'.", + new_variant.name, old_variant.name + )], + ), + (false, true) => { + let mut errors: Vec = vec![]; + + for (i, (old_field, new_field)) in old_variant + .fields + .iter() + .zip(new_variant.fields.iter()) + .enumerate() + { + if old_field != new_field { + errors.push(format!( + "Mismatched field {}, expected {}.", + field_to_string(old_field).0, + field_to_string(new_field).2 + )); + } + } + + (Declarations::FieldMismatch.into(), errors) + } + (false, false) => unreachable!("Variants should not be the same"), + } +} + /// Return a diagnostic for a type parameter mismatch /// start by checking the lengths of the type parameters and return a diagnostic if they are different /// if the lengths are the same check each type parameter piece wise and return a diagnostic for each mismatch @@ -1599,65 +1695,26 @@ fn enum_variant_mismatch_diag( .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"), - }; + let (code, labels) = + enum_variant_field_error(old_variant, new_variant, variant_loc, def_loc); - 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 \ + for label in labels { + diags.add(Diagnostic::new( + code.clone(), + (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 {} for \ + .to_string(), + format!( + "Restore the original enum's {} for \ enum '{enum_name}' including the ordering.", - singular_or_plural(old_enum.variants.len(), "variant", "variants") - ), - ], - )); + singular_or_plural(old_enum.variants.len(), "variant", "variants") + ), + ], + )); + } } } @@ -1901,6 +1958,7 @@ fn type_parameter_diag( Ok(diags) } +/// Return a diagnostic for a type parameter constrant mismatch fn type_param_constraint_labels( old_constraints: AbilitySet, new_constraints: AbilitySet, @@ -1928,6 +1986,7 @@ fn type_param_constraint_labels( )) } +/// Return a diagnostic for a type parameter phantom mismatch fn type_param_phantom_labels(old_phantom: bool, new_phantom: bool) -> Option<(String, String)> { if old_phantom == new_phantom { return None; @@ -1946,6 +2005,7 @@ fn type_param_phantom_labels(old_phantom: bool, new_phantom: bool) -> Option<(St }) } +/// Format a list of items into a human-readable string. fn format_list( items: impl IntoIterator, noun_singular_plural: Option<(&str, &str)>, @@ -1972,6 +2032,7 @@ fn format_list( } } +/// Return a string with the singular or plural form of a word. fn singular_or_plural(n: usize, singular: &str, plural: &str) -> String { if n == 1 { singular.to_string()