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 f0e18832d1e33..bd127d042773b 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 @@ -85,7 +85,7 @@ module upgrades::upgrades { A, // add u8 B(u8), // to be changed to u16 C(u8, u8), // remove u8 - D(u8) // remove last u8 + D(u8) // remove u8 from last variant } public struct ChangeFieldA { 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 887ecdf66d2e6..12d2a04872009 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 @@ -83,9 +83,9 @@ module upgrades::upgrades { public enum EnumChangePositionalType { A(u8), // add u8 - B(u16), // to be changed to u16 + B(u16), // changed to u16 C(u8), // removed u8 - D, // removed last u8 + D, // removed u8 from variant } public struct ChangeFieldA { 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 59862adc7b9c1..7d62c259ddfec 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 @@ -90,4 +90,17 @@ module upgrades::upgrades { public struct ChangeNameNestedStruct { a: ChangeFieldA, // change to ChangeFieldB } + + // nested struct type param field mismatch + public struct NamedBox { x: A } + public struct NamedTwoBox { x: B, y: C } + + public struct NamedBoxInBox { x: NamedBox> } + public struct NamedBoxInTwoBox { x: NamedTwoBox, NamedBox> } + + public struct PositionalBox(G) + public struct PositionalTwoBox(H, I) + + public struct PositionalBoxInBox(PositionalBox>) + public struct PositionalBoxInTwoBox(PositionalTwoBox, PositionalBox>) } 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 9995e75d06701..9502846dcf0e4 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 @@ -90,4 +90,17 @@ module upgrades::upgrades { public struct ChangeNameNestedStruct { a: ChangeFieldB, // changed to ChangeFieldB } + + + public struct NamedBox { x: u32 } + public struct NamedTwoBox { x: u32, y: u32 } + + public struct NamedBoxInBox { x: u32 } + public struct NamedBoxInTwoBox { x: u32 } + + public struct PositionalBox(u32) + public struct PositionalTwoBox(u32, u32) + + public struct PositionalBoxInBox(u32) + public struct PositionalBoxInTwoBox(u32) } diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__additive.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__additive.snap index 6dfd7a009a152..0402b496262cf 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__additive.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__additive.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: normalize_path(err.to_string()) --- -error[Compatibility E01008]: missing declaration +error[Compatibility E01007]: missing declaration ┌─ /fixtures/upgrade_errors/additive_errors_v2/sources/UpgradeErrors.move:4:18 │ 4 │ module upgrades::upgrades { @@ -11,7 +11,7 @@ error[Compatibility E01008]: missing declaration = enums cannot be removed or changed during an 'additive' or 'dependency only' upgrade. = add missing enum 'EnumToRemove' back to the module 'upgrades'. -error[Compatibility E01008]: missing declaration +error[Compatibility E01007]: missing declaration ┌─ /fixtures/upgrade_errors/additive_errors_v2/sources/UpgradeErrors.move:4:18 │ 4 │ module upgrades::upgrades { @@ -20,7 +20,7 @@ error[Compatibility E01008]: missing declaration = functions cannot be removed or changed during an 'additive' or 'dependency only' upgrade. = add missing function 'function_to_remove' back to the module 'upgrades'. -error[Compatibility E01008]: missing declaration +error[Compatibility E01007]: missing declaration ┌─ /fixtures/upgrade_errors/additive_errors_v2/sources/UpgradeErrors.move:4:18 │ 4 │ module upgrades::upgrades { diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__addresses_first.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__addresses_first.snap index a9745ea1dd86c..9a60d301b7166 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__addresses_first.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__addresses_first.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: output --- -error[Compatibility E01007]: module missing +error[Compatibility E01006]: module missing ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/addresses_first/Move.toml:4:1 │ 4 │ ╭ [package] 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 9bfb076c465fb..eb65354488d68 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 @@ -38,7 +38,7 @@ error[Compatibility E01001]: missing public declaration = structs are part of a module's public interface and cannot be removed or changed during a 'compatible' upgrade. = add missing struct 'StructToBeRemoved' back to the module 'struct_'. -error[Compatibility E01007]: module missing +error[Compatibility E01006]: module missing ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/declaration_errors_v2/Move.toml:1:1 │ 1 │ ╭ [package] diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__emoji.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__emoji.snap index a24bfcb9eb8a9..ac2c0a7a1c9fd 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__emoji.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__emoji.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: output --- -error[Compatibility E01007]: module missing +error[Compatibility E01006]: module missing ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/emoji/Move.toml:2:2 │ 2 │ 😀[package]😀 diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__empty.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__empty.snap index 3fcbbedc9638f..1d573f94245bb 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__empty.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__empty.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: output --- -error[Compatibility E01007]: module missing +error[Compatibility E01006]: module missing ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/empty/Move.toml:1:1 │ 1 │ 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 4e5820f7e0a71..1084b0213f057 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 @@ -57,7 +57,7 @@ error[Compatibility E02001]: variant mismatch │ ----------------- Enum definition 32 │ A, 33 │ C, // changed from B - │ ^ Mismatched variant name 'C', expected 'B'. + │ ^ Mismatched variant 'C', expected 'B'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangeVariant' including the ordering. @@ -69,7 +69,7 @@ error[Compatibility E02001]: variant mismatch │ ----------------------- Enum definition 37 │ A, 38 │ C, // to be changed to C - │ ^ Mismatched variant name 'C', expected 'B'. + │ ^ Mismatched variant 'C', expected 'B'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangeAndAddVariant' including the ordering. @@ -114,7 +114,7 @@ error[Compatibility E02001]: variant mismatch │ -------------------------- Enum definition 43 │ A, 44 │ C, // changed to C - │ ^ Mismatched variant name 'C', expected 'B'. + │ ^ Mismatched variant 'C', expected 'B'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangeAndRemoveVariant' including the ordering. @@ -165,7 +165,7 @@ error[Compatibility E02001]: variant mismatch │ -------------------------- Enum definition 68 │ A { a: u8 }, 69 │ C { b: u8 }, // changed to C - │ ^^^^^^^^^^^ Mismatched variant name 'C', expected 'B'. + │ ^^^^^^^^^^^ Mismatched variant 'C', expected 'B'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangeVariantWithTypes' including the ordering. @@ -177,7 +177,7 @@ error[Compatibility E02001]: variant mismatch │ -------------------------------- Enum definition 73 │ A { a: u8 }, 74 │ C { b: u8 }, // to be changed to C - │ ^^^^^^^^^^^ Mismatched variant name 'C', expected 'B'. + │ ^^^^^^^^^^^ Mismatched variant 'C', expected 'B'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangeAndAddVariantWithTypes' including the ordering. @@ -222,7 +222,7 @@ error[Compatibility E02001]: variant mismatch │ --------------------------------------------- Enum definition 79 │ A(u8), 80 │ C(u8), // changed to C - │ ^^^^^ Mismatched variant name 'C', expected 'B'. + │ ^^^^^ Mismatched variant 'C', expected 'B'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangeAndRemoveVariantWithPositionalTypes' including the ordering. @@ -238,14 +238,14 @@ error[Compatibility E01004]: field mismatch = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering. -error[Compatibility E01004]: field mismatch +error[Compatibility E01002]: type 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'. +86 │ B(u16), // changed to u16 + │ ^^^^^^ Mismatched field type 'u16', expected 'u8'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering. @@ -268,24 +268,24 @@ error[Compatibility E01004]: field mismatch 84 │ public enum EnumChangePositionalType { │ ------------------------ Enum definition · -88 │ D, // removed last u8 +88 │ D, // removed u8 from variant │ ^ Mismatched variant field count, expected 1, found 0. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variants for enum 'EnumChangePositionalType' including the ordering. -error[Compatibility E01004]: field mismatch +error[Compatibility E01002]: type 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'. + │ ^^^^^^^^^^^^^^^ Mismatched field type '0x0::upgrades::ChangeFieldB', expected '0x0::upgrades::ChangeFieldA'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variant for enum 'EnumWithPositionalChanged' including the ordering. -error[Compatibility E01004]: field mismatch +error[Compatibility E01002]: type mismatch ┌─ /fixtures/upgrade_errors/enum_errors_v2/sources/UpgradeErrors.move:104:9 │ 103 │ public enum EnumWithNamedChanged { @@ -295,7 +295,7 @@ error[Compatibility E01004]: field mismatch 106 │ │ y: ChangeFieldA, 107 │ │ z: ChangeFieldB, // changed to ChangeFieldB 108 │ │ }, - │ ╰─────────^ Mismatched field 'z: 0x0::upgrades::ChangeFieldA', expected '0x0::upgrades::ChangeFieldB'. + │ ╰─────────^ Mismatched field type '0x0::upgrades::ChangeFieldB', expected '0x0::upgrades::ChangeFieldA'. │ = Enums are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original enum's variant for enum 'EnumWithNamedChanged' including the ordering. diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__package_no_name.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__package_no_name.snap index 03cd875ac4992..000b3a5a550ff 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__package_no_name.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__package_no_name.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: output --- -error[Compatibility E01007]: module missing +error[Compatibility E01006]: module missing ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/package_no_name/Move.toml:1:1 │ 1 │ [package] diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__starts_second_line.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__starts_second_line.snap index 9984efcd08db3..0a8fbb124048b 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__starts_second_line.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__starts_second_line.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: output --- -error[Compatibility E01007]: module missing +error[Compatibility E01006]: module missing ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/starts_second_line/Move.toml:2:1 │ 2 │ ╭ [package] 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 dabb1ac7c25e5..9dd94bfd7df8d 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 @@ -90,7 +90,7 @@ error[Compatibility E01004]: field mismatch │ --------------- Struct definition 41 │ a: u64, 42 │ c: u64, // changed from b to c - │ ^ Mismatched field name 'c', expected 'b'. + │ ^ Mismatched field 'c', expected 'b'. │ = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original struct's fields for struct 'ChangeFieldName' including the ordering. @@ -170,7 +170,7 @@ 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. + │ ----------------- ^ Mismatched field 'a', expected a positional field. │ │ │ Struct definition │ @@ -210,5 +210,115 @@ error[Compatibility E01002]: type mismatch = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. = Restore the original struct's field for struct 'ChangeNameNestedStruct' including the ordering. +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:95:33 + │ +95 │ public struct NamedBox { x: u32 } + │ -------- ^ Mismatched field type 'u32', expected 'A'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's field for struct 'NamedBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:96:39 + │ +96 │ public struct NamedTwoBox { x: u32, y: u32 } + │ ----------- ^ Mismatched field type 'u32', expected 'B'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's fields for struct 'NamedTwoBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:96:47 + │ +96 │ public struct NamedTwoBox { x: u32, y: u32 } + │ ----------- ^ Mismatched field type 'u32', expected 'C'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's fields for struct 'NamedTwoBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:98:38 + │ +98 │ public struct NamedBoxInBox { x: u32 } + │ ------------- ^ Mismatched field type 'u32', expected '0x0::upgrades::NamedBox<0x0::upgrades::NamedBox>'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's field for struct 'NamedBoxInBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:99:44 + │ +99 │ public struct NamedBoxInTwoBox { x: u32 } + │ ---------------- ^ Mismatched field type 'u32', expected '0x0::upgrades::NamedTwoBox<0x0::upgrades::NamedBox, 0x0::upgrades::NamedBox>'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's field for struct 'NamedBoxInTwoBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:101:36 + │ +101 │ public struct PositionalBox(u32) + │ ------------- ^^^ Mismatched field type 'u32', expected 'G'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's field for struct 'PositionalBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:102:42 + │ +102 │ public struct PositionalTwoBox(u32, u32) + │ ---------------- ^^^ Mismatched field type 'u32', expected 'H'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's fields for struct 'PositionalTwoBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:102:47 + │ +102 │ public struct PositionalTwoBox(u32, u32) + │ ---------------- ^^^ Mismatched field type 'u32', expected 'I'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's fields for struct 'PositionalTwoBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:104:41 + │ +104 │ public struct PositionalBoxInBox(u32) + │ ------------------ ^^^ Mismatched field type 'u32', expected '0x0::upgrades::PositionalBox<0x0::upgrades::PositionalBox>'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's field for struct 'PositionalBoxInBox' including the ordering. + +error[Compatibility E01002]: type mismatch + ┌─ /fixtures/upgrade_errors/struct_errors_v2/sources/UpgradeErrors.move:105:47 + │ +105 │ public struct PositionalBoxInTwoBox(u32) + │ --------------------- ^^^ Mismatched field type 'u32', expected '0x0::upgrades::PositionalTwoBox<0x0::upgrades::PositionalBox, 0x0::upgrades::PositionalBox>'. + │ │ + │ Struct definition + │ + = Structs are part of a module's public interface and cannot be removed or changed during an upgrade. + = Restore the original struct's field for struct 'PositionalBoxInTwoBox' 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__version_mismatch.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__version_mismatch.snap index ad31f8daf1b4f..0f1d6f9ceff1c 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__version_mismatch.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__version_mismatch.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: normalize_path(result.unwrap_err().to_string()) --- -error[Compatibility E01009]: file format version downgrade +error[Compatibility E01008]: file format version downgrade ┌─ /fixtures/upgrade_errors/deponly_errors_v2/sources/UpgradeErrors.move:4:18 │ 4 │ module upgrades::upgrades { diff --git a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__whitespace.snap b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__whitespace.snap index b76c5aa7047b6..933bf7e129589 100644 --- a/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__whitespace.snap +++ b/crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__whitespace.snap @@ -2,7 +2,7 @@ source: crates/sui/src/unit_tests/upgrade_compatibility_tests.rs expression: output --- -error[Compatibility E01007]: module missing +error[Compatibility E01006]: module missing ┌─ /Users/jordanjennings/code/sui/crates/sui/src/unit_tests/fixtures/upgrade_errors/missing_module_toml/whitespace/Move.toml:1:1 │ 1 │ ╭ diff --git a/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs b/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs index 4e79efa4b50bd..74c9f22b1d4b8 100644 --- a/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs +++ b/crates/sui/src/unit_tests/upgrade_compatibility_tests.rs @@ -1,17 +1,19 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 - -use crate::upgrade_compatibility::{compare_packages, missing_module_diag}; use insta::assert_snapshot; +use std::fs; +use std::path::PathBuf; +use std::str::FromStr; +use std::sync::Arc; + +use crate::upgrade_compatibility::{compare_packages, missing_module_diag, FormattedField}; + +use move_binary_format::normalized::{Field, Type}; use move_binary_format::CompiledModule; use move_command_line_common::files::FileHash; use move_compiler::diagnostics::report_diagnostics_to_buffer; use move_compiler::shared::files::{FileName, FilesSourceText}; use move_core_types::identifier::Identifier; -use std::fs; -use std::path::PathBuf; -use std::str::FromStr; -use std::sync::Arc; use sui_move_build::BuildConfig; use sui_move_build::CompiledPackage; use sui_types::move_package::UpgradePolicy; @@ -167,6 +169,18 @@ fn test_missing_module_toml() { } } +#[test] +fn positional_formatting() { + let name = Identifier::new("pos999").unwrap(); + let field = Field { + name, + type_: Type::Bool, + }; + + let ff = FormattedField::new(&field, &[]); + assert_eq!(format!("{}", ff), "'bool' at position 999"); +} + fn get_packages(name: &str) -> (Vec, CompiledPackage, PathBuf) { let mut path: PathBuf = PathBuf::from(env!("CARGO_MANIFEST_DIR")); path.push("src/unit_tests/fixtures/upgrade_errors/"); diff --git a/crates/sui/src/upgrade_compatibility/formatting.rs b/crates/sui/src/upgrade_compatibility/formatting.rs new file mode 100644 index 0000000000000..de7eb8f0a8599 --- /dev/null +++ b/crates/sui/src/upgrade_compatibility/formatting.rs @@ -0,0 +1,168 @@ +// Copyright (c) Mysten Labs, Inc. +// SPDX-License-Identifier: Apache-2.0 + +use anyhow::{Context, Error}; +use move_binary_format::normalized::{Field, Type}; +use move_bytecode_source_map::source_map::SourceName; +use move_core_types::identifier::Identifier; +use move_ir_types::location::Loc; +use regex::Regex; +use std::fmt; +use std::sync::LazyLock; + +pub(super) struct FormattedType<'f> { + type_: &'f Type, + type_params: &'f [SourceName], +} + +impl<'f> fmt::Display for FormattedType<'f> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + if f.alternate() { + write!( + f, + "{}", + format_param(self.type_, self.type_params.to_vec(), &mut Vec::new()) + .map_err(|_| fmt::Error)?, + ) + } else { + write!( + f, + "'{}'", + format_param(self.type_, self.type_params.to_vec(), &mut Vec::new()) + .map_err(|_| fmt::Error)?, + ) + } + } +} + +pub(super) enum FormattedIdentifier<'f> { + Positional(usize), + Named(&'f Identifier), +} + +pub(super) struct FormattedField<'f> { + pub(super) identifier: FormattedIdentifier<'f>, + pub(super) type_: FormattedType<'f>, +} + +impl<'f> FormattedField<'f> { + pub(super) fn new(f: &'f Field, type_params: &'f [SourceName]) -> Self { + static RE_POS: LazyLock = LazyLock::new(|| Regex::new(r"^pos(\d+)$").unwrap()); + let identifier = if let Some(ix) = RE_POS + .captures(f.name.as_str()) + .and_then(|c| c.get(1)) + .and_then(|m| m.as_str().parse::().ok()) + { + FormattedIdentifier::Positional(ix) + } else { + FormattedIdentifier::Named(&f.name) + }; + + FormattedField { + identifier, + type_: FormattedType { + type_: &f.type_, + type_params, + }, + } + } +} + +impl<'f> fmt::Display for FormattedField<'f> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use FormattedIdentifier as FI; + match self.identifier { + FI::Positional(_) if f.alternate() => write!(f, "a positional field"), + FI::Named(name) if f.alternate() => write!(f, "'{name}'"), + + FI::Positional(ix) => write!(f, "'{:#}' at position {ix}", self.type_), + FI::Named(name) => write!(f, "'{name}: {:#}'", self.type_), + } + } +} + +/// Returns a string representation of a parameter and updates its secondary label to include its location. +pub(super) fn format_param( + param: &Type, + type_params: Vec, + secondary: &mut Vec<(Loc, String)>, +) -> Result { + Ok(match param { + Type::TypeParameter(t) => { + let type_param = type_params + .get(*t as usize) + .context("Unable to get type param location")?; + + secondary.push(( + type_param.1, + format!("Type parameter '{}' is defined here", &type_param.0), + )); + type_param.0.to_string() + } + Type::Vector(t) => { + format!("vector<{}>", format_param(t, type_params, secondary)?) + } + Type::MutableReference(t) => { + format!("&mut {}", format_param(t, type_params, secondary)?) + } + Type::Reference(t) => { + format!("&{}", format_param(t, type_params, secondary)?) + } + Type::Struct { + address, + module, + name, + type_arguments, + .. + } if type_arguments.len() > 0 => { + format!( + "{}::{}::{}<{}>", + address.to_hex_literal(), + module, + name, + type_arguments + .iter() + .map(|t| format_param(t, type_params.clone(), secondary)) + .collect::, _>>()? + .join(", ") + ) + } + _ => format!("{}", param), + }) +} + +/// Format a list of items into a human-readable string. +pub(super) fn format_list( + items: impl IntoIterator, + noun_singular_plural: Option<(&str, &str)>, +) -> String { + let items: Vec<_> = items.into_iter().map(|i| i.to_string()).collect(); + let items_string = match items.len() { + 0 => "none".to_string(), + 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().expect("unexpected empty list"); + format!("{}, and {}", all_but_last, last) + } + }; + if let Some((singular, plural)) = noun_singular_plural { + format!( + "{}: {}", + singular_or_plural(items.len(), singular, plural), + items_string, + ) + } else { + items_string + } +} + +/// Returns a string with the singular or plural form of a word based on a count. +pub(super) fn singular_or_plural(n: usize, singular: &str, plural: &str) -> String { + if n == 1 { + singular.to_string() + } else { + plural.to_string() + } +} diff --git a/crates/sui/src/upgrade_compatibility.rs b/crates/sui/src/upgrade_compatibility/mod.rs similarity index 87% rename from crates/sui/src/upgrade_compatibility.rs rename to crates/sui/src/upgrade_compatibility/mod.rs index 63a534e45f7da..f3a09e490c52a 100644 --- a/crates/sui/src/upgrade_compatibility.rs +++ b/crates/sui/src/upgrade_compatibility/mod.rs @@ -1,12 +1,14 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 -#[path = "unit_tests/upgrade_compatibility_tests.rs"] +mod formatting; +#[path = "../unit_tests/upgrade_compatibility_tests.rs"] #[cfg(test)] mod upgrade_compatibility_tests; +use formatting::{format_list, format_param, singular_or_plural, FormattedField}; + use anyhow::{anyhow, Context, Error}; -use regex::Regex; use std::collections::{BTreeMap, HashMap, HashSet}; use std::fs; use std::path::PathBuf; @@ -47,20 +49,13 @@ use sui_sdk::SuiClient; use sui_types::move_package::UpgradePolicy; use sui_types::{base_types::ObjectID, execution_config_utils::to_binary_config}; -/// Errors that can occur during upgrade compatibility checks. -/// one-to-one related to the underlying trait functions see: [`CompatibilityMode`] -/// Except for the `ModuleMismatch` which is a special case for additive and dependency only policies and `ModuleMissing` +/// Errors that can occur during upgrade compatibility checks, +/// one-to-one related to the underlying trait functions see: [`CompatibilityMode`]. #[derive(Debug, Clone)] pub(crate) enum UpgradeCompatibilityModeError { ModuleMissing { name: Identifier, }, - /// The upgrade is not compatible with the existing package due to the policy. - /// This error is used for additive and dependency only policies where modules - /// are either not allowed to add declarations or change them. - ModuleMismatch { - policy: UpgradePolicy, - }, StructMissing { name: Identifier, }, @@ -164,14 +159,13 @@ pub(crate) enum UpgradeCompatibilityModeError { FriendMissing, } -/// Check if a specifc error breaks 'compatible' upgrades +/// Check if an `UpgradeCompatibilityModeError` variant breaks 'compatible' upgrades. fn breaks_compatibility( error: &UpgradeCompatibilityModeError, compatability: &Compatibility, ) -> bool { match error { - UpgradeCompatibilityModeError::ModuleMissing { .. } - | UpgradeCompatibilityModeError::ModuleMismatch { .. } => true, + UpgradeCompatibilityModeError::ModuleMissing { .. } => true, UpgradeCompatibilityModeError::StructAbilityMismatch { .. } | UpgradeCompatibilityModeError::StructTypeParamMismatch { .. } @@ -219,7 +213,8 @@ fn breaks_compatibility( } } -/// Check if a specifc error breaks inclusion checks for 'additive' (Subset) or 'dependency only' (Equal) upgrades +/// Check if an `UpgradeCompatibilityModeError` variant breaks inclusion checks for 'additive' (Subset) +/// or 'dependency only' (Equal) upgrades. fn breaks_inclusion_check( error: &UpgradeCompatibilityModeError, inclusion_check: &InclusionCheck, @@ -236,7 +231,6 @@ fn breaks_inclusion_check( } UpgradeCompatibilityModeError::ModuleMissing { .. } - | UpgradeCompatibilityModeError::ModuleMismatch { .. } | UpgradeCompatibilityModeError::StructMissing { .. } | UpgradeCompatibilityModeError::StructAbilityMismatch { .. } | UpgradeCompatibilityModeError::StructTypeParamMismatch { .. } @@ -262,7 +256,7 @@ fn breaks_inclusion_check( } } -/// A compatibility mode that collects errors as a vector of enums which describe the error causes +/// Compatibility mode used for 'compatible' upgrades, collects all errors and returns them as a single error. #[derive(Default)] pub(crate) struct CliCompatibilityMode { errors: Vec, @@ -270,7 +264,7 @@ pub(crate) struct CliCompatibilityMode { impl CompatibilityMode for CliCompatibilityMode { type Error = Vec; - // ignored, address is not populated pre-tx + // Ignored, address is not populated pre-tx, `compare_packages` function prevents name mismatches fn module_id_mismatch( &mut self, _old_addr: &AccountAddress, @@ -440,7 +434,8 @@ impl CompatibilityMode for CliCompatibilityMode { } } -/// A Compatibility mode used for checking inclusion checks for 'additive' (Subset) or 'dependency only' (Equal) upgrades +/// Compatibility mode used during inclusion checks: 'additive' (Subset) or 'dependency only' (Equal) upgrades, +/// collects all errors and returns them as a single error. #[derive(Default)] struct CliInclusionCheckMode { errors: Vec, @@ -449,13 +444,13 @@ struct CliInclusionCheckMode { impl InclusionCheckMode for CliInclusionCheckMode { type Error = Vec; - // ignored address is not populated pre-tx + // Ignored, address is not populated pre-tx, `compare_packages` function prevents name mismatches fn module_id_mismatch( &mut self, - old_address: &AccountAddress, - old_name: &IdentStr, - new_address: &AccountAddress, - new_name: &IdentStr, + _old_address: &AccountAddress, + _old_name: &IdentStr, + _new_address: &AccountAddress, + _new_name: &IdentStr, ) { } @@ -483,7 +478,7 @@ impl InclusionCheckMode for CliInclusionCheckMode { }); } - fn struct_missing(&mut self, name: &Identifier, old_struct: &Struct) { + fn struct_missing(&mut self, name: &Identifier, _old_struct: &Struct) { self.errors .push(UpgradeCompatibilityModeError::StructMissing { name: name.clone() }); } @@ -502,7 +497,7 @@ impl InclusionCheckMode for CliInclusionCheckMode { }); } - fn enum_missing(&mut self, name: &Identifier, old_enum: &Enum) { + fn enum_missing(&mut self, name: &Identifier, _old_enum: &Enum) { self.errors .push(UpgradeCompatibilityModeError::EnumMissing { name: name.clone() }); } @@ -524,7 +519,7 @@ impl InclusionCheckMode for CliInclusionCheckMode { }); } - fn function_missing(&mut self, name: &Identifier, old_func: &Function) { + fn function_missing(&mut self, name: &Identifier, _old_func: &Function) { self.errors .push(UpgradeCompatibilityModeError::FunctionMissing { name: name.clone() }); } @@ -559,7 +554,7 @@ struct IdentifierTableLookup { function_identifier_to_index: BTreeMap, } -/// creates an index to allow looking up the table index of a struct, enum, or function by its identifier +/// Creates an index to allow looking up the table index of a struct, enum, or function by its identifier. fn table_index(compiled_module: &CompiledModule) -> IdentifierTableLookup { // for each in compiled module let struct_identifier_to_index: BTreeMap = compiled_module @@ -658,7 +653,6 @@ upgrade_codes!( AbilityMismatch: { msg: "ability mismatch" }, FieldMismatch: { msg: "field mismatch" }, TypeParamMismatch: { msg: "type parameter mismatch" }, - ModuleMismatch: { msg: "module incompatible" }, ModuleMissing: { msg: "module missing" }, Missing: { msg: "missing declaration" }, VersionMismatch: { msg: "file format version downgrade" }, @@ -840,24 +834,13 @@ fn modules_into_diags( Ok(diags) } -fn errors_or_empty_vec( - name: Identifier, - result: Result<(), Vec>, -) -> Vec<(Identifier, UpgradeCompatibilityModeError)> { - match result { - Ok(_) => vec![], - Err(errors) => errors.into_iter().map(|e| (name.clone(), e)).collect(), - } -} - -/// Convert an error to a diagnostic using the specific error type's function. +/// Convert an error to a vector of diagnostics using the error specific function. fn compatibility_diag_from_error( error: &UpgradeCompatibilityModeError, compiled_unit_with_source: &CompiledUnitWithSource, is_compatible: bool, lookup: &IdentifierTableLookup, ) -> Result { - let module_name = compiled_unit_with_source.unit.name.as_str(); match error { UpgradeCompatibilityModeError::StructMissing { name, .. } => { missing_definition_diag("struct", name, is_compatible, compiled_unit_with_source) @@ -993,11 +976,6 @@ fn compatibility_diag_from_error( UpgradeCompatibilityModeError::FunctionEntryCompatibility { name, old_function, .. } => function_entry_mismatch(name, old_function, compiled_unit_with_source, lookup), - // Specifically handles additive and dep only policies where modules - // are either not allowed to add declarations or change them. - UpgradeCompatibilityModeError::ModuleMismatch { policy } => { - module_compatibility_error_diag(*policy, compiled_unit_with_source) - } UpgradeCompatibilityModeError::ModuleMissing { .. } => { unreachable!("Module Missing should be handled by outer function") } @@ -1048,45 +1026,7 @@ fn compatibility_diag_from_error( } } -// give specifics about the declarations which do not match -fn module_compatibility_error_diag( - policy: UpgradePolicy, - compiled_unit_with_source: &CompiledUnitWithSource, -) -> Result { - let mut diags = Diagnostics::new(); - - let loc = compiled_unit_with_source - .unit - .source_map - .definition_location; - - diags.add(Diagnostic::new( - Declarations::ModuleMismatch, - ( - loc, - format!( - "The upgrade is not compatible with the existing package due to {} policy.", - match policy { - UpgradePolicy::Additive => "additive", - UpgradePolicy::DepOnly => "dependency only", - _ => unreachable!("Invalid upgrade policy for this error type"), - } - ), - ), - Vec::<(Loc, String)>::new(), - vec![ - "The upgrade is not compatible with the existing package.".to_string(), - format!( - "The upgrade policy is set to '{}'.", - policy.to_string().to_lowercase() - ), - ], - )); - - Ok(diags) -} - -/// Return a diagnostic when a module is missing from a package. +/// Returns a diagnostic when a module is missing from a package. fn missing_module_diag( module_name: &Identifier, move_toml_hash: &FileHash, @@ -1120,7 +1060,7 @@ fn missing_module_diag( Ok(diags) } -/// Return a diagnostic for a missing definition. +/// Returns a diagnostic for a missing definition. fn missing_definition_diag( declaration_kind: &str, identifier_name: &Identifier, @@ -1182,7 +1122,7 @@ fn missing_definition_diag( Ok(diags) } -/// Return a diagnostic for a function which has lost its public visibility +/// Returns a diagnostic for a function which has lost its public visibility. fn function_lost_public( function_name: &Identifier, compiled_unit_with_source: &CompiledUnitWithSource, @@ -1224,39 +1164,9 @@ fn function_lost_public( Ok(diags) } -fn format_param( - param: &Type, - type_params: Vec, - secondary: &mut Vec<(Loc, String)>, -) -> Result { - Ok(match param { - Type::TypeParameter(t) => { - let type_param = type_params - .get(*t as usize) - .context("Unable to get type param location")?; - - secondary.push(( - type_param.1, - format!("Type parameter '{}' is defined here", &type_param.0), - )); - type_param.0.to_string() - } - Type::Vector(t) => { - format!("vector<{}>", format_param(t, type_params, secondary)?) - } - Type::MutableReference(t) => { - format!("&mut {}", format_param(t, type_params, secondary)?) - } - Type::Reference(t) => { - format!("&{}", format_param(t, type_params, secondary)?) - } - _ => format!("{}", param), - }) -} - -/// 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. +/// Returns diagnostics for each signature mismatch in the given function. +/// Start by checking the lengths of the parameters and returns and add a diagnostic if they are different. +/// If the lengths are the same check each parameter piece wise and add a diagnostic for each mismatch. fn function_signature_mismatch_diag( function_name: &Identifier, old_function: &Function, @@ -1571,7 +1481,7 @@ fn function_entry_mismatch( Ok(diags) } -/// Return a label string for an ability mismatch. +/// Returns a label string for an ability mismatch. fn ability_mismatch_label( old_abilities: AbilitySet, new_abilities: AbilitySet, @@ -1613,7 +1523,7 @@ fn ability_mismatch_label( } } -/// Return a diagnostic for an ability mismatch. +/// Returns a diagnostic for a given struct's ability mismatch. fn struct_ability_mismatch_diag( struct_name: &Identifier, old_struct: &Struct, @@ -1682,63 +1592,43 @@ 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 +/// Returns an error code and label for the given field. +fn field_mismatch_message( + old_field: &Field, + new_field: &Field, + type_params: Vec, +) -> Result<(Declarations, String), Error> { + let old_ff = FormattedField::new(old_field, &type_params); + let new_ff = FormattedField::new(new_field, &type_params); + + Ok( + match ( + old_field.name != new_field.name, + old_field.type_ != new_field.type_, + ) { + (true, true) => ( + Declarations::FieldMismatch, + format!("Mismatched field {new_ff}, expected {old_ff}."), ), - ), - (true, false) => ( - Declarations::FieldMismatch, - format!( - "Mismatched field name {}, expected {}.", - new_field_name, old_field_name + (true, false) => ( + Declarations::FieldMismatch, + format!("Mismatched field {new_ff:#}, expected {old_ff:#}."), ), - ), - (false, true) => ( - Declarations::TypeMismatch, - format!( - "Mismatched field type {}, expected {}.", - new_field_type, old_field_type + (false, true) => ( + Declarations::TypeMismatch, + format!( + "Mismatched field type {}, expected {}.", + new_ff.type_, old_ff.type_ + ), ), - ), - (false, false) => unreachable!("Fields should not be the same"), - } + (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 +/// Returns diagnostics for each field mismatch in the given struct. +/// 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, @@ -1762,16 +1652,21 @@ fn struct_field_mismatch_diag( let def_loc = struct_sourcemap.definition_location; + let dummy_field = Field { + name: Identifier::new("dummy_field") + .context("unexpected error with identifier constructor")?, + type_: Type::Bool, + }; let old_fields: Vec<&Field> = old_struct .fields .iter() - .filter(|f| f.name != Identifier::new("dummy_field").unwrap() && f.type_ != Type::Bool) + .filter(|f| f != &&dummy_field) .collect(); let new_fields: Vec<&Field> = new_struct .fields .iter() - .filter(|f| f.name != Identifier::new("dummy_field").unwrap() && f.type_ != Type::Bool) + .filter(|f| f != &&dummy_field) .collect(); let reason = if public_visibility_related_error { @@ -1811,7 +1706,11 @@ fn struct_field_mismatch_diag( .get(i) .context("Unable to get field location")?; - let (code, label) = field_mismatch_message(old_field, new_field); + let (code, label) = field_mismatch_message( + old_field, + new_field, + struct_sourcemap.type_parameters.clone(), + )?; diags.add(Diagnostic::new( code, @@ -1833,9 +1732,9 @@ fn struct_field_mismatch_diag( Ok(diags) } -/// 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 +/// Returns diagnostics for each type parameter mismatch in the given struct. +/// 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. fn struct_type_param_mismatch_diag( name: &Identifier, old_struct: &Struct, @@ -1868,7 +1767,7 @@ fn struct_type_param_mismatch_diag( ) } -/// Return a diagnostic for a new variant in an enum +/// Returns a diagnostic for enum ability mismatches. fn enum_ability_mismatch_diag( enum_name: &Identifier, old_enum: &Enum, @@ -1940,69 +1839,64 @@ fn enum_ability_mismatch_diag( Ok(diags) } -/// Returns the error code and label for mismatched, missing, or unexpected variant -fn enum_variant_field_error( +/// Returns the error code and label for mismatched, missing, or unexpected variants. +fn enum_variant_field_message( old_variant: &Variant, new_variant: &Variant, - variant_loc: Loc, - def_loc: Loc, -) -> (DiagnosticInfo, Vec) { +) -> Result, Error> { if old_variant.fields.len() != new_variant.fields.len() { - return ( + return Ok(vec![( Declarations::FieldMismatch.into(), - vec![format!( + 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 + Ok( + match ( + old_variant.name != new_variant.name, + old_variant.fields != new_variant.fields, + ) { + (true, true) => vec![( + Enums::VariantMismatch.into(), + 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 + (true, false) => vec![( + Enums::VariantMismatch.into(), + format!( + "Mismatched variant '{}', 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 - )); + (false, true) => { + let mut errors: Vec<(DiagnosticInfo, String)> = vec![]; + + for (old_field, new_field) in + old_variant.fields.iter().zip(new_variant.fields.iter()) + { + if old_field != new_field { + let (code, label) = + field_mismatch_message(old_field, new_field, Vec::new())?; + errors.push((code.into(), label)); + } } - } - (Declarations::FieldMismatch.into(), errors) - } - (false, false) => unreachable!("Variants should not be the same"), - } + 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 +/// Returns diagnostics for each variant mismatch in the given enum. +/// Start by checking the lengths of the variants 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. fn enum_variant_mismatch_diag( enum_name: &Identifier, old_enum: &Enum, @@ -2040,12 +1934,11 @@ fn enum_variant_mismatch_diag( .0 .1; - let (code, labels) = - enum_variant_field_error(old_variant, new_variant, variant_loc, def_loc); + let messages = enum_variant_field_message(old_variant, new_variant)?; - for label in labels { + for (code, label) in messages { diags.add(Diagnostic::new( - code.clone(), + code, (variant_loc, label), vec![(def_loc, "Enum definition".to_string())], vec![ @@ -2072,7 +1965,7 @@ fn enum_variant_mismatch_diag( Ok(diags) } -/// Return a diagnostic for a new variant in an enum +/// Returns diagnostics for each new variant in an enum. fn enum_new_variant_diag( enum_name: &Identifier, old_enum: &Enum, @@ -2134,7 +2027,7 @@ fn enum_new_variant_diag( Ok(diags) } -/// Return a diagnostic for a missing variant in an enum +/// Returns diagnostics for each missing variant in an enum. fn enum_variant_missing_diag( enum_name: &Identifier, old_enum: &Enum, @@ -2182,6 +2075,7 @@ fn enum_variant_missing_diag( Ok(diags) } +/// Returns a diagnostic for an unexpected struct. fn struct_new_diag( struct_name: &Identifier, compiled_unit_with_source: &CompiledUnitWithSource, @@ -2211,6 +2105,7 @@ fn struct_new_diag( Ok(diags) } +/// Returns a diagnostic for an unexpected struct changed. fn struct_changed_diag( struct_name: &Identifier, old_struct: &Struct, @@ -2256,6 +2151,7 @@ fn struct_changed_diag( Ok(diags) } +/// Returns a diagnostic for an unexpected new enum. fn enum_new_diag( enum_name: &Identifier, _new_enum: &Enum, @@ -2286,6 +2182,7 @@ fn enum_new_diag( Ok(diags) } +/// Returns a diagnostic for an unexpected enum change. fn enum_changed_diag( enum_name: &Identifier, old_enum: &Enum, @@ -2331,6 +2228,7 @@ fn enum_changed_diag( Ok(diags) } +/// Returns a diagnostic for an unexpected new function. fn function_new_diag( function_name: &Identifier, compiled_unit_with_source: &CompiledUnitWithSource, @@ -2363,6 +2261,7 @@ fn function_new_diag( Ok(diags) } +/// Returns a diagnostic for an unexpected function changed. fn function_changed_diag( function_name: &Identifier, old_function: &Function, @@ -2386,7 +2285,7 @@ fn function_changed_diag( Ok(diags) } -/// Return a diagnostic for a type parameter mismatch +/// Returns a diagnostic for an enum type parameter mismatch. fn enum_type_param_mismatch( enum_name: &Identifier, old_enum: &Enum, @@ -2419,7 +2318,7 @@ fn enum_type_param_mismatch( ) } -/// Return a diagnostic for a type parameter mismatch +/// Returns a diagnostic for a type parameter mismatch. fn type_parameter_diag( declaration_kind: &str, name: &Identifier, @@ -2437,7 +2336,7 @@ fn type_parameter_diag( .enumerate() .map(|(i, c)| { if i == 0 { - c.to_uppercase().next().unwrap() + c.to_uppercase().next().unwrap_or(c) } else { c } @@ -2523,7 +2422,7 @@ fn type_parameter_diag( Ok(diags) } -/// Return a diagnostic for a type parameter constrant mismatch +/// Returns a label for a type parameter constraint mismatch. fn type_param_constraint_labels( old_constraints: AbilitySet, new_constraints: AbilitySet, @@ -2551,7 +2450,7 @@ fn type_param_constraint_labels( )) } -/// Return a diagnostic for a type parameter phantom mismatch +/// Returns a label 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; @@ -2570,7 +2469,7 @@ fn type_param_phantom_labels(old_phantom: bool, new_phantom: bool) -> Option<(St }) } -/// Return a diagnostic for package file format version mismatch +/// Returns a diagnostic for package file format version mismatch. fn file_format_version_downgrade_diag( old_version: &u32, new_version: &u32, @@ -2602,7 +2501,7 @@ fn file_format_version_downgrade_diag( Ok(diags) } -/// Return a diagnostic for a friend link mismatch +/// Returns a diagnostic for a friend link mismatch. fn friend_link_diag( compiled_unit_with_source: &CompiledUnitWithSource, ) -> Result { @@ -2623,44 +2522,8 @@ fn friend_link_diag( Ok(diags) } -/// Format a list of items into a human-readable string. -fn format_list( - items: impl IntoIterator, - noun_singular_plural: Option<(&str, &str)>, -) -> String { - let items: Vec<_> = items.into_iter().map(|i| i.to_string()).collect(); - let items_string = match items.len() { - 0 => "none".to_string(), - 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) - } - }; - if let Some((singular, plural)) = noun_singular_plural { - format!( - "{}: {}", - singular_or_plural(items.len(), singular, plural), - items_string, - ) - } else { - items_string - } -} - -/// 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() - } else { - plural.to_string() - } -} - -/// Helper function to determine if colors should be used in the output. -/// disables colors in tests +/// Helper function to determine if colors should be used in the output and +/// disables colors in tests. fn use_colors() -> bool { #[cfg(test)] {