ryo_analysis/query/

builder_dataflow_v2.rs

//! DataFlowBuilder: Construct DataFlowGraphV2 from PureFile ASTs
//!
//! Key improvements over V1:
//! - **String-free**: Uses SymbolId for variable names (no String allocation)
//! - **Proper scoping**: Uses $param and $var segments for variables
//! - **VarId-based**: Direct VarId indexing, no petgraph
//! - **V2 API**: Uses `ImHashMap<WorkspaceFilePath, Arc<PureFile>>` for consistency

use super::dataflow_v2::{
    DataFlowGraphV2, FlowData, FlowKind, Guard, GuardKind, ScopeData, ScopeId, ScopeKind, VarData,
    VarKind,
};
use super::lock_v2::{AccessKind, LockAcquisitionV2, LockType};
use super::var_id::VarId;
use crate::ast::ASTRegistry;
use crate::symbol::{SymbolId, SymbolPath, SymbolRegistry, VarScope};
use im::HashMap as ImHashMap;
use ryo_source::pure::{
    PureBlock, PureExpr, PureFile, PureImplItem, PureItem, PureParam, PurePattern, PureStmt,
};
use ryo_symbol::{SymbolPathResolver, WorkspaceFilePath};
use smallvec::smallvec;
use std::collections::{HashMap, HashSet};
use std::sync::Arc;

/// Builder for constructing DataFlowGraphV2 using WorkspaceFilePath-keyed files.
pub struct DataFlowBuilderWorkspace<'a> {
    registry: &'a SymbolRegistry,
    files: &'a ImHashMap<WorkspaceFilePath, Arc<PureFile>>,
    crate_name: &'a str,
}

impl<'a> DataFlowBuilderWorkspace<'a> {
    /// Create a new workspace builder.
    pub fn new(
        registry: &'a SymbolRegistry,
        files: &'a ImHashMap<WorkspaceFilePath, Arc<PureFile>>,
        crate_name: &'a str,
    ) -> Self {
        Self {
            registry,
            files,
            crate_name,
        }
    }

    /// Build the DataFlowGraphV2.
    pub fn build(self) -> DataFlowGraphV2 {
        let estimated_vars = self.files.len() * 10;
        let estimated_flows = self.files.len() * 5;
        let mut graph = DataFlowGraphV2::with_capacity(estimated_vars, estimated_flows);

        let resolver = SymbolPathResolver::new(self.crate_name);
        for (path, file) in self.files {
            // Derive module path from workspace file path
            // Note: Using module_path_str() + parse() to bridge ryo-symbol and ryo-analysis SymbolPath types
            let module_path_str = resolver.module_path_str(path);
            let module_path = SymbolPath::parse(&module_path_str).ok();

            let mut collector = FlowCollectorV2::new(module_path, self.registry, self.crate_name);
            collector.visit_file(file);
            collector.apply_to(&mut graph);
        }

        graph
    }

    /// Incrementally add data from specified files to an existing graph.
    ///
    /// This is used for incremental updates after mutations:
    /// 1. Call `graph.clear_for_symbols()` to remove old data for affected symbols
    /// 2. Call this method to add new data from modified files
    ///
    /// # Arguments
    /// * `graph` - The existing DataFlowGraphV2 to update
    /// * `modified_files` - Only process these files (must be subset of self.files)
    ///
    /// **Deprecated**: Use `build_incremental_by_symbols` for symbol-based updates.
    #[deprecated(
        since = "0.1.0",
        note = "Use build_incremental_by_symbols() for symbol-based updates without file I/O."
    )]
    pub fn build_incremental(
        self,
        graph: &mut DataFlowGraphV2,
        modified_files: &[WorkspaceFilePath],
    ) {
        let resolver = SymbolPathResolver::new(self.crate_name);

        for path in modified_files {
            if let Some(file) = self.files.get(path) {
                let module_path_str = resolver.module_path_str(path);
                let module_path = SymbolPath::parse(&module_path_str).ok();

                let mut collector =
                    FlowCollectorV2::new(module_path, self.registry, self.crate_name);
                collector.visit_file(file);
                collector.apply_to(graph);
            }
        }
    }

    /// Incrementally add data from ASTRegistry for specified symbols (Phase 2).
    ///
    /// This is the symbol-based incremental update path:
    /// 1. Call `graph.clear_for_symbols()` to remove old data for affected symbols
    /// 2. Call this method to add new data directly from ASTRegistry
    ///
    /// # Arguments
    /// * `graph` - The existing DataFlowGraphV2 to update
    /// * `ast_registry` - The ASTRegistry containing PureItem ASTs
    /// * `affected_ids` - Only process these symbol IDs
    pub fn build_incremental_by_symbols(
        &self,
        graph: &mut DataFlowGraphV2,
        ast_registry: &ASTRegistry,
        affected_ids: &[SymbolId],
    ) {
        for &id in affected_ids {
            // Get module path from symbol path
            let module_path = self.registry.resolve(id).and_then(|path| {
                // Get parent path (module containing this symbol)
                let path_str = path.to_string();
                path_str
                    .rsplit_once("::")
                    .and_then(|(parent, _)| SymbolPath::parse(parent).ok())
            });

            // Get AST from ASTRegistry
            if let Some(item) = ast_registry.get(id) {
                let mut collector =
                    FlowCollectorV2::new(module_path, self.registry, self.crate_name);
                collector.visit_item(item);
                collector.apply_to(graph);
            }
        }
    }
}

/// Collected variable info before adding to graph.
struct CollectedVarV2 {
    data: VarData,
    name: String,
    temp_id: usize,
    /// The variable's SymbolId from SymbolRegistry (None for local variables).
    var_symbol_id: Option<SymbolId>,
}

/// Collected flow info before adding to graph.
struct CollectedFlowV2 {
    from_id: usize,
    to_id: usize,
    data: FlowData,
}

/// Collects data flows from a PureFile (V2 - String-free).
struct FlowCollectorV2<'a> {
    /// Module path for this file (e.g., ryo_cli::commands::graph).
    module_path: Option<SymbolPath>,
    registry: &'a SymbolRegistry,
    /// Crate name for fallback paths.
    crate_name: String,
    /// Current function/method scope path.
    current_scope_path: Option<SymbolPath>,
    /// Current symbol ID for the scope.
    current_symbol: Option<SymbolId>,
    /// Collected variables with temporary IDs.
    vars: Vec<CollectedVarV2>,
    /// Variable name -> temp_id map for deduplication (local to current scope).
    var_name_map: HashMap<String, usize>,
    /// Collected flows.
    flows: Vec<CollectedFlowV2>,
    /// Collected scopes.
    collected_scopes: Vec<ScopeData>,
    /// Scope stack (current scope chain).
    scope_stack: Vec<ScopeId>,
    /// Next temp ID.
    next_id: usize,
    /// Current line number.
    current_line: u32,
    /// Collected lock acquisitions (transferred to graph in apply_to).
    collected_locks: Vec<CollectedLockV2>,
    /// Guard temp_ids (for tracking field accesses within critical sections).
    guard_temp_ids: HashSet<usize>,
    /// Collected field accesses on guard variables.
    collected_field_accesses: Vec<CollectedFieldAccessV2>,
    /// Skip lock detection in visit_expr_for_assignments (set during PureStmt::Local init).
    in_local_init: bool,
}

/// Collected lock acquisition info before adding to graph.
struct CollectedLockV2 {
    /// Temp ID of the lock variable (receiver of .lock()/.read()/.write()).
    lock_temp_id: usize,
    /// Temp ID of the guard variable (let guard = ...).
    guard_temp_id: usize,
    /// Type of lock.
    lock_type: LockType,
    /// Line where the lock is acquired.
    line: u32,
    /// Whether this is a try_lock (non-blocking).
    is_try: bool,
    /// Name of the lock variable.
    lock_name: String,
    /// Name of the guard variable.
    guard_name: String,
    /// The function that owns this lock acquisition.
    owner_fn: Option<SymbolId>,
}

/// Collected field access on a guard variable.
struct CollectedFieldAccessV2 {
    /// Temp ID of the guard variable.
    guard_temp_id: usize,
    /// Name of the accessed field.
    field_name: String,
    /// Kind of access (read or write).
    access_kind: AccessKind,
}

impl<'a> FlowCollectorV2<'a> {
    fn new(
        module_path: Option<SymbolPath>,
        registry: &'a SymbolRegistry,
        crate_name: &str,
    ) -> Self {
        Self {
            module_path,
            registry,
            crate_name: crate_name.to_string(),
            current_scope_path: None,
            current_symbol: None,
            vars: Vec::new(),
            var_name_map: HashMap::new(),
            flows: Vec::new(),
            collected_scopes: Vec::new(),
            scope_stack: Vec::new(),
            next_id: 0,
            current_line: 0,
            collected_locks: Vec::new(),
            guard_temp_ids: HashSet::new(),
            collected_field_accesses: Vec::new(),
            in_local_init: false,
        }
    }

