brianhansonxyz/rspade_system
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add incremental manifest-time code quality checks and JS duplicate method detection

Browse Source 
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
root
2025-12-26 21:49:28 +00:00
parent 0ea0341aeb
commit b54484c7ed
32 changed files with 589 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

40
app/RSpade/CodeQuality/CLAUDE.md
View File

@@ -480,6 +480,46 @@ By default, code quality rules run only when `php artisan rsx:check` is executed
2. Need to provide immediate feedback before code execution
3. Have been specifically requested by framework maintainers
### Incremental vs Cross-File Rules
Manifest-time rules support two processing modes via `is_incremental()`:
**Incremental Rules** (`is_incremental() = true`, default):
- Check each file independently
- Only changed files are processed during incremental manifest rebuilds
- More efficient for per-file validation (syntax, patterns, structure)
- Example: `JqhtmlInlineScriptRule`, `MassAssignmentRule`
**Cross-File Rules** (`is_incremental() = false`):
- Need to see relationships between files or check the full manifest
- Run once per manifest build with access to `Manifest::get_all()`
- Use for rules that validate naming across files, check for duplicates, etc.
- Example: `ScssClassScope_CodeQualityRule`, `InstanceMethods_CodeQualityRule`
To make a rule cross-file, override `is_incremental()`:
```php
public function is_incremental(): bool
{
return false; // This rule needs cross-file context
}
```
Cross-file rules should use a static guard to ensure they only run once:
```php
public function check(string $file_path, string $contents, array $metadata = []): void
{
static $already_checked = false;
if ($already_checked) return;
$already_checked = true;
// Access all files via Manifest::get_all()
$files = Manifest::get_all();
// ... check relationships between files
}
```
### Current Manifest-Time Rules
Only the following rules are approved for manifest-time execution:

22
app/RSpade/CodeQuality/Rules/CodeQualityRule_Abstract.php
View File

@@ -92,6 +92,28 @@ abstract class CodeQualityRule_Abstract
{
return false;
}
/**
* Whether this rule checks files incrementally or needs cross-file context
*
* This method is only relevant for rules where is_called_during_manifest_scan() = true.
*
* INCREMENTAL (true - default):
* - Rule checks each file independently
* - Only changed files are passed during incremental manifest rebuilds
* - More efficient for per-file validation rules
*
* CROSS-FILE (false):
* - Rule needs to see relationships between files or check the full manifest
* - Runs once per manifest build with access to all files via Manifest::get_all()
* - Use for rules that validate naming across files, check for duplicates, etc.
*
* @return bool True for per-file rules, false for cross-file rules
*/
public function is_incremental(): bool
{
return true;
}
/**
* Get default severity for this rule

8
app/RSpade/CodeQuality/Rules/JavaScript/DecoratorIdentifierParam_CodeQualityRule.php
View File

@@ -56,6 +56,14 @@ class DecoratorIdentifierParam_CodeQualityRule extends CodeQualityRule_Abstract
return true;
}
/**
* This is a cross-file rule - needs full manifest context
*/
public function is_incremental(): bool
{
return false;
}
/**
* Check the manifest for decorator identifier parameter violations
*/

142
app/RSpade/CodeQuality/Rules/JavaScript/JsDuplicateMethod_CodeQualityRule.php Executable file
View File

