upgrade errors advanced function signatures (#20663)

## Description 

function signature upgrade errors test and improve formatting for:
- type params in functions
- vectors
- &
- mut
- structs

## Test plan 

How did you test the new or updated feature?

---

## 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): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
- [ ] REST API:

crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v1/sources/UpgradeErrors.move

@@ -24,5 +24,37 @@ module upgrades::upgrades {
     public fun func_with_wrong_return_length(): (u64, u64) {
         (0,0)
     }
-}
 
+    public struct StructA has drop {
+        x: u64
+    }
+
+    public struct StructB has drop {
+        x: u32
+    }
+
+    // change argument from A to B
+    public fun func_with_wrong_struct_param(a: StructA): u64 {
+        0
+    }
+
+    // change from reference to value
+    public fun ref_to_value(a: &u32): u64 {
+        0
+    }
+
+    // value to ref u32
+    public fun value_to_ref(a: u32): u64 {
+        0
+    }
+
+    // mutable to immutable reference
+    public fun mutable_to_immutable_ref(a: &mut u32): u64 {
+        0
+    }
+
+    // immutable to mutable reference
+    public fun immutable_to_mutable_ref(a: &u32): u64 {
+        0
+    }
+}

crates/sui/src/unit_tests/fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move

@@ -29,5 +29,38 @@ module upgrades::upgrades {
     public fun func_with_wrong_return_length(): u64 {
         0
     }
+
+    public struct StructA has drop {
+        x: u64
+    }
+
+    public struct StructB has drop {
+        x: u32
+    }
+
+    // changed argument from A to B
+    public fun func_with_wrong_struct_param(a: StructB): u64 {
+        0
+    }
+
+    // changed from reference to value
+    public fun ref_to_value(a: u32): u64 {
+        0
+    }
+
+    // u32 as ref
+    public fun value_to_ref(a: &u32): u64 {
+        0
+    }
+
+    // mutable to immutable reference
+    public fun mutable_to_immutable_ref(a: &u32): u64 {
+        0
+    }
+
+    // immutable to mutable reference
+    public fun immutable_to_mutable_ref(a: &mut u32): u64 {
+        0
+    }
 }
 

crates/sui/src/unit_tests/fixtures/upgrade_errors/type_param_errors_v1/sources/UpgradeErrors.move

@@ -53,5 +53,16 @@ module upgrades::upgrades {
 
     // remove constraint (no effect)
     public fun remove_constraint<T: copy>(a: T): T { return a }
-}
 
+    // swap type params
+    public fun swap_type_params<T: drop, U: drop + copy>(a: T, b: U): T { return a }
+
+    // swap return type params
+    public fun swap_type_params_return<T: drop, U: drop + copy>(a: T, b: U): T { return a }
+
+    // change type on vector
+    public fun vec_changed(_: vector<u32>) {}
+
+    // change type param on vector
+    public fun vec_changed_type_param<T: drop, U: drop + copy>(_: vector<T>) {}
+}

crates/sui/src/unit_tests/fixtures/upgrade_errors/type_param_errors_v2/sources/UpgradeErrors.move

@@ -53,5 +53,17 @@ module upgrades::upgrades {
 
     // constraint removed (no effect)
     public fun remove_constraint<T>(a: T): T { return a }
+
+    // type params swapped
+    public fun swap_type_params<T: drop, U: drop + copy>(a: U, b: T): T { return b }
+
+    // return type params swapped U and T
+    public fun swap_type_params_return<T: drop, U: drop + copy>(a: T, b: U): U { return b }
+
+    // change type on vector
+    public fun vec_changed(_: vector<u32>) {}
+
+    // change type param on vector
+    public fun vec_changed_type_param<T: drop, U: drop + copy>(_: vector<U>) {}
 }
 

crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__all.snap

@@ -165,7 +165,7 @@ error[Compatibility E03001]: function signature mismatch
    ┌─ /fixtures/upgrade_errors/all_v2/sources/UpgradeErrors.move:88:36
    │
 88 │     public fun function_change_arg(a: u8) {} // now has u8 instead of u64
-   │                                    ^ Unexpected parameter u8, expected u64
+   │                                    ^ Unexpected parameter 'u8', expected 'u64'
    │
    = Functions are part of a module's public interface and cannot be changed during an upgrade.
    = Restore the original function's parameter for function 'function_change_arg'.

crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__function.snap

@@ -6,7 +6,7 @@ error[Compatibility E03001]: 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
+  │                                      ^ 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 parameter for function 'func_with_wrong_param'.
@@ -15,7 +15,7 @@ error[Compatibility E03001]: function signature mismatch
    ┌─ /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
+   │                                          ^^^ 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 type for function 'func_with_wrong_return'.
@@ -24,7 +24,7 @@ error[Compatibility E03001]: function signature mismatch
    ┌─ /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
+   │                                                 ^ 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 parameter for function 'func_with_wrong_param_and_return'.
@@ -33,7 +33,7 @@ error[Compatibility E03001]: function signature mismatch
    ┌─ /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
