29c657f7a7
Add 100+ automated unit tests from .expect file specifications Add session system test Add rsx:constants:regenerate command test Add rsx:logrotate command test Add rsx:clean command test Add rsx:manifest:stats command test Add model enum system test Add model mass assignment prevention test Add rsx:check command test Add migrate:status command test 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
446 lines
16 KiB
PHP
446 lines
16 KiB
PHP
<?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);
|
|
}
|
|
}
|