[move compiler] Report unnecessary mutable references (MystenLabs#14216)

## Description 

- Report unused mutable references
- The system is quite exhaustive and reports even locally created
mutable references. It piggy-backs on the borrow checker but globally
collects usages instead of keeping it in the abstract state. This is
done mostly for simplicity
- A mutable reference is considered unused if:
  - It is mutated
- It is passed to a function (since freeze removes any subtyping issues)
  - It is returned 
- Copies share the same abstract reference ID

## Test Plan 

- Added new tests

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [X] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

With this change, Move will now suggest switching mutable references
`&mut` to immutable ones `&` when the reference is not modified, or
passed somewhere where the mutability is required.
This can be suppressed for parameters by prefixing the parameter name
with an underscore `_`.

crates/sui-move-build/tests/linter/custom_state_change.exp

@@ -1,7 +1,7 @@
 warning[Lint W02001]: potentially unenforceable custom transfer/share/freeze policy
    ┌─ tests/linter/custom_state_change.move:15:16
    │
-15 │     public fun custom_transfer_bad(o: S1, ctx: &mut TxContext) {
+15 │     public fun custom_transfer_bad(o: S1, ctx: &TxContext) {
    │                ^^^^^^^^^^^^^^^^^^^ - An instance of a module-private type with a store ability to be transferred coming from here
    │                │                    
    │                Potential unintended implementation of a custom transfer function.

crates/sui-move-build/tests/linter/custom_state_change.move

@@ -12,7 +12,7 @@ module 0x42::test {
     }
 
     #[lint_allow(self_transfer)]
-    public fun custom_transfer_bad(o: S1, ctx: &mut TxContext) {
+    public fun custom_transfer_bad(o: S1, ctx: &TxContext) {
         transfer::transfer(o, tx_context::sender(ctx))
     }
 

external-crates/move/move-compiler/src/cfgir/borrows/mod.rs

@@ -6,20 +6,25 @@ mod state;
 
 use super::absint::*;
 use crate::{
+    diag,
     diagnostics::Diagnostics,
-    hlir::ast::*,
+    hlir::{
+        ast::*,
+        translate::{display_var, DisplayVar},
+    },
     parser::ast::BinOp_,
     shared::{unique_map::UniqueMap, CompilationEnv},
 };
 use state::{Value, *};
-use std::collections::BTreeMap;
+use std::{cell::RefCell, collections::BTreeMap, rc::Rc};
 
 //**************************************************************************************************
 // Entry and trait bindings
 //**************************************************************************************************
 
 struct BorrowSafety {
     local_numbers: UniqueMap<Var, usize>,
+    mutably_used: RefExpInfoMap,
 }
 
 impl BorrowSafety {
@@ -28,7 +33,10 @@ impl BorrowSafety {
         for (idx, (v, _)) in local_types.key_cloned_iter().enumerate() {
             local_numbers.add(v, idx).unwrap();
         }
-        Self { local_numbers }
+        Self {
+            local_numbers,
+            mutably_used: Rc::new(RefCell::new(BTreeMap::new())),
+        }
     }
 }
 
@@ -63,10 +71,11 @@ impl TransferFunctions for BorrowSafety {
     fn execute(
         &mut self,
         pre: &mut Self::State,
-        _lbl: Label,
-        _idx: usize,
+        lbl: Label,
+        idx: usize,
         cmd: &Command,
     ) -> Diagnostics {
+        pre.start_command(lbl, idx);
         let mut context = Context::new(self, pre);
         command(&mut context, cmd);
         context
@@ -90,17 +99,52 @@ pub fn verify(
         ..
     } = context;
     let acquires = *acquires;
+    let mut safety = BorrowSafety::new(locals);
+
     // check for existing errors
     let has_errors = compilation_env.has_errors();
-    let mut initial_state = BorrowState::initial(locals, acquires.clone(), has_errors);
+    let mut initial_state = BorrowState::initial(
+        locals,
+        safety.mutably_used.clone(),
+        acquires.clone(),
+        has_errors,
+    );
     initial_state.bind_arguments(&signature.parameters);
-    let mut safety = BorrowSafety::new(locals);
     initial_state.canonicalize_locals(&safety.local_numbers);
     let (final_state, ds) = safety.analyze_function(cfg, initial_state);
     compilation_env.add_diags(ds);
+    unused_mut_borrows(compilation_env, safety.mutably_used);
     final_state
 }
 
+fn unused_mut_borrows(compilation_env: &mut CompilationEnv, mutably_used: RefExpInfoMap) {
+    for info in RefCell::borrow(&mutably_used).values() {
+        let RefExpInfo {
+            loc,
+            is_mut,
+            used_mutably,
+            param_name,
+        } = info;
+        if *is_mut && !*used_mutably {
+            let msg = "Mutable reference is never used mutably, \
+            consider switching to an immutable reference '&' instead";
+            let mut diag = diag!(UnusedItem::MutReference, (*loc, msg));
+            let display_param = param_name
+                .as_ref()
+                .map(|v| (v.loc(), display_var(v.value())));
+            if let Some((param_loc, DisplayVar::Orig(v))) = display_param {
+                let param_msg = format!(
+                    "For parameters, this can be silenced by prefixing \
+                    the name with an underscore, e.g. '_{v}'"
+                );
+                diag.add_secondary_label((param_loc, param_msg))
+            }
+
+            compilation_env.add_diag(diag)
+        }
+    }
+}
+
 //**************************************************************************************************
 // Command
 //**************************************************************************************************