+   │                                                          ^^^ 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 type for function 'func_with_wrong_param_and_return'.
@@ -56,5 +56,50 @@ error[Compatibility E03001]: function signature mismatch
    = 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'.
 
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:42:45
+   │
+42 │     public fun func_with_wrong_struct_param(a: StructB): u64 {
+   │                                             ^ Unexpected parameter '0x0::upgrades::StructB', expected '0x0::upgrades::StructA'
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameter for function 'func_with_wrong_struct_param'.
+
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:47:29
+   │
+47 │     public fun ref_to_value(a: u32): u64 {
+   │                             ^ Unexpected parameter 'u32', expected '&u32'
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameter for function 'ref_to_value'.
+
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:52:29
+   │
+52 │     public fun value_to_ref(a: &u32): u64 {
+   │                             ^ Unexpected parameter '&u32', expected 'u32'
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameter for function 'value_to_ref'.
+
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:57:41
+   │
+57 │     public fun mutable_to_immutable_ref(a: &u32): u64 {
+   │                                         ^ Unexpected parameter '&u32', expected '&mut u32'
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameter for function 'mutable_to_immutable_ref'.
+
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/function_errors_v2/sources/UpgradeErrors.move:62:41
+   │
+62 │     public fun immutable_to_mutable_ref(a: &mut u32): u64 {
+   │                                         ^ Unexpected parameter '&mut u32', expected '&u32'
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameter for function 'immutable_to_mutable_ref'.
+
 
 Upgrade failed, this package requires changes to be compatible with the existing package. Its upgrade policy is set to 'compatible'.

crates/sui/src/unit_tests/snapshots/sui__upgrade_compatibility__upgrade_compatibility_tests__type_param.snap

@@ -111,5 +111,49 @@ error[Compatibility E01005]: type parameter mismatch
    = Functions are part of a module's public interface and cannot be changed during an upgrade.
    = Restore the original function's type parameter for function 'add_constraint'.
 
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/type_param_errors_v2/sources/UpgradeErrors.move:58:58
+   │
+58 │     public fun swap_type_params<T: drop, U: drop + copy>(a: U, b: T): T { return b }
+   │                                 -                        ^ Unexpected parameter 'U', expected 'T'
+   │                                 │                         
+   │                                 Type parameter 'T' is defined here
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameters for function 'swap_type_params'.
+
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/type_param_errors_v2/sources/UpgradeErrors.move:58:64
+   │
+58 │     public fun swap_type_params<T: drop, U: drop + copy>(a: U, b: T): T { return b }
+   │                                          -                     ^ Unexpected parameter 'T', expected 'U'
+   │                                          │                      
+   │                                          Type parameter 'U' is defined here
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameters for function 'swap_type_params'.
+
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/type_param_errors_v2/sources/UpgradeErrors.move:61:78
+   │
+61 │     public fun swap_type_params_return<T: drop, U: drop + copy>(a: T, b: U): U { return b }
+   │                                        -                                     ^ Unexpected return type 'U', expected 'T'
+   │                                        │                                      
+   │                                        Type parameter 'T' is defined here
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's return type for function 'swap_type_params_return'.
+
+error[Compatibility E03001]: function signature mismatch
+   ┌─ /fixtures/upgrade_errors/type_param_errors_v2/sources/UpgradeErrors.move:67:64
+   │
+67 │     public fun vec_changed_type_param<T: drop, U: drop + copy>(_: vector<U>) {}
+   │                                       -                        ^ Unexpected parameter 'vector<U>', expected 'vector<T>'
+   │                                       │                         
+   │                                       Type parameter 'T' is defined here
+   │
+   = Functions are part of a module's public interface and cannot be changed during an upgrade.
+   = Restore the original function's parameter for function 'vec_changed_type_param'.
+
 
 Upgrade failed, this package requires changes to be compatible with the existing package. Its upgrade policy is set to 'compatible'.

crates/sui/src/upgrade_compatibility.rs

@@ -15,6 +15,7 @@ use move_binary_format::file_format::{
     AbilitySet, DatatypeTyParameter, EnumDefinitionIndex, FunctionDefinitionIndex,
     StructDefinitionIndex, TableIndex,
 };
+use move_binary_format::normalized::Type;
 use move_binary_format::{
     compatibility::Compatibility,
     compatibility_mode::CompatibilityMode,
@@ -895,6 +896,36 @@ fn function_lost_public(
     Ok(diags)
 }
 
+fn format_param(
+    param: &Type,
+    type_params: Vec<SourceName>,
+    secondary: &mut Vec<(Loc, String)>,
+) -> Result<String, Error> {
+    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
@@ -961,13 +992,25 @@ fn function_signature_mismatch_diag(
                     .context("Unable to get parameter location")?
                     .1;
 
+                let mut secondary = Vec::new();
+
+                let old_param = format_param(
+                    old_param,
+                    func_sourcemap.type_parameters.clone(),
+                    &mut secondary,
+                )?;
+                let new_param = format_param(
+                    new_param,
+                    func_sourcemap.type_parameters.clone(),
+                    &mut Vec::new(),
+                )?;
+
+                let label = format!("Unexpected parameter '{new_param}', expected '{old_param}'");
+
                 diags.add(Diagnostic::new(
                     Functions_::SignatureMismatch,
-                    (
-                        param_loc,
-                        format!("Unexpected parameter {new_param}, expected {old_param}"),
-                    ),
-                    Vec::<(Loc, String)>::new(),
+                    (param_loc, label),
+                    secondary,
                     vec![
                         "Functions are part of a module's public interface \
                         and cannot be changed during an upgrade."
@@ -1117,23 +1160,31 @@ fn function_signature_mismatch_diag(
                 .context("Unable to get return location")?;
 
             if old_return != new_return {
+                let mut secondary = Vec::new();
+
+                let old_return = format_param(
+                    old_return,
+                    func_sourcemap.type_parameters.clone(),
+                    &mut secondary,
+                )?;
+                let new_return = format_param(
+                    new_return,
+                    func_sourcemap.type_parameters.clone(),
+                    &mut Vec::new(),
+                )?;
+
+                let label = 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}'"
+                    )
+                };
+
                 diags.add(Diagnostic::new(
                     Functions_::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(),
+                    (*return_, label),
+                    secondary,
                     vec![
                         "Functions are part of a module's public interface \
                         and cannot be changed during an upgrade."