perl_tdd_support/tdd/

test_generator.rs

//! Test generator for TDD workflow support
//!
//! This module provides automatic test generation for Perl code,
//! supporting the red-green-refactor cycle of Test-Driven Development.
//!
//! ## LSP Workflow Integration
//!
//! Test generation operates within the **LSP workflow**:
//! **Parse → Index → Navigate → Complete → Analyze**
//!
//! - **Parse Stage**: Analyzes Perl functions/methods to determine test requirements
//! - **Index Stage**: Standardizes test patterns and identifies edge cases
//! - **Navigate Stage**: Links related test cases and maintains test dependency relationships
//! - **Complete Stage**: Generates test code using appropriate test frameworks (Test::More, Test2::V0)
//! - **Analyze Stage**: Updates test coverage metrics and integrates with workspace symbols
//!
//! Essential for maintaining high-quality test coverage in enterprise Perl development
//! systems where reliability and correctness are critical business requirements.
//!
//! ## Usage Examples
//!
//! ```rust,ignore
//! use perl_parser::test_generator::{TestGenerator, TestFramework};
//! use perl_parser::{Parser, ast::Node};
//!
//! # fn main() -> Result<(), Box<dyn std::error::Error>> {
//! let mut parser = Parser::new("sub add { my ($a, $b) = @_; return $a + $b; }");
//! let ast = parser.parse()?;
//! let generator = TestGenerator::new(TestFramework::TestMore);
//! let test_cases = generator.generate_tests(&ast, "sub add { my ($a, $b) = @_; return $a + $b; }");
//! println!("Generated {} test cases", test_cases.len());
//! # Ok(())
//! # }
//! ```

use crate::ast::{Node, NodeKind};
use std::collections::HashMap;

/// Test generator for creating unit tests from code
///
/// Automatically generates comprehensive test suites with support for multiple
/// test frameworks and TDD workflow patterns.
pub struct TestGenerator {
    /// Test framework to use (Test::More, Test2::V0, etc.)
    framework: TestFramework,
    /// Options for test generation
    options: TestGeneratorOptions,
}

/// Supported Perl test frameworks
///
/// Different test frameworks provide varying levels of functionality and syntax.
#[derive(Debug, Clone, PartialEq)]
pub enum TestFramework {
    /// Test::More - Classic Perl testing framework
    TestMore,
    /// Test2::V0 - Modern Test2 ecosystem
    Test2V0,
    /// Test::Simple - Basic testing framework
    TestSimple,
    /// Test::Class - Class-based testing framework
    TestClass,
}

/// Configuration options for test generation
///
/// Controls various aspects of test generation including coverage,
/// performance testing, and mock usage.
#[derive(Debug, Clone)]
pub struct TestGeneratorOptions {
    /// Generate tests for private methods
    pub test_private: bool,
    /// Generate edge case tests
    pub edge_cases: bool,
    /// Generate mock objects for dependencies
    pub use_mocks: bool,
    /// Generate data-driven tests
    pub data_driven: bool,
    /// Generate performance tests
    pub perf_tests: bool,
    /// Expected return values for generated tests
    pub expected_values: HashMap<String, String>,
    /// Performance thresholds in seconds for benchmarks
    pub perf_thresholds: HashMap<String, f64>,
}

impl Default for TestGeneratorOptions {
    fn default() -> Self {
        Self {
            test_private: false,
            edge_cases: true,
            use_mocks: true,
            data_driven: true,
            perf_tests: false,
            expected_values: HashMap::new(),
            perf_thresholds: HashMap::new(),
        }
    }
}

/// Generated test case
#[derive(Debug, Clone)]
pub struct TestCase {
    /// Unique identifier for the test (e.g., "test_add")
    pub name: String,
    /// Human-readable description of what the test validates
    pub description: String,
    /// Generated Perl test code
    pub code: String,
    /// Whether the test is marked as TODO (needs user input)
    pub is_todo: bool,
}

impl TestGenerator {
    /// Create a new test generator for Perl parsing workflow test automation
    ///
    /// # Arguments
    ///
    /// * `framework` - Test framework to use for generating test code
    ///
    /// # Returns
    ///
    /// A configured test generator ready for Perl script test generation
    ///
    /// # Examples
    ///
    /// ```rust,ignore
    /// use perl_parser::{TestGenerator, TestFramework};
    ///
    /// let generator = TestGenerator::new(TestFramework::TestMore);
    /// // Generator ready for Perl parsing workflow test generation
    /// ```
    pub fn new(framework: TestFramework) -> Self {
        Self { framework, options: TestGeneratorOptions::default() }
    }

