Sprite resolution, asset wiring, shift-assign, unreachable state warning

Sprite/asset pipeline:
- Linker::link_with_assets() places sprite CHR data in ROM at correct tile
- assets::resolve_sprites() walks Program for inline sprite bytes
- CodeGen::with_sprites() maps sprite names to tile indices
- gen_draw() uses correct tile index from sprite declarations
- main.rs wires the full resolution pipeline

Shift-assign operators (<<= and >>=):
- AssignOp::ShiftLeftAssign and ShiftRightAssign variants
- Parser handles in both statement and array index contexts
- Codegen emits ASL A / LSR A
- IR lowering maps to ShiftLeft/ShiftRight ops

Unreachable state warning (W0104):
- BFS from start state finds reachable states via transitions
- States not reached produce W0104 warning

Error polish helpers:
- suggest_var_name() for "did you mean" suggestions
- emit_undefined_var() for E0502 with typo hints
- Used by analyzer for better diagnostics

242 tests pass, clippy clean.

https://claude.ai/code/session_01W6eQFStA66EuMKHUFo2rx3

src/analyzer/mod.rs

@@ -3,7 +3,7 @@ mod tests;
 
 use std::collections::{HashMap, HashSet};
 
-use crate::errors::{Diagnostic, ErrorCode};
+use crate::errors::{Diagnostic, ErrorCode, Label, Level};
 use crate::lexer::Span;
 use crate::parser::ast::*;
 
@@ -48,6 +48,7 @@ pub fn analyze(program: &Program) -> AnalysisResult {
         max_depths: HashMap::new(),
         stack_depth_limit: DEFAULT_STACK_DEPTH,
         in_loop: false,
+        used_vars: HashSet::new(),
     };
     analyzer.analyze_program(program);
 
@@ -70,6 +71,9 @@ struct Analyzer {
     max_depths: HashMap<String, u32>,
     stack_depth_limit: u32,
     in_loop: bool,
+    /// Names of variables that have been read somewhere in the program.
+    /// Used for the W0103 unused-variable warning.
+    used_vars: HashSet<String>,
 }
 
 impl Analyzer {
@@ -121,9 +125,34 @@ impl Analyzer {
             }
         }
 
-        // Type-check function bodies
+        // Type-check function bodies. Parameters are registered as
+        // symbols for the duration of the body check so that identifier
+        // references (and the W0103 used-variable tracker) can resolve
+        // them. They are unregistered afterwards to avoid leaking into
+        // the global scope. Parameters are also pre-marked as "used" so
+        // we do not emit W0103 for unused function arguments (which are
+        // a common and deliberate pattern).
         for fun in &program.functions {
+            let mut added_params = Vec::new();
+            for param in &fun.params {
+                if !self.symbols.contains_key(&param.name) {
+                    self.symbols.insert(
+                        param.name.clone(),
+                        Symbol {
+                            name: param.name.clone(),
+                            sym_type: param.param_type.clone(),
+                            is_const: false,
+                            span: fun.span,
+                        },
+                    );
+                    added_params.push(param.name.clone());
+                }
+                self.mark_var_used(&param.name);
+            }
             self.check_block(&fun.body, &state_names);
+            for name in &added_params {
+                self.symbols.remove(name);
+            }
         }
 
         // Build call graph
