forge_guard/security/

engine.rs

//! Security engine — coordinates all vulnerability checks.

use crate::chains::ChainRegistry;
use crate::core::{Finding, ProjectConfig, SecurityScores, Severity};
use crate::plugins::{PluginContext, PluginRegistry};
use crate::security::checks;
use std::path::Path;
use std::sync::Arc;

/// The security engine drives all vulnerability analysis.
#[derive(Clone)]
pub struct SecurityEngine {
    config: Arc<crate::core::config::SecurityConfig>,
    project_config: Arc<ProjectConfig>,
    plugin_registry: Arc<PluginRegistry>,
    check_registry: Vec<&'static checks::SecurityCheckMeta>,
}

impl SecurityEngine {
    /// Create a new security engine with the given configuration.
    pub fn new(
        config: &ProjectConfig,
        plugin_registry: &PluginRegistry,
    ) -> Result<Self, crate::core::ForgeGuardError> {
        let mut engine = Self {
            config: Arc::new(config.security.clone()),
            project_config: Arc::new(config.clone()),
            plugin_registry: Arc::new(plugin_registry.clone()),
            check_registry: Vec::new(),
        };

        // Register built-in checks based on config
        engine.register_builtin_checks();
        // Register plugin checks
        engine.register_plugin_checks()?;

        Ok(engine)
    }

    fn register_builtin_checks(&mut self) {
        for check in checks::ALL_CHECKS {
            let should_register = match check.severity {
                "critical" | "high" => self.config.enable_high,
                "medium" => self.config.enable_medium,
                "low" => self.config.enable_low,
                "informational" => self.config.enable_info,
                _ => true,
            };

            // Check severity overrides
            let effective_severity = self
                .config
                .severity_overrides
                .get(check.name)
                .map(|s| s.as_str())
                .unwrap_or(check.severity);

            if should_register && self.filter_by_severity(effective_severity) {
                self.check_registry.push(check);
            }
        }
    }

    fn filter_by_severity(&self, severity: &str) -> bool {
        match severity {
            "critical" | "high" => self.config.enable_high,
            "medium" => self.config.enable_medium,
            "low" => self.config.enable_low,
            "informational" => self.config.enable_info,
            _ => true,
        }
    }

    fn register_plugin_checks(&mut self) -> Result<(), crate::core::ForgeGuardError> {
        // Plugin checks are loaded at runtime via `execute_all()` in `analyze_files`.
        // This method exists as an extension point for future dynamic check registration.
        Ok(())
    }

    /// Analyze a set of Solidity source files and return findings.
    /// Includes findings from registered plugins (both built-in and external).
    pub fn analyze_files(
        &self,
        files: &[std::path::PathBuf],
        _chain_registry: &ChainRegistry,
    ) -> Result<Vec<Finding>, crate::core::ForgeGuardError> {
        use rayon::prelude::*;
        let max_findings = self.config.max_findings_per_check;

        // Process files in parallel using Rayon
        let mut all_findings: Vec<Finding> = files
            .par_iter()
            .flat_map(|file| {
                let content = match std::fs::read_to_string(file) {
                    Ok(c) => c,
                    Err(_) => return Vec::new(),
                };

                self.analyze_file(file, &content, max_findings)
            })
            .collect();

        // ── Collect plugin findings ──
        // Run enabled plugins against all source files at once
        let plugin_ctx = PluginContext::new(&self.project_config, files.to_vec());
        let plugin_results = self.plugin_registry.execute_all(&plugin_ctx);

        for result in plugin_results {
            if result.success {
                all_findings.extend(result.findings);
            } else if let Some(err) = &result.error {
                eprintln!("  ⚠️  Plugin '{}' failed: {}", result.plugin_name, err);
            }
        }

        Ok(all_findings)
    }

    /// Analyze a set of Solidity source files in quick mode.
    /// Skips parser-heavy checks (reentrancy, access control) and only runs
    /// pattern-based HIGH/CRITICAL checks for lightning-fast results.
    /// Plugins are also skipped in quick mode.
    pub fn analyze_files_quick(
        &self,
        files: &[std::path::PathBuf],
        _chain_registry: &ChainRegistry,
    ) -> Result<Vec<Finding>, crate::core::ForgeGuardError> {
        use rayon::prelude::*;
        // In quick mode: cap findings lower and only run high-severity checks
        let max_findings = self.config.max_findings_per_check.min(15);

        // Only register high/critical severity checks that use line-based patterns
        let quick_checks: Vec<&&'static checks::SecurityCheckMeta> = self
            .check_registry
            .iter()
            .filter(|check| {
                // Only HIGH/CRITICAL checks
                let sev = check.severity;
                (sev == "high" || sev == "critical")
                    // Skip parser-heavy checks (reentrancy, access control)
                    && check.id != "FA-H-001"
                    && check.id != "FA-H-002"
            })
            .collect();

        let all_findings: Vec<Finding> = files
            .par_iter()
            .flat_map(|file| {
                let content = match std::fs::read_to_string(file) {
                    Ok(c) => c,
                    Err(_) => return Vec::new(),
                };

                let file_name = file.to_string_lossy().to_string();
                let lines: Vec<&str> = content.lines().collect();
                let mut findings = Vec::new();

                for check in &quick_checks {
                    if findings.len() >= max_findings {
                        break;
                    }
                    let check_findings = self.execute_check(check, &file_name, &lines, &content);
                    findings.extend(check_findings);
                }

                findings
            })
            .collect();

        // Skip plugins in quick mode
        Ok(all_findings)
    }

    /// Analyze a single Solidity file for vulnerabilities.
    fn analyze_file(&self, file: &Path, content: &str, max_findings: usize) -> Vec<Finding> {
        let file_name = file.to_string_lossy().to_string();
        let lines: Vec<&str> = content.lines().collect();
        let mut findings = Vec::new();

        // Run each registered check against this file
        for check in &self.check_registry {
            if findings.len() >= max_findings {
                break;
            }

            let check_findings = self.execute_check(check, &file_name, &lines, content);
            findings.extend(check_findings);
        }

        findings
    }