    /// Create a new test generator with custom options
    ///
    /// # Arguments
    ///
    /// * `framework` - Test framework to use for generating test code
    /// * `options` - Custom configuration options for test generation
    pub fn with_options(framework: TestFramework, options: TestGeneratorOptions) -> Self {
        Self { framework, options }
    }

    /// Generate tests for a given AST
    pub fn generate_tests(&self, ast: &Node, source: &str) -> Vec<TestCase> {
        let mut tests = Vec::new();

        // Find all subroutines
        let subs = self.find_subroutines(ast);

        for sub in subs {
            // Generate basic test
            tests.push(self.generate_basic_test(&sub, source));

            // Generate edge case tests if enabled
            if self.options.edge_cases {
                tests.extend(self.generate_edge_cases(&sub, source));
            }

            // Generate data-driven tests if enabled
            if self.options.data_driven {
                if let Some(test) = self.generate_data_driven_test(&sub, source) {
                    tests.push(test);
                }
            }

            // Generate performance test if enabled
            if self.options.perf_tests {
                tests.push(self.generate_perf_test(&sub, source));
            }
        }

        // Generate module-level tests
        tests.extend(self.generate_module_tests(ast, source));

        tests
    }

    /// Find all subroutines in the AST
    fn find_subroutines(&self, node: &Node) -> Vec<SubroutineInfo> {
        let mut subs = Vec::new();
        self.find_subroutines_recursive(node, &mut subs);
        subs
    }

    fn find_subroutines_recursive(&self, node: &Node, subs: &mut Vec<SubroutineInfo>) {
        match &node.kind {
            NodeKind::Subroutine { name, signature, .. } => {
                if let Some(name) = name {
                    let is_private = name.starts_with('_');
                    if !is_private || self.options.test_private {
                        // Extract parameters from signature
                        let params = self.extract_parameters(signature.as_deref());
                        subs.push(SubroutineInfo {
                            name: name.clone(),
                            params,
                            node: node.clone(),
                            is_private,
                        });
                    }
                }
            }
            _ => {
                for child in node.children() {
                    self.find_subroutines_recursive(child, subs);
                }
            }
        }
    }

    /// Generate a basic test for a subroutine
    fn generate_basic_test(&self, sub: &SubroutineInfo, _source: &str) -> TestCase {
        let test_name = format!("test_{}", sub.name);
        let description = format!("Basic test for {}", sub.name);

        let code = match self.framework {
            TestFramework::TestMore => self.generate_test_more_basic(&sub.name, &sub.params),
            TestFramework::Test2V0 => self.generate_test2_basic(&sub.name, &sub.params),
            TestFramework::TestSimple => self.generate_test_simple_basic(&sub.name, &sub.params),
            TestFramework::TestClass => self.generate_test_class_basic(&sub.name, &sub.params),
        };

        TestCase { name: test_name, description, code, is_todo: false }
    }

    fn generate_test_more_basic(&self, name: &str, params: &Option<Vec<String>>) -> String {
        let mut code = String::new();
        code.push_str("use Test::More;\n\n");

        code.push_str(&format!("subtest '{}' => sub {{\n", name));

        if let Some(params) = params {
            let args = self.generate_sample_args(params.len());
            code.push_str(&format!("    my $result = {}({});\n", name, args));
        } else {
            code.push_str(&format!("    my $result = {}();\n", name));
        }

        if let Some(expected) = self.options.expected_values.get(name) {
            code.push_str(&format!("    is($result, {}, 'Returns expected value');\n", expected));
        } else {
            code.push_str("    ok(defined $result, 'Function returns defined value');\n");
        }

        if name.starts_with("is_") || name.starts_with("has_") {
            code.push_str("    ok($result == 0 || $result == 1, 'Returns boolean');\n");
        }

        code.push_str("};\n");
        code
    }

    fn generate_test2_basic(&self, name: &str, params: &Option<Vec<String>>) -> String {
        let mut code = String::new();
        code.push_str("use Test2::V0;\n\n");

        code.push_str(&format!("subtest '{}' => sub {{\n", name));

        if let Some(params) = params {
            let args = self.generate_sample_args(params.len());
            code.push_str(&format!("    my $result = {}({});\n", name, args));
            code.push_str("    ok($result, 'Function returns value');\n");
        } else {
            code.push_str(&format!("    my $result = {}();\n", name));
            code.push_str("    ok($result, 'Function returns value');\n");
        }

        code.push_str("};\n");
        code
    }

    fn generate_test_simple_basic(&self, name: &str, params: &Option<Vec<String>>) -> String {
        let mut code = String::new();
        code.push_str("use Test::Simple tests => 1;\n\n");

        if let Some(params) = params {
            let args = self.generate_sample_args(params.len());
            code.push_str(&format!("ok({}({}), 'Test {}');\n", name, args, name));
        } else {
            code.push_str(&format!("ok({}(), 'Test {}');\n", name, name));
        }

        code
    }