@@ -0,0 +1,142 @@
<?php
namespace App\RSpade\CodeQuality\Rules\JavaScript;
use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;
/**
* JsDuplicateMethodRule - Detects duplicate method definitions in ES6 classes
*
* JavaScript allows defining the same method name multiple times in a class,
* but later definitions silently overwrite earlier ones. This is almost always
* a mistake - commonly caused by copy/paste errors or merge conflicts.
*
* Example of problematic code:
* class MyComponent extends Component {
* on_render() { ... } // First definition
* // ... lots of code ...
* on_render() { ... } // Overwrites the first one silently!
* }
*/
class JsDuplicateMethod_CodeQualityRule extends CodeQualityRule_Abstract
{
public function get_id(): string
{
return 'JS-DUPLICATE-METHOD-01';
}
public function get_name(): string
{
return 'Duplicate Method Definition';
}
public function get_description(): string
{
return 'Detects ES6 class methods that are defined more than once (later definitions overwrite earlier ones)';
}
public function get_file_patterns(): array
{
return ['*.js'];
}
public function get_default_severity(): string
{
return 'critical';
}
/**
* This rule runs during manifest scan for immediate feedback
*/
public function is_called_during_manifest_scan(): bool
{
return true;
}
/**
* Check JavaScript file for duplicate method definitions
*/
public function check(string $file_path, string $contents, array $metadata = []): void
{
// Skip if no duplicate methods detected by parser
if (empty($metadata['duplicate_methods'])) {
return;
}
$class_name = $metadata['class'] ?? 'unknown';
foreach ($metadata['duplicate_methods'] as $duplicate) {
$method_name = $duplicate['name'];
$is_static = $duplicate['static'] ?? false;
$first_line = $duplicate['firstLine'] ?? 1;
$second_line = $duplicate['secondLine'] ?? 1;
$static_prefix = $is_static ? 'static ' : '';
$method_type = $is_static ? 'Static method' : 'Method';
$this->add_violation(
$file_path,
$second_line,
"{$method_type} '{$method_name}' is defined multiple times in class {$class_name}",
$this->extract_code_lines($contents, $first_line, $second_line),
$this->get_remediation($method_name, $class_name, $is_static, $first_line, $second_line),
'critical'
);
}
}
/**
* Extract code snippets for both method definitions
*/
private function extract_code_lines(string $contents, int $first_line, int $second_line): string
{
$lines = explode("\n", $contents);
$snippets = [];
if (isset($lines[$first_line - 1])) {
$snippets[] = "Line {$first_line}: " . trim($lines[$first_line - 1]);
}
if (isset($lines[$second_line - 1])) {
$snippets[] = "Line {$second_line}: " . trim($lines[$second_line - 1]);
}
return implode("\n", $snippets);
}
/**
* Get remediation instructions
*/
private function get_remediation(string $method_name, string $class_name, bool $is_static, int $first_line, int $second_line): string
{
$static_prefix = $is_static ? 'static ' : '';
return "DUPLICATE METHOD DEFINITION: '{$method_name}' is defined twice in {$class_name}
PROBLEM:
JavaScript allows defining the same method name multiple times in a class,
but later definitions silently overwrite earlier ones. This means:
- The first definition at line {$first_line} is IGNORED
- Only the second definition at line {$second_line} will be executed
This is almost always a mistake caused by:
- Copy/paste errors
- Merge conflicts
- Forgetting an earlier implementation exists
SOLUTION:
1. Review both definitions to understand what each was intended to do
2. Merge the logic if both are needed, OR
3. Remove the duplicate definition if one is obsolete
FIRST DEFINITION (line {$first_line}) - THIS IS BEING IGNORED:
Look at what this implementation does - is any of this logic needed?
SECOND DEFINITION (line {$second_line}) - THIS IS THE ACTIVE ONE:
This is the only version that will actually run.
COMMON PATTERNS:
- If both have different logic: merge them into a single method
- If they're identical: remove one (preferably keep the better-documented one)
- If one is outdated: remove the obsolete version";
}
}

8
app/RSpade/CodeQuality/Rules/Manifest/InstanceMethods_CodeQualityRule.php
View File

@@ -57,6 +57,14 @@ class InstanceMethods_CodeQualityRule extends CodeQualityRule_Abstract
return true;
}
/**
* This is a cross-file rule - needs full manifest context
*/
public function is_incremental(): bool
{
return false;
}
/**
* Check the manifest for instance method violations
*/

8
app/RSpade/CodeQuality/Rules/Manifest/Monoprogenic_CodeQualityRule.php
View File

