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

Implement BEM-style enum naming and fetch() anti-aliasing policy

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 02:17:31 +00:00
parent a289eecf0f
commit 7d379b2402
50 changed files with 1041 additions and 577 deletions
Download Patch File Download Diff File Expand all files Collapse all files

429
app/RSpade/CodeQuality/Rules/PHP/FieldAliasing_CodeQualityRule.php
View File

@@ -5,33 +5,34 @@ namespace App\RSpade\CodeQuality\Rules\PHP;
use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;
/**
* FieldAliasingRule - Detects confusing field name shortenings
* FieldAliasingRule - Enforces fetch() anti-aliasing policy
*
* This rule catches cases where a field name is SHORTENED by dropping parts,
* which creates confusion about what the field actually represents.
* fetch() exists for SECURITY (removing private data), not aliasing.
*
* 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
* VALID PATTERNS:
* 1. Model method with MATCHING name:
* 'full_name' => $model->full_name()
* 'unread_count' => $this->unread_count()
*
* 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
* 2. Conditional with matching property/method or literals:
* 'foo' => $condition ? $model->foo : null
* 'secret' => $user->is_admin ? $model->secret : '[REDACTED]'
*
* ALLOWED - Transformations:
* 'type_id_label_upper' => strtoupper($contact->type_id_label),
* INVALID PATTERNS:
* 1. Any property alias (key != property):
* 'type_label' => $model->type_id__label // BAD
*
* 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.
* 2. Method call with mismatched name:
* 'addr' => $model->formatted_address() // BAD - name must match
*
* 3. Redundant explicit assignments (unnecessary):
* 'id' => $model->id // Already in toArray()
*
* Applies to:
* - Controller methods with #[Ajax_Endpoint] attribute
* - Model fetch() methods with #[Ajax_Endpoint_Model_Fetch] attribute
*
* NOT checked (controllers are an escape hatch for custom responses):
* - Controller methods with #[Ajax_Endpoint] attribute
*/
class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
{
@@ -48,7 +49,7 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
*/
public function get_name(): string
{
return 'Field Aliasing Prohibition';
return 'Fetch Anti-Aliasing Policy';
}
/**
@@ -56,7 +57,7 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
*/
public function get_description(): string
{
return 'Prohibits renaming fields during serialization - field names must be consistent across all application layers';
return 'Enforces fetch() anti-aliasing policy - fetch() is for security, not aliasing';
}
/**
@@ -72,7 +73,7 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
*/
public function is_called_during_manifest_scan(): bool
{
return false; // Only run during rsx:check
return true; // Immediate feedback on aliasing violations
}
/**
@@ -101,21 +102,16 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
return;
}
// Determine file type and get relevant methods
// Only check models - controllers are an escape hatch for custom responses
$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
if ($extends !== 'Rsx_Model_Abstract') {
return;
}
// Check fetch() method with #[Ajax_Endpoint_Model_Fetch]
$methods_to_check = $this->get_model_fetch_methods($metadata);
if (empty($methods_to_check)) {
return;
}
@@ -138,29 +134,6 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
}
}
/**
* 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
*/
@@ -196,54 +169,82 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
$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)
$actual_line_num = $method_start_line + $offset;
// Match: 'key_name' => $something->property_name
// or: 'key_name' => $something['property_name']
// Without function wrapping
// Check for line-level exception
if ($this->line_has_exception($original_lines, $actual_line_num)) {
continue;
}
// 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];
// Pattern: 'key' => ...
// We need to analyze what's on the right side
if (!preg_match("/['\"]([a-zA-Z_][a-zA-Z0-9_]*)['\"]\\s*=>/", $line, $key_match)) {
continue;
}
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;
}
$key = $key_match[1];
// Get the value part (everything after =>)
$arrow_pos = strpos($line, '=>');
if ($arrow_pos === false) {
continue;
}
$value_part = trim(substr($line, $arrow_pos + 2));
// Check for ternary operator
if ($this->is_ternary_expression($value_part)) {
$this->check_ternary($file_path, $actual_line_num, $line, $key, $value_part);
continue;
}
// Check for method call: $var->method() or $this->method()
if (preg_match('/\$([a-zA-Z_][a-zA-Z0-9_]*)->([a-zA-Z_][a-zA-Z0-9_]*)\s*\(/', $value_part, $method_match)) {
$method_called = $method_match[2];
if ($key !== $method_called) {
$this->add_violation(
$file_path,
$actual_line_num,
"Field name shortened by dropping parts: '{$key}' is missing parts from '{$property}'",
"Method call key must match method name: '{$key}' != '{$method_called}()'",
trim($line),
$this->build_suggestion($key, $property),
$this->build_method_mismatch_suggestion($key, $method_called),
'high'
);
}
continue;
}
// 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];
// Check for property access: $var->property or $var['property']
$property = null;
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;
}
// Object property: $var->prop
if (preg_match('/\$([a-zA-Z_][a-zA-Z0-9_]*)->([a-zA-Z_][a-zA-Z0-9_]*)(?:\s*[,;\]\)]|$)/', $value_part, $prop_match)) {
$property = $prop_match[2];
}
// Array access: $var['prop']
elseif (preg_match('/\$([a-zA-Z_][a-zA-Z0-9_]*)\[[\'"]([a-zA-Z_][a-zA-Z0-9_]*)[\'"]\]/', $value_part, $arr_match)) {
$property = $arr_match[2];
}
if ($property !== null) {
if ($key === $property) {
// Redundant assignment - already in toArray()
$this->add_violation(
$file_path,
$actual_line_num,
"Field name shortened by dropping parts: '{$key}' is missing parts from '{$property}'",
"Redundant assignment: '{$key}' is already included by toArray()",
trim($line),
$this->build_suggestion($key, $property),
$this->build_redundant_suggestion($key),
'medium'
);
} else {
// Aliasing - key != property
$this->add_violation(
$file_path,
$actual_line_num,
"Field aliasing prohibited: '{$key}' != '{$property}'",
trim($line),
$this->build_alias_suggestion($key, $property),
'high'
);
}
@@ -252,85 +253,112 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
}
/**
* 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)
* Check if expression contains a ternary operator (not inside a string)
*/
private function is_problematic_alias(string $key, string $property): bool
private function is_ternary_expression(string $value): 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;
// Remove string contents to avoid false positives
$no_strings = preg_replace('/(["\'])(?:[^\\\\]|\\\\.)*?\1/', '', $value);
return str_contains($no_strings, '?') && str_contains($no_strings, ':');
}
/**
* Check if the value has a transformation applied (function call wrapping it)
* Check ternary expression for valid patterns
*/
private function has_transformation(string $line, string $matched_portion): bool
private function check_ternary(string $file_path, int $line_num, string $line, string $key, string $value_part): void
{
// Find where the matched portion starts in the line
$pos = strpos($line, $matched_portion);
if ($pos === false) {
return false;
// Extract the true and false branches
// This is simplified - a full parser would be needed for nested ternaries
$no_strings = preg_replace('/(["\'])(?:[^\\\\]|\\\\.)*?\1/', '""', $value_part);
// Find the ? and : positions
$q_pos = strpos($no_strings, '?');
$c_pos = strpos($no_strings, ':');
if ($q_pos === false || $c_pos === false || $c_pos < $q_pos) {
return; // Can't parse
}
// 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
$true_branch = trim(substr($value_part, $q_pos + 1, $c_pos - $q_pos - 1));
$false_branch = trim(substr($value_part, $c_pos + 1));
// Remove trailing punctuation from false branch
$false_branch = rtrim($false_branch, ',;)');
// Check each branch - must be either:
// 1. A literal (string, number, null, true, false)
// 2. A property/method access with matching key name
$true_valid = $this->is_valid_ternary_branch($key, $true_branch);
$false_valid = $this->is_valid_ternary_branch($key, $false_branch);
if (!$true_valid || !$false_valid) {
$this->add_violation(
$file_path,
$line_num,
"Ternary branches must use matching property/method name or literals",
trim($line),
$this->build_ternary_suggestion($key),
'high'
);
}
}
/**
* Check if a ternary branch is valid
*/
private function is_valid_ternary_branch(string $key, string $branch): bool
{
$branch = trim($branch);
// Literal values are always valid
if ($this->is_literal($branch)) {
return true;
}
// Check for method chaining or casting
if (preg_match("/=>\\s*\\([^)]+\\)\\s*\\$/", $line)) {
// Cast like (string)$var->prop
// Method call: $var->method() - method must match key
if (preg_match('/\$[a-zA-Z_][a-zA-Z0-9_]*->([a-zA-Z_][a-zA-Z0-9_]*)\s*\(/', $branch, $m)) {
return $m[1] === $key;
}
// Property access: $var->prop - prop must match key
if (preg_match('/\$[a-zA-Z_][a-zA-Z0-9_]*->([a-zA-Z_][a-zA-Z0-9_]*)(?:\s*$|[^(])/', $branch, $m)) {
return $m[1] === $key;
}
// Array access: $var['prop'] - prop must match key
if (preg_match('/\$[a-zA-Z_][a-zA-Z0-9_]*\[[\'"]([a-zA-Z_][a-zA-Z0-9_]*)[\'"]\]/', $branch, $m)) {
return $m[1] === $key;
}
// Other expressions (function calls, etc.) - can't validate easily, allow
return true;
}
/**
* Check if value is a literal
*/
private function is_literal(string $value): bool
{
$value = trim($value);
// null, true, false
if (in_array(strtolower($value), ['null', 'true', 'false'])) {
return true;
}
// Check for string concatenation
if (preg_match("/=>\\s*['\"].*['\"]\\s*\\.\\s*\\$/", $line) || preg_match("/\\$[^,]+\\.\\s*['\"]/", $line)) {
// Number
if (is_numeric($value)) {
return true;
}
// Check for ternary operator
if (strpos($line, '?') !== false && strpos($line, ':') !== false) {
// String literal
if (preg_match('/^(["\']).*\1$/', $value)) {
return true;
}
// Check for null coalescing
if (strpos($line, '??') !== false) {
// Empty array
if ($value === '[]') {
return true;
}
@@ -413,33 +441,100 @@ class FieldAliasing_CodeQualityRule extends CodeQualityRule_Abstract
}
/**
* Build suggestion for fixing the violation
* Build suggestion for method name mismatch
*/
private function build_suggestion(string $key, string $property): string
private function build_method_mismatch_suggestion(string $key, string $method): 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", [
"PROBLEM: Method call key doesn't match method name.",
"",
"fetch() anti-aliasing policy requires method keys to match method names.",
"This ensures a single source of truth and consistent naming across PHP/JS.",
"",
"FIX: Use the method name as the key:",
"",
" // WRONG",
" '{$key}' => \$model->{$method}(),",
"",
" // CORRECT",
" '{$method}' => \$model->{$method}(),",
"",
"See: php artisan rsx:man model_fetch",
]);
}
return implode("\n", $suggestions);
/**
* Build suggestion for redundant assignment
*/
private function build_redundant_suggestion(string $key): string
{
return implode("\n", [
"PROBLEM: Redundant explicit assignment.",
"",
"This field is already included automatically by toArray().",
"Explicit assignment is unnecessary and adds maintenance burden.",
"",
"FIX: Remove this line - the field is already in the output.",
"",
" // UNNECESSARY - remove this line",
" '{$key}' => \$model->{$key},",
"",
"toArray() automatically includes all model fields, enum properties,",
"and the __MODEL marker for JavaScript hydration.",
"",
"See: php artisan rsx:man model_fetch",
]);
}
/**
* Build suggestion for property aliasing
*/
private function build_alias_suggestion(string $key, string $property): string
{
return implode("\n", [
"PROBLEM: Field aliasing is prohibited.",
"",
"fetch() exists for SECURITY (removing private data), not aliasing.",
"Aliasing breaks grep searches and obscures data sources.",
"",
"OPTIONS:",
"",
"1. Use the original property name:",
" '{$property}' => \$model->{$property},",
"",
"2. If this is a computed value, create a model method:",
" // In model:",
" public function {$key}() { return ...; }",
"",
" // In fetch:",
" '{$key}' => \$model->{$key}(),",
"",
"3. If this is an enum property, use the full BEM-style name:",
" // Instead of 'type_label', use 'type_id__label'",
"",
"See: php artisan rsx:man model_fetch",
]);
}
/**
* Build suggestion for ternary violations
*/
private function build_ternary_suggestion(string $key): string
{
return implode("\n", [
"PROBLEM: Ternary branches must use matching names or literals.",
"",
"Conditional assignments in fetch() are allowed, but both branches",
"must use the same property/method name as the key, or be literals.",
"",
"VALID patterns:",
" '{$key}' => \$condition ? \$model->{$key} : null,",
" '{$key}' => \$model->can_see() ? \$model->{$key} : '[HIDDEN]',",
"",
"INVALID patterns:",
" '{$key}' => \$condition ? \$model->other_field : null,",
"",
"See: php artisan rsx:man model_fetch",
]);
}
}