    fn generate_test_class_basic(&self, name: &str, params: &Option<Vec<String>>) -> String {
        let mut code = String::new();
        code.push_str("use Test::Class::Most;\n\n");

        code.push_str(&format!("sub test_{} : Test {{\n", name));
        code.push_str("    my $self = shift;\n");

        if let Some(params) = params {
            let args = self.generate_sample_args(params.len());
            code.push_str(&format!("    my $result = $self->{}({});\n", name, args));
        } else {
            code.push_str(&format!("    my $result = $self->{}();\n", name));
        }

        code.push_str("    ok($result, 'Function works');\n");
        code.push_str("}\n");

        code
    }

    /// Generate edge case tests
    fn generate_edge_cases(&self, sub: &SubroutineInfo, _source: &str) -> Vec<TestCase> {
        let mut tests = Vec::new();

        if sub.params.is_some() {
            // Test with undef parameters
            tests.push(self.generate_undef_test(sub));

            // Test with empty parameters
            tests.push(self.generate_empty_test(sub));

            // Test with wrong type parameters
            tests.push(self.generate_type_test(sub));
        }

        tests
    }

    fn generate_undef_test(&self, sub: &SubroutineInfo) -> TestCase {
        let test_name = format!("test_{}_undef", sub.name);
        let description = format!("Test {} with undef parameters", sub.name);

        let code = match self.framework {
            TestFramework::TestMore => {
                format!(
                    "use Test::More;\n\n\
                     subtest '{} with undef' => sub {{\n    \
                     eval {{ {}(undef) }};\n    \
                     ok(!$@, 'Handles undef gracefully');\n\
                     }};\n",
                    sub.name, sub.name
                )
            }
            _ => String::new(), // Simplified for other frameworks
        };

        TestCase { name: test_name, description, code, is_todo: false }
    }

    fn generate_empty_test(&self, sub: &SubroutineInfo) -> TestCase {
        let test_name = format!("test_{}_empty", sub.name);
        let description = format!("Test {} with empty parameters", sub.name);

        let code = match self.framework {
            TestFramework::TestMore => {
                format!(
                    "use Test::More;\n\n\
                     subtest '{} with empty params' => sub {{\n    \
                     eval {{ {}('', [], {{}}) }};\n    \
                     ok(!$@, 'Handles empty values');\n\
                     }};\n",
                    sub.name, sub.name
                )
            }
            _ => String::new(),
        };

        TestCase { name: test_name, description, code, is_todo: false }
    }

    fn generate_type_test(&self, sub: &SubroutineInfo) -> TestCase {
        let test_name = format!("test_{}_types", sub.name);
        let description = format!("Test {} with different types", sub.name);

        let code = match self.framework {
            TestFramework::TestMore => {
                format!(
                    "use Test::More;\n\n\
                     subtest '{} type checking' => sub {{\n    \
                     # Test with different types\n    \
                     eval {{ {}(123) }};\n    \
                     eval {{ {}('string') }};\n    \
                     eval {{ {}([1,2,3]) }};\n    \
                     eval {{ {}({{a=>1}}) }};\n    \
                     pass('Handles different types');\n\
                     }};\n",
                    sub.name, sub.name, sub.name, sub.name, sub.name
                )
            }
            _ => String::new(),
        };

        TestCase { name: test_name, description, code, is_todo: false }
    }

    /// Generate data-driven test
    fn generate_data_driven_test(&self, sub: &SubroutineInfo, _source: &str) -> Option<TestCase> {
        sub.params.as_ref()?;

        let test_name = format!("test_{}_data_driven", sub.name);
        let description = format!("Data-driven test for {}", sub.name);

        let code = match self.framework {
            TestFramework::TestMore => {
                format!(
                    "use Test::More;\n\n\
                     my @test_cases = (\n    \
                     {{ input => 'test1', expected => 'result1' }},\n    \
                     {{ input => 'test2', expected => 'result2' }},\n    \
                     {{ input => 'test3', expected => 'result3' }},\n\
                     );\n\n\
                     for my $case (@test_cases) {{\n    \
                     my $result = {}($case->{{input}});\n    \
                     is($result, $case->{{expected}}, \n       \
                     \"{}($case->{{input}}) returns $case->{{expected}}\");\n\
                     }}\n",
                    sub.name, sub.name
                )
            }
            _ => String::new(),
        };

        Some(TestCase {
            name: test_name,
            description,
            code,
            is_todo: true, // Mark as TODO since user needs to fill in test data
        })
    }

