ryo-suggest 0.1.0

//! UnnecessaryClone - Detect clone() calls where ownership is not needed
//!
//! This rule analyzes data flow to find clone() operations that could be
//! replaced with references or removed entirely.
//!
//! # Rule Code
//! RP001 (Ryo Performance)
//!
//! # Detection Strategy
//!
//! 1. Find all `FlowKind::Clone` edges in DataFlowGraphV2
//! 2. Analyze how the cloned value is used downstream
//! 3. Report if ownership is not actually needed
//!
//! # Patterns Detected
//!
//! ## Clone immediately dropped (confidence 0.9)
//! ```ignore
//! let x = data.clone();
//! // x is never used - unnecessary
//! ```
//!
//! ## Clone used only as reference (confidence 0.7)
//! ```ignore
//! let x = data.clone();
//! foo(&x);  // Only borrowed - could use &data directly
//! ```
//!
//! ## Clone passed as argument (confidence 0.7, callee-aware)
//! ```ignore
//! let x = data.clone();
//! process(x);  // Checks if process() accepts &T
//! // Reported only when ALL callees accept references
//! ```
//!
//! # FP Suppression Mechanisms
//!
//! The following heuristics suppress false positives:
//!
//! 1. **`let mut` binding** — `let mut x = y.clone()` indicates the clone will
//!    be mutated as a working copy. Suppresses Case 2 (OnlySharedBorrow).
//!
//! 2. **Source consumption** — If the clone source variable has Move, Argument,
//!    MutBorrow, Write, or Assign flows, the clone is likely a snapshot preserving
//!    state before the source is consumed. Suppresses Case 2.
//!
//! 3. **Receiver type heuristic** — Method calls ending in `_mut` (e.g. `get_mut`,
//!    `iter_mut`) classify the receiver as MutBorrow, preventing false reports on
//!    variables used via mutable method chains.
//!
//! 4. **Argument flow classification** — `&expr` → SharedBorrow, `&mut expr` →
//!    MutBorrow (not generic Argument). Prevents false reports when clone is
//!    passed as `&mut` to a function.
//!
//! # Known Limitations
//!
//! **Borrow checker constraint clones**: When a clone is needed to satisfy
//! Rust's borrowing rules (not ownership), this rule may still report it.
//! Example:
//! ```ignore
//! let project = storage.get_project(path);  // borrows &storage
//! let id = project.id.clone();              // clone to release borrow
//! storage.remove(&id);                      // needs &mut storage
//! ```
//! The clone source (`project`) has no mutation flows — it's the *source's
//! source* (`storage`) that requires `&mut`. DataFlow tracks only 1-level
//! source relationships, so this transitive borrow conflict is not detected.

use ryo_analysis::context::AnalysisContext;
use ryo_analysis::{FlowKind, SymbolId, VarId, VarKind};

use super::{PerformanceDetails, PerformanceSuggest};
use crate::{
    LintSeverity, MutationSpec, OpportunityId, SafetyLevel, Suggest, SuggestCategory,
    SuggestLocation, SuggestOpportunity, SuggestResult,
};

/// UnnecessaryClone detection rule
///
/// Detects clone() calls where the cloned value doesn't actually need ownership.
/// See module-level documentation for detection strategy, FP suppression
/// mechanisms, and known limitations.
///
/// # Detection Confidence Levels
///
/// - **High (0.9)**: Clone immediately dropped (no successors)
/// - **Medium (0.7)**: Clone used only via SharedBorrow (suppressed if `let mut`
///   or source consumed)
/// - **Medium (0.7)**: Clone passed as argument, all callees accept references
pub struct UnnecessaryClone {
    /// Minimum confidence threshold for reporting
    min_confidence: f32,
}

impl UnnecessaryClone {
    pub fn new() -> Self {
        Self {
            min_confidence: 0.5,
        }
    }

    /// Set minimum confidence threshold
    pub fn with_min_confidence(mut self, threshold: f32) -> Self {
        self.min_confidence = threshold.clamp(0.0, 1.0);
        self
    }

