# RSpade Code Quality System

## Overview

The Code Quality system is a modular, extensible framework for enforcing coding standards and best practices across the RSpade codebase. It replaces a monolithic 1921-line checker with a clean, maintainable architecture using Manifest-based auto-discovery.

## Architecture

### Core Components

1. **CodeQualityChecker** (`CodeQualityChecker.php`)
   - Main orchestrator that discovers and runs all rules
   - Auto-discovers rules via RuleDiscovery::discover_rules()
   - Handles file scanning, caching, and violation collection
   - Performs syntax linting for PHP, JavaScript, and JSON files

2. **CodeQualityRule_Abstract** (`Rules/CodeQualityRule_Abstract.php`)
   - Base class for all code quality rules
   - Defines the interface: `get_id()`, `get_name()`, `check()`, etc.
   - Provides `add_violation()` helper method
   - Rules self-register by extending this class

3. **Violation** (`Violation.php`)
   - Data class representing a code violation
   - Contains: rule_id, file_path, line_number, message, severity, code_snippet, suggestion
   - Provides `to_array()` for serialization

### Support Classes

- **ViolationCollector** - Aggregates violations from all rules
- **CacheManager** - Caches sanitized file contents to improve performance
- **FileSanitizer** - Removes comments and strings for accurate code analysis

## Rule Categories

### PHP Rules (`Rules/PHP/`)

1. **NamingConventionRule** (PHP-NAMING-01)
   - Enforces underscore_case for methods and variables
   - Excludes Laravel framework methods (toArray, firstOrCreate, etc.)
   - Severity: Medium

2. **MassAssignmentRule** (PHP-MASS-01)
   - Prohibits use of $fillable property
   - Ensures $guarded = ['*'] or removal
   - Severity: High

3. **PhpFallbackLegacyRule** (PHP-FALLBACK-01)
   - Detects "fallback" or "legacy" in comments/function names
   - Enforces fail-loud principle
   - Severity: Critical

4. **DbTableUsageRule** (PHP-DB-01)
   - Prohibits DB::table() usage
   - Requires ORM models for database access
   - Severity: High

5. **FunctionExistsRule** (PHP-FUNC-01)
   - Prohibits function_exists() checks
   - Enforces predictable runtime environment
   - Severity: High

### Jqhtml Rules (`Rules/Jqhtml/`)

1. **JqhtmlInlineScriptRule** (JQHTML-INLINE-01)
   - Prohibits inline <script> and <style> tags in .jqhtml templates
   - Enforces component class pattern with Component
   - Requires separate .js and .scss files
   - Severity: Critical
   - Runs at manifest-time

### JavaScript Rules (`Rules/JavaScript/`)

1. **VarUsageRule** (JS-VAR-01)
   - Prohibits 'var' keyword, requires let/const
   - Severity: Medium

2. **DefensiveCodingRule** (JS-DEFENSIVE-01)
   - Prohibits typeof checks for core classes
   - Core classes always exist in runtime
   - Severity: High

3. **InstanceMethodsRule** (JS-STATIC-01)
   - Enforces static methods in JavaScript classes
   - Exceptions allowed with @instance-class comment
   - Severity: Medium

4. **JQueryUsageRule** (JS-JQUERY-01)
   - Enforces $ over jQuery
   - Detects deprecated methods (live, die, bind, etc.)
   - Severity: Medium

5. **ThisUsageRule** (JS-THIS-01)
   - Detects problematic 'this' usage
   - Suggests class reference pattern
   - Severity: Medium

6. **DocumentReadyRule** (JS-READY-01)
   - Prohibits jQuery ready patterns
   - Requires ES6 class with static init()
   - Severity: High

7. **JsFallbackLegacyRule** (JS-FALLBACK-01)
   - JavaScript version of fallback/legacy detection
   - Severity: Critical

### Common Rules (`Rules/Common/`)

1. **FilenameCaseRule** (FILE-CASE-01)
   - Enforces lowercase filenames
   - Severity: Low

2. **FilenameEnhancedRule** (FILE-NAME-01)
   - Validates controller/model naming conventions
   - Checks file-class name consistency
   - Severity: Medium

3. **RootFilesRule** (FILE-ROOT-01)
   - Restricts files in project root
   - Maintains clean project structure
   - Severity: Medium

4. **RsxTestFilesRule** (FILE-RSX-01)
   - Prevents test files directly in rsx/
   - Enforces proper test organization
   - Severity: Medium

5. **RouteExistsRule** (ROUTE-EXISTS-01)
   - Validates Rsx::Route() calls reference existing routes
   - Checks controller/method combinations exist in manifest
   - Suggests placeholder URLs for unimplemented routes
   - Severity: High

### Sanity Check Rules (`Rules/SanityChecks/`)

1. **PhpSanityCheckRule** (PHP-SC-001)
   - Complex pattern detection (currently disabled)
   - Detects suspicious code patterns
   - Severity: Critical