    /// Generate performance test
    fn generate_perf_test(&self, sub: &SubroutineInfo, _source: &str) -> TestCase {
        let test_name = format!("test_{}_performance", sub.name);
        let description = format!("Performance test for {}", sub.name);

        let code = match self.framework {
            TestFramework::TestMore => {
                let mut snippet = String::new();
                snippet.push_str("use Test::More;\n");
                snippet.push_str("use Benchmark qw(timeit);\n\n");
                snippet.push_str(&format!("subtest '{} performance' => sub {{\n", sub.name));
                snippet.push_str(&format!(
                    "    my $result = timeit(1000, sub {{ {}() }});\n",
                    sub.name
                ));
                if let Some(threshold) = self.options.perf_thresholds.get(&sub.name) {
                    snippet.push_str(&format!(
                        "    cmp_ok($result->real, '<', {}, 'Executes under threshold');\n",
                        threshold
                    ));
                } else {
                    snippet.push_str("    ok($result->real >= 0, 'Execution time recorded');\n");
                }
                snippet.push_str("};\n");
                snippet
            }
            _ => String::new(),
        };

        TestCase { name: test_name, description, code, is_todo: true }
    }

    /// Generate module-level tests
    fn generate_module_tests(&self, ast: &Node, _source: &str) -> Vec<TestCase> {
        let mut tests = Vec::new();

        // Find package declaration
        if let Some(package_name) = self.find_package_name(ast) {
            // Generate module load test
            tests.push(self.generate_module_load_test(&package_name));

            // Generate export test if module exports functions
            if self.has_exports(ast) {
                tests.push(self.generate_export_test(&package_name));
            }

            // Generate new() test if it's an OO module
            if self.has_constructor(ast) {
                tests.push(self.generate_constructor_test(&package_name));
            }
        }

        tests
    }

    fn find_package_name(&self, node: &Node) -> Option<String> {
        match &node.kind {
            NodeKind::Package { name, .. } => Some(name.clone()),
            _ => {
                for child in node.children() {
                    if let Some(name) = self.find_package_name(child) {
                        return Some(name);
                    }
                }
                None
            }
        }
    }

    fn has_exports(&self, node: &Node) -> bool {
        // Check if module uses Exporter
        self.find_use_statement(node, "Exporter").is_some()
    }

    fn has_constructor(&self, node: &Node) -> bool {
        // Check if module has a new() method
        self.find_subroutine(node, "new").is_some()
    }

    fn find_use_statement(&self, node: &Node, module: &str) -> Option<Node> {
        match &node.kind {
            NodeKind::Use { module: m, .. } if m == module => Some(node.clone()),
            _ => {
                for child in node.children() {
                    if let Some(result) = self.find_use_statement(child, module) {
                        return Some(result);
                    }
                }
                None
            }
        }
    }

    fn find_subroutine(&self, node: &Node, name: &str) -> Option<Node> {
        match &node.kind {
            NodeKind::Subroutine { name: Some(n), .. } if n == name => Some(node.clone()),
            _ => {
                for child in node.children() {
                    if let Some(result) = self.find_subroutine(child, name) {
                        return Some(result);
                    }
                }
                None
            }
        }
    }

    fn generate_module_load_test(&self, package: &str) -> TestCase {
        let test_name = "test_module_loads".to_string();
        let description = format!("Test that {} loads correctly", package);

        let code = match self.framework {
            TestFramework::TestMore => {
                format!(
                    "use Test::More;\n\n\
                     BEGIN {{\n    \
                     use_ok('{}') || print \"Bail out!\\n\";\n\
                     }}\n\n\
                     diag(\"Testing {} ${}::VERSION, Perl $], $^X\");\n",
                    package, package, package
                )
            }
            _ => String::new(),
        };

        TestCase { name: test_name, description, code, is_todo: false }
    }

    fn generate_export_test(&self, package: &str) -> TestCase {
        let test_name = "test_exports".to_string();
        let description = format!("Test {} exports", package);

        let code = match self.framework {
            TestFramework::TestMore => {
                format!(
                    "use Test::More;\n\
                     use {};\n\n\
                     can_ok('main', @{}::EXPORT);\n",
                    package, package
                )
            }
            _ => String::new(),
        };

        TestCase { name: test_name, description, code, is_todo: false }
    }

    fn generate_constructor_test(&self, package: &str) -> TestCase {
        let test_name = "test_constructor".to_string();
        let description = format!("Test {} constructor", package);

        let code = match self.framework {
            TestFramework::TestMore => {
                format!(
                    "use Test::More;\n\
                     use {};\n\n\
                     subtest 'constructor' => sub {{\n    \
                     my $obj = {}->new();\n    \
                     isa_ok($obj, '{}');\n    \
                     can_ok($obj, 'new');\n\
                     }};\n",
                    package, package, package
                )
            }
            _ => String::new(),
        };

        TestCase { name: test_name, description, code, is_todo: false }
    }

