Unused Declaration Detection (#4377)

# Unused Declaration Detection

Detection of unused program declarations with compiler warnings.

Program example `example.mo`:
```
import Array "mo:base/Array";
import Debug "mo:base/Debug";

actor {
    var variable1 = 0;
    var variable2 = "TEST";

    func testUnusedFunction(parameter1 : Bool, parameter2 : Int) {
        var variable2 = 2;
        var variable3 = 3;
        let variable4 = 4;
        if (variable1 == 0 and variable3 == 3) {
            let variable2 = parameter1;
            Debug.print(debug_show(variable2));
        };
    };
};
```

Compiler messages:
```
example.mo:1.8-1.13: warning [M0194], Unused declaration Array
example.mo:6.9-6.18: warning [M0194], Unused declaration variable2
example.mo:8.10-8.28: warning [M0194], Unused declaration testUnusedFunction
example.mo:8.48-8.58: warning [M0194], Unused declaration parameter2
example.mo:9.13-9.22: warning [M0194], Unused declaration variable2
example.mo:11.13-11.22: warning [M0194], Unused declaration variable4
```

## Coverage

The analysis detects the following unused declarations:
* Variables
* Parameters, including shared context
* Functions
* Classes
* Objects
* Modules
* Imports
* Private fields in objects and classes

Special aspects:
* System functions are considered implicitly used.
* Non-accessed stable variables are considered unused, even if they could be accessed in a future upgraded program version.

## Warnings

The warning of an unused declaration can be suppressed by prefixing the identifier by an underscore.

Example:

```
object Silence {
    public func log(_message: Text) { // Suppress the warning for the unused `_message` parameter.
    }
}
```
## Tweaks from #4407 

* don't warn about unused declarations in code from packages (assuming packaces are third party you can't silence them anyway):
  * annotate LibPath Ast nodes with source package, if any, as tracked and determined during import resolution.
  * predicate unused declaration warnings on package origin.
* don't reject unused declarations in the repl treating top-level  code as belonging to fake package "<top-level>" (a mild hack).
   The repl can't know the rest of the interaction so any warning is premature and a nuisance. 
* change terminology of declarations/variables to bindings/indentifiers (for consistency with rest of code)
* add error-code description in M0194.md
* add changelog entry.

Future: we could suppress all warnings, not just unused declarations - from imported package code this way, should we want to.  A --lint mode could re-enable them for further auditing. The rationale is that the warnings are of interest to and actionable on only by the author of the package, not the client. 

## Future Work

The following analyses are not yet implemented but would be beneficial to support:
* Unused recursive function calls (direct or indirect recursion).
* Unused type definitions, unused type parameters
* Unused branch labels
* Unused variant options
* Unused public fields: Additional aspects to consider:
    - Accesses via paths outside the declaration scope.
    - Possible usage before declaration.
    - Polymorphism of structural typing.
    - A library module may expose more (directly or indirectly) public declarations than used.
* Write-only mutable variables: Mutable variables that are never read but only written
* Unnecessary mutability of read-only variables: Recommend `let` instead of `var`.

Changelog.md

@@ -1,9 +1,19 @@
 # Motoko compiler changelog
 
+
 ## Unreleased
 
 * motoko (`moc`)
 