    /// Apply collected vars, flows, and scopes to a DataFlowGraphV2.
    fn apply_to(self, graph: &mut DataFlowGraphV2) {
        // Transfer scopes first (ScopeIds are stable indices)
        for scope_data in self.collected_scopes {
            graph.add_scope(scope_data);
        }

        let mut id_to_var: HashMap<usize, VarId> = HashMap::new();

        // Add variables
        for collected in self.vars {
            let var_id = graph.add_var(collected.data, collected.name, collected.var_symbol_id);
            id_to_var.insert(collected.temp_id, var_id);
        }

        // Add flows
        for flow in self.flows {
            if let (Some(&from_var), Some(&to_var)) =
                (id_to_var.get(&flow.from_id), id_to_var.get(&flow.to_id))
            {
                graph.add_flow(from_var, to_var, flow.data);
            }
        }

        // Transfer lock acquisitions
        for lock in self.collected_locks {
            if let (Some(&lock_var), Some(&guard_var)) = (
                id_to_var.get(&lock.lock_temp_id),
                id_to_var.get(&lock.guard_temp_id),
            ) {
                let mut acq = LockAcquisitionV2::new(
                    lock_var,
                    guard_var,
                    lock.lock_type,
                    lock.line,
                    lock.lock_name,
                    lock.guard_name,
                );
                if lock.is_try {
                    acq = acq.with_try();
                }
                if let Some(owner) = lock.owner_fn {
                    acq = acq.with_owner_fn(owner);
                }
                graph.lock_tracker_mut().acquire(acq);
            }
        }

        // Transfer field accesses on guard variables to lock tracker
        for fa in self.collected_field_accesses {
            if let Some(&guard_var) = id_to_var.get(&fa.guard_temp_id) {
                graph.lock_tracker_mut().record_field_access(
                    guard_var,
                    &fa.field_name,
                    fa.access_kind,
                    0, // PureAST has no line info
                );
            }
        }

        // Flush active lock sections to completed.
        // PureAST lacks scope/span info, so release() is never called per-guard.
        // Flushing here ensures stats() counts all acquisitions.
        graph.lock_tracker_mut().flush_active_sections();
    }

    /// Lookup an existing variable by name in current scope.
    ///
    /// Returns the temp ID if found, None otherwise.
    fn lookup_existing_var(&self, name: &str) -> Option<usize> {
        self.var_name_map.get(name).copied()
    }

    /// Lookup existing variable or create new one.
    ///
    /// For path expressions (variable references), this first checks if the
    /// variable already exists as a parameter or local, avoiding duplicate creation.
    /// Returns `None` if outside of a function scope.
    fn lookup_or_create_var(&mut self, name: &str, default_kind: VarKind) -> Option<usize> {
        // First, try to find existing variable
        if let Some(id) = self.lookup_existing_var(name) {
            return Some(id);
        }

        // Not found, create new variable
        self.get_or_create_var(name, default_kind)
    }

    /// Get or create a variable and temp ID.
    ///
    /// Uses name-based deduplication within current scope.
    /// Returns `None` if `current_symbol` is not set (outside function scope).
    fn get_or_create_var(&mut self, name: &str, kind: VarKind) -> Option<usize> {
        // Skip if not inside a function scope
        let parent = self.current_symbol?;

        // Check if we already have this var by name
        if let Some(&id) = self.var_name_map.get(name) {
            return Some(id);
        }

        // Create new var entry
        let id = self.next_id;
        self.next_id += 1;

        // Determine VarScope from VarKind
        let var_scope = match kind {
            VarKind::Parameter => VarScope::Param,
            VarKind::Field => VarScope::Field,
            VarKind::Local | VarKind::Temp | VarKind::Return | VarKind::Static => VarScope::Local,
        };

        // Try to lookup existing variable SymbolId from registry.
        // Returns None for local variables not registered in the registry.
        let var_symbol_id = self.lookup_var_symbol(parent, var_scope, name);

        let data = VarData {
            parent,
            kind,
            line: self.current_line,
            is_mut: false,
        };

        self.vars.push(CollectedVarV2 {
            data,
            name: name.to_string(),
            temp_id: id,
            var_symbol_id,
        });
        self.var_name_map.insert(name.to_string(), id);
        Some(id)
    }

    /// Lookup a variable's SymbolId from the registry.
    ///
    /// Constructs the expected path (parent::$scope::name) and looks it up.
    fn lookup_var_symbol(&self, parent: SymbolId, scope: VarScope, name: &str) -> Option<SymbolId> {
        // Get parent's path
        let parent_path = self.registry.resolve(parent)?;

        // Build the variable path using internal API
        let var_path = parent_path.with_var_scope(scope, name).ok()?;

        // Lookup in registry
        self.registry.lookup(&var_path)
    }

    /// Mark a variable as `mut` binding (e.g. `let mut x = ...`).
    fn set_var_mut(&mut self, temp_id: usize) {
        if let Some(var) = self.vars.iter_mut().find(|v| v.temp_id == temp_id) {
            var.data.is_mut = true;
        }
    }

    /// Get or create the usage sink variable for the current scope.
    ///
    /// All non-assignment usage flows (Argument, Read, Return) point to this
    /// single Temp variable. The important property is that the SOURCE variable
    /// gets an outgoing flow, making it visible to analyses like impact/provenance.
    fn usage_sink(&mut self) -> Option<usize> {
        self.get_or_create_var("$usage", VarKind::Temp)
    }

    /// Track argument usage flows for function/method call arguments.
    ///
    /// Classifies each argument by its syntactic form:
    /// - `&expr`     → `SharedBorrow` (callee receives `&T`)
    /// - `&mut expr` → `MutBorrow`    (callee receives `&mut T`)
    /// - `expr`      → `Argument`     (callee receives `T` by value)
    ///
    /// This classification enables UnnecessaryClone to distinguish between
    /// passing a clone as `&x` (unnecessary) vs `x` (possibly necessary).
    fn track_call_args(&mut self, args: &[PureExpr]) {
        let Some(sink) = self.usage_sink() else {
            return;
        };
        for arg in args {
            // Classify flow kind based on how the argument is passed:
            // - &expr  → SharedBorrow (callee receives immutable reference)
            // - &mut expr → MutBorrow  (callee receives mutable reference)
            // - expr   → Argument     (callee receives ownership / copy)
            let (inner, kind) = match arg {
                PureExpr::Ref {
                    is_mut: false,
                    expr: inner,
                } => (inner.as_ref(), FlowKind::SharedBorrow),
                PureExpr::Ref {
                    is_mut: true,
                    expr: inner,
                } => (inner.as_ref(), FlowKind::MutBorrow),
                other => (other, FlowKind::Argument),
            };
            let sources = self.collect_expr_sources(inner);
            for source_id in sources {
                self.add_flow(source_id, sink, kind);
            }
        }
    }

    /// Track read flow for method call receiver.
    fn track_receiver_read(&mut self, receiver: &PureExpr) {
        let Some(sink) = self.usage_sink() else {
            return;
        };
        let sources = self.collect_expr_sources(receiver);
        for source_id in sources {
            self.add_flow(source_id, sink, FlowKind::Read);
        }
    }

    /// Track receiver as mutably borrowed (for methods like `get_mut`, `iter_mut`).
    fn track_receiver_mut_borrow(&mut self, receiver: &PureExpr) {
        let Some(sink) = self.usage_sink() else {
            return;
        };
        let sources = self.collect_expr_sources(receiver);
        for source_id in sources {
            self.add_flow(source_id, sink, FlowKind::MutBorrow);
        }
    }

    /// Track closure/async capture flows.
    ///
    /// Collects ALL variable references from the body (not just tail expression).
    /// Variables that already exist in the outer scope are considered captured:
    /// - `move` closure/async: Move flow
    /// - non-move: Read flow
    fn track_closure_captures(&mut self, body: &PureExpr, is_move: bool) {
        let Some(sink) = self.usage_sink() else {
            return;
        };
        let existing_vars: Vec<String> = self.var_name_map.keys().cloned().collect();
        // Collect ALL variable names referenced anywhere in the body
        let mut ref_names = Vec::new();
        Self::collect_all_referenced_names(body, &mut ref_names);
        let kind = if is_move {
            FlowKind::Move
        } else {
            FlowKind::Read
        };
        for name in ref_names {
            if existing_vars.contains(&name) {
                if let Some(&var_id) = self.var_name_map.get(&name) {
                    self.add_flow(var_id, sink, kind);
                }
            }
        }
    }

