Add JS-CATCH-FALLBACK-01 rule and update npm packages
Add PHP-ALIAS-01 rule: prohibit field aliasing in serialization 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
root
365
app/RSpade/CodeQuality/Rules/JavaScript/JqhtmlCatchFallback_CodeQualityRule.php
Executable file
365
app/RSpade/CodeQuality/Rules/JavaScript/JqhtmlCatchFallback_CodeQualityRule.php
Executable file
@@ -0,0 +1,365 @@
|
||||
<?php
|
||||
|
||||
namespace App\RSpade\CodeQuality\Rules\JavaScript;
|
||||
|
||||
use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;
|
||||
use App\RSpade\CodeQuality\Support\FileSanitizer;
|
||||
|
||||
/**
|
||||
* Detect fallback data assignments in catch blocks within on_load() methods
|
||||
*
|
||||
* This is a CRITICAL violation of the fail-loud principle. When a network error
|
||||
* occurs during data loading, the application should SHOW THE ERROR, not silently
|
||||
* substitute hardcoded values that will cause bizarre undefined behavior.
|
||||
*
|
||||
* FORBIDDEN PATTERN:
|
||||
* async on_load() {
|
||||
* try {
|
||||
* this.data.roles = await Controller.get_roles();
|
||||
* } catch (e) {
|
||||
* this.data.roles = [{id: 1, name: 'Manager'}]; // VIOLATION
|
||||
* }
|
||||
* }
|
||||
*
|
||||
* CORRECT PATTERN:
|
||||
* async on_load() {
|
||||
* try {
|
||||
* this.data.roles = await Controller.get_roles();
|
||||
* } catch (e) {
|
||||
* this.data.error_data = e; // Surface the error
|
||||
* }
|
||||
* }
|
||||
*
|
||||
* This is like putting black tape over a car's check engine light. The error
|
||||
* is hidden, never noticed, never fixed. The application shows "working" UI
|
||||
* but the data is stale/wrong/invented.
|
||||
*/
|
||||
class JqhtmlCatchFallback_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
{
|
||||
/**
|
||||
* Allowed property names for this.data.X = in catch blocks
|
||||
*/
|
||||
private const ALLOWED_ERROR_PROPERTIES = [
|
||||
'error',
|
||||
'error_data',
|
||||
'error_message',
|
||||
'load_error',
|
||||
'loading_error',
|
||||
'fetch_error',
|
||||
];
|
||||
|
||||
/**
|
||||
* Properties that can be set to boolean/null values in catch blocks
|
||||
* These are state flags, not data
|
||||
*/
|
||||
private const ALLOWED_STATE_PROPERTIES = [
|
||||
'loading',
|
||||
'loaded',
|
||||
'is_loading',
|
||||
'is_loaded',
|
||||
'fetching',
|
||||
'refreshing',
|
||||
];
|
||||
|
||||
public function get_id(): string
|
||||
{
|
||||
return 'JS-CATCH-FALLBACK-01';
|
||||
}
|
||||
|
||||
public function get_name(): string
|
||||
{
|
||||
return 'Catch Block Fallback Data Assignment';
|
||||
}
|
||||
|
||||
public function get_description(): string
|
||||
{
|
||||
return 'Prohibits assigning fallback data in catch blocks - errors must be surfaced, not hidden';
|
||||
}
|
||||
|
||||
public function get_file_patterns(): array
|
||||
{
|
||||
return ['*.js'];
|
||||
}
|
||||
|
||||
/**
|
||||
* Run during manifest build for immediate feedback
|
||||
*/
|
||||
public function is_called_during_manifest_scan(): bool
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
public function get_default_severity(): string
|
||||
{
|
||||
return 'critical';
|
||||
}
|
||||
|
||||
public function check(string $file_path, string $contents, array $metadata = []): void
|
||||
{
|
||||
// Skip if not a JavaScript file
|
||||
if (!isset($metadata['extension']) || $metadata['extension'] !== 'js') {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip vendor and node_modules
|
||||
if (str_contains($file_path, '/vendor/') || str_contains($file_path, '/node_modules/')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip CodeQuality directory
|
||||
if (str_contains($file_path, '/CodeQuality/')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if this is a Component subclass (Component, Spa_Action, Spa_Layout, or their children)
|
||||
$extends = $metadata['extends'] ?? null;
|
||||
if (!in_array($extends, ['Component', 'Spa_Action', 'Spa_Layout'])) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check for file-level exception
|
||||
if (str_contains($contents, '@' . $this->get_id() . '-EXCEPTION')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Get sanitized content (comments removed)
|
||||
$sanitized_data = FileSanitizer::sanitize_javascript($file_path);
|
||||
$sanitized_content = $sanitized_data['content'];
|
||||
|
||||
// Get class name
|
||||
$class_name = $metadata['class'] ?? 'Unknown';
|
||||
|
||||
// Find the on_load method
|
||||
if (!preg_match('/(?:async\s+)?on_load\s*\([^)]*\)\s*\{/i', $sanitized_content, $method_match, PREG_OFFSET_CAPTURE)) {
|
||||
return; // No on_load method
|
||||
}
|
||||
|
||||
$method_start = $method_match[0][1];
|
||||
|
||||
// Extract the on_load method body
|
||||
$method_body = $this->extract_brace_block($sanitized_content, $method_start);
|
||||
if (empty($method_body)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Find catch blocks within on_load
|
||||
$this->check_catch_blocks($file_path, $contents, $method_body, $method_start, $class_name);
|
||||
}
|
||||
|
||||
/**
|
||||
* Find and check all catch blocks within the on_load method
|
||||
*/
|
||||
private function check_catch_blocks(string $file_path, string $original_contents, string $method_body, int $method_offset, string $class_name): void
|
||||
{
|
||||
// Find all catch blocks
|
||||
$offset = 0;
|
||||
while (preg_match('/\bcatch\s*\([^)]*\)\s*\{/', $method_body, $match, PREG_OFFSET_CAPTURE, $offset)) {
|
||||
$catch_start = $match[0][1];
|
||||
$catch_body = $this->extract_brace_block($method_body, $catch_start);
|
||||
|
||||
if (!empty($catch_body)) {
|
||||
$this->check_catch_body($file_path, $original_contents, $catch_body, $method_offset + $catch_start, $class_name);
|
||||
}
|
||||
|
||||
// Move past this catch block
|
||||
$offset = $catch_start + strlen($match[0][0]);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check a catch block body for forbidden data assignments
|
||||
*/
|
||||
private function check_catch_body(string $file_path, string $original_contents, string $catch_body, int $catch_offset, string $class_name): void
|
||||
{
|
||||
$lines = explode("\n", $catch_body);
|
||||
$original_lines = explode("\n", $original_contents);
|
||||
|
||||
// Calculate line offset from file start
|
||||
$line_offset = substr_count(substr($original_contents, 0, $catch_offset), "\n");
|
||||
|
||||
foreach ($lines as $line_num => $line) {
|
||||
$actual_line_number = $line_offset + $line_num + 1;
|
||||
$trimmed = trim($line);
|
||||
|
||||
// Skip empty lines
|
||||
if (empty($trimmed)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Check for this.data.X = patterns
|
||||
if (preg_match('/\bthis\.data\.(\w+)\s*=\s*(.+)/', $line, $matches)) {
|
||||
$property_name = $matches[1];
|
||||
$value_part = trim($matches[2]);
|
||||
|
||||
// Allow error-related properties
|
||||
if (in_array($property_name, self::ALLOWED_ERROR_PROPERTIES)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Allow any property to be set to null (clearing data is OK)
|
||||
if (preg_match('/^null\s*[;,]?\s*$/', $value_part)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Allow state properties when set to boolean
|
||||
if (in_array($property_name, self::ALLOWED_STATE_PROPERTIES)) {
|
||||
if (preg_match('/^(true|false)\s*[;,]?\s*$/', $value_part)) {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
// Check for line-level exception in original file
|
||||
if ($this->line_has_exception($original_lines, $actual_line_number)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Get the actual line from original content for display
|
||||
$display_line = isset($original_lines[$actual_line_number - 1])
|
||||
? trim($original_lines[$actual_line_number - 1])
|
||||
: trim($line);
|
||||
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$actual_line_number,
|
||||
"CRITICAL: Fallback data assigned in catch block. Setting 'this.data.{$property_name}' hides the error instead of surfacing it.",
|
||||
$display_line,
|
||||
$this->build_suggestion($property_name, $class_name),
|
||||
'critical'
|
||||
);
|
||||
}
|
||||
|
||||
// Also check for this.data = (full object assignment)
|
||||
if (preg_match('/\bthis\.data\s*=\s*\{/', $line)) {
|
||||
// Check for line-level exception in original file
|
||||
if ($this->line_has_exception($original_lines, $actual_line_number)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$display_line = isset($original_lines[$actual_line_number - 1])
|
||||
? trim($original_lines[$actual_line_number - 1])
|
||||
: trim($line);
|
||||
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$actual_line_number,
|
||||
"CRITICAL: Fallback data object assigned in catch block. This hides the error instead of surfacing it.",
|
||||
$display_line,
|
||||
$this->build_suggestion('...', $class_name),
|
||||
'critical'
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract a brace-delimited block starting at the given position
|
||||
*/
|
||||
private function extract_brace_block(string $content, int $start_pos): string
|
||||
{
|
||||
// Find the opening brace
|
||||
$brace_pos = strpos($content, '{', $start_pos);
|
||||
if ($brace_pos === false) {
|
||||
return '';
|
||||
}
|
||||
|
||||
$brace_count = 0;
|
||||
$pos = $brace_pos;
|
||||
$length = strlen($content);
|
||||
|
||||
while ($pos < $length) {
|
||||
$char = $content[$pos];
|
||||
|
||||
if ($char === '{') {
|
||||
$brace_count++;
|
||||
} elseif ($char === '}') {
|
||||
$brace_count--;
|
||||
if ($brace_count === 0) {
|
||||
return substr($content, $brace_pos, $pos - $brace_pos + 1);
|
||||
}
|
||||
}
|
||||
$pos++;
|
||||
}
|
||||
|
||||
return '';
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a line has an exception comment
|
||||
*/
|
||||
private function line_has_exception(array $lines, int $line_num): bool
|
||||
{
|
||||
$line_index = $line_num - 1;
|
||||
|
||||
// Check current line
|
||||
if (isset($lines[$line_index]) && str_contains($lines[$line_index], '@' . $this->get_id() . '-EXCEPTION')) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check previous line
|
||||
if ($line_index > 0 && isset($lines[$line_index - 1]) && str_contains($lines[$line_index - 1], '@' . $this->get_id() . '-EXCEPTION')) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build an extremely clear suggestion about why this is wrong
|
||||
*/
|
||||
private function build_suggestion(string $property_name, string $class_name): string
|
||||
{
|
||||
$lines = [];
|
||||
$lines[] = "================================================================================";
|
||||
$lines[] = "BLACK TAPE OVER THE CHECK ENGINE LIGHT";
|
||||
$lines[] = "================================================================================";
|
||||
$lines[] = "";
|
||||
$lines[] = "This code HIDES network errors by substituting hardcoded fallback data.";
|
||||
$lines[] = "When the server fails, users see 'working' UI with WRONG DATA.";
|
||||
$lines[] = "";
|
||||
$lines[] = "WHAT HAPPENS:";
|
||||
$lines[] = " 1. Server returns error (500, timeout, network failure)";
|
||||
$lines[] = " 2. Your code catches the error and assigns fake data";
|
||||
$lines[] = " 3. User sees UI that looks normal but contains INVENTED values";
|
||||
$lines[] = " 4. User makes decisions based on WRONG information";
|
||||
$lines[] = " 5. Error is never noticed, never fixed, never logged properly";
|
||||
$lines[] = " 6. Application behavior becomes NON-DETERMINISTIC";
|
||||
$lines[] = "";
|
||||
$lines[] = "WHY THIS IS CATASTROPHIC:";
|
||||
$lines[] = " - Enumerated values (roles, statuses, types) MUST have ONE source of truth";
|
||||
$lines[] = " - Hardcoded fallbacks create SHADOW DATA that diverges from the database";
|
||||
$lines[] = " - Errors should be VISIBLE so they can be FIXED";
|
||||
$lines[] = " - Silent failures are WORSE than loud failures";
|
||||
$lines[] = "";
|
||||
$lines[] = "WRONG (your code):";
|
||||
$lines[] = " async on_load() {";
|
||||
$lines[] = " try {";
|
||||
$lines[] = " this.data.{$property_name} = await Controller.get_data();";
|
||||
$lines[] = " } catch (e) {";
|
||||
$lines[] = " console.error('Failed:', e);";
|
||||
$lines[] = " this.data.{$property_name} = [{...hardcoded fallback...}]; // DISASTER";
|
||||
$lines[] = " }";
|
||||
$lines[] = " }";
|
||||
$lines[] = "";
|
||||
$lines[] = "CORRECT:";
|
||||
$lines[] = " async on_load() {";
|
||||
$lines[] = " try {";
|
||||
$lines[] = " this.data.{$property_name} = await Controller.get_data();";
|
||||
$lines[] = " } catch (e) {";
|
||||
$lines[] = " this.data.error_data = e; // SURFACE THE ERROR";
|
||||
$lines[] = " }";
|
||||
$lines[] = " }";
|
||||
$lines[] = "";
|
||||
$lines[] = "Template shows error state when this.data.error_data is set:";
|
||||
$lines[] = " <% if (this.data.error_data) { %>";
|
||||
$lines[] = " <Universal_Error_Page_Component \$error_data=this.data.error_data />";
|
||||
$lines[] = " <% } else { %>";
|
||||
$lines[] = " ... normal content ...";
|
||||
$lines[] = " <% } %>";
|
||||
$lines[] = "";
|
||||
$lines[] = "================================================================================";
|
||||
$lines[] = "FIX: Remove the fallback data. Assign this.data.error_data = e instead.";
|
||||
$lines[] = "================================================================================";
|
||||
|
||||
return implode("\n", $lines);
|
||||
}
|
||||
}
|
||||
445
app/RSpade/CodeQuality/Rules/PHP/FieldAliasing_CodeQualityRule.php
Executable file
445
app/RSpade/CodeQuality/Rules/PHP/FieldAliasing_CodeQualityRule.php
Executable file
@@ -0,0 +1,445 @@
|
||||
<?php
|
||||
|
||||
namespace App\RSpade\CodeQuality\Rules\PHP;
|
||||
|
||||
use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;
|
||||
|
||||
/**
|
||||
* FieldAliasingRule - Detects confusing field name shortenings
|
||||
*
|
||||
* This rule catches cases where a field name is SHORTENED by dropping parts,
|
||||
* which creates confusion about what the field actually represents.
|
||||
*
|
||||
* VIOLATION - Dropping parts from a name (confusing):
|
||||
* 'type_label' => $contact->type_id_label, // Dropped "id" - what happened to it?
|
||||
* 'client_label' => $record->client_id_label, // Dropped "id" - confusing
|
||||
* 'name' => $user->display_name, // Dropped "display" - loses context
|
||||
*
|
||||
* ALLOWED - Renaming to a completely different concept:
|
||||
* 'value' => $client->id, // "value" is a UI concept, not a shortened "id"
|
||||
* 'label' => $client->name, // "label" is a UI concept, not a shortened "name"
|
||||
* 'client_id' => $client->id, // Adding context, not dropping it
|
||||
* 'id' => $client->id, // Same name - no aliasing
|
||||
*
|
||||
* ALLOWED - Transformations:
|
||||
* 'type_id_label_upper' => strtoupper($contact->type_id_label),
|
||||
*
|
||||
* The detection logic: Flag when the key's underscore-separated parts are a
|
||||
* PROPER SUBSET of the source property's parts (all key parts exist in source,
|
||||
* but source has additional parts). This catches "dropping parts" without
|
||||
* flagging legitimate renames to different concepts.
|
||||
*
|
||||
* Applies to:
|
||||
* - Controller methods with #[Ajax_Endpoint] attribute
|
||||
* - Model fetch() methods with #[Ajax_Endpoint_Model_Fetch] attribute
|
||||
*/
|
||||
class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
{
|
||||
/**
|
||||
* Get the unique rule identifier
|
||||
*/
|
||||
public function get_id(): string
|
||||
{
|
||||
return 'PHP-ALIAS-01';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get human-readable rule name
|
||||
*/
|
||||
public function get_name(): string
|
||||
{
|
||||
return 'Field Aliasing Prohibition';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get rule description
|
||||
*/
|
||||
public function get_description(): string
|
||||
{
|
||||
return 'Prohibits renaming fields during serialization - field names must be consistent across all application layers';
|
||||
}
|
||||
|
||||
/**
|
||||
* Get file patterns this rule applies to
|
||||
*/
|
||||
public function get_file_patterns(): array
|
||||
{
|
||||
return ['*.php'];
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this rule is called during manifest scan
|
||||
*/
|
||||
public function is_called_during_manifest_scan(): bool
|
||||
{
|
||||
return false; // Only run during rsx:check
|
||||
}
|
||||
|
||||
/**
|
||||
* Get default severity for this rule
|
||||
*/
|
||||
public function get_default_severity(): string
|
||||
{
|
||||
return 'high';
|
||||
}
|
||||
|
||||
/**
|
||||
* Check a file for violations
|
||||
*/
|
||||
public function check(string $file_path, string $contents, array $metadata = []): void
|
||||
{
|
||||
// Read original file content for exception checking
|
||||
$original_contents = file_get_contents($file_path);
|
||||
|
||||
// Skip if file-level exception comment is present
|
||||
if (strpos($original_contents, '@' . $this->get_id() . '-EXCEPTION') !== false) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip archived files
|
||||
if (str_contains($file_path, '/archive/') || str_contains($file_path, '/archived/')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Determine file type and get relevant methods
|
||||
$extends = $metadata['extends'] ?? null;
|
||||
$methods_to_check = [];
|
||||
|
||||
if ($extends === 'Rsx_Controller_Abstract') {
|
||||
// Controller - check methods with #[Ajax_Endpoint]
|
||||
$methods_to_check = $this->get_ajax_endpoint_methods($metadata);
|
||||
} elseif ($extends === 'Rsx_Model_Abstract') {
|
||||
// Model - check fetch() method with #[Ajax_Endpoint_Model_Fetch]
|
||||
$methods_to_check = $this->get_model_fetch_methods($metadata);
|
||||
} else {
|
||||
// Not a controller or model we care about
|
||||
return;
|
||||
}
|
||||
|
||||
if (empty($methods_to_check)) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check each relevant method
|
||||
foreach ($methods_to_check as $method_name => $method_info) {
|
||||
$method_body = $this->extract_method_body($contents, $method_name);
|
||||
if (!$method_body) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$method_line = $method_info['line'] ?? 1;
|
||||
|
||||
// Check if method has exception comment
|
||||
if ($this->method_has_exception($original_contents, $method_name, $method_line)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->check_method_body($file_path, $method_body, $method_name, $method_line, $original_contents);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get methods with #[Ajax_Endpoint] attribute from controller
|
||||
*/
|
||||
private function get_ajax_endpoint_methods(array $metadata): array
|
||||
{
|
||||
$methods = $metadata['public_static_methods'] ?? [];
|
||||
$result = [];
|
||||
|
||||
foreach ($methods as $method_name => $method_info) {
|
||||
$attributes = $method_info['attributes'] ?? [];
|
||||
|
||||
foreach ($attributes as $attr_name => $attr_data) {
|
||||
$short_name = basename(str_replace('\\', '/', $attr_name));
|
||||
if ($short_name === 'Ajax_Endpoint') {
|
||||
$result[$method_name] = $method_info;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get fetch() methods with #[Ajax_Endpoint_Model_Fetch] attribute from model
|
||||
*/
|
||||
private function get_model_fetch_methods(array $metadata): array
|
||||
{
|
||||
$methods = $metadata['public_static_methods'] ?? [];
|
||||
$result = [];
|
||||
|
||||
if (!isset($methods['fetch'])) {
|
||||
return $result;
|
||||
}
|
||||
|
||||
$fetch_info = $methods['fetch'];
|
||||
$attributes = $fetch_info['attributes'] ?? [];
|
||||
|
||||
foreach ($attributes as $attr_name => $attr_data) {
|
||||
$short_name = basename(str_replace('\\', '/', $attr_name));
|
||||
if ($short_name === 'Ajax_Endpoint_Model_Fetch') {
|
||||
$result['fetch'] = $fetch_info;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
return $result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check method body for aliasing violations
|
||||
*/
|
||||
private function check_method_body(string $file_path, string $method_body, string $method_name, int $method_start_line, string $original_contents): void
|
||||
{
|
||||
$lines = explode("\n", $method_body);
|
||||
$original_lines = explode("\n", $original_contents);
|
||||
|
||||
foreach ($lines as $offset => $line) {
|
||||
// Pattern: 'key' => $var->property or 'key' => $var['property']
|
||||
// We need to detect when key != property (with no transformation)
|
||||
|
||||
// Match: 'key_name' => $something->property_name
|
||||
// or: 'key_name' => $something['property_name']
|
||||
// Without function wrapping
|
||||
|
||||
// Pattern for object property access: 'key' => $var->prop
|
||||
if (preg_match("/['\"]([a-zA-Z_][a-zA-Z0-9_]*)['\"]\\s*=>\\s*\\$[a-zA-Z_][a-zA-Z0-9_]*->([a-zA-Z_][a-zA-Z0-9_]*)\\s*[,\\]\\)]/", $line, $matches)) {
|
||||
$key = $matches[1];
|
||||
$property = $matches[2];
|
||||
|
||||
if ($this->is_problematic_alias($key, $property) && !$this->has_transformation($line, $matches[0])) {
|
||||
// Check for line-level exception
|
||||
$actual_line_num = $method_start_line + $offset;
|
||||
if ($this->line_has_exception($original_lines, $actual_line_num)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$actual_line_num,
|
||||
"Field name shortened by dropping parts: '{$key}' is missing parts from '{$property}'",
|
||||
trim($line),
|
||||
$this->build_suggestion($key, $property),
|
||||
'high'
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Pattern for array access: 'key' => $var['prop']
|
||||
if (preg_match("/['\"]([a-zA-Z_][a-zA-Z0-9_]*)['\"]\\s*=>\\s*\\$[a-zA-Z_][a-zA-Z0-9_]*\\[['\"]([a-zA-Z_][a-zA-Z0-9_]*)['\"]\\]\\s*[,\\]\\)]/", $line, $matches)) {
|
||||
$key = $matches[1];
|
||||
$property = $matches[2];
|
||||
|
||||
if ($this->is_problematic_alias($key, $property) && !$this->has_transformation($line, $matches[0])) {
|
||||
// Check for line-level exception
|
||||
$actual_line_num = $method_start_line + $offset;
|
||||
if ($this->line_has_exception($original_lines, $actual_line_num)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$actual_line_num,
|
||||
"Field name shortened by dropping parts: '{$key}' is missing parts from '{$property}'",
|
||||
trim($line),
|
||||
$this->build_suggestion($key, $property),
|
||||
'high'
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the key is a problematic alias of the property
|
||||
*
|
||||
* Returns true if the key is a PROPER SUBSET of the property's parts,
|
||||
* meaning all parts of the key exist in the property, but the property
|
||||
* has additional parts that were dropped.
|
||||
*
|
||||
* Examples:
|
||||
* 'type_label', 'type_id_label' -> true (dropped "id")
|
||||
* 'name', 'display_name' -> true (dropped "display")
|
||||
* 'value', 'id' -> false (different concept)
|
||||
* 'client_id', 'id' -> false (adding context)
|
||||
* 'id', 'id' -> false (same name)
|
||||
*/
|
||||
private function is_problematic_alias(string $key, string $property): bool
|
||||
{
|
||||
// Same name is never a violation
|
||||
if ($key === $property) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Split by underscores
|
||||
$key_parts = explode('_', strtolower($key));
|
||||
$property_parts = explode('_', strtolower($property));
|
||||
|
||||
// Check if ALL key parts exist in the property parts
|
||||
foreach ($key_parts as $key_part) {
|
||||
if (!in_array($key_part, $property_parts)) {
|
||||
// Key has a part that doesn't exist in property
|
||||
// This means it's a rename to a different concept, not a shortening
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// At this point, all key parts exist in property parts
|
||||
// Check if property has additional parts (making key a proper subset)
|
||||
if (count($property_parts) > count($key_parts)) {
|
||||
// Property has more parts than key - parts were dropped
|
||||
return true;
|
||||
}
|
||||
|
||||
// Same number of parts (just reordered?) - not a violation
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if the value has a transformation applied (function call wrapping it)
|
||||
*/
|
||||
private function has_transformation(string $line, string $matched_portion): bool
|
||||
{
|
||||
// Find where the matched portion starts in the line
|
||||
$pos = strpos($line, $matched_portion);
|
||||
if ($pos === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Get everything after '=>' and before the matched value
|
||||
if (preg_match("/=>\\s*([a-zA-Z_][a-zA-Z0-9_]*)\\s*\\(/", $line, $fn_match)) {
|
||||
// There's a function call before the value
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check for method chaining or casting
|
||||
if (preg_match("/=>\\s*\\([^)]+\\)\\s*\\$/", $line)) {
|
||||
// Cast like (string)$var->prop
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check for string concatenation
|
||||
if (preg_match("/=>\\s*['\"].*['\"]\\s*\\.\\s*\\$/", $line) || preg_match("/\\$[^,]+\\.\\s*['\"]/", $line)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check for ternary operator
|
||||
if (strpos($line, '?') !== false && strpos($line, ':') !== false) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check for null coalescing
|
||||
if (strpos($line, '??') !== false) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if method has exception comment in docblock
|
||||
*/
|
||||
private function method_has_exception(string $contents, string $method_name, int $method_line): bool
|
||||
{
|
||||
$lines = explode("\n", $contents);
|
||||
|
||||
// Check the 15 lines before the method definition for exception
|
||||
$start_line = max(0, $method_line - 16);
|
||||
$end_line = $method_line - 1;
|
||||
|
||||
for ($i = $start_line; $i <= $end_line && $i < count($lines); $i++) {
|
||||
$line = $lines[$i];
|
||||
if (str_contains($line, '@' . $this->get_id() . '-EXCEPTION')) {
|
||||
return true;
|
||||
}
|
||||
// Stop if we hit another method definition
|
||||
if (preg_match('/^\s*public\s+static\s+function\s+/', $line) && !str_contains($line, $method_name)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a specific line has an exception comment
|
||||
*/
|
||||
private function line_has_exception(array $lines, int $line_num): bool
|
||||
{
|
||||
$line_index = $line_num - 1;
|
||||
|
||||
// Check current line
|
||||
if (isset($lines[$line_index]) && str_contains($lines[$line_index], '@' . $this->get_id() . '-EXCEPTION')) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check previous line
|
||||
if ($line_index > 0 && isset($lines[$line_index - 1]) && str_contains($lines[$line_index - 1], '@' . $this->get_id() . '-EXCEPTION')) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract method body from file contents
|
||||
*/
|
||||
private function extract_method_body(string $contents, string $method_name): ?string
|
||||
{
|
||||
// Pattern to match method definition
|
||||
$pattern = '/public\s+static\s+function\s+' . preg_quote($method_name, '/') . '\s*\([^)]*\)[^{]*\{/s';
|
||||
|
||||
if (!preg_match($pattern, $contents, $matches, PREG_OFFSET_CAPTURE)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$start_pos = $matches[0][1] + strlen($matches[0][0]) - 1;
|
||||
$brace_count = 1;
|
||||
$pos = $start_pos + 1;
|
||||
$length = strlen($contents);
|
||||
|
||||
while ($pos < $length && $brace_count > 0) {
|
||||
$char = $contents[$pos];
|
||||
if ($char === '{') {
|
||||
$brace_count++;
|
||||
} elseif ($char === '}') {
|
||||
$brace_count--;
|
||||
}
|
||||
$pos++;
|
||||
}
|
||||
|
||||
return substr($contents, $start_pos, $pos - $start_pos);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build suggestion for fixing the violation
|
||||
*/
|
||||
private function build_suggestion(string $key, string $property): string
|
||||
{
|
||||
$suggestions = [];
|
||||
$suggestions[] = "PROBLEM: Field name shortened by dropping parts.";
|
||||
$suggestions[] = "";
|
||||
$suggestions[] = "The key '{$key}' contains only some parts of '{$property}'.";
|
||||
$suggestions[] = "This is confusing because it obscures what was removed.";
|
||||
$suggestions[] = "";
|
||||
$suggestions[] = "FIX: Use the full property name:";
|
||||
$suggestions[] = "";
|
||||
$suggestions[] = " // WRONG - parts dropped, confusing";
|
||||
$suggestions[] = " '{$key}' => \$model->{$property},";
|
||||
$suggestions[] = "";
|
||||
$suggestions[] = " // CORRECT - full name preserved";
|
||||
$suggestions[] = " '{$property}' => \$model->{$property},";
|
||||
$suggestions[] = "";
|
||||
$suggestions[] = "NOTE: Renaming to a DIFFERENT concept is allowed:";
|
||||
$suggestions[] = "";
|
||||
$suggestions[] = " // OK - 'value'/'label' are UI concepts, not shortenings";
|
||||
$suggestions[] = " 'value' => \$model->id,";
|
||||
$suggestions[] = " 'label' => \$model->name,";
|
||||
$suggestions[] = "";
|
||||
$suggestions[] = " // OK - adding context, not dropping it";
|
||||
$suggestions[] = " 'client_id' => \$client->id,";
|
||||
|
||||
return implode("\n", $suggestions);
|
||||
}
|
||||
}
|
||||
@@ -889,90 +889,6 @@ class Debugger
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Forced console debug - Always outputs in development
|
||||
*
|
||||
* Same as console_debug() but bypasses all filters and configuration.
|
||||
* Always outputs to BOTH CLI and HTTP contexts simultaneously.
|
||||
* Only works in non-production mode - silently ignored in production.
|
||||
*
|
||||
* Use for critical development notices that developers must see,
|
||||
* such as "manifest does not exist, full rebuild required".
|
||||
*
|
||||
* @param string $channel Channel category
|
||||
* @param mixed ...$values Values to output
|
||||
* @return void
|
||||
*/
|
||||
public static function console_debug_force(string $channel, ...$values): void
|
||||
{
|
||||
// Never output in production
|
||||
if (app()->environment('production')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Normalize channel name
|
||||
$channel = static::__normalize_channel($channel);
|
||||
|
||||
// Get caller information
|
||||
$trace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, 3);
|
||||
|
||||
// Detect if called via helper function
|
||||
$caller_index = 0;
|
||||
if (isset($trace[0]['function']) && $trace[0]['function'] === 'console_debug_force' &&
|
||||
isset($trace[0]['file']) && str_ends_with($trace[0]['file'], '/app/RSpade/helpers.php')) {
|
||||
// Called via helper, skip one level
|
||||
$caller_index = 1;
|
||||
}
|
||||
|
||||
$caller = $trace[$caller_index] ?? null;
|
||||
|
||||
// Build location prefix
|
||||
$location_prefix = '';
|
||||
if ($caller && isset($caller['file']) && isset($caller['line'])) {
|
||||
$file = str_replace(base_path() . '/', '', $caller['file']);
|
||||
$location_prefix = "[{$file}:{$caller['line']}] ";
|
||||
}
|
||||
|
||||
// CLI output - always output
|
||||
if (app()->runningInConsole()) {
|
||||
$prefix = $location_prefix . "[{$channel}]";
|
||||
// ANSI color codes: \033[33m = yellow (more visible), \033[0m = reset
|
||||
fwrite(STDERR, "\033[33m" . $prefix . "\033[0m");
|
||||
|
||||
foreach ($values as $value) {
|
||||
if (is_string($value) || is_numeric($value) || is_bool($value) || is_null($value)) {
|
||||
$output = is_bool($value) ? ($value ? 'true' : 'false') :
|
||||
(is_null($value) ? 'null' : (string)$value);
|
||||
fwrite(STDERR, ' ' . $output);
|
||||
} else {
|
||||
$json_value = static::_prepare_value_for_json($value);
|
||||
$json = json_encode($json_value, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
|
||||
fwrite(STDERR, "\n" . $json);
|
||||
}
|
||||
}
|
||||
fwrite(STDERR, "\n");
|
||||
}
|
||||
|
||||
// HTTP output - always output
|
||||
if (!app()->runningInConsole()) {
|
||||
// Prepare values for JSON
|
||||
$prepared_values = [];
|
||||
foreach ($values as $value) {
|
||||
$prepared_values[] = static::_prepare_value_for_json($value);
|
||||
}
|
||||
|
||||
// Store message
|
||||
$channel_with_prefixes = $location_prefix . "[{$channel}]";
|
||||
static::$console_messages[] = [$channel_with_prefixes, $prepared_values];
|
||||
|
||||
// Register shutdown function if not already registered
|
||||
if (!static::$shutdown_registered && !static::$trace_mode) {
|
||||
register_shutdown_function([static::class, 'dump_console_debug_messages_to_html']);
|
||||
static::$shutdown_registered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepare a value for JSON encoding
|
||||
*
|
||||
|
||||
@@ -1100,7 +1100,7 @@ class Manifest
|
||||
// Incremental updates are normal and don't need logging
|
||||
$cache_file = self::__get_cache_file_path();
|
||||
if (!$cache_loaded) {
|
||||
console_debug_force('MANIFEST', 'Manifest cache does not exist, performing full rebuild', $cache_file);
|
||||
console_debug('MANIFEST', 'Manifest cache does not exist, performing full rebuild', $cache_file);
|
||||
}
|
||||
|
||||
// zug zug
|
||||
@@ -1118,7 +1118,7 @@ class Manifest
|
||||
'permissions' => $file_perms,
|
||||
]);
|
||||
} else {
|
||||
console_debug_force('MANIFEST', 'WARNING: Cache file does not exist after rebuild!', $cache_file);
|
||||
console_debug('MANIFEST', 'WARNING: Cache file does not exist after rebuild!', $cache_file);
|
||||
}
|
||||
} else {
|
||||
console_debug('MANIFEST', 'Manifest cache is valid, no rebuild needed');
|
||||
|
||||
@@ -83,24 +83,6 @@ function console_debug(string $channel, ...$values)
|
||||
\App\RSpade\Core\Debug\Debugger::console_debug($channel, ...$values);
|
||||
}
|
||||
|
||||
/**
|
||||
* Forced console debug output - Always visible in development
|
||||
*
|
||||
* Same as console_debug() but ALWAYS outputs to both web and CLI,
|
||||
* bypassing the console_debug enabled/disabled config and filters.
|
||||
* Only works in non-production mode - silently ignored in production.
|
||||
*
|
||||
* Use for critical development notices that developers must see,
|
||||
* such as "manifest does not exist, performing full scan".
|
||||
*
|
||||
* @param string $channel Channel category
|
||||
* @param mixed ...$values Values to output
|
||||
* @return void
|
||||
*/
|
||||
function console_debug_force(string $channel, ...$values)
|
||||
{
|
||||
\App\RSpade\Core\Debug\Debugger::console_debug_force($channel, ...$values);
|
||||
}
|
||||
|
||||
/**
|
||||
* Sanity check failure handler
|
||||
|
||||
Reference in New Issue
Block a user