    /// Execute a single security check against source content.
    fn execute_check(
        &self,
        check: &&'static checks::SecurityCheckMeta,
        file_name: &str,
        lines: &[&str],
        content: &str,
    ) -> Vec<Finding> {
        match check.id {
            // ── HIGH severity checks ───────────────────────
            "FA-H-001" => self.check_reentrancy(file_name, lines, content),
            "FA-H-002" => self.check_access_control(file_name, lines, content),
            "FA-H-003" => self.check_delegatecall(file_name, lines, content),
            "FA-H-004" => self.check_tx_origin(file_name, lines, content),
            "FA-H-005" => self.check_create2(file_name, lines, content),
            "FA-H-006" => self.check_dos(file_name, lines, content),
            "FA-H-007" => self.check_storage_collision(file_name, lines, content),
            "FA-H-008" => self.check_unsafe_assembly(file_name, lines, content),
            "FA-H-009" => self.check_selfdestruct(file_name, lines, content),
            "FA-H-010" => self.check_proxy_vulnerabilities(file_name, lines, content),
            "FA-H-011" => self.check_oracle_manipulation(file_name, lines, content),
            "FA-H-012" => self.check_signature_vulnerabilities(file_name, lines, content),
            "FA-H-013" => self.check_replay_attacks(file_name, lines, content),
            "FA-H-014" => self.check_erc20_issues(file_name, lines, content),
            "FA-H-015" => self.check_bridge_vulnerabilities(file_name, lines, content),
            "FA-H-016" => self.check_flash_loan_issues(file_name, lines, content),
            "FA-H-017" => self.check_mev_issues(file_name, lines, content),
            "FA-H-018" => self.check_cross_chain_issues(file_name, lines, content),
            "FA-H-019" => self.check_dependency_vulnerabilities(file_name, lines, content),
            "FA-H-020" => self.check_unsafe_imports(file_name, lines, content),
            "FA-H-021" => self.check_unsafe_initializers(file_name, lines, content),
            "FA-H-022" => self.check_unsafe_upgrade_paths(file_name, lines, content),
            "FA-H-023" => self.check_clone_vulnerabilities(file_name, lines, content),

            // ── MEDIUM severity checks ─────────────────────
            "FA-M-001" => self.check_gas_problems(file_name, lines, content),
            "FA-M-002" => self.check_unsafe_casting(file_name, lines, content),
            "FA-M-003" => self.check_timestamp_manipulation(file_name, lines, content),
            "FA-M-004" => self.check_storage_inefficiencies(file_name, lines, content),
            "FA-M-005" => self.check_unsafe_events(file_name, lines, content),
            "FA-M-006" => self.check_poor_visibility(file_name, lines, content),
            "FA-M-007" => self.check_bad_modifiers(file_name, lines, content),
            "FA-M-008" => self.check_unsafe_math(file_name, lines, content),
            "FA-M-009" => self.check_poor_access_patterns(file_name, lines, content),

            // ── LOW severity checks ────────────────────────
            "FA-L-001" => self.check_naming_issues(file_name, lines, content),
            "FA-L-002" => self.check_code_duplication(file_name, lines, content),
            "FA-L-003" => self.check_optimization(file_name, lines, content),

            // ── INFO checks ───────────────────────────────
            "FA-I-001" => self.check_style_issues(file_name, lines, content),
            "FA-I-002" => self.check_documentation(file_name, lines, content),

            _ => Vec::new(),
        }
    }

    /// Calculate security scores based on findings.
    pub fn calculate_scores(&self, findings: &[Finding]) -> SecurityScores {
        let mut scores = SecurityScores::perfect();

        for finding in findings {
            let deduction = match finding.severity {
                Severity::Critical => 30,
                Severity::High => 15,
                Severity::Medium => 8,
                Severity::Low => 3,
                Severity::Informational => 1,
            };

            match finding.category.as_str() {
                "Access Control" => {
                    scores.access_control = scores.access_control.saturating_sub(deduction)
                }
                "DeFi" => {
                    scores.security = scores.security.saturating_sub(deduction);
                    scores.exploit_resistance = scores.exploit_resistance.saturating_sub(deduction);
                }
                "Upgradeability" => {
                    scores.upgradeability = scores.upgradeability.saturating_sub(deduction);
                    scores.proxy_safety = scores.proxy_safety.saturating_sub(deduction);
                }
                "Gas" => scores.gas = scores.gas.saturating_sub(deduction),
                "Deployment" => scores.deployment = scores.deployment.saturating_sub(deduction),
                "Dependencies" => {
                    scores.dependencies = scores.dependencies.saturating_sub(deduction)
                }
                "Cross-Chain" => {
                    scores.chain_compatibility =
                        scores.chain_compatibility.saturating_sub(deduction)
                }
                "Logic" | "Security" => scores.security = scores.security.saturating_sub(deduction),
                "Cryptography" => scores.security = scores.security.saturating_sub(deduction),
                _ => scores.security = scores.security.saturating_sub(deduction),
            }

            // General deductions
            scores.production_readiness = scores.production_readiness.saturating_sub(deduction / 2);
            scores.architecture = scores.architecture.saturating_sub(deduction / 3);
        }

        scores
    }

    // ─────────────────────────────────────────────────────────
    // Pattern-based check implementations
    // ─────────────────────────────────────────────────────────

    fn find_lines_containing<'a>(
        &self,
        lines: &[&'a str],
        patterns: &[&str],
    ) -> Vec<(usize, &'a str)> {
        let mut results = Vec::new();
        for (i, line) in lines.iter().enumerate() {
            for pattern in patterns {
                if line.contains(pattern)
                    && !line.trim_start().starts_with("//")
                    && !line.trim_start().starts_with("/*")
                {
                    results.push((i + 1, line.trim()));
                    break;
                }
            }
        }
        results
    }

    #[allow(dead_code)]
    fn find_lines_containing_excluding<'a>(
        &self,
        lines: &[&'a str],
        pattern: &str,
        exclude: &[&str],
    ) -> Vec<(usize, &'a str)> {
        let mut results = Vec::new();
        for (i, line) in lines.iter().enumerate() {
            if line.contains(pattern) && !line.trim_start().starts_with("//") {
                let excluded = exclude.iter().any(|e| line.contains(e));
                if !excluded {
                    results.push((i + 1, line.trim()));
                }
            }
        }
        results
    }

    fn make_finding(
        &self,
        check: &'static checks::SecurityCheckMeta,
        file: &str,
        line: usize,
        snippet: &str,
    ) -> Finding {
        Finding::builder()
            .id(&format!("{}-{}", check.id, line))
            .title(check.name)
            .description(check.description)
            .severity(match check.severity {
                "critical" => Severity::Critical,
                "high" => Severity::High,
                "medium" => Severity::Medium,
                "low" => Severity::Low,
                _ => Severity::Informational,
            })
            .file(file)
            .location(line, 0)
            .code(snippet)
            .recommendation(check.remediation)
            .category(check.category)
            .blocks_deployment(check.blocks_deployment)
            .build()
    }

    // ── Checks: classification helpers ────────────────────

    /// Check if a function body contains ERC-777/ERC-1155 style callback patterns
    /// that could introduce reentrancy via token transfers.
    fn has_callback_reentrancy_indicators(
        &self,
        func: &crate::parser::FunctionDef,
        contract: &crate::parser::Contract,
    ) -> bool {
        // ERC-777: _mint / _burn / _transfer call tokensReceived() on sender/recipient if they are contracts
        // ERC-721: _mint / _safeMint
        // ERC-1155: _mint / _safeTransferFrom
        let has_mint = func.body.iter().any(|s| {
            s.text.contains("_mint(")
                || s.text.contains("_safeMint(")
                || s.text.contains("_transfer(")
        });
        // Check if contract is ERC-1155 or ERC-777 compatible
        let is_safe_mint_contract = contract.is_erc721()
            || contract.is_erc1155()
            || contract.inheritance.iter().any(|i| {
                let l = i.to_lowercase();
                l.contains("erc777")
                    || l.contains("erc721")
                    || l.contains("erc1155")
                    || l.contains("erc721upgradeable")
                    || l.contains("erc1155upgradeable")
            });
        is_safe_mint_contract && has_mint
    }