    /// Recursively collect all variable names referenced in an expression tree.
    ///
    /// Unlike `collect_expr_sources` which only finds value-producing sources,
    /// this traverses the entire AST including all statements and sub-expressions.
    fn collect_all_referenced_names(expr: &PureExpr, names: &mut Vec<String>) {
        match expr {
            PureExpr::Path(name) => {
                let simple = name.split("::").last().unwrap_or(name);
                names.push(simple.to_string());
            }
            PureExpr::Field { expr, .. } => {
                Self::collect_all_referenced_names(expr, names);
            }
            PureExpr::Binary { left, right, .. } => {
                Self::collect_all_referenced_names(left, names);
                Self::collect_all_referenced_names(right, names);
            }
            PureExpr::Unary { expr, .. }
            | PureExpr::Ref { expr, .. }
            | PureExpr::Await(expr)
            | PureExpr::Try(expr)
            | PureExpr::Cast { expr, .. } => {
                Self::collect_all_referenced_names(expr, names);
            }
            PureExpr::Call { func, args } => {
                Self::collect_all_referenced_names(func, names);
                for arg in args {
                    Self::collect_all_referenced_names(arg, names);
                }
            }
            PureExpr::MethodCall { receiver, args, .. } => {
                Self::collect_all_referenced_names(receiver, names);
                for arg in args {
                    Self::collect_all_referenced_names(arg, names);
                }
            }
            PureExpr::Tuple(exprs) | PureExpr::Array(exprs) => {
                for e in exprs {
                    Self::collect_all_referenced_names(e, names);
                }
            }
            PureExpr::Struct { fields, .. } => {
                for (_, e) in fields {
                    Self::collect_all_referenced_names(e, names);
                }
            }
            PureExpr::Index { expr, index } => {
                Self::collect_all_referenced_names(expr, names);
                Self::collect_all_referenced_names(index, names);
            }
            PureExpr::Block { block, .. } | PureExpr::Unsafe(block) => {
                Self::collect_all_referenced_names_block(block, names);
            }
            PureExpr::Async { body, .. } => {
                Self::collect_all_referenced_names_block(body, names);
            }
            PureExpr::If {
                cond,
                then_branch,
                else_branch,
            } => {
                Self::collect_all_referenced_names(cond, names);
                Self::collect_all_referenced_names_block(then_branch, names);
                if let Some(else_expr) = else_branch {
                    Self::collect_all_referenced_names(else_expr, names);
                }
            }
            PureExpr::Match { expr, arms } => {
                Self::collect_all_referenced_names(expr, names);
                for arm in arms {
                    if let Some(guard) = &arm.guard {
                        Self::collect_all_referenced_names(guard, names);
                    }
                    Self::collect_all_referenced_names(&arm.body, names);
                }
            }
            PureExpr::Loop { body, .. } => {
                Self::collect_all_referenced_names_block(body, names);
            }
            PureExpr::While { cond, body, .. } => {
                Self::collect_all_referenced_names(cond, names);
                Self::collect_all_referenced_names_block(body, names);
            }
            PureExpr::For { expr, body, .. } => {
                Self::collect_all_referenced_names(expr, names);
                Self::collect_all_referenced_names_block(body, names);
            }
            PureExpr::Closure { body, .. } => {
                Self::collect_all_referenced_names(body, names);
            }
            PureExpr::Return(Some(expr))
            | PureExpr::Break {
                expr: Some(expr), ..
            } => {
                Self::collect_all_referenced_names(expr, names);
            }
            PureExpr::Let { expr, .. } => {
                Self::collect_all_referenced_names(expr, names);
            }
            _ => {} // Lit, Macro, Continue, Return(None), Break(None), etc.
        }
    }

    /// Collect all referenced variable names from a block's statements.
    fn collect_all_referenced_names_block(block: &PureBlock, names: &mut Vec<String>) {
        for stmt in &block.stmts {
            match stmt {
                PureStmt::Expr(e) | PureStmt::Semi(e) => {
                    Self::collect_all_referenced_names(e, names);
                }
                PureStmt::Local { init, .. } => {
                    if let Some(expr) = init {
                        Self::collect_all_referenced_names(expr, names);
                    }
                }
                PureStmt::Item(_) => {}
            }
        }
    }

    /// Extract variable references from macro tokens heuristically.
    ///
    /// Scans tokens for identifiers matching known variables and creates Read flows.
    /// Handles common macros like `format!`, `println!`, `vec!`, etc.
    fn track_macro_var_refs(&mut self, tokens: &str) {
        let Some(sink) = self.usage_sink() else {
            return;
        };
        // Extract potential identifiers from tokens (simple word boundary scan)
        for token in tokens.split(|c: char| !c.is_alphanumeric() && c != '_') {
            if token.is_empty() || token.starts_with(|c: char| c.is_ascii_digit()) {
                continue;
            }
            // Check if this token matches a known variable
            if let Some(&var_id) = self.var_name_map.get(token) {
                self.add_flow(var_id, sink, FlowKind::Read);
            }
        }
    }

    /// Bind pattern variables from match arm patterns.
    ///
    /// Creates local variables for identifier patterns and assigns them
    /// from the scrutinee expression sources.
    fn bind_pattern_vars(&mut self, pattern: &PurePattern, scrutinee_sources: &[usize]) {
        match pattern {
            PurePattern::Ident { name, .. } => {
                if let Some(target_id) = self.get_or_create_var(name, VarKind::Local) {
                    for &source_id in scrutinee_sources {
                        self.add_flow(source_id, target_id, FlowKind::Assign);
                    }
                }
            }
            PurePattern::Tuple(pats) | PurePattern::Slice(pats) | PurePattern::Or(pats) => {
                for pat in pats {
                    self.bind_pattern_vars(pat, scrutinee_sources);
                }
            }
            PurePattern::Struct { fields, .. } => {
                for (_, pat) in fields {
                    self.bind_pattern_vars(pat, scrutinee_sources);
                }
            }
            PurePattern::Ref { pattern, .. } => {
                self.bind_pattern_vars(pattern, scrutinee_sources);
            }
            _ => {} // Wild, Lit, Path, Range, Rest, Other — no variable binding
        }
    }

    /// Track return flow for return expressions.
    fn track_return(&mut self, expr: &PureExpr) {
        let Some(sink) = self.usage_sink() else {
            return;
        };
        let sources = self.collect_expr_sources(expr);
        for source_id in sources {
            self.add_flow(source_id, sink, FlowKind::Return);
        }
    }

    /// Add a data flow edge (tagged with current scope).
    fn add_flow(&mut self, from_id: usize, to_id: usize, kind: FlowKind) {
        let scope = self.current_scope();
        self.flows.push(CollectedFlowV2 {
            from_id,
            to_id,
            data: FlowData {
                kind,
                line: self.current_line,
                scope,
            },
        });
    }

    // ========== Scope Management ==========

    /// Get the current scope (top of scope stack).
    fn current_scope(&self) -> ScopeId {
        self.scope_stack
            .last()
            .copied()
            .unwrap_or(ScopeId::from_raw(0))
    }

    /// Push a new scope onto the stack.
    fn push_scope(&mut self, kind: ScopeKind, guard: Option<Guard>) {
        let parent = self.scope_stack.last().copied();
        let id = ScopeId::from_raw(self.collected_scopes.len() as u32);
        self.collected_scopes.push(ScopeData {
            parent,
            kind,
            guard,
        });
        self.scope_stack.push(id);
    }

    /// Pop the current scope from the stack.
    fn pop_scope(&mut self) {
        self.scope_stack.pop();
    }

    // ========== Guard Parsing ==========

    /// Parse a condition expression to extract guard constraints.
    ///
    /// Handles: `a < b`, `a <= b`, `a > b`, `a >= b`,
    /// `expr.is_some()`, `expr.is_ok()`, `if let Some(x) = expr`, `if let Ok(x) = expr`.
    fn parse_guard(&mut self, cond: &PureExpr) -> Option<Guard> {
        match cond {
            // a < b, a <= b, a > b, a >= b
            PureExpr::Binary { op, left, right } => {
                let (kind, left_expr, _right_expr) = match op.as_str() {
                    "<" => (GuardKind::LessThan, left, right),
                    "<=" => (GuardKind::LessEqual, left, right),
                    // a > b => b < a (constrained var is right)
                    ">" => (GuardKind::LessThan, right, left),
                    ">=" => (GuardKind::LessEqual, right, left),
                    // && — merge both sides
                    "&&" => {
                        // Return the first parseable guard (P0 simplification)
                        return self.parse_guard(left).or_else(|| self.parse_guard(right));
                    }
                    _ => return None,
                };
                // Extract constrained variable
                let var_name = extract_expr_name(left_expr)?;
                Some(Guard {
                    kind,
                    var_names: smallvec![var_name],
                })
            }
            // expr.is_some(), expr.is_ok()
            PureExpr::MethodCall {
                receiver, method, ..
            } => {
                let kind = match method.as_str() {
                    "is_some" => GuardKind::IsSome,
                    "is_ok" => GuardKind::IsOk,
                    _ => return None,
                };
                let var_name = extract_expr_name(receiver)?;
                Some(Guard {
                    kind,
                    var_names: smallvec![var_name],
                })
            }
            // if let Some(x) = expr / if let Ok(x) = expr
            PureExpr::Let { pattern, expr } => {
                let kind = match pattern {
                    PurePattern::Struct { path, .. } | PurePattern::Path(path) => {
                        if path == "Some" || path.ends_with("::Some") {
                            GuardKind::LetSome
                        } else if path == "Ok" || path.ends_with("::Ok") {
                            GuardKind::LetOk
                        } else {
                            return None;
                        }
                    }
                    _ => return None,
                };
                let var_name = extract_expr_name(expr)?;
                Some(Guard {
                    kind,
                    var_names: smallvec![var_name],
                })
            }
            _ => None,
        }
    }

    /// Parse guard from a match arm pattern (e.g., Some(x) → LetSome on scrutinee).
    fn parse_match_arm_guard(&self, _pattern: &PurePattern) -> Option<Guard> {
        // TODO: P1 — extract variant guards from match patterns
        None
    }

    /// Emit FlowKind::Conditional from condition variable references.
    fn emit_conditional_flow(&mut self, cond: &PureExpr) {
        let mut names = Vec::new();
        FlowCollectorV2::collect_all_referenced_names(cond, &mut names);
        // Create pairwise Conditional flows between condition variables
        let var_ids: Vec<usize> = names
            .iter()
            .filter_map(|name| self.lookup_existing_var(name))
            .collect();
        // Emit Conditional from each var to a synthetic "condition" node
        // For P0: emit a self-referencing Conditional to mark these vars as condition participants
        for &var_id in &var_ids {
            self.add_flow(var_id, var_id, FlowKind::Conditional);
        }
    }

    fn visit_file(&mut self, file: &PureFile) {
        for item in &file.items {
            self.visit_item(item);
        }
    }