    fn generate_sample_args(&self, count: usize) -> String {
        let args: Vec<String> = (0..count).map(|i| format!("'arg{}'", i + 1)).collect();
        args.join(", ")
    }

    /// Extract parameters from a subroutine signature
    fn extract_parameters(&self, signature: Option<&Node>) -> Option<Vec<String>> {
        if let Some(sig) = signature {
            if let NodeKind::Signature { parameters } = &sig.kind {
                let mut param_names = Vec::new();
                for param in parameters {
                    match &param.kind {
                        NodeKind::MandatoryParameter { variable }
                        | NodeKind::OptionalParameter { variable, .. } => {
                            if let NodeKind::Variable { name, .. } = &variable.kind {
                                param_names.push(name.clone());
                            }
                        }
                        _ => {}
                    }
                }
                if param_names.is_empty() { None } else { Some(param_names) }
            } else {
                None
            }
        } else {
            None
        }
    }
}

#[derive(Debug, Clone)]
struct SubroutineInfo {
    name: String,
    params: Option<Vec<String>>,
    #[allow(dead_code)]
    node: Node,
    #[allow(dead_code)]
    is_private: bool,
}

/// Test runner integration for executing Perl tests
///
/// Provides functionality for running test suites, watch mode for continuous
/// testing, and coverage tracking integration.
pub struct TestRunner {
    /// Command to run tests (e.g., "prove -l")
    test_command: String,
    /// Whether watch mode is enabled for continuous testing
    watch_mode: bool,
    /// Whether coverage tracking is enabled
    coverage: bool,
}

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

impl TestRunner {
    /// Create a new test runner with default settings
    ///
    /// Uses "prove -l" as the default test command.
    pub fn new() -> Self {
        Self { test_command: "prove -l".to_string(), watch_mode: false, coverage: false }
    }

    /// Create a new test runner with a custom command
    ///
    /// # Arguments
    ///
    /// * `command` - The shell command to execute tests (e.g., "prove -v -l")
    pub fn with_command(command: String) -> Self {
        Self { test_command: command, watch_mode: false, coverage: false }
    }

    /// Run tests and return results
    pub fn run_tests(&self, test_files: &[String]) -> TestResults {
        let mut results = TestResults::default();

        // Build command
        let mut cmd = self.test_command.clone();

        if self.coverage {
            cmd = format!("cover -test {}", cmd);
        }

        for file in test_files {
            cmd.push(' ');
            cmd.push_str(file);
        }

        // Execute tests (simplified - would use std::process::Command in real impl)
        results.total = test_files.len();
        results.passed = test_files.len(); // Assume all pass for now

        results
    }

    /// Run tests in watch mode
    pub fn watch(&self, _test_files: &[String]) -> Result<(), String> {
        if !self.watch_mode {
            return Err("Watch mode not enabled".to_string());
        }

        // Would implement file watching here
        Ok(())
    }

    /// Get test coverage
    pub fn get_coverage(&self) -> Option<CoverageReport> {
        if !self.coverage {
            return None;
        }

        Some(CoverageReport {
            line_coverage: 85.0,
            branch_coverage: 75.0,
            function_coverage: 90.0,
            uncovered_lines: vec![],
        })
    }
}

/// Results from executing a test suite
#[derive(Debug, Default, Clone)]
pub struct TestResults {
    /// Total number of tests executed
    pub total: usize,
    /// Number of tests that passed
    pub passed: usize,
    /// Number of tests that failed
    pub failed: usize,
    /// Number of tests that were skipped
    pub skipped: usize,
    /// Number of tests marked as TODO
    pub todo: usize,
    /// Error messages from failed tests
    pub errors: Vec<String>,
}

/// Code coverage metrics from test execution
#[derive(Debug)]
pub struct CoverageReport {
    /// Percentage of lines covered (0.0 to 100.0)
    pub line_coverage: f64,
    /// Percentage of branches covered (0.0 to 100.0)
    pub branch_coverage: f64,
    /// Percentage of functions covered (0.0 to 100.0)
    pub function_coverage: f64,
    /// Line numbers that were not covered by tests
    pub uncovered_lines: Vec<usize>,
}

/// Refactoring suggestions for the green-to-refactor phase
///
/// Analyzes code to identify opportunities for improvement such as
/// duplicate code, complex methods, and naming issues.
pub struct RefactoringSuggester {
    /// Collection of refactoring suggestions found during analysis
    suggestions: Vec<RefactoringSuggestion>,
}