    /// Check if a function has read-only reentrancy patterns.
    /// Read-only reentrancy occurs when an external call queries state that a reentering
    /// call could modify before the original function completes.
    fn has_read_only_reentrancy(&self, func: &crate::parser::FunctionDef) -> bool {
        // Pattern: external call, then read state that the call could affect
        let mut has_ext_call = false;
        let mut state_read_after_call = false;
        for stmt in &func.body {
            match &stmt.kind {
                crate::parser::StatementKind::ExternalCall => {
                    has_ext_call = true;
                }
                crate::parser::StatementKind::StateRead if has_ext_call => {
                    state_read_after_call = true;
                }
                _ => {}
            }
        }
        // Common read-only reentrancy targets: balanceOf, ownerOf, totalSupply
        if has_ext_call && state_read_after_call {
            let reads_critical = func.body.iter().any(|s| {
                matches!(s.kind, crate::parser::StatementKind::StateRead)
                    && (s.text.contains("balanceOf")
                        || s.text.contains("totalSupply")
                        || s.text.contains("ownerOf")
                        || s.text.contains("getBalance")
                        || s.text.contains(".balance"))
            });
            return reads_critical;
        }
        false
    }

    /// Check if external call targets are to user-controlled addresses (higher risk).
    fn is_external_call_to_user_address(&self, stmts: &[crate::parser::Statement]) -> bool {
        stmts.iter().any(|s| {
            if matches!(s.kind, crate::parser::StatementKind::ExternalCall) {
                let target = crate::parser::extract_call_target(&s.text);
                matches!(target, crate::parser::CallTarget::LocalVariable(_))
                    || matches!(target, crate::parser::CallTarget::Address)
            } else {
                false
            }
        })
    }

    /// Check if a function's external calls target the contract itself (self-call).
    fn has_self_call_reentrancy(&self, func: &crate::parser::FunctionDef) -> bool {
        func.body.iter().any(|s| {
            if matches!(s.kind, crate::parser::StatementKind::ExternalCall) {
                let target = crate::parser::extract_call_target(&s.text);
                matches!(target, crate::parser::CallTarget::SelfCall)
            } else {
                false
            }
        })
    }

    // ── Reentrancy ─────────────────────────────────────────
    fn check_reentrancy(&self, file: &str, _lines: &[&str], content: &str) -> Vec<Finding> {
        // Use the parser to perform proper CEI (Checks-Effects-Interactions) analysis
        let source_file = crate::parser::parse_source(content);
        let mut findings = Vec::new();

        for contract in &source_file.contracts {
            // Resolve inheritance for reentrancy guard
            let has_global_guard = source_file.inheritance_includes_reentrancy_guard(contract)
                || contract.has_reentrancy_guard_modifier();

            for func in &contract.functions {
                // Skip view/pure functions — they cannot modify state
                if !func.modifies_state() {
                    continue;
                }

                // Skip functions with explicit nonReentrant modifier
                if func.has_reentrancy_guard() {
                    continue;
                }

                // ── Check 1: ERC-777 / ERC-721 / ERC-1155 callback reentrancy ──
                // _safeMint, _mint in ERC-721/1155 call onERC721Received/onERC1155Received
                // which can re-enter the contract before state changes propagate
                if self.has_callback_reentrancy_indicators(func, contract) {
                    findings.push(
                        Finding::builder()
                            .id(&format!("FA-H-001-CB-{}", func.line))
                            .title(checks::REENTRANCY.name)
                            .description(&format!(
                                "{} — Callback reentrancy: function '{}' uses safe mint/transfer patterns that invoke receiver callbacks, enabling reentrancy via ERC-777/721/1155 hooks",
                                checks::REENTRANCY.description, func.name
                            ))
                            .severity(Severity::High)
                            .file(file)
                            .location(func.line, 0)
                            .code(&format!("Function {} uses safe mint/transfer with callback hooks", func.name))
                            .recommendation("Apply the Checks-Effects-Interactions pattern: update state before _safeMint/_mint. Consider using _mint instead of _safeMint when safe, or add a nonReentrant modifier. For ERC-1155, ensure balances are set before _safeTransferFrom.")
                            .category("Logic")
                            .blocks_deployment(true)
                            .build()
                    );
                    continue;
                }

                // ── Check 2: Read-only reentrancy ──
                // External call followed by reads of state that could be manipulated
                if self.has_read_only_reentrancy(func) {
                    findings.push(
                        Finding::builder()
                            .id(&format!("FA-H-001-RO-{}", func.line))
                            .title(checks::REENTRANCY.name)
                            .description(&format!(
                                "{} — Read-only reentrancy: function '{}' makes external calls then reads state (balanceOf, totalSupply, etc.) that the re-entering call could have modified",
                                checks::REENTRANCY.description, func.name
                            ))
                            .severity(Severity::High)
                            .file(file)
                            .location(func.line, 0)
                            .code(&format!("Function {} reads manipulated state after external call", func.name))
                            .recommendation("Use nonReentrant modifier for functions that read state after external calls. Consider using a snapshot pattern to isolate pre-call state from post-call reads.")
                            .category("Logic")
                            .blocks_deployment(true)
                            .build()
                    );
                    continue;
                }

                // ── Check 3: Self-call reentrancy ──
                // Functions that call this.function() which could re-enter
                if self.has_self_call_reentrancy(func) {
                    findings.push(
                        Finding::builder()
                            .id(&format!("FA-H-001-SC-{}", func.line))
                            .title(checks::REENTRANCY.name)
                            .description(&format!(
                                "{} — Self-call reentrancy path: function '{}' calls this.function() which could re-enter",
                                checks::REENTRANCY.description, func.name
                            ))
                            .severity(Severity::Medium)
                            .file(file)
                            .location(func.line, 0)
                            .code(&format!("Function {} contains this.function() call — possible reentrancy path", func.name))
                            .recommendation("Use nonReentrant modifier or restructure to avoid self-calls that could create reentrancy paths.")
                            .category("Logic")
                            .blocks_deployment(false)
                            .build()
                    );
                    continue;
                }

                // ── Check 4: If the whole contract has ReentrancyGuard, check for missing nonReentrant ──
                if has_global_guard
                    && func
                        .body
                        .iter()
                        .any(|s| matches!(s.kind, crate::parser::StatementKind::ExternalCall))
                {
                    if let Some(ext_call_stmt) = func
                        .body
                        .iter()
                        .find(|s| matches!(s.kind, crate::parser::StatementKind::ExternalCall))
                    {
                        findings.push(
                            Finding::builder()
                                .id(&format!("FA-H-001-MG-{}", func.line))
                                .title(checks::REENTRANCY.name)
                                .description(&format!(
                                    "Function '{}' makes external calls (line {}) but lacks nonReentrant modifier, despite contract having ReentrancyGuard",
                                    func.name, ext_call_stmt.line
                                ))
                                .severity(Severity::Medium)
                                .file(file)
                                .location(func.line, 0)
                                .code(&format!("Function {} makes external call at line {} without nonReentrant", func.name, ext_call_stmt.line))
                                .recommendation(checks::REENTRANCY.remediation)
                                .category("Logic")
                                .blocks_deployment(false)
                                .build()
                        );
                        continue;
                    }
                }

                // ── Check 5: Proper CEI (Checks-Effects-Interactions) analysis ──
                // VIOLATION: State write AFTER an external call
                let mut findings_from_cei = self.analyze_cei_pattern(func, file);
                findings.append(&mut findings_from_cei);

                // ── Check 6: External calls to user-controlled addresses ──
                if !findings.iter().any(|f| f.id.starts_with("FA-H-001")) {
                    // No other reentrancy finding yet — check for high-risk call targets
                    if self.is_external_call_to_user_address(&func.body) {
                        findings.push(
                            Finding::builder()
                                .id(&format!("FA-H-001-UC-{}", func.line))
                                .title(checks::REENTRANCY.name)
                                .description(&format!(
                                    "{} — External call to user-controlled address in function '{}'. User-supplied addresses are high risk for reentrancy attacks",
                                    checks::REENTRANCY.description, func.name
                                ))
                                .severity(Severity::High)
                                .file(file)
                                .location(func.line, 0)
                                .code(&format!("Function {} calls external address from user-supplied input", func.name))
                                .recommendation("Use nonReentrant modifier for functions that make external calls to user-supplied addresses. Validate call targets against a whitelist when possible.")
                                .category("Logic")
                                .blocks_deployment(true)
                                .build()
                        );
                    }
                }
            }
        }

        findings
    }

