repotoire 0.8.2

//! Mutual recursion detector using call-graph SCCs.
//!
//! Identifies groups of functions that form call cycles (mutually recursive
//! sets). These create tight coupling and make reasoning, testing, and
//! refactoring difficult — especially when combined with high complexity.

use crate::detectors::base::{Detector, DetectorConfig, DetectorScope};
use crate::detectors::is_line_suppressed_for;
use crate::models::{Finding, Severity};
use anyhow::Result;
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use tracing::debug;

/// Detects mutual recursion via call-graph strongly connected components.
///
/// Functions that call each other in a cycle create mutual recursion.
/// This detector reports call cycles with configurable size limits and
/// severity based on cycle size and aggregate complexity.
///
/// Uses pre-computed graph primitives:
/// - `call_cycles_idx()`: SCCs in the call graph with size >= 2
/// - `node_idx()`: node lookup for complexity and file path
pub struct MutualRecursionDetector {
    config: DetectorConfig,
    /// Maximum cycle size to report (skip very large SCCs).
    max_cycle_size: usize,
}

detector_constructors! {
    MutualRecursionDetector {
        max_cycle_size: usize = config_opt("max_cycle_size", 50),
    }
}

impl Detector for MutualRecursionDetector {
    fn name(&self) -> &'static str {
        "MutualRecursionDetector"
    }

    fn description(&self) -> &'static str {
        "Detects mutual recursion (call cycles) between functions"
    }

    fn category(&self) -> &'static str {
        "code_smell"
    }

    fn config(&self) -> Option<&DetectorConfig> {
        Some(&self.config)
    }

    fn detector_scope(&self) -> DetectorScope {
        DetectorScope::GraphWide
    }

    fn is_deterministic(&self) -> bool {
        true
    }

    fn detect(
        &self,
        ctx: &crate::detectors::analysis_context::AnalysisContext,
    ) -> Result<Vec<Finding>> {
        let graph = ctx.graph;
        let gi = graph.interner();

        let cycles = &graph.primitives().call_cycles;

        if cycles.is_empty() {
            return Ok(vec![]);
        }

        debug!(
            "MutualRecursionDetector: examining {} call cycles",
            cycles.len()
        );

        let mut findings = Vec::new();

        for cycle in cycles {
            if cycle.len() > self.max_cycle_size {
                debug!(
                    "Skipping large call cycle with {} functions (max: {})",
                    cycle.len(),
                    self.max_cycle_size
                );
                continue;
            }

            // Sum complexity across all functions in the cycle.
            // Note: call_cycles uses NodeIndex; convert to QN for node_idx lookups.
            let total_complexity: u32 = cycle
                .iter()
                .filter_map(|&idx| graph.node_idx(idx))
                .map(|n| n.complexity as u32)
                .sum();

            let cycle_size = cycle.len();

            // Skip trivial cycles: small size AND low complexity are usually intentional
            // Data: 2-function cycles with complexity 2 are boilerplate (12 on repotoire, all noise)
            //       Cycles with complexity 15+ or 4+ functions are real architectural issues
            if cycle_size < 4 && total_complexity < 20 {
                continue;
            }

            // Same-file 2-cycles: cap severity at Info regardless of complexity.
            //
            // Rationale: a 2-cycle entirely within one file is overwhelmingly
            // dominated by recursive-descent / visitor / classifier patterns
            // (e.g. AST classifiers that recurse through node-kind dispatch).
            // These are structurally tree-recursive over a finite AST and
            // never problematic. Real architectural mutual recursion almost
            // always crosses module boundaries.
            //
            // The previous heuristic used a name-prefix allowlist
            // (`visit*`, `walk*`, `classify*`, etc.). That approach was
            // brittle in both directions: it missed legitimate visitor verbs
            // (e.g. `classify_*`, `lower_*`, `fold_*`) and false-allowed
            // accidental cycles whose names happened to match.
            //
            // A proper structural fix would inspect parameter types (e.g.
            // detect `Node<'_>`-receiving functions). That requires graph
            // extraction to capture parameter type strings, which is out of
            // scope here. Until then, severity capping is the calibrated
            // signal.
            let cycle_files: HashSet<&str> = cycle
                .iter()
                .filter_map(|&idx| graph.node_idx(idx).map(|n| n.path(gi)))
                .collect();
            let same_file_pair = cycle_files.len() == 1 && cycle_size == 2;

            // Severity based on cycle size and aggregate complexity, with
            // same-file 2-cycles capped at Info.
            let severity = if same_file_pair {
                Severity::Info
            } else if cycle_size > 5 || total_complexity > 30 {
                Severity::High
            } else if cycle_size > 3 || total_complexity > 20 {
                Severity::Medium
            } else {
                Severity::Low
            };

            // Collect function names, file paths, and line numbers.
            let mut func_names = Vec::new();
            let mut affected_files: HashSet<PathBuf> = HashSet::new();
            let mut func_lines: Vec<(PathBuf, u32)> = Vec::new();

            for &idx in cycle {
                if let Some(node) = graph.node_idx(idx) {
                    func_names.push(node.qn(gi).to_string());
                    let p = PathBuf::from(node.path(gi));
                    affected_files.insert(p.clone());
                    func_lines.push((p, node.line_start));
                }
            }

            // Honor `repotoire:ignore[mutual-recursion]` suppression comments
            // on any function in the cycle. A single annotated function
            // suppresses the whole finding.
            if is_cycle_suppressed(&func_lines, ctx.files.as_ref()) {
                debug!(
                    "Skipping suppressed cycle of {} functions: {}",
                    cycle_size,
                    func_names.first().map(|s| s.as_str()).unwrap_or("?")
                );
                continue;
            }

            let cycle_display = if func_names.len() <= 8 {
                func_names.join(" -> ")
            } else {
                let first_few: Vec<&str> = func_names.iter().take(6).map(|s| s.as_str()).collect();
                format!(
                    "{} ... (+{} more)",
                    first_few.join(" -> "),
                    func_names.len() - 6
                )
            };

            let description = format!(
                "Mutual recursion detected: {} functions form a call cycle. \
                 Cycle: {}. Aggregate complexity: {}.",
                cycle_size, cycle_display, total_complexity,
            );

            findings.push(Finding {
                id: String::new(),
                detector: "mutual-recursion".to_string(),
                severity,
                confidence: Some(0.95),
                deterministic: true, // Graph-theoretic: Tarjan SCC is mathematically provable
                title: format!(
                    "Mutual recursion: {} functions in call cycle (complexity {})",
                    cycle_size, total_complexity,
                ),
                description,
                affected_files: affected_files.into_iter().collect(),
                line_start: cycle
                    .first()
                    .and_then(|&idx| graph.node_idx(idx))
                    .map(|n| n.line_start),
                line_end: None,
                suggested_fix: Some(if cycle_size == 2 {
                    "Consider refactoring to eliminate direct mutual calls. \
                     Common strategies: merge the two functions, use a callback parameter, \
                     or introduce a shared data structure that both functions operate on."
                        .to_string()
                } else {
                    "Break the cycle by extracting shared logic into a common helper, \
                     introducing an event system, or restructuring the call chain \
                     to be unidirectional."
                        .to_string()
                }),
                estimated_effort: Some(if cycle_size > 5 {
                    "Large (1-3 days)".to_string()
                } else if cycle_size > 2 {
                    "Medium (4-8 hours)".to_string()
                } else {
                    "Small (1-4 hours)".to_string()
                }),
                category: Some("code_smell".to_string()),
                why_it_matters: Some(
                    "Mutual recursion creates tight coupling between functions, making them \
                     impossible to understand, test, or refactor independently. It can also \
                     cause stack overflows if the recursion depth is unbounded."
                        .to_string(),
                ),
                ..Default::default()
            });
        }

        // Sort by severity (highest first).
        findings.sort_by_key(|f| std::cmp::Reverse(f.severity));

        debug!("MutualRecursionDetector found {} findings", findings.len());

        Ok(findings)
    }
}

