Fix unused variable check to detect write-only variables

Assignment targets (e.g. `x = 2`) were incorrectly marked as "used",
so variables that were written to but never read weren't flagged. Now
the check correctly distinguishes reads from writes: only variable
reads count as uses.

For variables with assignments, no autofix is offered since neither
removing the let nor renaming would be safe (both would leave
dangling assignment references).

https://claude.ai/code/session_01QdfZKyPKuP6bvX4A2r3pWw

Author: claude

src/checks/unused_vars.rs

@@ -1,4 +1,4 @@
-use rustc_hash::FxHashMap;
+use rustc_hash::{FxHashMap, FxHashSet};
 
 use crate::diagnostics::{Autofix, Diagnostic, Severity};
 use crate::parser::ast::{
@@ -49,8 +49,9 @@ struct UnusedLocalVar {
     name: SymbolName,
     /// Position of the variable name (for the warning).
     name_position: Position,
-    /// How to fix this unused variable.
-    fix: UnusedVarFix,
+    /// How to fix this unused variable. `None` when no safe autofix
+    /// is available (e.g. variable is assigned to after binding).
+    fix: Option<UnusedVarFix>,
 }
 
 /// The type of fix to apply for an unused variable.
@@ -85,6 +86,12 @@ struct UnusedVariableVisitor {
     /// Key is the interned symbol ID.
     let_removal_positions: FxHashMap<InternedSymbolId, Position>,
 
+    /// Variables that have been assigned to after their initial binding.
+    /// We can't safely offer autofixes for these (removing the `let`
+    /// would leave dangling assignments, and renaming would miss the
+    /// assignment targets).
+    assigned_variables: FxHashSet<InternedSymbolId>,
+
     method_this_type_hint: Option<TypeHint>,
 
     /// Type parameter tracking for the current function/method.
@@ -112,6 +119,7 @@ impl UnusedVariableVisitor {
             file_bindings: FxHashMap::default(),
             unused: vec![],
             let_removal_positions: FxHashMap::default(),
+            assigned_variables: FxHashSet::default(),
             method_this_type_hint: None,
             type_param_info: vec![],
             unused_type_params: vec![],
@@ -123,17 +131,18 @@ impl UnusedVariableVisitor {
 
         // Unused local variables
         for unused_var in &self.unused {
-            let fix = match &unused_var.fix {
-                UnusedVarFix::Rename => Autofix {
+            let fixes = match &unused_var.fix {
+                Some(UnusedVarFix::Rename) => vec![Autofix {
                     description: format!("Rename to `_{}`", unused_var.name),
                     position: unused_var.name_position.clone(),
                     new_text: format!("_{}", unused_var.name),
-                },
-                UnusedVarFix::RemoveLet { removal_position } => Autofix {
+                }],
+                Some(UnusedVarFix::RemoveLet { removal_position }) => vec![Autofix {
                     description: "Remove this let binding".to_owned(),
                     position: removal_position.clone(),
                     new_text: String::new(),
-                },
+                }],
+                None => vec![],
             };
 
             diagnostics.push(Diagnostic {
@@ -144,7 +153,7 @@ impl UnusedVariableVisitor {
                     msgtext!(" is unused."),
                 ]),
                 position: unused_var.name_position.clone(),
-                fixes: vec![fix],
+                fixes,
             });
         }
 
@@ -264,11 +273,16 @@ impl UnusedVariableVisitor {
             }
 
             if let UseState::NotUsed(position) = use_state {
-                // Check if this is a let binding with removal info
-                let fix = if let Some(removal_position) = self.let_removal_positions.remove(&id) {
-                    UnusedVarFix::RemoveLet { removal_position }
+                // If this variable has been assigned to, we can't
+                // safely offer autofixes (removing the let would
+                // leave dangling assignments, and renaming would miss
+                // the assignment targets).
+                let fix = if self.assigned_variables.contains(&id) {
+                    None
+                } else if let Some(removal_position) = self.let_removal_positions.remove(&id) {
+                    Some(UnusedVarFix::RemoveLet { removal_position })
                 } else {
-                    UnusedVarFix::Rename
+                    Some(UnusedVarFix::Rename)
                 };
 
                 self.unused.push(UnusedLocalVar {
@@ -570,7 +584,15 @@ impl Visitor for UnusedVariableVisitor {
     }
 
     fn visit_expr_assign(&mut self, var: &Symbol, expr: &Expression) {
-        self.check_symbol(var);
+        // An assignment is a write, not a read. Don't mark the
+        // variable as used, so we can detect write-only variables.
+        if self.is_locally_bound(var) {
+            self.assigned_variables.insert(var.interned_id);
+            // Can't safely remove the let binding when there are
+            // later assignments to the variable.
+            self.let_removal_positions.remove(&var.interned_id);
+        }
+
         self.visit_expr(expr);
     }
 

src/test_files/check/assign_returns_unit.gdn

@@ -4,3 +4,13 @@ public fun foo(): Unit {
 }
 
 // args: check
+// expected exit status: 1
+// expected stdout:
+// Warning: `i` is unused.
+// -| src/test_files/check/assign_returns_unit.gdn:2:9
+// 1| public fun foo(): Unit {
+// 2|     let i = 0
+//  |         ^
+// 3|     i = 2
+// 4| }
+

src/test_files/check/assign_wrong_type.gdn

@@ -6,6 +6,14 @@ public fun foo() {
 // args: check
 // expected exit status: 1
 // expected stdout:
+// Warning: `x` is unused.
+// -| src/test_files/check/assign_wrong_type.gdn:2:9
+// 1| public fun foo() {
+// 2|     let x = 1
+//  |         ^
+// 3|     x = ""
+// 4| }
+// 
 // Error: Expected `Int`, but got `String`.
 // -| src/test_files/check/assign_wrong_type.gdn:3:9
 // 1| public fun foo() {

src/test_files/check/unused_assigned_var.gdn

@@ -0,0 +1,61 @@
+// A variable that is only written to (assigned) but never read
+// should be reported as unused.
+fun foo(): Int {
+    let x = 1
+    x = 2
+    0
+}
+
+// But a variable that is read after being assigned should not warn.
+fun bar(): Int {
+    let x = 1
+    x = 2
+    x
+}
+
+// Warning: `foo` is never called.
+// --| src/test_files/check/unused_assigned_var.gdn:3:5
+//  1| // A variable that is only written to (assigned) but never read
+//  2| // should be reported as unused.
+//  3| fun foo(): Int {
+//   |     ^^^
+//  4|     let x = 1
+//  5|     x = 2
+//
+// Warning: `bar` is never called.
+// --| src/test_files/check/unused_assigned_var.gdn:10:5
+//  9| // But a variable that is read after being assigned should not warn.
+// 10| fun bar(): Int {
+//   |     ^^^
+// 11|     let x = 1
+// 12|     x = 2
+
+// args: check
+// expected exit status: 1
+// expected stdout:
+// Warning: `x` is unused.
+// --| src/test_files/check/unused_assigned_var.gdn:4:9
+//  2| // should be reported as unused.
+//  3| fun foo(): Int {
+//  4|     let x = 1
+//   |         ^
+//  5|     x = 2
+//  6|     0
+// 
+// Warning: `foo` is never called.
+// --| src/test_files/check/unused_assigned_var.gdn:3:5
+//  1| // A variable that is only written to (assigned) but never read
+//  2| // should be reported as unused.
+//  3| fun foo(): Int {
+//   |     ^^^
+//  4|     let x = 1
+//  5|     x = 2
+// 
+// Warning: `bar` is never called.
+// --| src/test_files/check/unused_assigned_var.gdn:10:5
+//  9| // But a variable that is read after being assigned should not warn.
+// 10| fun bar(): Int {
+//   |     ^^^
+// 11|     let x = 1
+// 12|     x = 2
+