    /// Perform CEI (Checks-Effects-Interactions) analysis on a function body.
    fn analyze_cei_pattern(&self, func: &crate::parser::FunctionDef, file: &str) -> Vec<Finding> {
        let mut seen_external_call = false;
        let mut first_ext_call_line = 0;
        let mut cei_violations = Vec::new();

        for stmt in &func.body {
            match &stmt.kind {
                crate::parser::StatementKind::ExternalCall => {
                    seen_external_call = true;
                    if first_ext_call_line == 0 {
                        first_ext_call_line = stmt.line;
                    }
                }
                crate::parser::StatementKind::StateWrite if seen_external_call => {
                    // CEI violation: state write after external call
                    // Filter out known guard patterns (reentrancy status toggle)
                    let is_guard_assignment = stmt.text.contains("_status")
                        || stmt.text.contains("_ENTERED")
                        || stmt.text.contains("_NOT_ENTERED")
                        || stmt.text.contains("locked");
                    if !is_guard_assignment {
                        cei_violations.push((stmt.line, stmt.text.clone()));
                    }
                }
                _ => {}
            }
        }

        if !cei_violations.is_empty() {
            let snippet = cei_violations
                .iter()
                .map(|(line, text)| format!("line {}: {}", line, text))
                .collect::<Vec<_>>()
                .join("; ");
            vec![
                Finding::builder()
                    .id(&format!("FA-H-001-{}", func.line))
                    .title(checks::REENTRANCY.name)
                    .description(&format!(
                        "{} — CEI violation: state modification after external call in function '{}' (first external call at line {})",
                        checks::REENTRANCY.description, func.name, first_ext_call_line
                    ))
                    .severity(Severity::High)
                    .file(file)
                    .location(func.line, 0)
                    .code(&snippet)
                    .recommendation(checks::REENTRANCY.remediation)
                    .category("Logic")
                    .blocks_deployment(true)
                    .build()
            ]
        } else {
            Vec::new()
        }
    }

    // ── Access Control ─────────────────────────────────────
    fn check_access_control(&self, file: &str, _lines: &[&str], content: &str) -> Vec<Finding> {
        // Use the parser to analyze access control with inheritance and modifier tracking
        let source_file = crate::parser::parse_source(content);
        let mut findings = Vec::new();

        // Sensitive function name patterns that typically need access control
        let sensitive_prefixes = [
            "withdraw",
            "transfer",
            "mint",
            "burn",
            "set",
            "update",
            "initialize",
            "upgradeTo",
            "upgrade",
            "pause",
            "unpause",
            "freeze",
            "unfreeze",
            "destroy",
            "kill",
            "recover",
            "drain",
            "swap",
            "add",
            "remove",
            "grant",
            "revoke",
            "change",
            "configure",
            "deposit",
            "stake",
            "unstake",
            "claim",
            "collect",
            "distribute",
            "allocate",
            "delegate",
            "execute",
            "send",
            "approve",
            "reset",
            "toggle",
            "blacklist",
            "whitelist",
            "ban",
            "suspend",
            "close",
            "open",
            "lock",
            "unlock",
            "renounce",
            "rescue",
            "emergency",
            "shutdown",
            "finalize",
            "override",
        ];

        for contract in &source_file.contracts {
            // Use SourceFile to resolve inheritance chain for access control
            let inherits_ac = source_file.inheritance_includes_access_control(contract)
                || contract.inherits_access_control();

            // Check for known access control patterns in the contract
            let has_known_ac_modifiers = self.contract_has_ac_modifiers(contract);

            for func in &contract.functions {
                // Only check public/external functions
                if func.visibility != crate::parser::Visibility::Public
                    && func.visibility != crate::parser::Visibility::External
                {
                    continue;
                }

                // Skip constructors
                if func.is_constructor || func.is_fallback || func.is_receive {
                    continue;
                }

                // Skip view/pure functions as they can't modify state
                if func.mutability == crate::parser::Mutability::Pure
                    || func.mutability == crate::parser::Mutability::View
                {
                    continue;
                }

                // Check if the function name matches sensitive patterns
                let is_sensitive = sensitive_prefixes
                    .iter()
                    .any(|prefix| func.name.to_lowercase().starts_with(prefix));

                if !is_sensitive {
                    // Also check for functions that write to state variables but seem unprotected
                    // Functions that contain external calls to transfer value
                    let has_value_transfer = func.body.iter().any(|s| {
                        s.text.contains(".transfer(")
                            || s.text.contains(".send(")
                            || s.text.contains(".call{value")
                    });
                    // Also check for functions that access important state variables
                    let writes_critical = func
                        .body
                        .iter()
                        .filter(|s| matches!(s.kind, crate::parser::StatementKind::StateWrite))
                        .count()
                        > 1;
                    if !has_value_transfer && !writes_critical {
                        continue;
                    }
                }

                // ── Check 1: Has explicit access control modifier ──
                let has_ac_modifier = func.has_access_control(contract);

                // ── Check 2: Has inline access control in function body ──
                let has_inline_ac = self.function_has_inline_access_control(func);

                // ── Check 3: Role-specific patterns (OpenZeppelin AccessControl v4/v5) ──
                let has_role_ac = self.function_has_role_based_access(func);

                // ── Check 4: Initialize function specific checks ──
                if func.name.to_lowercase().starts_with("initialize") {
                    self.check_initializer_access(
                        func,
                        contract,
                        &source_file,
                        file,
                        &mut findings,
                    );
                    continue;
                }

                // Determine overall access control status
                let has_any_ac = has_ac_modifier || has_inline_ac || has_role_ac;
                let has_any_modifier = !func.modifiers.is_empty();

                if !has_any_ac {
                    if inherits_ac && !has_any_modifier {
                        // Contract inherits Ownable/AccessControl but this function has no modifier
                        findings.push(self.make_finding(
                            &checks::ACCESS_CONTROL, file, func.line, &format!(
                                "Function '{}' is public/external but lacks any access control modifier (contract inherits {})",
                                func.name,
                                contract.inheritance.join(", ")
                            )
                        ));
                    } else if !inherits_ac && !has_any_modifier {
                        // No access control at all
                        findings.push(self.make_finding(
                            &checks::ACCESS_CONTROL, file, func.line, &format!(
                                "Function '{}' is public/external with no access control modifier — any caller can execute this sensitive function",
                                func.name
                            )
                        ));
                    } else if has_any_modifier && !has_ac_modifier && !has_inline_ac {
                        // Function has modifiers but none are access control
                        let mods = func.modifiers.join(", ");
                        findings.push(self.make_finding(
                            &checks::ACCESS_CONTROL, file, func.line, &format!(
                                "Function '{}' has modifiers ({}) but none appear to be access control",
                                func.name, mods
                            )
                        ));
                    }
                }
            }

            // ── Post-contract analysis: check for missing _disableInitializers ──
            if has_known_ac_modifiers || inherits_ac {
                self.check_missing_disable_initializers(contract, file, &mut findings);
            }
        }

        findings
    }