/// Check whether ANY function in the cycle has a `repotoire:ignore[mutual-recursion]`
/// (or unscoped `repotoire:ignore`) suppression comment on or immediately above
/// its definition line.
///
/// Suppression is intentionally permissive: a single annotated function in the
/// cycle suppresses the entire finding. This matches user intent ("this cycle
/// is known and intentional") without requiring annotations on every member.
///
/// Returns `false` (do not suppress) if `files` is `None` or the file/line
/// cannot be located — i.e. fail open: when in doubt, report.
fn is_cycle_suppressed(
    func_lines: &[(PathBuf, u32)],
    files: &crate::detectors::file_index::FileIndex,
) -> bool {
    for (path, line_start) in func_lines {
        if line_start_is_suppressed(path, *line_start, files) {
            return true;
        }
    }
    false
}

/// Check whether `path:line_start` (1-indexed) carries a suppression comment
/// for `mutual-recursion` (either inline on the line or as a comment on the
/// line directly above).
fn line_start_is_suppressed(
    path: &Path,
    line_start: u32,
    files: &crate::detectors::file_index::FileIndex,
) -> bool {
    if line_start == 0 {
        return false;
    }
    let Some(entry) = files.get(path) else {
        return false;
    };
    let content: &str = &entry.content;
    let target_idx = (line_start as usize).saturating_sub(1);
    let lines: Vec<&str> = content.lines().collect();
    let Some(line) = lines.get(target_idx) else {
        return false;
    };
    let prev = if target_idx > 0 {
        lines.get(target_idx - 1).copied()
    } else {
        None
    };
    is_line_suppressed_for(line, prev, "mutual-recursion")
}