    /// Analyze if a cloned variable actually needs ownership
    fn analyze_clone_necessity(
        &self,
        ctx: &AnalysisContext,
        cloned_var: VarId,
    ) -> Option<CloneAnalysis> {
        let dataflow = &ctx.dataflow_graph;

        // Get outgoing flows from the cloned variable
        let outgoing_flows = dataflow.outgoing(cloned_var);

        // Case 1: No outgoing flows = clone immediately dropped
        if outgoing_flows.is_empty() {
            return Some(CloneAnalysis {
                pattern: ClonePattern::ImmediatelyDropped,
                confidence: 0.9,
                suggestion: "Remove unused clone()".to_string(),
            });
        }

        // Analyze how the cloned value is used
        let mut has_move = false;
        let mut has_mut_borrow = false;
        let mut has_shared_borrow = false;
        let mut has_return = false;
        let mut has_argument = false;

        for &flow_id in outgoing_flows {
            if let Some(flow_data) = dataflow.flow(flow_id) {
                match flow_data.kind {
                    FlowKind::Move => has_move = true,
                    FlowKind::MutBorrow => has_mut_borrow = true,
                    FlowKind::SharedBorrow => has_shared_borrow = true,
                    FlowKind::Return => has_return = true,
                    FlowKind::Argument => has_argument = true,
                    _ => {}
                }
            }
        }

        // Case 2: Only used via SharedBorrow
        //
        // The cloned value is never moved, mutably borrowed, or returned — it's
        // only read through `&x`. In theory, `&source` could be used directly.
        //
        // Suppression rules (return None instead of reporting):
        //   (a) `let mut x = source.clone()` — the `mut` binding signals intent
        //       to mutate the working copy (e.g. `x.set_extension()`).
        //   (b) Source variable has Move/Argument/MutBorrow/Write/Assign flows —
        //       the clone is a snapshot preserving state before source mutation.
        //
        // Known residual FP: borrow-checker-required clones where the source
        // is a reference into a container that is later mutably borrowed.
        // See module docs "Known Limitations".
        if has_shared_borrow && !has_move && !has_mut_borrow && !has_return {
            let var_is_mut = dataflow.var(cloned_var).is_some_and(|v| v.is_mut);
            if var_is_mut || self.source_is_consumed_or_mutated(ctx, cloned_var) {
                return None;
            }
            return Some(CloneAnalysis {
                pattern: ClonePattern::OnlySharedBorrow,
                confidence: 0.7,
                suggestion: "Consider using reference instead of clone()".to_string(),
            });
        }

        // Case 3: Passed as argument — check callee parameter types
        // Skip if the cloned value is mutably borrowed: the clone creates a
        // working copy that is modified in-place, so ownership is required.
        if has_argument && !has_return && !has_move && !has_mut_borrow {
            if let Some(analysis) = self.analyze_argument_escape(ctx, cloned_var) {
                return Some(analysis);
            }
            // Could not confirm clone is unnecessary → suppress
            return None;
        }

        // Case 4: Ownership is actually needed
        None
    }

    /// Check callee parameter types to determine if a clone passed as argument
    /// is actually necessary.
    ///
    /// Returns Some(CloneAnalysis) only when ALL callees of the parent function
    /// accept references (not owned). Returns None to suppress the report when:
    /// - Any callee takes owned T (clone might be needed)
    /// - No callee info available (conservative)
    fn analyze_argument_escape(
        &self,
        ctx: &AnalysisContext,
        cloned_var: VarId,
    ) -> Option<CloneAnalysis> {
        let dataflow = &ctx.dataflow_graph;

        // Get the parent function containing the clone
        let var_data = dataflow.var(cloned_var)?;
        let parent_fn = var_data.parent;

        // Get all callees of the parent function
        let callees: Vec<SymbolId> = ctx.code_graph.callees_of(parent_fn).collect();

        // No callee info → can't determine, suppress
        if callees.is_empty() {
            return None;
        }

        // Check if ANY callee has a non-self parameter that takes owned T
        for callee_id in &callees {
            let Some(fn_detail) = ctx.detail_store.function(*callee_id) else {
                // No detail info for this callee → parameter types unknown.
                // Be conservative: the callee might take ownership.
                return None;
            };
            for param in &fn_detail.params {
                // Skip self parameters
                if param.is_self {
                    continue;
                }
                // If parameter type is NOT a reference, the callee takes ownership
                let ty = param.ty.trim();
                if !ty.starts_with('&') {
                    // At least one callee takes owned T → clone might be needed
                    return None;
                }
            }
        }

        // ALL known callees accept references → clone is likely unnecessary.
        // Note: the callee list may be incomplete for method calls on external
        // types, but the `else { return None }` guard above ensures we only
        // reach here when every callee has detail info.
        Some(CloneAnalysis {
            pattern: ClonePattern::ArgumentNoEscape,
            confidence: 0.7,
            suggestion: "All called functions accept references - clone() is likely unnecessary"
                .to_string(),
        })
    }