    /// Check if a contract has known access control modifiers (Ownable, AccessControl, etc.).
    fn contract_has_ac_modifiers(&self, contract: &crate::parser::Contract) -> bool {
        // Check modifier definitions for access control patterns
        contract.modifiers_defs.iter().any(|m| {
            let lower = m.name.to_lowercase();
            lower.contains("only") || lower.contains("auth") || lower == "whennotpaused"
        }) || contract.functions.iter().any(|f| {
            f.modifiers.iter().any(|mod_name| {
                let lower = mod_name.to_lowercase();
                lower.contains("only") || lower.contains("role") || lower == "whennotpaused"
            })
        })
    }

    /// Check if a function uses inline access control via require() checks in the function body.
    fn function_has_inline_access_control(&self, func: &crate::parser::FunctionDef) -> bool {
        func.body.iter().any(|s| {
            if matches!(s.kind, crate::parser::StatementKind::Guard) {
                let text_lower = s.text.to_lowercase();
                // Check for access-control require patterns
                text_lower.contains("msg.sender == owner")
                    || text_lower.contains("msg.sender == ") && text_lower.contains("owner")
                    || text_lower.contains("msg.sender == addresses")
                    || text_lower.contains("hasrole(")
                    || text_lower.contains("onlyowner")
                    || text_lower.contains("_isowner")
                    || text_lower.contains("_authorized")
                    || text_lower.contains("isowner")
                    || text_lower.contains("authorized")
                    || text_lower.contains("isadmin")
                    || text_lower.contains("_isadmin")
                    || text_lower.contains("onlyrole")
                    || text_lower.contains("hasrole")
                    || text_lower.contains("allowed to")
                    || text_lower.contains("not authorized")
                    || text_lower.contains("unauthorized")
                    || text_lower.contains("caller not")
                    || text_lower.contains("_checkrole")
            } else {
                false
            }
        })
    }

    /// Check if a function uses OpenZeppelin's AccessControl role-based patterns.
    fn function_has_role_based_access(&self, func: &crate::parser::FunctionDef) -> bool {
        // Check for _checkRole(ROLE, address) calls
        func.body.iter().any(|s| {
            s.text.contains("_checkRole(") || s.text.contains("_grantRole(")
                || s.text.contains("_revokeRole(") || s.text.contains("onlyRole")
        })
            // Also check modifiers that reference role constants
            || func.modifiers.iter().any(|m| {
                let lower = m.to_lowercase();
                lower.contains("role") || lower == "onlyrole"
            })
    }

    /// Perform specialized access control checks for initialize() functions.
    fn check_initializer_access(
        &self,
        func: &crate::parser::FunctionDef,
        _contract: &crate::parser::Contract,
        _source_file: &crate::parser::SourceFile,
        file: &str,
        findings: &mut Vec<Finding>,
    ) {
        let body = &func.body;
        let has_initializer_modifier = func.modifiers.iter().any(|m| {
            let lower = m.to_lowercase();
            lower == "initializer" || lower == "reinitializer" || lower.contains("initializer")
        });

        // Check for Ownable 2-step initialize pattern
        let calls_ownable_init = body.iter().any(|s| {
            s.text.contains("__Ownable_init(") || s.text.contains("__Ownable_init_unchain(")
        });
        let calls_accesscontrol_init = body.iter().any(|s| {
            s.text.contains("__AccessControl_init(")
                || s.text.contains("__AccessControl_init_unchain(")
        });
        let calls_reentrancyguard_init = body
            .iter()
            .any(|s| s.text.contains("__ReentrancyGuard_init("));

        if !has_initializer_modifier {
            let ac_methods = [
                (calls_ownable_init, "__Ownable_init"),
                (calls_accesscontrol_init, "__AccessControl_init"),
                (calls_reentrancyguard_init, "__ReentrancyGuard_init"),
            ];
            let init_calls: Vec<&str> = ac_methods
                .iter()
                .filter(|(called, _)| *called)
                .map(|(_, name)| *name)
                .collect();
            if !init_calls.is_empty() {
                findings.push(self.make_finding(
                    &checks::ACCESS_CONTROL,
                    file,
                    func.line,
                    &format!(
                        "Initialize function '{}' calls {} but lacks the 'initializer' modifier — can be front-run",
                        func.name,
                        init_calls.join(", ")
                    )
                ));
            }
        }

        // Check for onlyOwner-like direct assignment in initialize
        let assigns_owner = body.iter().any(|s| {
            let lower = s.text.to_lowercase();
            (lower.contains("owner") || lower.contains("admin"))
                && (s.text.contains('=') || s.text.contains(" := "))
                && !matches!(s.kind, crate::parser::StatementKind::Guard)
        });

        if assigns_owner && !has_initializer_modifier {
            findings.push(self.make_finding(
                &checks::ACCESS_CONTROL,
                file,
                func.line,
                &format!(
                    "Function '{}' assigns owner/admin directly but lacks 'initializer' modifier — unprotected",
                    func.name
                )
            ));
        }
    }