@@ -141,6 +170,145 @@ impl Analyzer {
 
         // Compute max call depths from entry points (state handlers)
         self.compute_max_depths(program);
+
+        // Check for unused global variables (W0103). Variables whose names
+        // start with '_' are exempt by convention. State-local variables are
+        // left out for now to avoid noise during early development.
+        for var in &program.globals {
+            if var.name.starts_with('_') {
+                continue;
+            }
+            if !self.used_vars.contains(&var.name) {
+                self.diagnostics.push(Diagnostic {
+                    level: Level::Warning,
+                    code: ErrorCode::W0103,
+                    message: format!("unused variable '{}'", var.name),
+                    span: var.span,
+                    labels: Vec::<Label>::new(),
+                    help: Some(
+                        "prefix with '_' to silence this warning, or remove the declaration".into(),
+                    ),
+                    note: None,
+                });
+            }
+        }
+
+        // Check for unreachable states (W0104).
+        self.check_unreachable_states(program);
+    }
+
+    /// Mark a variable name as having been read somewhere in the program.
+    fn mark_var_used(&mut self, name: &str) {
+        self.used_vars.insert(name.to_string());
+    }
+
+    /// Recursively walk an expression tree and mark every identifier that
+    /// appears as an `Expr::Ident` (or as an `Expr::ArrayIndex` base) as
+    /// "read". Used by the W0103 unused-variable analysis. Also emits
+    /// E0502 for any identifier that is not defined in the symbol table.
+    fn walk_expr_reads(&mut self, expr: &Expr) {
+        match expr {
+            Expr::Ident(name, span) => {
+                if self.symbols.contains_key(name) {
+                    self.mark_var_used(name);
+                } else {
+                    self.emit_undefined_var(name, *span);
+                }
+            }
+            Expr::ArrayIndex(name, idx, span) => {
+                // Array base is a read; index may contain more reads.
+                if self.symbols.contains_key(name) {
+                    self.mark_var_used(name);
+                } else {
+                    self.emit_undefined_var(name, *span);
+                }
+                self.walk_expr_reads(idx);
+            }
+            Expr::BinaryOp(lhs, _, rhs, _) => {
+                self.walk_expr_reads(lhs);
+                self.walk_expr_reads(rhs);
+            }
+            Expr::UnaryOp(_, inner, _) | Expr::Cast(inner, _, _) => {
+                self.walk_expr_reads(inner);
+            }
+            Expr::Call(_, args, _) => {
+                // Function name is validated separately via E0503; here we
+                // just recurse into argument expressions so their reads
+                // get tracked (and undefined-var errors surface).
+                for arg in args {
+                    self.walk_expr_reads(arg);
+                }
+            }
+            Expr::ArrayLiteral(elems, _) => {
+                for e in elems {
+                    self.walk_expr_reads(e);
+                }
+            }
+            Expr::IntLiteral(_, _) | Expr::BoolLiteral(_, _) | Expr::ButtonRead(_, _, _) => {}
+        }
+    }
+
+    /// Suggest a similarly-named symbol for undefined-variable errors.
+    /// Uses a simple heuristic: same first character and similar length.
+    fn suggest_var_name(&self, unknown: &str) -> Option<String> {
+        let first = unknown.chars().next()?;
+        self.symbols
+            .keys()
+            .filter(|name| {
+                name.starts_with(first)
+                    && name.len().abs_diff(unknown.len()) <= 2
+                    && name.as_str() != unknown
+            })
+            .min_by_key(|name| name.len().abs_diff(unknown.len()))
+            .cloned()
+    }
+
+    /// Emit E0502 for an undefined variable reference, with a "did you mean"
+    /// suggestion if a similar symbol exists.
+    fn emit_undefined_var(&mut self, name: &str, span: Span) {
+        let mut diag = Diagnostic::error(
+            ErrorCode::E0502,
+            format!("undefined variable '{name}'"),
+            span,
+        );
+        if let Some(suggestion) = self.suggest_var_name(name) {
+            diag = diag.with_help(format!("did you mean '{suggestion}'?"));
+        }
+        self.diagnostics.push(diag);
+    }
+
+    /// Reachability analysis for states. Performs a BFS from the start state
+    /// through every transition in state handlers and emits W0104 for any
+    /// state that is never reached.
+    fn check_unreachable_states(&mut self, program: &Program) {
+        let mut reachable: HashSet<String> = HashSet::new();
+        let mut queue: Vec<String> = vec![program.start_state.clone()];
+
+        while let Some(state_name) = queue.pop() {
+            if !reachable.insert(state_name.clone()) {
+                continue;
+            }
+            if let Some(state) = program.states.iter().find(|s| s.name == state_name) {
+                collect_transitions_from_state(state, &mut queue);
+            }
+        }
+
+        for state in &program.states {
+            if !reachable.contains(&state.name) {
+                self.diagnostics.push(Diagnostic {
+                    level: Level::Warning,
+                    code: ErrorCode::W0104,
+                    message: format!("state '{}' is unreachable from start state", state.name),
+                    span: state.span,
+                    labels: Vec::<Label>::new(),
+                    help: Some(
+                        "add a 'transition' to this state from a reachable state, or remove it"
+                            .into(),
+                    ),
+                    note: None,
+                });
+            }
+        }
     }
 
     fn register_const(&mut self, c: &ConstDecl) {
@@ -307,6 +475,7 @@ impl Analyzer {
             Statement::VarDecl(var) => {
                 self.register_var(var);
                 if let Some(init) = &var.init {
+                    self.walk_expr_reads(init);
                     self.check_expr_type(init, &var.var_type);
                 }
             }
@@ -324,7 +493,7 @@ impl Analyzer {
                             }
                         }
                     }