impl crate::detectors::RegisteredDetector for MutualRecursionDetector {
    fn create(init: &crate::detectors::DetectorInit) -> Arc<dyn Detector> {
        Arc::new(Self::with_config(
            init.config_for("MutualRecursionDetector"),
        ))
    }
}

#[cfg(test)]
mod tests {
    use super::*;
    use crate::graph::{CodeEdge, CodeNode, GraphBuilder};

    #[test]
    fn test_detects_mutual_recursion_pair() {
        // f1 calls f2, f2 calls f1 — cross-module (different dirs).
        // Skip condition: size < 4 AND complexity < 20.
        // Use complexity 11+9=20 to pass the filter (not < 20) while staying <= 20
        // so severity stays Low (not Medium which requires complexity > 20).
        let mut builder = GraphBuilder::new();

        let mut f1_node = CodeNode::function("f1", "module_a/a.py");
        f1_node.complexity = 11;
        let mut f2_node = CodeNode::function("f2", "module_b/b.py");
        f2_node.complexity = 9;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);

        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f1, CodeEdge::calls());

        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();
        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert_eq!(findings.len(), 1, "Should detect exactly one call cycle");
        assert_eq!(
            findings[0].severity,
            Severity::Low,
            "Pair should be Low severity"
        );
        assert!(
            findings[0].description.contains("2 functions"),
            "Should mention 2 functions: {}",
            findings[0].description
        );
    }

    #[test]
    fn test_detects_triangle_recursion() {
        // f1 -> f2 -> f3 -> f1
        // Skip condition: size < 4 AND complexity < 20.
        // 3-function cycle: need total_complexity >= 20 to avoid being skipped.
        let mut builder = GraphBuilder::new();

        let mut f1_node = CodeNode::function("f1", "a.py");
        f1_node.complexity = 8;
        let mut f2_node = CodeNode::function("f2", "a.py");
        f2_node.complexity = 7;
        let mut f3_node = CodeNode::function("f3", "a.py");
        f3_node.complexity = 7;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);
        let f3 = builder.add_node(f3_node);

        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f3, CodeEdge::calls());
        builder.add_edge(f3, f1, CodeEdge::calls());

        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();
        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert_eq!(findings.len(), 1, "Should detect exactly one call cycle");
        assert_eq!(
            findings[0].severity,
            Severity::Medium,
            "Triangle should be Medium"
        );
    }

    #[test]
    fn test_no_cycle_in_dag() {
        // f1 -> f2 -> f3 (no back-edge)
        let mut builder = GraphBuilder::new();

        let f1 = builder.add_node(CodeNode::function("f1", "a.py"));
        let f2 = builder.add_node(CodeNode::function("f2", "a.py"));
        let f3 = builder.add_node(CodeNode::function("f3", "a.py"));

        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f3, CodeEdge::calls());

        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();
        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert!(findings.is_empty(), "DAG should have no mutual recursion");
    }

    #[test]
    fn test_skips_large_cycle() {
        // Create a cycle of 6 functions but set max_cycle_size=3.
        let mut builder = GraphBuilder::new();

        let mut nodes = Vec::new();
        for i in 0..6 {
            let n = builder.add_node(CodeNode::function(&format!("f{}", i), "big.py"));
            nodes.push(n);
        }
        for i in 0..6 {
            builder.add_edge(nodes[i], nodes[(i + 1) % 6], CodeEdge::calls());
        }

        let graph = builder.freeze();
        let config = DetectorConfig::new().with_option("max_cycle_size", serde_json::json!(3));
        let detector = MutualRecursionDetector::with_config(config);

        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert!(
            findings.is_empty(),
            "Should skip cycle larger than max_cycle_size"
        );
    }

    #[test]
    fn test_high_severity_for_large_or_complex_cycle() {
        // 6-function cycle should be High severity.
        let mut builder = GraphBuilder::new();

        let mut nodes = Vec::new();
        for i in 0..6 {
            let mut n = CodeNode::function(&format!("f{}", i), "complex.py");
            n.complexity = 3;
            nodes.push(builder.add_node(n));
        }
        for i in 0..6 {
            builder.add_edge(nodes[i], nodes[(i + 1) % 6], CodeEdge::calls());
        }

        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();

        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert_eq!(findings.len(), 1);
        assert_eq!(findings[0].severity, Severity::High);
    }

    #[test]
    fn test_empty_graph() {
        let builder = GraphBuilder::new();
        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();
        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");
        assert!(findings.is_empty());
    }

    #[test]
    fn test_scope_is_graph_wide() {
        let detector = MutualRecursionDetector::new();
        assert_eq!(detector.detector_scope(), DetectorScope::GraphWide);
    }

    #[test]
    fn test_category_is_code_smell() {
        let detector = MutualRecursionDetector::new();
        assert_eq!(detector.category(), "code_smell");
    }

    #[test]
    fn test_same_file_pair_capped_at_info() {
        // 2-function cycle within ONE file should be Info regardless of complexity.
        // This replaces the prior name-prefix allowlist heuristic.
        // The dogfood case: classify_command_arg_python ↔ classify_list_elements_py
        // (high aggregate complexity, same file, AST recursive descent).
        let mut builder = GraphBuilder::new();

        let mut f1_node = CodeNode::function("classify_command_arg_python", "command_injection.rs");
        f1_node.complexity = 25;
        let mut f2_node = CodeNode::function("classify_list_elements_py", "command_injection.rs");
        f2_node.complexity = 15;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);

        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f1, CodeEdge::calls());

        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();
        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert_eq!(findings.len(), 1, "Same-file 2-cycle is still reported");
        assert_eq!(
            findings[0].severity,
            Severity::Info,
            "Same-file 2-cycle should be Info regardless of names or complexity"
        );
    }

    #[test]
    fn test_same_file_triangle_not_capped() {
        // 3-function same-file cycle is NOT capped — only 2-cycles are.
        // Cross-file or larger cycles use the normal severity ladder.
        let mut builder = GraphBuilder::new();

        let mut f1_node = CodeNode::function("a", "same.py");
        f1_node.complexity = 8;
        let mut f2_node = CodeNode::function("b", "same.py");
        f2_node.complexity = 7;
        let mut f3_node = CodeNode::function("c", "same.py");
        f3_node.complexity = 7;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);
        let f3 = builder.add_node(f3_node);

        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f3, CodeEdge::calls());
        builder.add_edge(f3, f1, CodeEdge::calls());

        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();
        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert_eq!(findings.len(), 1);
        assert_eq!(
            findings[0].severity,
            Severity::Medium,
            "Same-file triangle is NOT capped — uses normal ladder (complexity 22 -> Medium)"
        );
    }

    #[test]
    fn test_cross_file_pair_not_capped() {
        // 2-function cycle across DIFFERENT files is NOT capped.
        // Cross-file mutual recursion is a real architectural smell.
        let mut builder = GraphBuilder::new();

        let mut f1_node = CodeNode::function("f1", "module_a/a.py");
        f1_node.complexity = 11;
        let mut f2_node = CodeNode::function("f2", "module_b/b.py");
        f2_node.complexity = 9;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);

        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f1, CodeEdge::calls());

        let graph = builder.freeze();
        let detector = MutualRecursionDetector::new();
        let ctx = crate::detectors::analysis_context::AnalysisContext::test(&graph);
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert_eq!(findings.len(), 1);
        assert_eq!(
            findings[0].severity,
            Severity::Low,
            "Cross-file 2-cycle uses normal ladder (not capped to Info)"
        );
    }

    #[test]
    fn test_suppression_comment_on_function_definition() {
        // A `repotoire:ignore[mutual-recursion]` comment above a function
        // definition suppresses the entire cycle. Single annotation suffices.
        let mut builder = GraphBuilder::new();

        // Triangle cycle that would otherwise fire as Medium
        // (3 functions same file, complexity 8+7+7=22).
        let mut f1_node = CodeNode::function("alpha", "src/a.rs");
        f1_node.complexity = 8;
        f1_node.line_start = 3; // 1-indexed line where `fn alpha` lives
        let mut f2_node = CodeNode::function("beta", "src/a.rs");
        f2_node.complexity = 7;
        f2_node.line_start = 7;
        let mut f3_node = CodeNode::function("gamma", "src/a.rs");
        f3_node.complexity = 7;
        f3_node.line_start = 11;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);
        let f3 = builder.add_node(f3_node);
        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f3, CodeEdge::calls());
        builder.add_edge(f3, f1, CodeEdge::calls());
        let graph = builder.freeze();

        // File content where `beta` (line 7) has a suppression comment
        // on the line directly above it (line 6).
        let content = "\
// line 1
// line 2
fn alpha() {}
// line 4
// line 5
// repotoire:ignore[mutual-recursion]
fn beta() {}
// line 8
// line 9
// line 10
fn gamma() {}
";
        let ctx = crate::detectors::analysis_context::AnalysisContext::test_with_mock_files(
            &graph,
            vec![("src/a.rs", content)],
        );

        let detector = MutualRecursionDetector::new();
        let findings = detector.detect(&ctx).expect("detection should succeed");

        assert!(
            findings.is_empty(),
            "Cycle suppressed by single annotation should not fire (got {} findings)",
            findings.len()
        );
    }

    #[test]
    fn test_unscoped_suppression_also_works() {
        // `repotoire:ignore` (no detector name) suppresses everything.
        let mut builder = GraphBuilder::new();

        let mut f1_node = CodeNode::function("a", "src/x.rs");
        f1_node.complexity = 11;
        f1_node.line_start = 2;
        let mut f2_node = CodeNode::function("b", "src/y.rs");
        f2_node.complexity = 9;
        f2_node.line_start = 1;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);
        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f1, CodeEdge::calls());
        let graph = builder.freeze();

        // Inline suppression on the function-definition line itself.
        let content_x = "\
// preamble
fn a() {} // repotoire:ignore
";
        let content_y = "fn b() {}\n";

        let ctx = crate::detectors::analysis_context::AnalysisContext::test_with_mock_files(
            &graph,
            vec![("src/x.rs", content_x), ("src/y.rs", content_y)],
        );

        let detector = MutualRecursionDetector::new();
        let findings = detector.detect(&ctx).expect("detection should succeed");
        assert!(
            findings.is_empty(),
            "Unscoped repotoire:ignore should suppress mutual-recursion finding"
        );
    }

    #[test]
    fn test_suppression_for_different_detector_does_not_apply() {
        // `repotoire:ignore[some-other-detector]` should NOT suppress
        // mutual-recursion findings.
        let mut builder = GraphBuilder::new();

        let mut f1_node = CodeNode::function("a", "src/p.rs");
        f1_node.complexity = 11;
        f1_node.line_start = 2;
        let mut f2_node = CodeNode::function("b", "src/q.rs");
        f2_node.complexity = 9;
        f2_node.line_start = 1;

        let f1 = builder.add_node(f1_node);
        let f2 = builder.add_node(f2_node);
        builder.add_edge(f1, f2, CodeEdge::calls());
        builder.add_edge(f2, f1, CodeEdge::calls());
        let graph = builder.freeze();

        let content_p = "\
// repotoire:ignore[surprisal]
fn a() {}
";
        let content_q = "fn b() {}\n";

        let ctx = crate::detectors::analysis_context::AnalysisContext::test_with_mock_files(
            &graph,
            vec![("src/p.rs", content_p), ("src/q.rs", content_q)],
        );

        let detector = MutualRecursionDetector::new();
        let findings = detector.detect(&ctx).expect("detection should succeed");
        assert_eq!(
            findings.len(),
            1,
            "Suppression scoped to another detector should not apply here"
        );
    }
}