## Configuration

### Config File (`config/rsx.php`)

```php
'code_quality' => [
    'enabled' => env('CODE_QUALITY_ENABLED', true),
    'cache_enabled' => true,
    'parallel_processing' => false,
    'excluded_directories' => [
        'vendor',
        'node_modules',
        'storage',
        'bootstrap/cache',
        'CodeQuality', // Exclude checker itself
    ],
    'rsx_test_whitelist' => [
        // Files allowed in rsx/ directory
        'main.php',
        'routes.php',
    ],
],
```

### Disabling Rules

Rules can be disabled by adding them to the disabled list:

```php
'disabled_rules' => [
    'PHP-SC-001', // Temporarily disabled
],
```

## Usage

### Command Line

```bash
# Run all checks
php artisan rsx:check

# Check specific directory
php artisan rsx:check rsx/

# Check specific file
php artisan rsx:check app/Models/User.php
```

### Exception Granting System

The code quality system supports granting exceptions to allow specific violations when justified. Exceptions are granted via specially formatted comments in the source files.

#### Exception Comment Format

```
@{RULE-ID}-EXCEPTION - Optional rationale
```

**Naming Convention:**
- Use the exact rule ID from `get_id()` method
- Add `-EXCEPTION` suffix
- Examples: `@PHP-NAMING-01-EXCEPTION`, `@JS-DEFENSIVE-01-EXCEPTION`, `@FILE-CASE-01-EXCEPTION`

#### Exception Placement

**Line-Level Exceptions** (most common):
Place the exception comment on the same line as the violation OR on the line immediately before it.

```php
// Same-line exception
if (key && key.startsWith('rsx::')) {  // @JS-DEFENSIVE-01-EXCEPTION - storage.key() can return null

// Previous-line exception
// @PHP-REFLECT-01-EXCEPTION - Test runner needs ReflectionClass for filtering
if ($reflection->isAbstract()) {
    continue;
}
```

**File-Level Exceptions** (for entire file):
Place at the top of the file, after namespace/use statements, before the main docblock.

```php
<?php

namespace App\RSpade\Core\Ajax;

use SomeClass;

// @FILE-SUBCLASS-01-EXCEPTION

/**
 * Class docblock
 */
class MyClass {
```

**Docblock Exceptions** (for method/class):
Place inside the docblock using JSDoc/PHPDoc style.

```php
/**
 * Check if a method is overriding a parent method
 *
 * @PHP-REFLECT-02-EXCEPTION: This method needs ReflectionClass to check parent methods
 * from Laravel framework classes which are not tracked in the manifest.
 */
protected function is_overriding_parent_method($class_name, $method_name) {
```

#### Implementing Exception Handling in Rules

**NOT all rules implement exception handling** - it must be added per-rule. Approximately 31 of 111 rules currently support exceptions.

**To check if a rule supports exceptions:**

1. Open the rule file in `/system/app/RSpade/CodeQuality/Rules/`
2. Search for `EXCEPTION` in the file
3. If found, the rule implements exception checking
4. If not found, exceptions will be ignored by that rule

**To implement exception handling in a rule:**

Add a check before calling `add_violation()`. The exact implementation depends on rule structure:

**Line-by-line checking pattern:**

```php
public function check(string $file_path, string $contents, array $metadata = []): void
{
    $lines = explode("\n", $contents);

    foreach ($lines as $line_number => $line) {
        $actual_line_number = $line_number + 1;

        // Skip if line has exception comment
        if (str_contains($line, '@' . $this->get_id() . '-EXCEPTION')) {
            continue;
        }

        // Check for violation
        if ($this->detect_violation($line)) {
            $this->add_violation(...);
        }
    }
}
```

**Multi-line or previous-line pattern:**

```php
// Check previous line for exception comment
$prev_line_index = $line_number - 1;
if ($prev_line_index >= 0 && str_contains($lines[$prev_line_index], '@' . $this->get_id() . '-EXCEPTION')) {
    continue;  // Skip this line
}
```

**File-level pattern:**

```php
public function check(string $file_path, string $contents, array $metadata = []): void
{
    // Check if entire file has exception
    if (str_contains($contents, '@' . $this->get_id() . '-EXCEPTION')) {
        return;  // Skip entire file
    }

    // ... rest of checking logic
}
```

#### Exception Rationale Guidelines

**Always include a rationale** explaining WHY the exception is needed:

```javascript
// @JS-DEFENSIVE-01-EXCEPTION - Browser API storage.key(i) can return null when i >= storage.length
```

**Good rationales:**
- Reference external API behavior: "Browser API returns null", "Laravel method signature requires this"
- Explain architectural necessity: "Task system uses direct queries for performance"
- Note optional/polymorphic patterns: "Array.find() returns undefined when no match"

**Bad rationales:**
- "TODO: fix later"
- "Not sure why this is needed"
- No rationale at all