-                    LValue::ArrayIndex(name, _) => {
+                    LValue::ArrayIndex(name, idx) => {
                         if let Some(sym) = self.symbols.get(name) {
                             if sym.is_const {
                                 self.diagnostics.push(Diagnostic::error(
@@ -334,17 +503,24 @@ impl Analyzer {
                                 ));
                             }
                         }
+                        // Indexing an array counts as a read of the array,
+                        // and the index expression itself may contain reads.
+                        self.mark_var_used(name);
+                        self.walk_expr_reads(idx);
                     }
                 }
+                self.walk_expr_reads(expr);
                 let ltype = self.lvalue_type(lvalue, *span);
                 if let Some(lt) = ltype {
                     self.check_expr_type(expr, &lt);
                 }
             }
             Statement::If(cond, then_block, else_ifs, else_block, _) => {
+                self.walk_expr_reads(cond);
                 self.check_expr_type(cond, &NesType::Bool);
                 self.check_block(then_block, state_names);
                 for (cond, block) in else_ifs {
+                    self.walk_expr_reads(cond);
                     self.check_expr_type(cond, &NesType::Bool);
                     self.check_block(block, state_names);
                 }
@@ -353,6 +529,7 @@ impl Analyzer {
                 }
             }
             Statement::While(cond, body, _) => {
+                self.walk_expr_reads(cond);
                 self.check_expr_type(cond, &NesType::Bool);
                 let was_in_loop = self.in_loop;
                 self.in_loop = true;
@@ -375,17 +552,21 @@ impl Analyzer {
                 }
             }
             Statement::Draw(draw) => {
+                self.walk_expr_reads(&draw.x);
+                self.walk_expr_reads(&draw.y);
                 self.check_expr_type(&draw.x, &NesType::U8);
                 self.check_expr_type(&draw.y, &NesType::U8);
                 if let Some(frame) = &draw.frame {
+                    self.walk_expr_reads(frame);
                     self.check_expr_type(frame, &NesType::U8);
                 }
             }
             Statement::Return(Some(expr), _) => {
                 // For M1, just validate the expression without checking return type
+                self.walk_expr_reads(expr);
                 let _ = self.infer_type(expr);
             }