/// A specific refactoring suggestion with context and actions
#[derive(Debug, Clone)]
pub struct RefactoringSuggestion {
    /// Short title summarizing the issue
    pub title: String,
    /// Detailed description of the problem and recommended fix
    pub description: String,
    /// Importance level for prioritizing refactoring efforts
    pub priority: Priority,
    /// Type of refactoring suggested
    pub category: RefactoringCategory,
    /// LSP code action identifier (e.g., "extract_method", "rename")
    pub code_action: Option<String>,
}

/// Priority level for refactoring suggestions
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub enum Priority {
    /// Minor improvement, can be addressed when convenient
    Low,
    /// Moderate issue that should be addressed
    Medium,
    /// Important issue requiring attention soon
    High,
    /// Urgent issue that should be fixed immediately
    Critical,
}

/// Categories of refactoring suggestions
#[derive(Debug, Clone, PartialEq)]
pub enum RefactoringCategory {
    /// Code that appears in multiple places and should be extracted
    DuplicateCode,
    /// Method with high cyclomatic complexity
    ComplexMethod,
    /// Method with too many lines of code
    LongMethod,
    /// Function with excessive parameter count
    TooManyParameters,
    /// Unreachable or unused code
    DeadCode,
    /// Inefficient code that could be optimized
    Performance,
    /// Poorly named variables or functions
    Naming,
    /// Structural issues with code organization
    Structure,
}

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

impl RefactoringSuggester {
    /// Create a new refactoring suggester
    pub fn new() -> Self {
        Self { suggestions: Vec::new() }
    }

    /// Analyze code and generate refactoring suggestions
    pub fn analyze(&mut self, ast: &Node, source: &str) -> Vec<RefactoringSuggestion> {
        self.suggestions.clear();

        // Check for duplicate code
        self.check_duplicate_code(ast, source);

        // Check for complex methods
        self.check_complex_methods(ast, source);

        // Check for long methods
        self.check_long_methods(ast, source);

        // Check for too many parameters
        self.check_parameter_count(ast);

        // Check for naming issues
        self.check_naming(ast);

        // Sort by priority
        self.suggestions.sort_by_key(|s| s.priority.clone());
        self.suggestions.reverse();

        self.suggestions.clone()
    }

    fn check_duplicate_code(&mut self, _ast: &Node, _source: &str) {
        // Simplified duplicate detection
        // In real implementation, would use similarity algorithms
    }

    fn check_complex_methods(&mut self, ast: &Node, _source: &str) {
        self.check_complex_methods_recursive(ast);
    }

    fn check_complex_methods_recursive(&mut self, node: &Node) {
        match &node.kind {
            NodeKind::Subroutine { name, .. } => {
                let complexity = self.calculate_cyclomatic_complexity(node);
                if complexity > 10 {
                    self.suggestions.push(RefactoringSuggestion {
                        title: format!("High complexity in {}", name.as_ref().unwrap_or(&"anonymous".to_string())),
                        description: format!("Cyclomatic complexity is {}. Consider breaking into smaller functions.", complexity),
                        priority: if complexity > 20 { Priority::High } else { Priority::Medium },
                        category: RefactoringCategory::ComplexMethod,
                        code_action: Some("extract_method".to_string()),
                    });
                }
            }
            _ => {
                for child in node.children() {
                    self.check_complex_methods_recursive(child);
                }
            }
        }
    }

    fn calculate_cyclomatic_complexity(&self, node: &Node) -> usize {
        let mut complexity = 1;
        self.count_decision_points(node, &mut complexity);
        complexity
    }

    fn count_decision_points(&self, node: &Node, complexity: &mut usize) {
        match &node.kind {
            NodeKind::If { .. }
            | NodeKind::While { .. }
            | NodeKind::For { .. }
            | NodeKind::Ternary { .. } => {
                *complexity += 1;
            }
            NodeKind::Binary { op: operator, .. } => {
                if operator == "&&" || operator == "||" || operator == "and" || operator == "or" {
                    *complexity += 1;
                }
            }
            _ => {}
        }

        for child in node.children() {
            self.count_decision_points(child, complexity);
        }
    }

    fn check_long_methods(&mut self, ast: &Node, source: &str) {
        self.check_long_methods_recursive(ast, source);
    }

    fn check_long_methods_recursive(&mut self, node: &Node, source: &str) {
        match &node.kind {
            NodeKind::Subroutine { name, .. } => {
                let lines = self.count_lines(node, source);
                if lines > 50 {
                    self.suggestions.push(RefactoringSuggestion {
                        title: format!(
                            "Long method: {}",
                            name.as_ref().unwrap_or(&"anonymous".to_string())
                        ),
                        description: format!(
                            "Method has {} lines. Consider breaking into smaller functions.",
                            lines
                        ),
                        priority: if lines > 100 { Priority::High } else { Priority::Medium },
                        category: RefactoringCategory::LongMethod,
                        code_action: Some("extract_method".to_string()),
                    });
                }
            }
            _ => {
                for child in node.children() {
                    self.check_long_methods_recursive(child, source);
                }
            }
        }
    }