    /// Check if the source variable of a Clone flow is consumed or mutated.
    ///
    /// This detects the "snapshot" pattern where a clone preserves state before
    /// the source is moved, mutably borrowed, or passed as an owned argument:
    /// ```ignore
    /// let snapshot = data.clone();  // snapshot for diff
    /// consume(data);               // source consumed → detected
    /// ```
    ///
    /// # Limitation
    ///
    /// Only checks **direct** outgoing flows of the clone source variable.
    /// Does not detect transitive borrow conflicts where the source is a
    /// reference into a container that is later `&mut`-borrowed:
    /// ```ignore
    /// let item = container.get(key);   // item borrows &container
    /// let val = item.field.clone();    // source = item, no mutation flows
    /// container.remove(key);           // needs &mut container — conflict!
    /// ```
    /// In this case, `item` has no Move/MutBorrow flows, so the check returns
    /// `false` even though the clone is required by the borrow checker.
    fn source_is_consumed_or_mutated(&self, ctx: &AnalysisContext, cloned_var: VarId) -> bool {
        let dataflow = &ctx.dataflow_graph;

        // Find the Clone flow incoming to cloned_var to get the source variable
        for &flow_id in dataflow.incoming(cloned_var) {
            let Some(flow_data) = dataflow.flow(flow_id) else {
                continue;
            };
            if flow_data.kind != FlowKind::Clone {
                continue;
            }
            let Some(edge) = dataflow.edge(flow_id) else {
                continue;
            };
            let source_var = edge.from;

            // Check if the source variable has any ownership-consuming or
            // mutating outgoing flows
            for &out_flow_id in dataflow.outgoing(source_var) {
                if let Some(out_flow) = dataflow.flow(out_flow_id) {
                    match out_flow.kind {
                        FlowKind::Move
                        | FlowKind::Argument
                        | FlowKind::MutBorrow
                        | FlowKind::Write
                        | FlowKind::Assign => return true,
                        _ => {}
                    }
                }
            }
        }

        false
    }
}

impl Default for UnnecessaryClone {
    fn default() -> Self {
        Self::new()
    }
}

impl PerformanceSuggest for UnnecessaryClone {
    fn code(&self) -> &'static str {
        "RP001"
    }

    fn default_severity(&self) -> LintSeverity {
        LintSeverity::Warning
    }
}