-            Statement::Call(name, _args, span) => {
+            Statement::Call(name, args, span) => {
                 if !self.symbols.contains_key(name) {
                     self.diagnostics.push(Diagnostic::error(
                         ErrorCode::E0503,
@@ -393,8 +574,13 @@ impl Analyzer {
                         *span,
                     ));
                 }
+                for arg in args {
+                    self.walk_expr_reads(arg);
+                }
             }
             Statement::Scroll(x, y, _) => {
+                self.walk_expr_reads(x);
+                self.walk_expr_reads(y);
                 self.check_expr_type(x, &NesType::U8);
                 self.check_expr_type(y, &NesType::U8);
             }
@@ -518,6 +704,49 @@ impl Analyzer {
     }
 }
 
+/// Collect every state name mentioned in a transition statement inside the
+/// given state's handlers and append them to `queue`. Used by the W0104
+/// unreachable-state check.
+fn collect_transitions_from_state(state: &StateDecl, queue: &mut Vec<String>) {
+    if let Some(block) = &state.on_enter {
+        collect_transitions_block(block, queue);
+    }
+    if let Some(block) = &state.on_exit {
+        collect_transitions_block(block, queue);
+    }
+    if let Some(block) = &state.on_frame {
+        collect_transitions_block(block, queue);
+    }
+    for (_, block) in &state.on_scanline {
+        collect_transitions_block(block, queue);
+    }
+}
+
+fn collect_transitions_block(block: &Block, queue: &mut Vec<String>) {
+    for stmt in &block.statements {
+        collect_transitions_stmt(stmt, queue);
+    }
+}
+
+fn collect_transitions_stmt(stmt: &Statement, queue: &mut Vec<String>) {
+    match stmt {
+        Statement::Transition(name, _) => queue.push(name.clone()),
+        Statement::If(_, then_b, elifs, else_b, _) => {
+            collect_transitions_block(then_b, queue);
+            for (_, b) in elifs {
+                collect_transitions_block(b, queue);
+            }
+            if let Some(b) = else_b {
+                collect_transitions_block(b, queue);
+            }
+        }
+        Statement::While(_, body, _) | Statement::Loop(body, _) => {
+            collect_transitions_block(body, queue);
+        }
+        _ => {}
+    }
+}
+
 /// Collect all function/call names from a block.
 fn collect_calls(block: &Block) -> Vec<String> {
     let mut calls = Vec::new();

src/analyzer/tests.rs

@@ -254,3 +254,179 @@ fn analyze_break_outside_loop() {
         "break outside loop should produce E0203, got: {errors:?}"
     );
 }
+
+#[test]
+fn analyze_unused_variable_warning() {
+    // `ghost` is declared but never read (only the initializer runs).
+    // It should trigger a W0103 warning.
+    let (prog, diags) = parser::parse(
+        r#"
+        game "Test" { mapper: NROM }
+        var ghost: u8 = 0
+        on frame { wait_frame }
+        start Main
+    "#,
+    );
+    assert!(diags.is_empty(), "parse errors: {diags:?}");
+    let result = analyze(&prog.unwrap());
+    assert!(
+        result.diagnostics.iter().any(|d| d.code == ErrorCode::W0103
+            && d.level == crate::errors::Level::Warning
+            && d.message.contains("ghost")),
+        "expected W0103 for unused var 'ghost', got: {:?}",
+        result.diagnostics
+    );
+    // And no hard errors.
+    assert!(
+        result.diagnostics.iter().all(|d| !d.is_error()),
+        "unexpected hard errors: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_unused_variable_no_warning_when_read() {
+    // `counter` is both written and read (in the `if` condition),
+    // so W0103 should NOT fire for it.
+    let (prog, diags) = parser::parse(
+        r#"
+        game "Test" { mapper: NROM }
+        var counter: u8 = 0
+        on frame {
+            counter = counter + 1
+            if counter > 60 { wait_frame }
+        }
+        start Main
+    "#,
+    );
+    assert!(diags.is_empty(), "parse errors: {diags:?}");
+    let result = analyze(&prog.unwrap());
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0103 && d.message.contains("counter")),
+        "did not expect W0103 for read variable 'counter', got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_unused_variable_underscore_prefix_silences() {
+    // A leading underscore silences the W0103 warning, matching Rust's
+    // convention for intentionally-unused names.
+    let (prog, diags) = parser::parse(
+        r#"
+        game "Test" { mapper: NROM }
+        var _reserved: u8 = 0
+        on frame { wait_frame }
+        start Main
+    "#,
+    );
+    assert!(diags.is_empty(), "parse errors: {diags:?}");
+    let result = analyze(&prog.unwrap());
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0103),
+        "did not expect W0103 for underscore-prefixed var, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_unreachable_state_warning() {
+    // `Orphan` is never reached from `Main` — W0104 should fire.
+    let (prog, diags) = parser::parse(
+        r#"
+        game "Test" { mapper: NROM }
+        state Main {
+            on frame { wait_frame }
+        }
+        state Orphan {
+            on frame { wait_frame }
+        }
+        start Main
+    "#,
+    );
+    assert!(diags.is_empty(), "parse errors: {diags:?}");
+    let result = analyze(&prog.unwrap());
+    assert!(
+        result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0104 && d.message.contains("Orphan")),
+        "expected W0104 for unreachable state 'Orphan', got: {:?}",
+        result.diagnostics
+    );
+    // And no hard errors.
+    assert!(
+        result.diagnostics.iter().all(|d| !d.is_error()),
+        "unexpected hard errors: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_reachable_state_no_warning() {
+    // Both states are reachable: Main transitions to Other, and Other
+    // transitions back to Main. Neither should trigger W0104.
+    let (prog, diags) = parser::parse(
+        r#"
+        game "Test" { mapper: NROM }
+        state Main {
+            on frame { transition Other }
+        }
+        state Other {
+            on frame { transition Main }
+        }
+        start Main
+    "#,
+    );
+    assert!(diags.is_empty(), "parse errors: {diags:?}");
+    let result = analyze(&prog.unwrap());
+    assert!(
+        !result
+            .diagnostics
+            .iter()
+            .any(|d| d.code == ErrorCode::W0104),
+        "did not expect any W0104 warnings, got: {:?}",
+        result.diagnostics
+    );
+}
+
+#[test]
+fn analyze_undefined_variable_emits_e0502() {
+    // `ghosy` does not exist; analyzer should emit E0502 and — thanks to
+    // the suggestion helper — hint at `ghost` which is the close match.
+    let (prog, diags) = parser::parse(
+        r#"
+        game "Test" { mapper: NROM }
+        var ghost: u8 = 0
+        var score: u8 = 0
+        on frame {
+            score = ghosy + 1
+        }
+        start Main
+    "#,
+    );
+    assert!(diags.is_empty(), "parse errors: {diags:?}");
+    let result = analyze(&prog.unwrap());
+    let diag = result
+        .diagnostics
+        .iter()
+        .find(|d| d.code == ErrorCode::E0502)
+        .expect("expected E0502 for undefined variable 'ghosy'");
+    assert!(
+        diag.message.contains("ghosy"),
+        "E0502 message should mention 'ghosy', got: {}",
+        diag.message
+    );
+    assert_eq!(
+        diag.help.as_deref(),
+        Some("did you mean 'ghost'?"),
+        "expected suggestion for 'ghost', got: {:?}",
+        diag.help
+    );
+}