    fn visit_item(&mut self, item: &PureItem) {
        match item {
            PureItem::Fn(f) => {
                // Build scope path: module_path::function_name
                let scope_path = self
                    .module_path
                    .as_ref()
                    .and_then(|mp| mp.child(&f.name).ok())
                    .unwrap_or_else(|| {
                        SymbolPath::parse(&format!("{}::unknown::{}", self.crate_name, f.name))
                            .unwrap_or_else(|_| {
                                SymbolPath::parse(&format!("{}::unknown", self.crate_name)).unwrap()
                            })
                    });

                self.current_scope_path = Some(scope_path.clone());
                self.current_line = 0;
                // Clear var_name_map for new scope
                self.var_name_map.clear();
                self.guard_temp_ids.clear();

                // Lookup symbol for this function (already registered in Phase 2)
                self.current_symbol = self.registry.lookup(&scope_path);

                // Push function scope
                self.push_scope(ScopeKind::Function, None);

                // Add parameters
                for param in &f.params {
                    if let Some(name) = extract_param_name(param) {
                        self.get_or_create_var(&name, VarKind::Parameter);
                    }
                }

                self.visit_block(&f.body);
                self.pop_scope();
                self.current_scope_path = None;
                self.current_symbol = None;
            }
            PureItem::Impl(i) => {
                for impl_item in &i.items {
                    self.visit_impl_item(impl_item, &i.self_ty, i.trait_.as_deref());
                }
            }
            PureItem::Mod(m) => {
                for item in &m.items {
                    self.visit_item(item);
                }
            }
            // Static and Const are outside function scope, skip dataflow tracking
            PureItem::Static(_) | PureItem::Const(_) => {}
            _ => {}
        }
    }

    fn visit_impl_item(&mut self, item: &PureImplItem, self_ty: &str, trait_: Option<&str>) {
        if let PureImplItem::Fn(f) = item {
            // Build scope path matching context.rs registration:
            // - Plain impl: module::Type::method
            // - Trait impl: module::<impl Trait for Type>::method
            let scope_path = self
                .module_path
                .as_ref()
                .and_then(|mp| {
                    let method_base = if let Some(trait_name) = trait_ {
                        mp.child_trait_impl(trait_name, self_ty)
                    } else {
                        // Strip generic parameters: "Router < S >" → "Router"
                        let base_type = self_ty.split('<').next().unwrap_or(self_ty).trim();
                        mp.child(base_type).ok()?
                    };
                    method_base.child(&f.name).ok()
                })
                .unwrap_or_else(|| {
                    SymbolPath::parse(&format!(
                        "{}::unknown::{}::{}",
                        self.crate_name, self_ty, f.name
                    ))
                    .unwrap_or_else(|_| {
                        SymbolPath::parse(&format!("{}::unknown", self.crate_name)).unwrap()
                    })
                });

            self.current_scope_path = Some(scope_path.clone());
            self.current_line = 0;
            // Clear var_name_map for new scope
            self.var_name_map.clear();

            // Lookup symbol for this method (already registered in Phase 2)
            self.current_symbol = self.registry.lookup(&scope_path);

            // Push function scope
            self.push_scope(ScopeKind::Function, None);

            // Add parameters (skip self)
            for param in &f.params {
                if let Some(name) = extract_param_name(param) {
                    if name != "self" {
                        self.get_or_create_var(&name, VarKind::Parameter);
                    }
                }
            }

            self.visit_block(&f.body);
            self.pop_scope();
            self.current_scope_path = None;
            self.current_symbol = None;
        }
    }

    fn visit_block(&mut self, block: &PureBlock) {
        for stmt in &block.stmts {
            self.visit_stmt(stmt);
        }
        // Treat tail expression (last Expr without semicolon) as implicit return
        if let Some(PureStmt::Expr(expr)) = block.stmts.last() {
            self.track_return(expr);
        }
    }

    fn visit_stmt(&mut self, stmt: &PureStmt) {
        match stmt {
            PureStmt::Local { pattern, init, .. } => {
                if let Some(name) = extract_pattern_name(pattern) {
                    if let Some(target_id) = self.get_or_create_var(&name, VarKind::Local) {
                        // Track `let mut x` binding mutability
                        if matches!(pattern, PurePattern::Ident { is_mut: true, .. }) {
                            self.set_var_mut(target_id);
                        }
                        if let Some(expr) = init {
                            // Check if this is a lock acquisition
                            // Patterns: mutex.lock(), mutex.lock().unwrap(), rwlock.read(), etc.
                            if let Some((lock_receiver, lock_type, is_try)) =
                                extract_lock_call(expr)
                            {
                                let lock_sources = self.collect_expr_sources(lock_receiver);
                                if let Some(&lock_source_id) = lock_sources.first() {
                                    let lock_name = extract_expr_name(lock_receiver)
                                        .unwrap_or_else(|| "?".to_string());
                                    self.collected_locks.push(CollectedLockV2 {
                                        lock_temp_id: lock_source_id,
                                        guard_temp_id: target_id,
                                        lock_type,
                                        line: self.current_line,
                                        is_try,
                                        lock_name,
                                        guard_name: name.clone(),
                                        owner_fn: self.current_symbol,
                                    });
                                    self.guard_temp_ids.insert(target_id);
                                }
                            }
                            // Check if this is a clone() call
                            if let Some(clone_receiver) = is_clone_call(expr) {
                                // Clone flow: receiver -> target
                                let sources = self.collect_expr_sources(clone_receiver);
                                for source_id in sources {
                                    self.add_flow(source_id, target_id, FlowKind::Clone);
                                }
                            } else if let Some((is_mut, inner)) = is_borrow_expr(expr) {
                                // Borrow flow: &source -> target or &mut source -> target
                                let kind = if is_mut {
                                    FlowKind::MutBorrow
                                } else {
                                    FlowKind::SharedBorrow
                                };
                                let sources = self.collect_expr_sources(inner);
                                for source_id in sources {
                                    self.add_flow(source_id, target_id, kind);
                                }
                            } else {
                                // Normal assignment flow
                                let sources = self.collect_expr_sources(expr);
                                for source_id in sources {
                                    self.add_flow(source_id, target_id, FlowKind::Assign);
                                }
                            }
                            // Also visit init expr for nested side effects
                            // (closures, calls, method calls, etc.)
                            // Skip lock detection since it was already handled above.
                            self.in_local_init = true;
                            self.visit_expr_for_assignments(expr);
                            self.in_local_init = false;
                        }
                    }
                } else if let Some(expr) = init {
                    // Wildcard or unnamed pattern (e.g., `let _ = foo(&x);`).
                    // Still visit the init expression for side effects so that
                    // variable usages inside it are tracked.
                    self.in_local_init = true;
                    self.visit_expr_for_assignments(expr);
                    self.in_local_init = false;
                }
            }
            PureStmt::Expr(expr) | PureStmt::Semi(expr) => {
                self.visit_expr_for_assignments(expr);
            }
            PureStmt::Item(item) => {
                self.visit_item(item);
            }
        }
    }