@@ -68,6 +68,14 @@ class Monoprogenic_CodeQualityRule extends CodeQualityRule_Abstract
return true;
}
/**
* This is a cross-file rule - needs full manifest context
*/
public function is_incremental(): bool
{
return false;
}
/**
* Check the manifest for Monoprogenic violations
*

8
app/RSpade/CodeQuality/Rules/Manifest/RsxControllerInheritance_CodeQualityRule.php
View File

@@ -53,6 +53,14 @@ class RsxControllerInheritance_CodeQualityRule extends CodeQualityRule_Abstract
return true;
}
/**
* This is a cross-file rule - needs full manifest context
*/
public function is_incremental(): bool
{
return false;
}
/**
* Check for RSX controllers extending Laravel's Controller
*/

8
app/RSpade/CodeQuality/Rules/Manifest/ScssClassScope_CodeQualityRule.php
View File

@@ -70,6 +70,14 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
return true;
}
/**
* This is a cross-file rule - needs full manifest context
*/
public function is_incremental(): bool
{
return false;
}
/**
* Get default severity for this rule
*/

8
app/RSpade/CodeQuality/Rules/Manifest/SpaAttributeMisuse_CodeQualityRule.php
View File

@@ -67,6 +67,14 @@ class SpaAttributeMisuse_CodeQualityRule extends CodeQualityRule_Abstract
return 'critical';
}
/**
* This is a cross-file rule - needs full manifest context
*/
public function is_incremental(): bool
{
return false;
}
/**
* Check the manifest for SPA + Route attribute combinations
*/

11
app/RSpade/Core/Js/Ajax.js
View File

@@ -50,9 +50,14 @@ class Ajax {
const error = event.reason;
console.error('Uncaught Ajax error:', error);
// Show Modal.error() for uncaught Ajax errors
if (typeof Modal !== 'undefined' && Modal.error) {
await Modal.error(error, 'Uncaught Ajax Error');
// Show error modal for uncaught Ajax errors
// In debug mode, use fatal_error() for detailed file/line info
if (typeof Modal !== 'undefined') {
if (window.rsxapp?.debug && Modal.fatal_error) {
await Modal.fatal_error(error, 'Uncaught Ajax Error');
} else if (Modal.error) {
await Modal.error(error, 'Uncaught Ajax Error');
}
}
}
});

33
app/RSpade/Core/Js/async.js
View File