+  * Warn on detection of unused identifiers (code `M0194`) (#4377).
+
+    - By design, warnings are not emitted for code imported from a package.
+    - A warning can be suppressed by replacing the identifier entirely by a wildcard `_`,
+      or by prefixing it with an `_`, e.g. replace `x` by `_x`.
+
+    **Limitations**: recursive and mutually recursive definitions are considered used,
+    even if never referenced outside the recursive definition.
+
   * Remove `__get_candid_interface_tmp_hack` endpoint. Candid interface is already stored as canister metadata, this temporary endpoint is redundant, thus removed. (#4386)
 
 ## 0.10.4 (2024-01-10)
@@ -12,7 +22,7 @@
 
   * Officializing the new **incremental garbage collector** after a successful beta testing phase.
     The incremental GC can be enabled by the `moc` flag `--incremental-gc` (#3837) and is designed to scale for large program heap sizes.
-    
+
     **Note**: While resolving scalability issues with regard to the instruction limit of the GC work, it is now possible to hit other scalability limits:
     - _Out of memory_: A program can run out of memory if it fills the entire memory space with live objects.
     - _Upgrade limits_: When using stable variables, the current mechanism of serialization and deserialization to and from stable memory can exceed the instruction limit or run out of memory.

src/exes/candid_tests.ml

@@ -76,7 +76,7 @@ let mo_of_test tenv test : (string * expected_behaviour, string) result =
 
   try
     let defs =
-      "import Prim \"mo:⛔\";" ^
+      "import _Prim \"mo:⛔\";" ^
       String.concat "" (List.map (fun (n,candid_typ) ->
         let mo_typ = Idl_to_mo.check_typ tenv candid_typ in
         "type " ^ n ^ " = " ^ Pretty.string_of_typ mo_typ ^ ";\n"

src/lang_utils/error_codes.ml

@@ -197,4 +197,5 @@ let error_codes : (string * string option) list =
     "M0191", None; (* Code requires Wasm features ... to execute *)
     "M0192", None; (* Object/Actor/Module body type mismatch *)
     "M0193", None; (* Can't declare actor class to have `async*` result *)
+    "M0194", Some([%blob "lang_utils/error_codes/M0194.md"]); (* Unused identifier warning *)
   ]

src/lang_utils/error_codes/M0194.md

@@ -0,0 +1,31 @@
+# M0194
+
+This warning means that you defined an identifier without
+referencing it later, a good indicator of dead code.
+
+Dubious code example:
+
+```motoko
+let nickname = "klutz";
+// code that never uses `nickname`
+```
+
+If you encounter this warning, you can either delete the definition (if the code has no other side-effect),
+
+```motoko
+// code that never uses `nickname`
+```
+
+replace it by a wildcard pattern:
+
+```motoko
+let _ = "klutz";
+// code that never uses `nickname`
+```
+
+or just prefix the identifier with an underscore:
+
+```motoko
+let _nickname = "klutz";
+// code that never uses `nickname`
+```

src/languageServer/declaration_index.ml

@@ -313,13 +313,14 @@ let list_files_recursively dir =
   in
   loop [] [ dir ]
 
-let scan_packages : unit -> string list =
+let scan_packages : unit -> lib_path list =
  fun _ ->
-  let scan_package path =
+  let scan_package p path =
     list_files_recursively path
     |> List.filter (fun f -> Filename.extension f = ".mo")
+    |> List.map (fun f -> { package = Some p; path = f })
   in
-  Flags.M.fold (fun _ v acc -> scan_package v @ acc) !Flags.package_urls []
+  Flags.M.fold (fun p v acc -> scan_package p v @ acc) !Flags.package_urls []
 
 let scan_actors : unit -> string list =
  fun _ ->
@@ -347,7 +348,7 @@ let index_from_scope : string -> t -> Syntax.lib list -> Scope.t -> t =
 
 let make_index_inner project_root vfs entry_points : t Diag.result =
   let package_paths =
-    List.map (fun f -> LibPath f @@ Source.no_region) (scan_packages ())
+    List.map (fun lp -> LibPath lp @@ Source.no_region) (scan_packages ())
   in
   let package_env =
     Pipeline.chase_imports

src/lowering/desugar.ml

@@ -1006,7 +1006,7 @@ let transform_import (i : S.import) : import_declaration =
   assert (t <> T.Pre);
   let rhs = match !ir with
     | S.Unresolved -> raise (Invalid_argument ("Unresolved import " ^ f))
-    | S.LibPath fp ->
+    | S.LibPath {path = fp; _} ->
       varE (var (id_of_full_path fp) t)
     | S.PrimPath ->
       varE (var (id_of_full_path "@prim") t)

src/mo_def/syntax.ml

@@ -12,9 +12,10 @@ let empty_typ_note = {note_typ = Type.Pre; note_eff = Type.Triv}
 
 (* Resolved imports (filled in separately after parsing) *)
 
+type lib_path = {package : string option; path : string}
 type resolved_import =
   | Unresolved
-  | LibPath of string
+  | LibPath of lib_path
   | IDLPath of (string * string) (* filepath * bytes *)
   | PrimPath (* the built-in prim module *)
 
@@ -125,6 +126,7 @@ and vis' =
   | System
 
 let is_public vis = match vis.Source.it with Public _ -> true | _ -> false
+let is_private vis = match vis.Source.it with Private -> true | _ -> false
 
 type stab = stab' Source.phrase
 and stab' = Stable | Flexible
@@ -296,8 +298,8 @@ open Source
 
 (* Identifiers *)
 
-let anon_id sort at = "anon-" ^ sort ^ "-" ^ string_of_pos at.left
-let is_anon_id id = Lib.String.chop_prefix "anon-" id.it <> None
+let anon_id sort at = "@anon-" ^ sort ^ "-" ^ string_of_pos at.left
+let is_anon_id id = Lib.String.chop_prefix "@anon-" id.it <> None
 
 (* Types & Scopes *)