    fn visit_expr_for_assignments(&mut self, expr: &PureExpr) {
        match expr {
            PureExpr::Binary { op, left, right } if op == "=" => {
                if let Some((name, kind)) = classify_lvalue(left) {
                    if let Some(target_id) = self.get_or_create_var(&name, kind) {
                        let sources = self.collect_expr_sources(right);

                        for source_id in sources {
                            self.add_flow(source_id, target_id, FlowKind::Assign);
                        }
                    }
                }
                // Track guard.field = expr as Write field access
                self.track_guard_field_access(left, AccessKind::Write);
                self.visit_expr_for_assignments(right);
            }
            PureExpr::Binary { left, right, .. } => {
                self.visit_expr_for_assignments(left);
                self.visit_expr_for_assignments(right);
            }
            PureExpr::Block { block, .. } => {
                self.push_scope(ScopeKind::Block, None);
                self.visit_block(block);
                self.pop_scope();
            }
            PureExpr::If {
                cond,
                then_branch,
                else_branch,
            } => {
                self.visit_expr_for_assignments(cond);
                // Emit Conditional flow for condition variables
                self.emit_conditional_flow(cond);
                // Parse guard from condition
                let guard = self.parse_guard(cond);
                self.push_scope(ScopeKind::IfThen, guard);
                self.visit_block(then_branch);
                self.pop_scope();
                if let Some(else_expr) = else_branch {
                    // TODO: negated guard for else branch
                    self.push_scope(ScopeKind::IfElse, None);
                    self.visit_expr_for_assignments(else_expr);
                    self.pop_scope();
                }
            }
            PureExpr::Match { expr, arms } => {
                self.visit_expr_for_assignments(expr);
                // Track scrutinee as Read
                self.track_receiver_read(expr);
                // Process each arm
                let scrutinee_sources = self.collect_expr_sources(expr);
                for arm in arms {
                    // Parse guard from match arm pattern
                    let arm_guard = self.parse_match_arm_guard(&arm.pattern);
                    self.push_scope(ScopeKind::MatchArm, arm_guard);
                    // Create pattern variable bindings from scrutinee
                    self.bind_pattern_vars(&arm.pattern, &scrutinee_sources);
                    if let Some(guard) = &arm.guard {
                        self.visit_expr_for_assignments(guard);
                    }
                    self.visit_expr_for_assignments(&arm.body);
                    self.pop_scope();
                }
            }
            PureExpr::Loop { body: block, .. } => {
                self.push_scope(ScopeKind::LoopBody, None);
                self.visit_block(block);
                self.pop_scope();
            }
            PureExpr::While { cond, body, .. } => {
                self.visit_expr_for_assignments(cond);
                self.track_receiver_read(cond);
                self.emit_conditional_flow(cond);
                let guard = self.parse_guard(cond);
                self.push_scope(ScopeKind::WhileBody, guard);
                self.visit_block(body);
                self.pop_scope();
            }
            PureExpr::For {
                pat, expr, body, ..
            } => {
                self.visit_expr_for_assignments(expr);
                // Track iterator → pattern variable binding
                if let Some(name) = extract_pattern_name(pat) {
                    if let Some(target_id) = self.get_or_create_var(&name, VarKind::Local) {
                        let sources = self.collect_expr_sources(expr);
                        for source_id in sources {
                            self.add_flow(source_id, target_id, FlowKind::Assign);
                        }
                    }
                }
                // Also track iterator as Read usage
                self.track_receiver_read(expr);
                // For loops implicitly bound the iterator variable
                self.push_scope(ScopeKind::ForBody, None);
                self.visit_block(body);
                self.pop_scope();
            }
            PureExpr::Closure { body, is_move, .. } => {
                // Track captured variables before visiting body
                self.track_closure_captures(body, *is_move);
                self.push_scope(ScopeKind::Closure, None);
                self.visit_expr_for_assignments(body);
                self.pop_scope();
            }
            PureExpr::Call { func, args } => {
                self.visit_expr_for_assignments(func);
                for arg in args {
                    self.visit_expr_for_assignments(arg);
                }
                // Track argument usage flows
                self.track_call_args(args);
            }
            PureExpr::MethodCall {
                receiver,
                method,
                args,
                ..
            } => {
                self.visit_expr_for_assignments(receiver);
                for arg in args {
                    self.visit_expr_for_assignments(arg);
                }
                // Detect lock calls in standalone expressions (not bound to let)
                // e.g. match &mut *self.mutex.lock().unwrap() { ... }
                // Skip if already detected in PureStmt::Local init.
                if !self.in_local_init {
                    if let Some((lock_receiver, lock_type, is_try)) = extract_lock_call(expr) {
                        let lock_sources = self.collect_expr_sources(lock_receiver);
                        if let Some(&lock_source_id) = lock_sources.first() {
                            let lock_name =
                                extract_expr_name(lock_receiver).unwrap_or_else(|| "?".to_string());
                            let guard_name = format!("_anon_guard_{}", self.current_line);
                            if let Some(guard_id) =
                                self.get_or_create_var(&guard_name, VarKind::Temp)
                            {
                                self.collected_locks.push(CollectedLockV2 {
                                    lock_temp_id: lock_source_id,
                                    guard_temp_id: guard_id,
                                    lock_type,
                                    line: self.current_line,
                                    is_try,
                                    lock_name,
                                    guard_name,
                                    owner_fn: self.current_symbol,
                                });
                                self.guard_temp_ids.insert(guard_id);
                            }
                        }
                    }
                }
                // Track receiver flow based on method name heuristic.
                //
                // Methods ending in `_mut` (get_mut, iter_mut, as_mut, etc.)
                // follow Rust's naming convention for `&mut self` receivers.
                // Classify those as MutBorrow so downstream analyses (e.g.
                // UnnecessaryClone) know the receiver is mutably borrowed.
                //
                // Limitation: mutating methods that don't follow the `_mut`
                // convention (e.g. `push`, `insert`, `set_extension`) are
                // classified as Read. For clone analysis, the `let mut`
                // binding check in UnnecessaryClone compensates for this.
                if method.ends_with("_mut") {
                    self.track_receiver_mut_borrow(receiver);
                } else {
                    self.track_receiver_read(receiver);
                }
                self.track_call_args(args);
            }
            PureExpr::Return(Some(expr)) => {
                self.visit_expr_for_assignments(expr);
                self.track_return(expr);
            }
            PureExpr::Return(None) => {}
            PureExpr::Break {
                expr: Some(expr), ..
            } => {
                self.visit_expr_for_assignments(expr);
                self.track_return(expr); // break-with-value is semantically similar to return
            }
            PureExpr::Struct { fields, .. } => {
                // Track field values as Read usage
                for (_, value_expr) in fields {
                    self.visit_expr_for_assignments(value_expr);
                    self.track_receiver_read(value_expr);
                }
            }
            PureExpr::Async { is_move, body } => {
                if *is_move {
                    let body_expr = PureExpr::Block {
                        label: None,
                        block: body.clone(),
                    };
                    self.track_closure_captures(&body_expr, true);
                }
                self.push_scope(ScopeKind::Block, None);
                self.visit_block(body);
                self.pop_scope();
            }
            PureExpr::Macro { tokens, .. } => {
                // Extract variable references from macro tokens heuristically
                self.track_macro_var_refs(tokens);
            }
            PureExpr::Index { expr, index } => {
                // Track collection[index] as Index flow
                self.visit_expr_for_assignments(expr);
                self.visit_expr_for_assignments(index);
                // Emit FlowKind::Index: index_var → collection_var
                if let Some(index_name) = extract_expr_name(index) {
                    if let Some(index_id) = self.lookup_existing_var(&index_name) {
                        if let Some(coll_name) = extract_expr_name(expr) {
                            if let Some(coll_id) = self.lookup_existing_var(&coll_name) {
                                self.add_flow(index_id, coll_id, FlowKind::Index);
                            }
                        }
                    }
                }
            }
            PureExpr::Let { expr, .. } => {
                // while let / if let: visit and track the inner expression
                self.visit_expr_for_assignments(expr);
                self.track_receiver_read(expr);
            }
            PureExpr::Field { .. } => {
                // Track guard.field as Read field access
                self.track_guard_field_access(expr, AccessKind::Read);
            }
            _ => {}
        }
    }

    /// Track field access on a guard variable (e.g., `guard.field`).
    ///
    /// If `expr` is `PureExpr::Field { expr: Path(guard_name), field }` and
    /// `guard_name` is a known lock guard, records the field access.
    fn track_guard_field_access(&mut self, expr: &PureExpr, access_kind: AccessKind) {
        if let PureExpr::Field {
            expr: receiver,
            field,
        } = expr
        {
            if let PureExpr::Path(name) = &**receiver {
                if let Some(&guard_id) = self.var_name_map.get(name.as_str()) {
                    if self.guard_temp_ids.contains(&guard_id) {
                        self.collected_field_accesses.push(CollectedFieldAccessV2 {
                            guard_temp_id: guard_id,
                            field_name: field.clone(),
                            access_kind,
                        });
                    }
                }
            }
        }
    }

    fn collect_expr_sources(&mut self, expr: &PureExpr) -> Vec<usize> {
        let mut sources = Vec::new();
        self.collect_sources_recursive(expr, &mut sources);
        sources
    }

    fn collect_sources_recursive(&mut self, expr: &PureExpr, sources: &mut Vec<usize>) {
        match expr {
            PureExpr::Path(name) => {
                let simple_name = name.split("::").last().unwrap_or(name);
                // Use lookup_or_create to avoid duplicating params as locals
                if let Some(id) = self.lookup_or_create_var(simple_name, VarKind::Local) {
                    sources.push(id);
                }
            }
            PureExpr::Field { expr, field } => {
                if is_self_expr(expr) {
                    // Use just the field name (not "self.field") since $field scope identifies it
                    if let Some(id) = self.get_or_create_var(field, VarKind::Field) {
                        sources.push(id);
                    }
                } else {
                    self.collect_sources_recursive(expr, sources);
                }
            }
            PureExpr::Binary { left, right, .. } => {
                self.collect_sources_recursive(left, sources);
                self.collect_sources_recursive(right, sources);
            }
            PureExpr::Unary { expr, .. } => {
                self.collect_sources_recursive(expr, sources);
            }
            PureExpr::Call { func, args } => {
                for arg in args {
                    self.collect_sources_recursive(arg, sources);
                }
                self.collect_sources_recursive(func, sources);
            }
            PureExpr::MethodCall { receiver, args, .. } => {
                self.collect_sources_recursive(receiver, sources);
                for arg in args {
                    self.collect_sources_recursive(arg, sources);
                }
            }
            PureExpr::If {
                cond,
                then_branch,
                else_branch,
            } => {
                self.collect_sources_recursive(cond, sources);
                if let Some(PureStmt::Expr(e)) = then_branch.stmts.last() {
                    self.collect_sources_recursive(e, sources);
                }
                if let Some(else_expr) = else_branch {
                    self.collect_sources_recursive(else_expr, sources);
                }
            }
            PureExpr::Block { block, .. } => {
                if let Some(PureStmt::Expr(e)) = block.stmts.last() {
                    self.collect_sources_recursive(e, sources);
                }
            }
            PureExpr::Match { expr, arms } => {
                self.collect_sources_recursive(expr, sources);
                for arm in arms {
                    self.collect_sources_recursive(&arm.body, sources);
                }
            }
            PureExpr::Tuple(exprs) | PureExpr::Array(exprs) => {
                for e in exprs {
                    self.collect_sources_recursive(e, sources);
                }
            }
            PureExpr::Index { expr, index } => {
                self.collect_sources_recursive(expr, sources);
                self.collect_sources_recursive(index, sources);
            }
            PureExpr::Ref { expr, .. } | PureExpr::Await(expr) | PureExpr::Try(expr) => {
                self.collect_sources_recursive(expr, sources);
            }
            PureExpr::Struct { fields, .. } => {
                for (_, e) in fields {
                    self.collect_sources_recursive(e, sources);
                }
            }
            _ => {}
        }
    }
}

fn extract_param_name(param: &PureParam) -> Option<String> {
    match param {
        PureParam::SelfValue { .. } => Some("self".to_string()),
        PureParam::Typed { name, .. } => Some(name.clone()),
    }
}

fn extract_pattern_name(pat: &PurePattern) -> Option<String> {
    match pat {
        PurePattern::Ident { name, .. } => Some(name.clone()),
        PurePattern::Ref { pattern, .. } => extract_pattern_name(pattern),
        _ => None,
    }
}