#### CRITICAL: AI Agent Exception Policy

**ABSOLUTE PROHIBITION - NEVER GRANT EXCEPTIONS AUTONOMOUSLY**

When you encounter a code quality violation, you are **FORBIDDEN** from granting exceptions without explicit programmer approval.

**Required procedure:**
1. **STOP** - Do not add exception comments
2. **ANALYZE** - Determine if the violation is:
   - Invalid defensive coding (should be removed)
   - Valid duck typing (needs exception)
   - External API constraint (needs exception)
3. **REPORT** - Present findings: "Found violation X in file Y. Analysis: [your reasoning]. Options: a) Remove the check (fail-loud), b) Grant exception (provide rationale), c) Refactor differently"
4. **WAIT** - Wait for programmer to decide
5. **NEVER** add `@RULE-ID-EXCEPTION` comments without explicit approval

**Only grant exceptions when:**
- Programmer explicitly requests it: "grant an exception for this"
- Programmer approves your recommendation: "yes, add the exception"
- You are implementing a fix that the programmer has already approved

**Exception grants are permanent code changes** - they suppress violations indefinitely and become part of the codebase. Do not make this decision autonomously.

## Development

### Creating New Rules

1. Create a new class extending `CodeQualityRule_Abstract`
2. Place in appropriate Rules subdirectory
3. Implement required methods:
   - `get_id()` - Unique rule identifier
   - `get_name()` - Human-readable name
   - `check()` - Violation detection logic
4. Add to Manifest scan directories if needed

Example:

```php
namespace App\RSpade\CodeQuality\Rules\PHP;

use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;

class MyNew_CodeQualityRule extends CodeQualityRule_Abstract
{
    public function get_id(): string
    {
        return 'PHP-NEW-01';
    }

    public function get_name(): string
    {
        return 'My New Rule';
    }

    public function get_description(): string
    {
        return 'Description of what this rule checks';
    }

    public function get_file_patterns(): array
    {
        return ['*.php'];
    }

    /**
     * Whether this rule is called during manifest scan
     *
     * IMPORTANT: This method should ALWAYS return false unless explicitly requested
     * by the framework developer. Manifest-time checks are reserved for critical
     * framework convention violations that need immediate developer attention.
     *
     * Rules executed during manifest scan will run on every file change in development,
     * potentially impacting performance. Only enable this for rules that:
     * - Enforce critical framework conventions that would break the application
     * - Need to provide immediate feedback before code execution
     * - Have been specifically requested to run at manifest-time by framework maintainers
     *
     * DEFAULT: Always return false unless you have explicit permission to do otherwise.
     */
    public function is_called_during_manifest_scan(): bool
    {
        return false; // Always false unless explicitly approved
    }

    public function check(string $file_path, string $contents, array $metadata = []): void
    {
        // Detection logic
        if ($violation_found) {
            $this->add_violation(
                $file_path,
                $line_number,
                "Violation message",
                $code_snippet,
                "How to fix",
                'medium'
            );
        }
    }
}
```

### Testing Rules

1. Create a temporary test file with violations
2. Run `php artisan rsx:check`
3. Verify violations are detected correctly
4. Clean up test files

## Migration from Monolith

The original 1921-line `CodeStandardsChecker.php` has been:
1. Archived to `/archived/CodeStandardsChecker.old.php`
2. Split into modular rule classes
3. Enhanced with auto-discovery via Manifest
4. Improved with better caching and performance

All original rule logic has been preserved exactly, ensuring no regression in code quality checks.

## Performance

- **Caching**: Sanitized file contents are cached to avoid repeated processing
- **Incremental Linting**: Files are only linted if changed since last check
- **Efficient Scanning**: Smart directory traversal skips excluded paths

## Manifest-Time Checking

By default, code quality rules run only when `php artisan rsx:check` is executed. However, certain critical rules can be configured to run during manifest builds to provide immediate feedback.

### When to Enable Manifest-Time Checking

**DO NOT** enable manifest-time checking unless you have explicit approval. Rules should only run at manifest-time if they:

1. Enforce critical framework conventions that would break the application
2. Need to provide immediate feedback before code execution
3. Have been specifically requested by framework maintainers

### Current Manifest-Time Rules

Only the following rules are approved for manifest-time execution:
- **BLADE-SCRIPT-01** (InlineScriptRule): Prevents inline JavaScript in Blade files (critical architecture violation)
- **JQHTML-INLINE-01** (JqhtmlInlineScriptRule): Prevents inline scripts/styles in Jqhtml template files (critical architecture violation)

All other rules should return `false` from `is_called_during_manifest_scan()`.

## Severity Levels

- **Critical**: Must fix immediately (e.g., fallback code)
- **High**: Should fix soon (e.g., mass assignment)
- **Medium**: Fix when convenient (e.g., naming conventions)
- **Low**: Minor issues (e.g., filename case)