@@ -26,17 +26,28 @@ function sleep(milliseconds = 0) {
/**
* Creates a debounced function with exclusivity and promise fan-in
*
* This function, when invoked, immediately runs the callback exclusively.
* For subsequent invocations, it applies a delay before running the callback exclusively again.
* The delay starts after the current asynchronous operation resolves.
* BEHAVIORAL GUARANTEES:
*
* If 'delay' is set to 0, the function will only prevent enqueueing multiple executions of the
* same method more than once, but will still run them immediately in an exclusive sequential manner.
* 1. IMMEDIATE START: The first call (or any call when idle and delay has passed) executes
* immediately without waiting.
*
* The most recent invocation of the function will be the parameters that get passed to the function
* when it invokes.
* 2. AWAITS COMPLETION: The callback is awaited - async operations (network requests, etc.)
* complete before the execution is considered finished.
*
* The function returns a promise that resolves when the next exclusive execution completes.
* 3. POST-EXECUTION DELAY: After callback completion, no further executions occur for at least
* `delay` milliseconds. The delay timer starts AFTER the callback resolves/rejects.
*
* 4. COALESCED QUEUING: If called during execution OR during the post-execution delay, requests
* are coalesced into ONE pending execution using the LAST provided arguments. When the delay
* expires, exactly one execution occurs with those final arguments.
*
* 5. GUARANTEED EXECUTION: Any call made during an active execution is guaranteed to trigger
* a subsequent execution (with that call's args or later args if more calls arrive).
*
* 6. PROMISE FAN-IN: All callers waiting for the same execution receive the same result/error.
*
* If 'delay' is set to 0, the function still enforces exclusivity (no parallel execution) but
* queued executions run immediately after the current one completes.
*
* Usage as function:
* const debouncedFn = debounce(myFunction, 250);
@@ -46,9 +57,9 @@ function sleep(milliseconds = 0) {
* myMethod() { ... }
*
* @param {function|number} callback_or_delay The callback function OR delay when used as decorator
* @param {number} delay The delay in milliseconds before subsequent invocations
* @param {boolean} immediate if true, the first time the action is called, the callback executes immediately
* @returns {function} A function that when invoked, runs the callback immediately and exclusively,
* @param {number} delay The delay in milliseconds after completion before next execution
* @param {boolean} immediate Unused - first call always runs immediately regardless of this value
* @returns {function} A debounced function that returns a Promise resolving to the callback result
*
* @decorator
*/

5
app/RSpade/Core/JsParsers/Js_Parser.php
View File

@@ -505,6 +505,11 @@ JS;
if (!empty($method_decorators)) {
$data['method_decorators'] = $method_decorators;
}
// Store duplicate methods if any found (methods defined more than once)
if (!empty($first_class['duplicateMethods'])) {
$data['duplicate_methods'] = $first_class['duplicateMethods'];
}
}
if (!empty($parsed['imports'])) {

21
app/RSpade/Core/JsParsers/resource/js-parser-server.js
View File

@@ -362,9 +362,13 @@ function parseFileContent(content, filePath, jsonOutput) {
public_static_methods: {},
properties: {},
staticProperties: {},
decorators: allDecorators
decorators: allDecorators,
duplicateMethods: [] // Track methods defined more than once
};
// Track seen method names to detect duplicates
const seenMethods = {}; // methodName -> { static: bool, line: number }
// Extract methods and properties
path.node.body.body.forEach(member => {
if (t.isClassMethod(member)) {
@@ -380,6 +384,21 @@ function parseFileContent(content, filePath, jsonOutput) {
// Determine visibility: private if starts with #, otherwise public
const methodName = member.key.name || (t.isPrivateName(member.key) ? '#' + member.key.id.name : 'unknown');
const visibility = methodName.startsWith('#') ? 'private' : 'public';
const methodLine = member.loc ? member.loc.start.line : null;
// Check for duplicate method definition
const methodKey = (member.static ? 'static:' : 'instance:') + methodName;
if (seenMethods[methodKey]) {
// This is a duplicate!
classInfo.duplicateMethods.push({
name: methodName,
static: member.static,
firstLine: seenMethods[methodKey].line,
secondLine: methodLine
});
} else {
seenMethods[methodKey] = { line: methodLine };
}
const methodInfo = {
// PHP-compatible fields

10
app/RSpade/Core/Manifest/CLAUDE.md
View File

@@ -141,6 +141,7 @@ private static $_manifest_loaded_at = null; // Load timestamp
- `get_all()` - Returns all files array
- `get_file($path)` - Get specific file metadata (throws if not found)
- `get_files_by_dir($dir)` - Get all files in directory
- `get_changed_files()` - Get files that changed in most recent manifest scan (for incremental code quality checks)
#### PHP Class Resolution
- `php_find_class($name)` - Find by simple class name
@@ -284,6 +285,15 @@ Register in `config/rsx.php`:
The manifest integrates with code quality checks via metadata storage. Rules that run during manifest scan store violations in `code_quality_metadata` field for each file.
### Incremental Code Quality Checks
Manifest-time code quality rules support incremental checking for better performance:
- **Incremental rules** (`is_incremental() = true`): Only check files that changed in this manifest scan
- **Cross-file rules** (`is_incremental() = false`): Run once per build, access full manifest via `get_all()`
The `get_changed_files()` API provides the list of files that changed, enabling rules to optimize their processing.
## Debugging
Enable verbose output:

184
app/RSpade/Core/Manifest/Manifest.php
View File

@@ -191,6 +191,9 @@ class Manifest
// Track if we've already shown the manifest rescan message this page load
protected static bool $__shown_rescan_message = false;
// Files that changed in the most recent manifest scan (for incremental code quality checks)
protected static array $_changed_files = [];
// ========================================
// Query Methods
// ========================================
@@ -216,6 +219,19 @@ class Manifest
return static::$data['data']['autoloader_class_map'] ?? [];
}
/**
* Get the list of files that changed in the most recent manifest scan
*
* Used by code quality rules that need to know which files changed for
* incremental processing. Returns empty array if manifest was fully rebuilt.
*
* @return array Array of relative file paths that changed
*/
public static function get_changed_files(): array
{
return static::$_changed_files;
}
/**
* Get data for a specific file
*/
@@ -1303,6 +1319,9 @@ class Manifest
}
}
// Store changed files for incremental code quality checks
static::$_changed_files = $files_to_process;
console_debug('MANIFEST', 'Phase 1: File Discovery - ' . count($files) . ' files, ' . count($files_to_process) . ' changed');
// If any files have changed and we're not in production, run auto-reformat
@@ -1518,7 +1537,7 @@ class Manifest
// Skip during migrations - database may not be provisioned yet
if (env('APP_ENV') !== 'production' && $changes && !static::__is_migration_context()) {
static::__verify_database_provisioned();
static::__run_manifest_time_code_quality_checks();
static::__run_manifest_time_code_quality_checks($files_to_process);
}
// Check if a file was auto-renamed and manifest needs to restart
@@ -3661,8 +3680,14 @@ class Manifest
* Run code quality checks during manifest scan
* Only runs in development mode after manifest changes
* Throws fatal exception on first violation found
*
* Supports two types of rules:
* - Incremental rules (is_incremental() = true): Only check changed files
* - Cross-file rules (is_incremental() = false): Run once with full manifest context
*
* @param array $changed_files Files that changed in this manifest scan
*/
protected static function __run_manifest_time_code_quality_checks(): void
protected static function __run_manifest_time_code_quality_checks(array $changed_files = []): void
{
// Create a minimal violation collector
$collector = new \App\RSpade\CodeQuality\Support\ViolationCollector();
@@ -3675,78 +3700,129 @@ class Manifest
true // Only get rules with is_called_during_manifest_scan() = true
);
foreach ($rules as $rule) {
// Get file patterns this rule applies to
$patterns = $rule->get_file_patterns();
// Separate rules by type
$incremental_rules = [];
$cross_file_rules = [];
// Check each file in the manifest
foreach (static::$data['data']['files'] as $file_path => $metadata) {
// Check if file matches any pattern
$matches = false;
foreach ($patterns as $pattern) {
$pattern = str_replace('*', '.*', $pattern);
if (preg_match('/^' . $pattern . '$/i', basename($file_path))) {
$matches = true;
break;
}
foreach ($rules as $rule) {
if ($rule->is_incremental()) {
$incremental_rules[] = $rule;
} else {
$cross_file_rules[] = $rule;
}
}
// Helper to throw violation exception
$throw_violation = function ($collector) {
$violations = $collector->get_all();
if (!empty($violations)) {
$first_violation = $violations[0];
$data = $first_violation->to_array();
static::_set_manifest_is_bad();
$relative_file = str_replace(base_path() . '/', '', $data['file']);
$error_message = "Code Quality Violation ({$data['type']}) - {$data['message']}\n\n";
$error_message .= "File: {$relative_file}:{$data['line']}\n\n";
if (!empty($data['code'])) {
$error_message .= "Code:\n " . $data['code'] . "\n\n";
}
if (!$matches) {
if (!empty($data['resolution'])) {
$error_message .= "Resolution:\n" . $data['resolution'];
}
throw new \App\RSpade\CodeQuality\RuntimeChecks\YoureDoingItWrongException(
$error_message,
0,
null,
$data['file'],
$data['line']
);
}
};
// Helper to check if file matches rule patterns
$file_matches_rule = function ($file_path, $rule) {
$patterns = $rule->get_file_patterns();
foreach ($patterns as $pattern) {
$pattern = str_replace('*', '.*', $pattern);
if (preg_match('/^' . $pattern . '$/i', basename($file_path))) {
return true;
}
}
return false;
};
// =========================================================
// INCREMENTAL RULES: Only check changed files
// =========================================================
// Determine which files to check - changed files only for incremental rebuild,
// or all files if this is a full rebuild (empty changed_files means check all)
$files_to_check = !empty($changed_files)
? array_flip($changed_files) // Use as lookup for O(1) checks
: null; // null = check all files
foreach ($incremental_rules as $rule) {
foreach (static::$data['data']['files'] as $file_path => $metadata) {
// Skip files that haven't changed (for incremental rebuilds)
if ($files_to_check !== null && !isset($files_to_check[$file_path])) {
continue;
}
// Get absolute path
$absolute_path = base_path($file_path);
if (!$file_matches_rule($file_path, $rule)) {
continue;
}
// Skip if file doesn't exist
$absolute_path = base_path($file_path);
if (!file_exists($absolute_path)) {
continue;
}
// Read file contents
$contents = file_get_contents($absolute_path);
// Run the rule check - include code_quality_metadata if available
$enhanced_metadata = $metadata;
if (isset(static::$data['data']['files'][$file_path]['code_quality_metadata'][$rule->get_id()])) {
$enhanced_metadata['rule_metadata'] = static::$data['data']['files'][$file_path]['code_quality_metadata'][$rule->get_id()];
}
$rule->check($absolute_path, $contents, $enhanced_metadata);
$throw_violation($collector);
}
}
// Check if any violations were found - throw on first violation
$violations = $collector->get_all();
if (!empty($violations)) {
// Get the first violation
$first_violation = $violations[0];
$data = $first_violation->to_array();
// Mark manifest as bad to force re-checking on next attempt
static::_set_manifest_is_bad();
// Format file path relative to project root
$relative_file = str_replace(base_path() . '/', '', $data['file']);
// Build error message
$error_message = "Code Quality Violation ({$data['type']}) - {$data['message']}\n\n";
$error_message .= "File: {$relative_file}:{$data['line']}\n\n";
if (!empty($data['code'])) {
$error_message .= "Code:\n " . $data['code'] . "\n\n";
}
if (!empty($data['resolution'])) {
$error_message .= "Resolution:\n" . $data['resolution'];
}
// Throw fatal exception with the first violation, passing file and line for accurate error display
throw new \App\RSpade\CodeQuality\RuntimeChecks\YoureDoingItWrongException(
$error_message,
0,
null,
$data['file'],
$data['line']
);
// =========================================================
// CROSS-FILE RULES: Run once with full manifest context
// =========================================================
// These rules internally access Manifest::get_all() to check all files
// We just need to trigger them once with any matching file
foreach ($cross_file_rules as $rule) {
// Find first file matching the rule's patterns to trigger the check
foreach (static::$data['data']['files'] as $file_path => $metadata) {
if (!$file_matches_rule($file_path, $rule)) {
continue;
}
$absolute_path = base_path($file_path);
if (!file_exists($absolute_path)) {
continue;
}
$contents = file_get_contents($absolute_path);
$enhanced_metadata = $metadata;
if (isset(static::$data['data']['files'][$file_path]['code_quality_metadata'][$rule->get_id()])) {
$enhanced_metadata['rule_metadata'] = static::$data['data']['files'][$file_path]['code_quality_metadata'][$rule->get_id()];
}
// Cross-file rules run once, they handle iteration internally
$rule->check($absolute_path, $contents, $enhanced_metadata);
$throw_violation($collector);
// Only trigger once per cross-file rule
break;
}
}
}