    fn count_lines(&self, node: &Node, source: &str) -> usize {
        let start = node.location.start;
        let end = node.location.end;

        let text = &source[start..end.min(source.len())];
        text.lines().count()
    }

    fn check_parameter_count(&mut self, ast: &Node) {
        self.check_parameter_count_recursive(ast);
    }

    fn check_parameter_count_recursive(&mut self, node: &Node) {
        match &node.kind {
            NodeKind::Subroutine { name, signature, .. } => {
                let params = self.extract_parameters(signature.as_deref());
                if let Some(params) = &params {
                    if params.len() > 5 {
                        self.suggestions.push(RefactoringSuggestion {
                            title: format!(
                                "Too many parameters in {}",
                                name.as_ref().unwrap_or(&"anonymous".to_string())
                            ),
                            description: format!(
                                "Function has {} parameters. Consider using a hash or object.",
                                params.len()
                            ),
                            priority: Priority::Medium,
                            category: RefactoringCategory::TooManyParameters,
                            code_action: Some("introduce_parameter_object".to_string()),
                        });
                    }
                }
            }
            _ => {
                for child in node.children() {
                    self.check_parameter_count_recursive(child);
                }
            }
        }
    }

    fn check_naming(&mut self, ast: &Node) {
        self.check_naming_recursive(ast);
    }

    fn check_naming_recursive(&mut self, node: &Node) {
        match &node.kind {
            NodeKind::Subroutine { name: Some(name), .. } => {
                if !self.is_good_name(name) {
                    self.suggestions.push(RefactoringSuggestion {
                        title: format!("Poor naming: {}", name),
                        description: "Consider using a more descriptive name".to_string(),
                        priority: Priority::Low,
                        category: RefactoringCategory::Naming,
                        code_action: Some("rename".to_string()),
                    });
                }
            }
            NodeKind::VariableDeclaration { variable, .. } => {
                if let NodeKind::Variable { name, .. } = &variable.kind {
                    if !self.is_good_variable_name(name) {
                        self.suggestions.push(RefactoringSuggestion {
                            title: format!("Poor variable name: {}", name),
                            description:
                                "Single letter variables should only be used for loop counters"
                                    .to_string(),
                            priority: Priority::Low,
                            category: RefactoringCategory::Naming,
                            code_action: Some("rename".to_string()),
                        });
                    }
                }
            }
            NodeKind::VariableListDeclaration { variables, .. } => {
                for var_node in variables {
                    if let NodeKind::Variable { name, .. } = &var_node.kind {
                        if !self.is_good_variable_name(name) {
                            self.suggestions.push(RefactoringSuggestion {
                                title: format!("Poor variable name: {}", name),
                                description:
                                    "Single letter variables should only be used for loop counters"
                                        .to_string(),
                                priority: Priority::Low,
                                category: RefactoringCategory::Naming,
                                code_action: Some("rename".to_string()),
                            });
                        }
                    }
                }
            }
            _ => {
                for child in node.children() {
                    self.check_naming_recursive(child);
                }
            }
        }
    }

    fn is_good_name(&self, name: &str) -> bool {
        // Check for meaningful names
        name.len() > 2 && !name.chars().all(|c| c.is_uppercase())
    }

    fn is_good_variable_name(&self, name: &str) -> bool {
        // Allow single letters for common loop variables
        if name.len() == 1 {
            return matches!(name, "$i" | "$j" | "$k" | "$n" | "$_");
        }

        // Remove sigil for checking
        let clean_name = name.trim_start_matches(['$', '@', '%', '*']);
        clean_name.len() > 1
    }