impl Suggest for UnnecessaryClone {
    fn name(&self) -> &'static str {
        "unnecessary-clone"
    }

    fn description(&self) -> &str {
        "Detects clone() calls where ownership is not actually needed"
    }

    fn category(&self) -> SuggestCategory {
        SuggestCategory::Performance
    }

    fn safety_level(&self) -> SafetyLevel {
        SafetyLevel::Confirm // Needs manual review
    }

    fn priority_weight(&self) -> f32 {
        1.2 // Medium-high priority for performance
    }

    fn detect(&self, ctx: &AnalysisContext, symbols: &[SymbolId]) -> Vec<SuggestOpportunity> {
        let mut opportunities = Vec::new();
        let mut next_id = 0u32;

        let dataflow = &ctx.dataflow_graph;

        // Determine which parent symbols to check
        let parent_symbols: Vec<SymbolId> = if symbols.is_empty() {
            // Check all functions/methods
            ctx.registry
                .iter_by_kind(ryo_analysis::SymbolKind::Function)
                .chain(ctx.registry.iter_by_kind(ryo_analysis::SymbolKind::Method))
                .collect()
        } else {
            symbols.to_vec()
        };

        // Iterate through flows to find Clone operations
        for (_flow_id, flow_data, edge) in dataflow.iter_flows() {
            if flow_data.kind != FlowKind::Clone {
                continue;
            }

            // Get the target variable (the cloned value)
            let cloned_var = edge.to;

            // Get parent symbol of the cloned variable
            let Some(var_data) = dataflow.var(cloned_var) else {
                continue;
            };

            // Skip if not in our target symbols
            if !symbols.is_empty() && !parent_symbols.contains(&var_data.parent) {
                continue;
            }

            // Skip temporary variables (often internal)
            if var_data.kind == VarKind::Temp {
                continue;
            }

            // Analyze if this clone is necessary
            let Some(analysis) = self.analyze_clone_necessity(ctx, cloned_var) else {
                continue;
            };

            // Skip if below confidence threshold
            if analysis.confidence < self.min_confidence {
                continue;
            }

            // Get location information
            let Some(location) = SuggestLocation::from_context(ctx, var_data.parent) else {
                continue;
            };

            // Get variable name for the message
            let var_name = dataflow
                .var_name(cloned_var)
                .unwrap_or("<unknown>")
                .to_string();

            let message = format!(
                "Unnecessary clone() for `{}`: {}",
                var_name,
                analysis.pattern.description()
            );

            let opp = self.create_performance_opportunity(
                OpportunityId::new(next_id),
                vec![var_data.parent],
                location,
                message,
                PerformanceDetails {
                    current_pattern: format!("let {} = source.clone()", var_name),
                    suggested_pattern: analysis.suggestion,
                    confidence: analysis.confidence,
                },
            );

            opportunities.push(opp);
            next_id += 1;
        }

        opportunities
    }

    fn to_mutation_specs(
        &self,
        _ctx: &AnalysisContext,
        _opportunity: &SuggestOpportunity,
    ) -> SuggestResult<Vec<MutationSpec>> {
        // For now, return empty - removing clone() requires careful analysis
        // of the surrounding code context.
        // Future: Could generate RemoveMethodCall or ReplaceWithReference specs
        Ok(Vec::new())
    }
}

/// Result of analyzing a clone operation
struct CloneAnalysis {
    pattern: ClonePattern,
    confidence: f32,
    suggestion: String,
}

/// Classification of unnecessary clone patterns
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum ClonePattern {
    /// Clone result is never used
    ImmediatelyDropped,
    /// Clone is only used via shared borrow
    OnlySharedBorrow,
    /// Clone is passed as argument but doesn't escape
    ArgumentNoEscape,
}

impl ClonePattern {
    fn description(&self) -> &'static str {
        match self {
            Self::ImmediatelyDropped => "cloned value is never used",
            Self::OnlySharedBorrow => "cloned value is only borrowed (not owned)",
            Self::ArgumentNoEscape => "cloned value doesn't escape function scope",
        }
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_clone_pattern_description() {
        assert!(ClonePattern::ImmediatelyDropped
            .description()
            .contains("never used"));
        assert!(ClonePattern::OnlySharedBorrow
            .description()
            .contains("borrowed"));
        assert!(ClonePattern::ArgumentNoEscape
            .description()
            .contains("escape"));
    }

    #[test]
    fn test_confidence_threshold() {
        let rule = UnnecessaryClone::new().with_min_confidence(0.8);
        assert!((rule.min_confidence - 0.8).abs() < f32::EPSILON);

        // Clamp to valid range
        let rule = UnnecessaryClone::new().with_min_confidence(1.5);
        assert!((rule.min_confidence - 1.0).abs() < f32::EPSILON);
    }

    #[test]
    fn test_rule_metadata() {
        let rule = UnnecessaryClone::new();
        assert_eq!(rule.code(), "RP001");
        assert_eq!(rule.name(), "unnecessary-clone");
        assert_eq!(rule.category(), SuggestCategory::Performance);
        assert_eq!(rule.safety_level(), SafetyLevel::Confirm);
    }
}