/// Extract a simple variable name from an expression.
///
/// Handles `Path("x")`, `Field { expr: Path("self"), field: "x" }`.
fn extract_expr_name(expr: &PureExpr) -> Option<String> {
    match expr {
        PureExpr::Path(name) => Some(name.clone()),
        PureExpr::Field { expr, field } if is_self_expr(expr) => Some(field.clone()),
        PureExpr::Field { expr, field } => {
            // Non-self field: "receiver.field" (e.g., shared.inner)
            if let Some(receiver_name) = extract_expr_name(expr) {
                Some(format!("{}.{}", receiver_name, field))
            } else {
                Some(field.clone())
            }
        }
        _ => None,
    }
}

fn classify_lvalue(expr: &PureExpr) -> Option<(String, VarKind)> {
    match expr {
        PureExpr::Path(name) => Some((name.clone(), VarKind::Local)),
        PureExpr::Field { expr, field } => {
            if is_self_expr(expr) {
                // Use just the field name (not "self.field") since $field scope identifies it
                Some((field.clone(), VarKind::Field))
            } else {
                None
            }
        }
        _ => None,
    }
}

fn is_self_expr(expr: &PureExpr) -> bool {
    matches!(expr, PureExpr::Path(name) if name == "self")
}

/// Check if an expression is a borrow (`&x` or `&mut x`).
///
/// Returns `(is_mut, inner_expr)` if this is a reference expression.
fn is_borrow_expr(expr: &PureExpr) -> Option<(bool, &PureExpr)> {
    match expr {
        PureExpr::Ref { is_mut, expr } => Some((*is_mut, expr)),
        _ => None,
    }
}

/// Check if an expression is a clone() method call.
///
/// Returns the receiver expression if this is a `.clone()` call.
fn is_clone_call(expr: &PureExpr) -> Option<&PureExpr> {
    match expr {
        PureExpr::MethodCall {
            receiver,
            method,
            args,
            ..
        } if method == "clone" && args.is_empty() => Some(receiver),
        _ => None,
    }
}

/// Classify a method name as a lock acquisition.
///
/// Returns `(LockType, is_try)` if the method is a lock-related call.
fn classify_lock_method(method: &str) -> Option<(LockType, bool)> {
    match method {
        "lock" => Some((LockType::Mutex, false)),
        "try_lock" => Some((LockType::Mutex, true)),
        "read" => Some((LockType::RwLockRead, false)),
        "try_read" => Some((LockType::RwLockRead, true)),
        "write" => Some((LockType::RwLockWrite, false)),
        "try_write" => Some((LockType::RwLockWrite, true)),
        "borrow" => Some((LockType::RefCell, false)),
        "borrow_mut" => Some((LockType::RefCellMut, false)),
        _ => None,
    }
}