    /// Check if the contract should have _disableInitializers() in its constructor.
    fn check_missing_disable_initializers(
        &self,
        contract: &crate::parser::Contract,
        file: &str,
        findings: &mut Vec<Finding>,
    ) {
        let has_constructor = contract.functions.iter().any(|f| f.is_constructor);
        let calls_disable = contract.functions.iter().any(|f| {
            f.body
                .iter()
                .any(|s| s.text.contains("disableInitializers"))
        });
        let is_upgradeable = contract.inheritance.iter().any(|i| {
            let l = i.to_lowercase();
            l.contains("initializable")
                || l.contains("uups")
                || l.contains("transparentupgradeable")
        });

        if is_upgradeable && has_constructor && !calls_disable {
            findings.push(self.make_finding(
                &checks::ACCESS_CONTROL,
                file,
                contract.line,
                &format!(
                    "Implementation contract '{}' is upgradeable but its constructor does not call _disableInitializers(), leaving it vulnerable to selfdestruct",
                    contract.name
                )
            ));
        }
    }

    // ── Delegatecall ───────────────────────────────────────
    fn check_delegatecall(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["delegatecall(", ".delegatecall("])
            .into_iter()
            .map(|(line, snippet)| self.make_finding(&checks::DELEGATECALL, file, line, snippet))
            .collect()
    }

    // ── tx.origin ──────────────────────────────────────────
    fn check_tx_origin(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["tx.origin"])
            .into_iter()
            .map(|(line, snippet)| self.make_finding(&checks::TX_ORIGIN, file, line, snippet))
            .collect()
    }

    // ── CREATE2 ────────────────────────────────────────────
    fn check_create2(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["CREATE2", "create2(", ".create2("])
            .into_iter()
            .map(|(line, snippet)| self.make_finding(&checks::CREATE2, file, line, snippet))
            .collect()
    }

    // ── DoS ────────────────────────────────────────────────
    fn check_dos(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["for (uint", "for (uint256", "while ("])
            .into_iter()
            .filter(|(_, snippet)| snippet.contains(".length"))
            .map(|(line, snippet)| self.make_finding(&checks::DOS, file, line, snippet))
            .collect()
    }

    // ── Storage Collision ──────────────────────────────────
    fn check_storage_collision(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        // Look for storage gap patterns in upgradeable contracts
        let has_storage_gap = lines
            .iter()
            .any(|l| l.contains("__gap") || l.contains("_gap"));
        let is_contract = lines.iter().any(|l| {
            l.contains("contract ")
                && l.contains("is ")
                && (l.contains("UUPS") || l.contains("Transparent") || l.contains("Beacon"))
        });

        if is_contract && !has_storage_gap {
            vec![self.make_finding(
                &checks::STORAGE_COLLISION,
                file,
                1,
                "Upgradeable contract without storage gap",
            )]
        } else {
            Vec::new()
        }
    }

    // ── Unsafe Assembly ────────────────────────────────────
    fn check_unsafe_assembly(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["assembly {"])
            .into_iter()
            .map(|(line, snippet)| self.make_finding(&checks::UNSAFE_ASSEMBLY, file, line, snippet))
            .collect()
    }

    // ── Selfdestruct ───────────────────────────────────────
    fn check_selfdestruct(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["selfdestruct(", "selfdestruct ("])
            .into_iter()
            .map(|(line, snippet)| self.make_finding(&checks::SELFDESTRUCT, file, line, snippet))
            .collect()
    }

    // ── Proxy Vulnerabilities ──────────────────────────────
    fn check_proxy_vulnerabilities(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        let mut findings = Vec::new();
        // Check for initialize function in implementation
        let has_initialize = lines.iter().any(|l| l.contains("function initialize"));
        let has_initializer_modifier = lines.iter().any(|l| l.contains("initializer"));
        let is_implementation = lines.iter().any(|l| {
            l.contains("is UUPSUpgradeable") || l.contains("is TransparentUpgradeableProxy")
        });

        if is_implementation && has_initialize && !has_initializer_modifier {
            findings.push(self.make_finding(
                &checks::PROXY_VULNERABILITIES,
                file,
                1,
                "Implementation contract has initialize() without initializer modifier",
            ));
        }

        // Check for disableInitializers call in constructor
        if is_implementation {
            let has_disable = lines.iter().any(|l| l.contains("disableInitializers"));
            let has_constructor = lines.iter().any(|l| l.contains("constructor("));
            if has_constructor && !has_disable {
                findings.push(self.make_finding(
                    &checks::PROXY_VULNERABILITIES,
                    file,
                    1,
                    "Implementation contract constructor does not call disableInitializers()",
                ));
            }
        }

        findings
    }

    // ── Oracle Manipulation ────────────────────────────────
    fn check_oracle_manipulation(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        let has_chainlink = lines
            .iter()
            .any(|l| l.contains("Chainlink") || l.contains("AggregatorV3Interface"));
        let has_price_reference = lines.iter().any(|l| {
            l.contains("price") || l.contains("oracle") || l.contains("twap") || l.contains("TWAP")
        });

        if has_price_reference
            && !has_chainlink
            && !lines
                .iter()
                .any(|l| l.contains("//") && l.contains("oracle"))
        {
            self.find_lines_containing(lines, &["price(", ".price(", "getPrice", "getLatestPrice"])
                .into_iter()
                .map(|(line, snippet)| {
                    self.make_finding(&checks::ORACLE_MANIPULATION, file, line, snippet)
                })
                .collect()
        } else {
            Vec::new()
        }
    }

    // ── Signature Vulnerabilities ──────────────────────────
    fn check_signature_vulnerabilities(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        self.find_lines_containing(lines, &["ecrecover(", "ECDSA", "SignatureChecker"])
            .into_iter()
            .map(|(line, snippet)| {
                self.make_finding(&checks::SIGNATURE_VULNERABILITIES, file, line, snippet)
            })
            .collect()
    }

    // ── Replay Attacks ─────────────────────────────────────
    fn check_replay_attacks(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let has_nonce = lines
            .iter()
            .any(|l| l.contains("nonce") || l.contains("Nonce"));
        let has_signing = lines
            .iter()
            .any(|l| l.contains("signature") || l.contains("ecrecover"));

        if has_signing && !has_nonce {
            self.find_lines_containing(lines, &["ecrecover(", "ECDSA"])
                .into_iter()
                .map(|(line, snippet)| {
                    self.make_finding(&checks::REPLAY_ATTACKS, file, line, snippet)
                })
                .collect()
        } else {
            Vec::new()
        }
    }

    // ── ERC20 Issues ───────────────────────────────────────
    fn check_erc20_issues(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let is_erc20 = lines
            .iter()
            .any(|l| l.contains("is IERC20") || l.contains("ERC20"));
        if !is_erc20 {
            return Vec::new();
        }

        let mut findings = Vec::new();

        // Check for approve without safe increase/decrease
        let has_approve = lines.iter().any(|l| l.contains("approve("));
        let has_safe_approve = lines
            .iter()
            .any(|l| l.contains("increaseAllowance") || l.contains("decreaseAllowance"));
        if has_approve && !has_safe_approve {
            findings.push(self.make_finding(
                &checks::ERC20_ISSUES,
                file,
                1,
                "ERC20 approve() used without safe increaseAllowance/decreaseAllowance pattern",
            ));
        }

        // Check for _mint in constructor (common frontrunning issue)
        let has_init_mint = lines.iter().any(|l| l.contains("_mint("));
        if has_init_mint {
            findings.push(self.make_finding(
                &checks::ERC20_ISSUES,
                file,
                1,
                "ERC20 tokens minted — verify initialization and distribution",
            ));
        }

        findings
    }

    // ── Bridge Vulnerabilities ─────────────────────────────
    fn check_bridge_vulnerabilities(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        let bridge_keywords = [
            "bridge",
            "relayer",
            "validator",
            "crossChain",
            "cross-chain",
            "message",
            "relay",
        ];
        let has_bridge = lines
            .iter()
            .any(|l| bridge_keywords.iter().any(|k| l.contains(k)));

        if has_bridge {
            vec![self.make_finding(
                &checks::BRIDGE_VULNERABILITIES,
                file,
                1,
                "Bridge/relayer pattern detected — review validator set and message signing",
            )]
        } else {
            Vec::new()
        }
    }

    // ── Flash Loan Issues ──────────────────────────────────
    fn check_flash_loan_issues(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let flash_keywords = [
            "flashLoan",
            "flashloan",
            "flash_loan",
            "onFlashLoan",
            "IFlashLoan",
        ];
        let has_flash = lines
            .iter()
            .any(|l| flash_keywords.iter().any(|k| l.contains(k)));

        if has_flash {
            vec![self.make_finding(
                &checks::FLASH_LOAN_ISSUES,
                file,
                1,
                "Flash loan pattern detected — verify price and balance checks",
            )]
        } else {
            Vec::new()
        }
    }

    // ── MEV Issues ─────────────────────────────────────────
    fn check_mev_issues(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let has_swap = lines
            .iter()
            .any(|l| l.contains("swap") || l.contains("Swap"));
        let has_slippage = lines.iter().any(|l| {
            l.contains("slippage")
                || l.contains("minAmount")
                || l.contains("minReturn")
                || l.contains("amountOutMin")
        });

        if has_swap && !has_slippage {
            self.find_lines_containing(lines, &[".swap", "swap("])
                .into_iter()
                .map(|(line, snippet)| self.make_finding(&checks::MEV_ISSUES, file, line, snippet))
                .collect()
        } else {
            Vec::new()
        }
    }

    // ── Cross Chain Issues ─────────────────────────────────
    fn check_cross_chain_issues(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let cross_chain_keywords = [
            "crossChain",
            "cross-chain",
            "LZ",
            "LayerZero",
            "CCIP",
            "Wormhole",
            "Hyperlane",
        ];
        let has_cross_chain = lines
            .iter()
            .any(|l| cross_chain_keywords.iter().any(|k| l.contains(k)));

        if has_cross_chain {
            vec![self.make_finding(&checks::CROSS_CHAIN_ISSUES, file, 1,
                "Cross-chain interaction detected — verify chain ID handling and message verification")]
        } else {
            Vec::new()
        }
    }

    // ── Dependency Vulnerabilities ─────────────────────────
    fn check_dependency_vulnerabilities(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        // Check imports for well-known vulnerable libraries
        let mut findings = Vec::new();
        if let Some(_line) = lines
            .iter()
            .find(|l| l.contains("import") && l.contains("../"))
        {
            findings.push(self.make_finding(
                &checks::DEPENDENCY_VULNERABILITIES,
                file,
                1,
                "Relative import detected — verify package version",
            ));
        }
        findings
    }

    // ── Unsafe Imports ─────────────────────────────────────
    fn check_unsafe_imports(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["import "])
            .into_iter()
            .filter(|(_, snippet)| {
                snippet.contains("http")
                    || snippet.contains("github.com/")
                        && !snippet.contains("OpenZeppelin")
                        && !snippet.contains("solmate")
                        && !snippet.contains("forge-std")
            })
            .map(|(line, snippet)| self.make_finding(&checks::UNSAFE_IMPORTS, file, line, snippet))
            .collect()
    }

    // ── Unsafe Initializers ────────────────────────────────
    fn check_unsafe_initializers(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        let has_initializer = lines
            .iter()
            .any(|l| l.contains("initializer") || l.contains("reinitializer"));
        let has_constructor = lines.iter().any(|l| l.contains("constructor("));

        if has_initializer && !has_constructor {
            // Has initializer but no constructor — could be upgradeable
            Vec::new()
        } else if has_initializer && has_constructor {
            let has_disable = lines.iter().any(|l| l.contains("disableInitializers"));
            if !has_disable {
                vec![self.make_finding(
                    &checks::UNSAFE_INITIALIZERS,
                    file,
                    1,
                    "Contract has initializer and constructor but no disableInitializers() call",
                )]
            } else {
                Vec::new()
            }
        } else {
            Vec::new()
        }
    }

    // ── Unsafe Upgrade Paths ──────────────────────────────
    fn check_unsafe_upgrade_paths(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        let is_upgradeable = lines.iter().any(|l| {
            l.contains("is UUPSUpgradeable")
                || l.contains("is TransparentUpgradeableProxy")
                || l.contains("upgradeTo")
        });
        if is_upgradeable {
            vec![self.make_finding(
                &checks::UNSAFE_UPGRADE_PATHS,
                file,
                1,
                "Upgradeable contract detected — verify upgrade path security",
            )]
        } else {
            Vec::new()
        }
    }

    // ── Clone Vulnerabilities ──────────────────────────────
    fn check_clone_vulnerabilities(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        self.find_lines_containing(lines, &["Clones.", "clone(", "ERC1167"])
            .into_iter()
            .map(|(line, snippet)| {
                self.make_finding(&checks::CLONE_VULNERABILITIES, file, line, snippet)
            })
            .collect()
    }

    // ── Gas Problems ───────────────────────────────────────
    fn check_gas_problems(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let mut findings = Vec::new();
        // Detect loops with length evaluated each iteration
        for (i, line) in lines.iter().enumerate() {
            let trimmed = line.trim();
            if trimmed.contains("for (uint")
                && trimmed.contains("i < ")
                && !trimmed.contains("memory ")
                && !trimmed.contains(".length")
            {
                // Check if the next lines verify the pattern
                findings.push(self.make_finding(&checks::GAS_PROBLEMS, file, i + 1, trimmed));
            }
        }
        findings
    }

    // ── Unsafe Casting ─────────────────────────────────────
    fn check_unsafe_casting(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["uint256(", "int256(", "address("])
            .into_iter()
            .filter(|(_, s)| {
                s.contains("uint8(")
                    || s.contains("uint16(")
                    || s.contains("uint32(")
                    || s.contains("uint64(")
                    || s.contains("uint128(")
                    || s.contains("int8(")
                    || s.contains("int16(")
            })
            .map(|(line, snippet)| self.make_finding(&checks::UNSAFE_CASTING, file, line, snippet))
            .collect()
    }

    // ── Timestamp Manipulation ─────────────────────────────
    fn check_timestamp_manipulation(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        let mut findings = Vec::new();
        for (i, line) in lines.iter().enumerate() {
            if !line.trim_start().starts_with("//")
                && (line.contains("block.timestamp") || line.contains("now "))
            {
                // Check if used in critical logic (not just logging)
                if line.contains("if ")
                    || line.contains("require")
                    || line.contains("==")
                    || line.contains(">=")
                    || line.contains("<=")
                {
                    findings.push(self.make_finding(
                        &checks::TIMESTAMP_MANIPULATION,
                        file,
                        i + 1,
                        line.trim(),
                    ));
                }
            }
        }
        findings
    }

    // ── Storage Inefficiencies ─────────────────────────────
    fn check_storage_inefficiencies(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        // Check for variables that could be packed (uint256 used when smaller type works)
        self.find_lines_containing(
            lines,
            &["uint256 public ", "uint256 internal ", "uint256 private "],
        )
        .into_iter()
        .filter(|(_, s)| !s.contains("address") && !s.contains("mapping") && s.len() < 100)
        .take(3)
        .map(|(line, snippet)| {
            self.make_finding(&checks::STORAGE_INEFFICIENCIES, file, line, snippet)
        })
        .collect()
    }

    // ── Unsafe Events ──────────────────────────────────────
    fn check_unsafe_events(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        self.find_lines_containing(lines, &["event ", "emit "])
            .into_iter()
            .filter(|(_, s)| {
                s.contains("password")
                    || s.contains("secret")
                    || s.contains("key")
                    || s.contains("privateKey")
                    || s.contains("mnemonic")
            })
            .map(|(line, snippet)| self.make_finding(&checks::UNSAFE_EVENTS, file, line, snippet))
            .collect()
    }

    // ── Poor Visibility ────────────────────────────────────
    fn check_poor_visibility(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        // Check for public state variables that could be private
        self.find_lines_containing(lines, &["public "])
            .into_iter()
            .filter(|(_, s)| s.contains("mapping(") && s.contains("public"))
            .map(|(line, snippet)| self.make_finding(&checks::POOR_VISIBILITY, file, line, snippet))
            .collect()
    }

    // ── Bad Modifiers ──────────────────────────────────────
    fn check_bad_modifiers(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        // Check for modifiers that make external calls
        self.find_lines_containing(lines, &["modifier "])
            .into_iter()
            .filter(|(_, s)| {
                s.contains(".call")
                    || s.contains(".transfer")
                    || s.contains(".send")
                    || s.contains("delegatecall")
            })
            .map(|(line, snippet)| self.make_finding(&checks::BAD_MODIFIERS, file, line, snippet))
            .collect()
    }

    // ── Unsafe Math ────────────────────────────────────────
    fn check_unsafe_math(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        // Check for unchecked block usage
        let mut in_unchecked = false;
        let mut findings = Vec::new();

        for (i, line) in lines.iter().enumerate() {
            let trimmed = line.trim();
            if trimmed.contains("unchecked {") || trimmed.contains("unchecked{") {
                in_unchecked = true;
                continue;
            }
            if trimmed.starts_with('}') && in_unchecked {
                in_unchecked = false;
                continue;
            }
            if in_unchecked
                && (trimmed.contains("++")
                    || trimmed.contains("--")
                    || trimmed.contains("+=")
                    || trimmed.contains("-=")
                    || trimmed.contains("*="))
            {
                findings.push(self.make_finding(&checks::UNSAFE_MATH, file, i + 1, trimmed));
            }
        }
        findings
    }

    // ── Poor Access Patterns ───────────────────────────────
    fn check_poor_access_patterns(
        &self,
        file: &str,
        lines: &[&str],
        _content: &str,
    ) -> Vec<Finding> {
        self.find_lines_containing(lines, &["storage ", "storage)"])
            .into_iter()
            .filter(|(_, s)| {
                !s.contains("memory") && (s.contains("memory") || s.contains("calldata"))
            })
            .take(3)
            .map(|(line, snippet)| {
                self.make_finding(&checks::POOR_ACCESS_PATTERNS, file, line, snippet)
            })
            .collect()
    }

    // ── Naming Issues ──────────────────────────────────────
    fn check_naming_issues(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let mut findings = Vec::new();
        for (i, line) in lines.iter().enumerate() {
            let trimmed = line.trim();
            // Check for contract/functions that don't follow camelCase
            if trimmed.starts_with("function ") && !trimmed.contains("(") {
                let name = trimmed.split_whitespace().nth(1).unwrap_or("");
                if name.contains('_') {
                    findings.push(self.make_finding(&checks::NAMING_ISSUES, file, i + 1, trimmed));
                    if findings.len() >= 3 {
                        break;
                    }
                }
            }
        }
        findings
    }

    // ── Code Duplication ──────────────────────────────────
    fn check_code_duplication(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        // Simple heuristic: check for repeated patterns across lines
        let mut findings = Vec::new();
        if lines.len() > 50 {
            let total_lines = lines.len();
            let unique_lines: std::collections::HashSet<&&str> = lines.iter().collect();
            let ratio = unique_lines.len() as f64 / total_lines as f64;
            if ratio < 0.5 {
                findings.push(self.make_finding(
                    &checks::CODE_DUPLICATION,
                    file,
                    1,
                    &format!(
                        "Low code diversity ({:.0}% unique lines) — possible duplication",
                        ratio * 100.0
                    ),
                ));
            }
        }
        findings
    }

    // ── Optimization ──────────────────────────────────────
    fn check_optimization(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let mut findings = Vec::new();
        for (i, line) in lines.iter().enumerate() {
            let trimmed = line.trim();
            // Suggest using calldata instead of memory
            if trimmed.contains("memory ")
                && !trimmed.contains("internal")
                && !trimmed.starts_with("//")
            {
                findings.push(self.make_finding(
                    &checks::OPTIMIZATION_ISSUES,
                    file,
                    i + 1,
                    trimmed,
                ));
                if findings.len() >= 2 {
                    break;
                }
            }
        }
        findings
    }

    // ── Style Issues ──────────────────────────────────────
    fn check_style_issues(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let mut findings = Vec::new();
        for (i, line) in lines.iter().enumerate() {
            if line.len() > 120 {
                findings.push(self.make_finding(
                    &checks::STYLE_ISSUES,
                    file,
                    i + 1,
                    "Line exceeds 120 characters",
                ));
                if findings.len() >= 2 {
                    break;
                }
            }
        }
        findings
    }

    // ── Documentation ─────────────────────────────────────
    fn check_documentation(&self, file: &str, lines: &[&str], _content: &str) -> Vec<Finding> {
        let mut findings = Vec::new();
        let mut has_natspec = false;

        for (i, line) in lines.iter().enumerate() {
            let trimmed = line.trim();

            if trimmed.starts_with("/**") {
                has_natspec = true;
                continue;
            }

            if trimmed.starts_with("function ") {
                if !has_natspec && i > 0 && !lines[i - 1].trim().starts_with("//") {
                    findings.push(self.make_finding(
                        &checks::DOCUMENTATION_ISSUES,
                        file,
                        i + 1,
                        trimmed,
                    ));
                    if findings.len() >= 3 {
                        break;
                    }
                }
                has_natspec = false;
            }

            if trimmed.ends_with("*/") || trimmed.starts_with("///") || trimmed.starts_with("* ") {
                has_natspec = true;
            }
        }
        findings
    }
}