    /// Extract parameters from a subroutine signature
    fn extract_parameters(&self, signature: Option<&Node>) -> Option<Vec<String>> {
        if let Some(sig) = signature {
            if let NodeKind::Signature { parameters } = &sig.kind {
                let mut param_names = Vec::new();
                for param in parameters {
                    match &param.kind {
                        NodeKind::MandatoryParameter { variable }
                        | NodeKind::OptionalParameter { variable, .. } => {
                            if let NodeKind::Variable { name, .. } = &variable.kind {
                                param_names.push(name.clone());
                            }
                        }
                        _ => {}
                    }
                }
                if param_names.is_empty() { None } else { Some(param_names) }
            } else {
                None
            }
        } else {
            None
        }
    }
}

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

    #[test]
    fn test_generate_basic_test() {
        let generator = TestGenerator::new(TestFramework::TestMore);
        let ast = Node::new(
            NodeKind::Subroutine {
                name: Some("add".to_string()),
                name_span: None,
                signature: Some(Box::new(Node::new(
                    NodeKind::Signature {
                        parameters: vec![
                            Node::new(
                                NodeKind::MandatoryParameter {
                                    variable: Box::new(Node::new(
                                        NodeKind::Variable {
                                            name: "$a".to_string(),
                                            sigil: "$".to_string(),
                                        },
                                        crate::ast::SourceLocation { start: 0, end: 0 },
                                    )),
                                },
                                crate::ast::SourceLocation { start: 0, end: 0 },
                            ),
                            Node::new(
                                NodeKind::MandatoryParameter {
                                    variable: Box::new(Node::new(
                                        NodeKind::Variable {
                                            name: "$b".to_string(),
                                            sigil: "$".to_string(),
                                        },
                                        crate::ast::SourceLocation { start: 0, end: 0 },
                                    )),
                                },
                                crate::ast::SourceLocation { start: 0, end: 0 },
                            ),
                        ],
                    },
                    crate::ast::SourceLocation { start: 0, end: 0 },
                ))),
                body: Box::new(Node::new(
                    NodeKind::Block { statements: vec![] },
                    crate::ast::SourceLocation { start: 0, end: 0 },
                )),
                attributes: vec![],
                prototype: None,
            },
            crate::ast::SourceLocation { start: 0, end: 0 },
        );

        let tests = generator.generate_tests(&ast, "sub add { }");
        assert!(!tests.is_empty());
        assert!(tests[0].code.contains("Test::More"));
        assert!(tests[0].code.contains("add"));
    }

    #[test]
    fn test_refactoring_suggestions() {
        let mut suggester = RefactoringSuggester::new();

        // Create a complex subroutine
        let ast = Node::new(
            NodeKind::Subroutine {
                name: Some("complex_function".to_string()),
                name_span: None,
                signature: Some(Box::new(Node::new(
                    NodeKind::Signature {
                        parameters: (0..7)
                            .map(|i| {
                                Node::new(
                                    NodeKind::MandatoryParameter {
                                        variable: Box::new(Node::new(
                                            NodeKind::Variable {
                                                name: format!("$param{}", i),
                                                sigil: "$".to_string(),
                                            },
                                            crate::ast::SourceLocation { start: 0, end: 0 },
                                        )),
                                    },
                                    crate::ast::SourceLocation { start: 0, end: 0 },
                                )
                            })
                            .collect(),
                    },
                    crate::ast::SourceLocation { start: 0, end: 0 },
                ))),
                body: Box::new(Node::new(
                    NodeKind::Block {
                        statements: vec![
                            // Add some if statements to increase complexity
                            Node::new(
                                NodeKind::If {
                                    condition: Box::new(Node::new(
                                        NodeKind::Variable {
                                            name: "$a".to_string(),
                                            sigil: "$".to_string(),
                                        },
                                        crate::ast::SourceLocation { start: 0, end: 0 },
                                    )),
                                    then_branch: Box::new(Node::new(
                                        NodeKind::Block { statements: vec![] },
                                        crate::ast::SourceLocation { start: 0, end: 0 },
                                    )),
                                    elsif_branches: vec![],
                                    else_branch: None,
                                },
                                crate::ast::SourceLocation { start: 0, end: 0 },
                            ),
                        ],
                    },
                    crate::ast::SourceLocation { start: 0, end: 0 },
                )),
                attributes: vec![],
                prototype: None,
            },
            crate::ast::SourceLocation { start: 0, end: 0 },
        );

        let suggestions = suggester.analyze(&ast, "sub complex_function { }");

        // Should suggest parameter object for too many params
        assert!(suggestions.iter().any(|s| s.category == RefactoringCategory::TooManyParameters));
    }

    #[test]
    fn test_cyclomatic_complexity() {
        let suggester = RefactoringSuggester::new();

        // Create a node with multiple decision points
        let node = Node::new(
            NodeKind::Block {
                statements: vec![Node::new(
                    NodeKind::If {
                        condition: Box::new(Node::new(
                            NodeKind::Variable { name: "$x".to_string(), sigil: "$".to_string() },
                            crate::ast::SourceLocation { start: 0, end: 0 },
                        )),
                        then_branch: Box::new(Node::new(
                            NodeKind::Block { statements: vec![] },
                            crate::ast::SourceLocation { start: 0, end: 0 },
                        )),
                        elsif_branches: vec![],
                        else_branch: None,
                    },
                    crate::ast::SourceLocation { start: 0, end: 0 },
                )],
            },
            crate::ast::SourceLocation { start: 0, end: 0 },
        );

        let complexity = suggester.calculate_cyclomatic_complexity(&node);
        assert_eq!(complexity, 2); // Base 1 + 1 if statement
    }
}