/// Extract a lock acquisition from an expression.
///
/// Handles common patterns:
/// - `receiver.lock()` / `receiver.read()` / `receiver.write()`
/// - `receiver.lock().unwrap()` / `receiver.lock().expect("...")`
/// - `receiver.lock().await` (tokio)
/// - `receiver.try_lock()`
///
/// Returns `(lock_receiver, LockType, is_try)`.
fn extract_lock_call(expr: &PureExpr) -> Option<(&PureExpr, LockType, bool)> {
    match expr {
        PureExpr::MethodCall {
            receiver,
            method,
            args,
            ..
        } => {
            // Direct lock call: receiver.lock(), receiver.read(), etc.
            if args.is_empty() {
                if let Some((lock_type, is_try)) = classify_lock_method(method) {
                    return Some((receiver, lock_type, is_try));
                }
            }
            // Chain suffix: receiver.lock().unwrap() / .expect("...") / .take() / etc.
            // Any no-arg method call on a lock result is traversed through.
            if args.is_empty() {
                return extract_lock_call(receiver);
            }
            None
        }
        // receiver.lock().await (tokio)
        PureExpr::Await(inner) => extract_lock_call(inner),
        // receiver.lock()? (try operator)
        PureExpr::Try(inner) => extract_lock_call(inner),
        _ => None,
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::symbol::SymbolKind;
    use ryo_source::pure::{
        MacroDelimiter, PureFn, PureGenerics, PureImpl, PureImplItem, PureMatchArm, PureVis,
    };

    /// Build a DataFlowGraphV2 from a single function definition.
    ///
    /// Registers the function in a fresh SymbolRegistry so that
    /// FlowCollectorV2 can resolve `current_symbol`.
    fn build_graph_for_fn(fn_def: PureFn) -> DataFlowGraphV2 {
        let mut registry = SymbolRegistry::new();
        let module_path = SymbolPath::parse("test_crate").unwrap();
        let fn_path = module_path.child(&fn_def.name).unwrap();
        registry.register(fn_path, SymbolKind::Function).unwrap();

        let file = PureFile {
            attrs: vec![],
            items: vec![PureItem::Fn(fn_def)],
        };

        let mut graph = DataFlowGraphV2::new();
        let mut collector = FlowCollectorV2::new(Some(module_path), &registry, "test_crate");
        collector.visit_file(&file);
        collector.apply_to(&mut graph);
        graph
    }

    fn make_fn(name: &str, stmts: Vec<PureStmt>) -> PureFn {
        PureFn {
            attrs: vec![],
            vis: PureVis::Public,
            is_async: false,
            is_async_inferred: false,
            is_const: false,
            is_unsafe: false,
            abi: None,
            name: name.to_string(),
            generics: PureGenerics::default(),
            params: vec![],
            ret: None,
            body: PureBlock { stmts },
        }
    }

    fn find_var(graph: &DataFlowGraphV2, name: &str) -> Option<VarId> {
        graph
            .iter_vars()
            .find(|(vid, _)| graph.var_name(*vid) == Some(name))
            .map(|(vid, _)| vid)
    }

    fn outgoing_kinds(graph: &DataFlowGraphV2, var: VarId) -> Vec<FlowKind> {
        graph
            .outgoing(var)
            .iter()
            .filter_map(|&fid| graph.flow(fid).map(|f| f.kind))
            .collect()
    }

    // ========== S-rank: Function call arguments ==========

    #[test]
    fn test_call_arg_generates_argument_flow() {
        // fn test_fn() { let x = 1; foo(x); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Call {
                func: Box::new(PureExpr::Path("foo".into())),
                args: vec![PureExpr::Path("x".into())],
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::Argument),
            "x passed to foo() should have Argument flow, got: {:?}",
            kinds
        );
    }

    // ========== S-rank: Method call receiver ==========

    #[test]
    fn test_method_receiver_generates_read_flow() {
        // fn test_fn() { let x = 1; x.push(1); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::MethodCall {
                receiver: Box::new(PureExpr::Path("x".into())),
                method: "push".into(),
                turbofish: None,
                args: vec![PureExpr::Lit("1".into())],
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::Read),
            "x used as method receiver should have Read flow, got: {:?}",
            kinds
        );
    }

    // ========== S-rank: Return expression ==========

    #[test]
    fn test_return_generates_return_flow() {
        // fn test_fn() { let x = 1; return x; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Return(Some(Box::new(PureExpr::Path("x".into()))))),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::Return),
            "x in return expression should have Return flow, got: {:?}",
            kinds
        );
    }

    // ========== S-rank: Borrow expressions ==========

    #[test]
    fn test_shared_borrow_generates_borrow_flow() {
        // fn test_fn() { let x = 1; let y = &x; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "y".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Ref {
                    is_mut: false,
                    expr: Box::new(PureExpr::Path("x".into())),
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::SharedBorrow),
            "x in &x should have SharedBorrow flow, got: {:?}",
            kinds
        );
    }

    #[test]
    fn test_mut_borrow_generates_mut_borrow_flow() {
        // fn test_fn() { let x = 1; let y = &mut x; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: true,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "y".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Ref {
                    is_mut: true,
                    expr: Box::new(PureExpr::Path("x".into())),
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::MutBorrow),
            "x in &mut x should have MutBorrow flow, got: {:?}",
            kinds
        );
    }

    // ========== Combined: Method call args ==========

    #[test]
    fn test_method_call_arg_generates_argument_flow() {
        // fn test_fn() { let x = 1; let y = 2; y.foo(x); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "y".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("2".into())),
            },
            PureStmt::Semi(PureExpr::MethodCall {
                receiver: Box::new(PureExpr::Path("y".into())),
                method: "foo".into(),
                turbofish: None,
                args: vec![PureExpr::Path("x".into())],
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::Argument),
            "x passed as method arg should have Argument flow, got: {:?}",
            kinds
        );
    }

    // ========== A-rank: For loop pattern binding ==========

    #[test]
    fn test_for_loop_creates_assign_to_pattern_var() {
        // fn test_fn() { let items = 1; for item in items { } }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "items".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::For {
                label: None,
                pat: PurePattern::Ident {
                    name: "item".into(),
                    is_mut: false,
                },
                expr: Box::new(PureExpr::Path("items".into())),
                body: PureBlock { stmts: vec![] },
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let items = find_var(&graph, "items").expect("variable 'items' should exist");
        let kinds = outgoing_kinds(&graph, items);

        assert!(
            kinds.contains(&FlowKind::Read),
            "items used as for-loop iterator should have Read flow, got: {:?}",
            kinds
        );

        let item = find_var(&graph, "item").expect("loop variable 'item' should exist");
        let incoming: Vec<FlowKind> = graph
            .incoming(item)
            .iter()
            .filter_map(|&fid| graph.flow(fid).map(|f| f.kind))
            .collect();
        assert!(
            incoming.contains(&FlowKind::Assign),
            "item should have incoming Assign from iterator, got: {:?}",
            incoming
        );
    }

    // ========== A-rank: Break with value ==========

    #[test]
    fn test_break_with_value_generates_read_flow() {
        // fn test_fn() { let x = 1; loop { break x; } }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Loop {
                label: None,
                body: PureBlock {
                    stmts: vec![PureStmt::Semi(PureExpr::Break {
                        label: None,
                        expr: Some(Box::new(PureExpr::Path("x".into()))),
                    })],
                },
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x in break expression should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== A-rank: Closure captures ==========

    #[test]
    fn test_closure_capture_generates_flow() {
        // fn test_fn() { let x = 1; let f = || x + 1; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "f".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Closure {
                    is_async: false,
                    is_move: false,
                    params: vec![],
                    ret: None,
                    body: Box::new(PureExpr::Binary {
                        op: "+".into(),
                        left: Box::new(PureExpr::Path("x".into())),
                        right: Box::new(PureExpr::Lit("1".into())),
                    }),
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x captured by closure should have outgoing flow, got: {:?}",
            kinds
        );
    }

    #[test]
    fn test_move_closure_capture_generates_move_flow() {
        // fn test_fn() { let x = 1; let f = move || x + 1; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "f".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Closure {
                    is_async: false,
                    is_move: true,
                    params: vec![],
                    ret: None,
                    body: Box::new(PureExpr::Binary {
                        op: "+".into(),
                        left: Box::new(PureExpr::Path("x".into())),
                        right: Box::new(PureExpr::Lit("1".into())),
                    }),
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::Move),
            "x captured by move closure should have Move flow, got: {:?}",
            kinds
        );
    }

    // ========== B-rank: Match arm pattern binding ==========

    #[test]
    fn test_match_arm_pattern_creates_variable_with_assign() {
        // fn test_fn() { let x = 1; match x { val => { } } }
        // Expected: x has outgoing Read (scrutinee), val exists with incoming Assign from x
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Match {
                expr: Box::new(PureExpr::Path("x".into())),
                arms: vec![PureMatchArm {
                    pattern: PurePattern::Ident {
                        name: "val".into(),
                        is_mut: false,
                    },
                    guard: None,
                    body: PureExpr::Block {
                        label: None,
                        block: PureBlock { stmts: vec![] },
                    },
                }],
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let val = find_var(&graph, "val").expect("match arm variable 'val' should exist");
        let incoming: Vec<FlowKind> = graph
            .incoming(val)
            .iter()
            .filter_map(|&fid| graph.flow(fid).map(|f| f.kind))
            .collect();
        assert!(
            incoming.contains(&FlowKind::Assign),
            "val should have incoming Assign from match scrutinee, got: {:?}",
            incoming
        );
    }

    // ========== B-rank: Match scrutinee read ==========

    #[test]
    fn test_match_scrutinee_generates_read_flow() {
        // fn test_fn() { let x = 1; match x { _ => {} } }
        // Expected: x has outgoing Read flow (scrutinee is read)
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Match {
                expr: Box::new(PureExpr::Path("x".into())),
                arms: vec![PureMatchArm {
                    pattern: PurePattern::Wild,
                    guard: None,
                    body: PureExpr::Block {
                        label: None,
                        block: PureBlock { stmts: vec![] },
                    },
                }],
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x used as match scrutinee should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== B-rank: Match as source in collect_sources_recursive ==========

    #[test]
    fn test_match_expr_as_init_collects_arm_body_sources() {
        // fn test_fn() { let a = 1; let b = 2; let result = match ... { _ => a, _ => b }; }
        // Expected: result has incoming Assign from both a and b
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "a".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "b".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("2".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "result".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Match {
                    expr: Box::new(PureExpr::Lit("0".into())),
                    arms: vec![
                        PureMatchArm {
                            pattern: PurePattern::Lit("1".into()),
                            guard: None,
                            body: PureExpr::Path("a".into()),
                        },
                        PureMatchArm {
                            pattern: PurePattern::Wild,
                            guard: None,
                            body: PureExpr::Path("b".into()),
                        },
                    ],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let result = find_var(&graph, "result").expect("variable 'result' should exist");
        let incoming: Vec<FlowKind> = graph
            .incoming(result)
            .iter()
            .filter_map(|&fid| graph.flow(fid).map(|f| f.kind))
            .collect();
        assert!(
            incoming.len() >= 2,
            "result should have incoming Assign from both arms, got: {:?}",
            incoming
        );
    }

    // ========== Struct expression: field values generate Read flow ==========

    #[test]
    fn test_struct_expr_field_values_generate_read_flow() {
        // fn test_fn() { let x = 1; let y = 2; let s = Foo { a: x, b: y }; }
        // Expected: x and y have outgoing Read flow (consumed by struct construction)
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "y".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("2".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "s".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Struct {
                    path: "Foo".into(),
                    fields: vec![
                        ("a".into(), PureExpr::Path("x".into())),
                        ("b".into(), PureExpr::Path("y".into())),
                    ],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let y = find_var(&graph, "y").expect("variable 'y' should exist");

        assert!(
            !outgoing_kinds(&graph, x).is_empty(),
            "x used in struct field should have outgoing flow, got: {:?}",
            outgoing_kinds(&graph, x)
        );
        assert!(
            !outgoing_kinds(&graph, y).is_empty(),
            "y used in struct field should have outgoing flow, got: {:?}",
            outgoing_kinds(&graph, y)
        );
    }

    // ========== Struct expression as statement (not in let init) ==========

    #[test]
    fn test_struct_expr_as_stmt_generates_read_flow() {
        // fn test_fn() { let x = 1; Foo { a: x }; }
        // Expected: x has outgoing Read flow
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Struct {
                path: "Foo".into(),
                fields: vec![("a".into(), PureExpr::Path("x".into()))],
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x used in struct expr statement should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== Field assignment: right-hand side generates Read flow ==========

    #[test]
    fn test_field_assign_rhs_generates_read_flow() {
        // fn test_fn() { let x = 1; self.field = x; }
        // Expected: x has outgoing flow (Assign to self.field, but also Read via the assignment)
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Binary {
                op: "=".into(),
                left: Box::new(PureExpr::Field {
                    expr: Box::new(PureExpr::Path("self".into())),
                    field: "data".into(),
                }),
                right: Box::new(PureExpr::Path("x".into())),
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x assigned to self.data should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== Async move block captures ==========

    #[test]
    fn test_async_move_block_captures_generate_flow() {
        // fn test_fn() { let x = 1; async move { x + 1 }; }
        // Expected: x has outgoing flow (captured by async move)
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Async {
                is_move: true,
                body: PureBlock {
                    stmts: vec![PureStmt::Expr(PureExpr::Binary {
                        op: "+".into(),
                        left: Box::new(PureExpr::Path("x".into())),
                        right: Box::new(PureExpr::Lit("1".into())),
                    })],
                },
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x captured by async move should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== Closure capture in non-tail position ==========

    #[test]
    fn test_closure_captures_var_in_non_tail_stmt() {
        // fn test_fn() { let x = 1; let f = move || { let y = x; }; }
        // Expected: x has outgoing flow (captured inside closure body, non-tail stmt)
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "f".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Closure {
                    is_async: false,
                    is_move: true,
                    params: vec![],
                    ret: None,
                    body: Box::new(PureExpr::Block {
                        label: None,
                        block: PureBlock {
                            stmts: vec![PureStmt::Local {
                                pattern: PurePattern::Ident {
                                    name: "y".into(),
                                    is_mut: false,
                                },
                                ty: None,
                                init: Some(PureExpr::Path("x".into())),
                            }],
                        },
                    }),
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            kinds.contains(&FlowKind::Move),
            "x captured in non-tail stmt of move closure should have Move flow, got: {:?}",
            kinds
        );
    }

    // ========== While condition reads variable ==========

    #[test]
    fn test_while_condition_generates_read_flow() {
        // fn test_fn() { let x = 1; while x > 0 { } }
        // Expected: x has outgoing Read flow from while condition
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::While {
                label: None,
                cond: Box::new(PureExpr::Binary {
                    op: ">".into(),
                    left: Box::new(PureExpr::Path("x".into())),
                    right: Box::new(PureExpr::Lit("0".into())),
                }),
                body: PureBlock { stmts: vec![] },
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x used in while condition should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== Tail expression treated as implicit return ==========

    #[test]
    fn test_tail_expr_generates_read_flow() {
        // fn test_fn() { let x = 1; (x, 2) }
        // Expected: x has outgoing flow (tail expression = implicit return)
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Expr(PureExpr::Tuple(vec![
                PureExpr::Path("x".into()),
                PureExpr::Lit("2".into()),
            ])),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x in tail expression should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== Macro expression variable extraction ==========

    #[test]
    fn test_macro_expr_variable_generates_read_flow() {
        // fn test_fn() { let x = 1; println!("{}", x); }
        // Expected: x has outgoing flow (referenced in macro tokens)
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::Macro {
                name: "println".into(),
                delimiter: MacroDelimiter::Paren,
                tokens: "\"{}\", x".into(),
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x referenced in macro tokens should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== while let condition reads variable ==========

    #[test]
    fn test_while_let_condition_generates_read_flow() {
        // fn test_fn() { let x = 1; while let Some(v) = x.parent() { } }
        // Expected: x has outgoing Read flow from while-let condition
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Semi(PureExpr::While {
                label: None,
                cond: Box::new(PureExpr::Let {
                    pattern: PurePattern::Ident {
                        name: "v".into(),
                        is_mut: false,
                    },
                    expr: Box::new(PureExpr::MethodCall {
                        receiver: Box::new(PureExpr::Path("x".into())),
                        method: "parent".into(),
                        turbofish: None,
                        args: vec![],
                    }),
                }),
                body: PureBlock { stmts: vec![] },
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let x = find_var(&graph, "x").expect("variable 'x' should exist");
        let kinds = outgoing_kinds(&graph, x);

        assert!(
            !kinds.is_empty(),
            "x used in while-let condition should have outgoing flow, got: {:?}",
            kinds
        );
    }

    // ========== Lock detection tests ==========

    #[test]
    fn test_lock_direct_call_detected() {
        // fn test_fn() { let m = 1; let guard = m.lock(); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "m".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "guard".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("m".into())),
                    method: "lock".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(
            tracker.acquisitions().len(),
            1,
            "should detect 1 lock acquisition"
        );
        assert_eq!(tracker.acquisitions()[0].lock_type, LockType::Mutex);
        assert!(!tracker.acquisitions()[0].is_try);
        assert_eq!(tracker.acquisitions()[0].lock_name, "m");
        assert_eq!(tracker.acquisitions()[0].guard_name, "guard");
    }

    #[test]
    fn test_lock_unwrap_pattern_detected() {
        // fn test_fn() { let m = 1; let guard = m.lock().unwrap(); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "m".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "guard".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::MethodCall {
                        receiver: Box::new(PureExpr::Path("m".into())),
                        method: "lock".into(),
                        turbofish: None,
                        args: vec![],
                    }),
                    method: "unwrap".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(
            tracker.acquisitions().len(),
            1,
            "should detect lock().unwrap() pattern"
        );
        assert_eq!(tracker.acquisitions()[0].lock_type, LockType::Mutex);
    }

    #[test]
    fn test_rwlock_read_write_detected() {
        // fn test_fn() { let rw = 1; let r = rw.read(); let w = rw.write(); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "rw".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "r".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("rw".into())),
                    method: "read".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "w".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("rw".into())),
                    method: "write".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(
            tracker.acquisitions().len(),
            2,
            "should detect both read and write locks"
        );
        assert_eq!(tracker.acquisitions()[0].lock_type, LockType::RwLockRead);
        assert_eq!(tracker.acquisitions()[1].lock_type, LockType::RwLockWrite);
    }

    #[test]
    fn test_try_lock_detected() {
        // fn test_fn() { let m = 1; let guard = m.try_lock(); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "m".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "guard".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("m".into())),
                    method: "try_lock".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(tracker.acquisitions().len(), 1, "should detect try_lock");
        assert!(tracker.acquisitions()[0].is_try);
    }

    #[test]
    fn test_lock_await_pattern_detected() {
        // fn test_fn() { let m = 1; let guard = m.lock().await; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "m".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "guard".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Await(Box::new(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("m".into())),
                    method: "lock".into(),
                    turbofish: None,
                    args: vec![],
                }))),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(
            tracker.acquisitions().len(),
            1,
            "should detect lock().await pattern"
        );
        assert_eq!(tracker.acquisitions()[0].lock_type, LockType::Mutex);
    }

    #[test]
    fn test_borrow_borrow_mut_detected() {
        // fn test_fn() { let cell = 1; let r = cell.borrow(); let w = cell.borrow_mut(); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "cell".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "r".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("cell".into())),
                    method: "borrow".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "w".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("cell".into())),
                    method: "borrow_mut".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(
            tracker.acquisitions().len(),
            2,
            "should detect borrow and borrow_mut"
        );
        assert_eq!(tracker.acquisitions()[0].lock_type, LockType::RefCell);
        assert_eq!(tracker.acquisitions()[1].lock_type, LockType::RefCellMut);
    }

    #[test]
    fn test_non_lock_method_not_detected() {
        // fn test_fn() { let x = 1; let y = x.foo(); }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "y".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("x".into())),
                    method: "foo".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(
            tracker.acquisitions().len(),
            0,
            "non-lock method should not be detected"
        );
    }

    #[test]
    fn test_guard_field_read_tracked() {
        // fn test_fn() { let m = 1; let guard = m.lock(); let _ = guard.counter; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "m".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "guard".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("m".into())),
                    method: "lock".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
            // let _ = guard.counter;
            PureStmt::Local {
                pattern: PurePattern::Wild,
                ty: None,
                init: Some(PureExpr::Field {
                    expr: Box::new(PureExpr::Path("guard".into())),
                    field: "counter".into(),
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(tracker.acquisitions().len(), 1);
        let sections = tracker.critical_sections();
        assert_eq!(sections.len(), 1, "should have 1 completed section");
        assert_eq!(
            sections[0].field_accesses.len(),
            1,
            "should track 1 field access"
        );
        assert_eq!(sections[0].field_accesses[0].field_name, "counter");
        assert_eq!(
            sections[0].field_accesses[0].access_kind,
            super::super::lock_v2::AccessKind::Read
        );
    }

    #[test]
    fn test_guard_field_write_tracked() {
        // fn test_fn() { let m = 1; let guard = m.lock(); guard.counter = 42; }
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "m".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "guard".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::MethodCall {
                    receiver: Box::new(PureExpr::Path("m".into())),
                    method: "lock".into(),
                    turbofish: None,
                    args: vec![],
                }),
            },
            // guard.counter = 42;
            PureStmt::Semi(PureExpr::Binary {
                op: "=".into(),
                left: Box::new(PureExpr::Field {
                    expr: Box::new(PureExpr::Path("guard".into())),
                    field: "counter".into(),
                }),
                right: Box::new(PureExpr::Lit("42".into())),
            }),
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(tracker.acquisitions().len(), 1);
        let sections = tracker.critical_sections();
        assert_eq!(sections.len(), 1);
        assert_eq!(sections[0].field_accesses.len(), 1);
        assert_eq!(sections[0].field_accesses[0].field_name, "counter");
        assert_eq!(
            sections[0].field_accesses[0].access_kind,
            super::super::lock_v2::AccessKind::Write
        );
    }

    #[test]
    fn test_non_guard_field_not_tracked() {
        // fn test_fn() { let x = 1; let _ = x.field; }
        // x is NOT a guard, so field access should not be tracked
        let stmts = vec![
            PureStmt::Local {
                pattern: PurePattern::Ident {
                    name: "x".into(),
                    is_mut: false,
                },
                ty: None,
                init: Some(PureExpr::Lit("1".into())),
            },
            PureStmt::Local {
                pattern: PurePattern::Wild,
                ty: None,
                init: Some(PureExpr::Field {
                    expr: Box::new(PureExpr::Path("x".into())),
                    field: "field".into(),
                }),
            },
        ];

        let graph = build_graph_for_fn(make_fn("test_fn", stmts));
        let tracker = graph.lock_tracker();
        assert_eq!(tracker.acquisitions().len(), 0);
        assert_eq!(tracker.critical_sections().len(), 0);
    }

    // ========== Generic impl block: lock detection ==========

    /// Build a DataFlowGraphV2 from an impl block.
    ///
    /// Registers the impl block and its methods in a fresh SymbolRegistry
    /// so that FlowCollectorV2 can resolve `current_symbol`.
    fn build_graph_for_impl(impl_block: PureImpl) -> DataFlowGraphV2 {
        let mut registry = SymbolRegistry::new();
        let module_path = SymbolPath::parse("test_crate").unwrap();

        // Register methods under the base type (strip generics)
        let base_type = impl_block
            .self_ty
            .split('<')
            .next()
            .unwrap_or(&impl_block.self_ty)
            .trim();
        let type_path = module_path.child(base_type).unwrap();

        for item in &impl_block.items {
            if let PureImplItem::Fn(f) = item {
                let method_path = type_path.child(&f.name).unwrap();
                registry.register(method_path, SymbolKind::Method).unwrap();
            }
        }

        let file = PureFile {
            attrs: vec![],
            items: vec![PureItem::Impl(impl_block)],
        };

        let mut graph = DataFlowGraphV2::new();
        let mut collector = FlowCollectorV2::new(Some(module_path), &registry, "test_crate");
        collector.visit_file(&file);
        collector.apply_to(&mut graph);
        graph
    }

    #[test]
    fn test_lock_in_generic_inherent_impl_detected() {
        // impl<S> Handle<S> { fn take(&self) { let g = self.write.lock(); } }
        let method = PureFn {
            attrs: vec![],
            vis: PureVis::Public,
            is_async: false,
            is_async_inferred: false,
            is_const: false,
            is_unsafe: false,
            abi: None,
            name: "take".to_string(),
            generics: PureGenerics::default(),
            params: vec![PureParam::SelfValue {
                is_ref: true,
                is_mut: false,
            }],
            ret: None,
            body: PureBlock {
                stmts: vec![PureStmt::Local {
                    pattern: PurePattern::Ident {
                        name: "g".into(),
                        is_mut: false,
                    },
                    ty: None,
                    init: Some(PureExpr::MethodCall {
                        receiver: Box::new(PureExpr::Field {
                            expr: Box::new(PureExpr::Path("self".into())),
                            field: "write".into(),
                        }),
                        method: "lock".into(),
                        turbofish: None,
                        args: vec![],
                    }),
                }],
            },
        };

        let impl_block = PureImpl {
            attrs: vec![],
            generics: PureGenerics::default(),
            is_unsafe: false,
            trait_: None,
            self_ty: "Handle < S >".to_string(), // generic self_ty
            items: vec![PureImplItem::Fn(method)],
        };

        let graph = build_graph_for_impl(impl_block);
        let tracker = graph.lock_tracker();
        assert_eq!(
            tracker.acquisitions().len(),
            1,
            "lock in generic inherent impl should be detected; \
             visit_impl_item must strip generics from self_ty"
        );
        assert_eq!(tracker.acquisitions()[0].lock_type, LockType::Mutex);
    }
}