Fix code quality violations and add VS Code extension features
Fix VS Code extension storage paths for new directory structure Fix jqhtml compiled files missing from bundle Fix bundle babel transformation and add rsxrealpath() function 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -186,6 +186,11 @@ class HardcodedInternalUrl_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
return false;
|
||||
}
|
||||
|
||||
// Allow exactly "/" (root/home URL) - common and acceptable
|
||||
if ($url === '/') {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Skip absolute URLs (with protocol)
|
||||
if (preg_match('#^//#', $url)) {
|
||||
return false;
|
||||
|
||||
@@ -98,8 +98,14 @@ class JQueryLengthCheck_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
}
|
||||
|
||||
if ($found) {
|
||||
// Check if .length is followed by comparison or assignment operators
|
||||
// These are valid uses: .length > 1, .length = x, etc.
|
||||
if (preg_match('/\.length\s*([><=!]+|[+\-*\/]=)/', $sanitized_line)) {
|
||||
continue; // Skip - this is a numeric comparison or assignment
|
||||
}
|
||||
|
||||
$original_line = $original_lines[$line_num] ?? $sanitized_line;
|
||||
|
||||
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$line_number,
|
||||
|
||||
@@ -51,13 +51,20 @@ class JQueryVariableNaming_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
];
|
||||
|
||||
/**
|
||||
* jQuery methods that return scalar values (not jQuery objects)
|
||||
* jQuery methods that return scalar values ONLY when called as getters (no arguments)
|
||||
* When called with arguments, these return jQuery object for chaining
|
||||
*/
|
||||
private const SCALAR_METHODS = [
|
||||
private const GETTER_METHODS = [
|
||||
'data', 'attr', 'val', 'text', 'html', 'prop', 'css',
|
||||
'offset', 'position', 'scrollTop', 'scrollLeft',
|
||||
'width', 'height', 'innerWidth', 'innerHeight',
|
||||
'outerWidth', 'outerHeight',
|
||||
];
|
||||
|
||||
/**
|
||||
* jQuery methods that ALWAYS return scalar values
|
||||
*/
|
||||
private const SCALAR_METHODS = [
|
||||
'index', 'size', 'length', 'get', 'toArray',
|
||||
'serialize', 'serializeArray',
|
||||
'is', 'hasClass', 'is_visible' // Custom RSpade methods
|
||||
@@ -155,7 +162,23 @@ class JQueryVariableNaming_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
|
||||
// Direct jQuery selector: $(...)
|
||||
if (preg_match('/^\$\s*\(/', $expr)) {
|
||||
// Check if followed by method chain
|
||||
// Check if it's creating an element: $('<element>')
|
||||
if (preg_match('/^\$\s*\(\s*[\'"]</', $expr)) {
|
||||
// Creating jQuery element - always returns jQuery object
|
||||
// Even with method chains like .text() or .attr(), the chaining continues
|
||||
// We only care about method chains that END with scalar methods
|
||||
if (preg_match('/^\$\s*\([^)]*\)(.*)/', $expr, $matches)) {
|
||||
$chain = trim($matches[1]);
|
||||
if ($chain === '') {
|
||||
return 'jquery'; // Just $('<element>') with no methods
|
||||
}
|
||||
// Only check if chain ENDS with a scalar method
|
||||
return $this->analyze_method_chain($chain);
|
||||
}
|
||||
return 'jquery';
|
||||
}
|
||||
|
||||
// Regular selector or other jQuery call
|
||||
if (preg_match('/^\$\s*\([^)]*\)(.*)/', $expr, $matches)) {
|
||||
$chain = trim($matches[1]);
|
||||
if ($chain === '') {
|
||||
@@ -201,25 +224,57 @@ class JQueryVariableNaming_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
|
||||
// Find the last method call in the chain
|
||||
// Match patterns like .method() or .method(args)
|
||||
// Also capture what's inside the parentheses
|
||||
$methods = [];
|
||||
preg_match_all('/\.([a-zA-Z_][a-zA-Z0-9_]*)\s*\([^)]*\)/', $chain, $methods);
|
||||
|
||||
if (empty($methods[1])) {
|
||||
preg_match_all('/\.([a-zA-Z_][a-zA-Z0-9_]*)\s*\(([^)]*)\)/', $chain, $methods, PREG_SET_ORDER);
|
||||
|
||||
if (empty($methods)) {
|
||||
// No method calls found
|
||||
return 'unknown';
|
||||
}
|
||||
|
||||
|
||||
// Check the last method to determine return type
|
||||
$last_method = end($methods[1]);
|
||||
|
||||
$last_method_data = end($methods);
|
||||
$last_method = $last_method_data[1];
|
||||
$last_args = trim($last_method_data[2] ?? '');
|
||||
|
||||
if (in_array($last_method, self::JQUERY_OBJECT_METHODS, true)) {
|
||||
return 'jquery';
|
||||
}
|
||||
|
||||
|
||||
if (in_array($last_method, self::SCALAR_METHODS, true)) {
|
||||
return 'scalar';
|
||||
}
|
||||
|
||||
|
||||
// Check getter methods - return scalar for getters, jQuery for setters
|
||||
if (in_array($last_method, self::GETTER_METHODS, true)) {
|
||||
// Count arguments by splitting on commas (simple heuristic)
|
||||
// Note: This won't handle nested function calls perfectly, but works for common cases
|
||||
$arg_count = $last_args === '' ? 0 : (substr_count($last_args, ',') + 1);
|
||||
|
||||
// Special handling for methods that take a key parameter
|
||||
// .data('key') - 1 arg = getter (returns value)
|
||||
// .data('key', value) - 2 args = setter (returns jQuery)
|
||||
// .attr('name') - 1 arg = getter (returns attribute value)
|
||||
// .attr('name', value) - 2 args = setter (returns jQuery)
|
||||
if (in_array($last_method, ['data', 'attr', 'prop', 'css'], true)) {
|
||||
if ($arg_count <= 1) {
|
||||
return 'scalar'; // Getter with key - returns scalar value
|
||||
} else {
|
||||
return 'jquery'; // Setter with key and value - returns jQuery for chaining
|
||||
}
|
||||
}
|
||||
|
||||
// For other getter methods (text, html, val, etc.)
|
||||
// .text() - no args = getter (returns text)
|
||||
// .text('value') - 1 arg = setter (returns jQuery)
|
||||
if ($last_args === '') {
|
||||
return 'scalar'; // Getter mode - returns scalar
|
||||
} else {
|
||||
return 'jquery'; // Setter mode - returns jQuery object for chaining
|
||||
}
|
||||
}
|
||||
|
||||
// Unknown method - could be custom plugin
|
||||
return 'unknown';
|
||||
}
|
||||
|
||||
@@ -7,23 +7,24 @@ use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;
|
||||
/**
|
||||
* JavaScript 'this' Usage Rule
|
||||
*
|
||||
* PHILOSOPHY: Remove ambiguity about what 'this' refers to in all contexts.
|
||||
* PHILOSOPHY: Enforce clear 'this' patterns in anonymous functions and static methods.
|
||||
*
|
||||
* RULES:
|
||||
* 1. Anonymous functions: Can use 'const $var = $(this)' as first line (jQuery pattern)
|
||||
* 2. Instance methods: Must use 'const that = this' as first line (constructors exempt)
|
||||
* 3. Static methods: Use Class_Name OR 'const CurrentClass = this' for polymorphism
|
||||
* 4. Arrow functions: Ignored (they inherit 'this' context)
|
||||
* 5. Constructors: Direct 'this' usage allowed for property assignment
|
||||
* 1. Anonymous functions: MUST use 'const $element = $(this)' or 'const that = this' as first line
|
||||
* 2. Static methods: MUST NOT use naked 'this' - use Class_Name or 'const CurrentClass = this'
|
||||
* 3. Instance methods: EXEMPT - can use 'this' directly (no aliasing required)
|
||||
* 4. Arrow functions: EXEMPT - they inherit 'this' context
|
||||
* 5. Constructors: EXEMPT - 'this' allowed directly for property assignment
|
||||
*
|
||||
* PATTERNS:
|
||||
* - jQuery: const $element = $(this) // Variable must start with $
|
||||
* - Instance: const that = this // Standard instance aliasing
|
||||
* - Static (exact): Use Class_Name // When you need exact class
|
||||
* - jQuery callback: const $element = $(this) // Variable must start with $
|
||||
* - Anonymous function: const that = this // Instance context aliasing
|
||||
* - Static (exact): Use Class_Name // When you need exact class
|
||||
* - Static (polymorphic): const CurrentClass = this // When inherited classes need different behavior
|
||||
*
|
||||
* This rule does NOT try to detect all jQuery callbacks - it offers the jQuery
|
||||
* pattern as an option when 'this' violations are found in anonymous functions.
|
||||
* INSTANCE METHODS POLICY:
|
||||
* Instance methods (on_ready, on_load, etc.) can use 'this' directly.
|
||||
* This rule only enforces aliasing for anonymous functions and prohibits naked 'this' in static methods.
|
||||
*/
|
||||
class ThisUsage_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
{
|
||||
|
||||
@@ -159,7 +159,22 @@ function analyzeFile(filePath) {
|
||||
let isAnonymousFunc = false;
|
||||
let isStaticMethod = false;
|
||||
let isConstructor = false;
|
||||
let isInstanceMethod = false;
|
||||
let hasMethodDefinition = false;
|
||||
|
||||
// First pass: check if we're in a MethodDefinition
|
||||
for (let i = ancestors.length - 1; i >= 0; i--) {
|
||||
const ancestor = ancestors[i];
|
||||
if (ancestor.type === 'MethodDefinition') {
|
||||
hasMethodDefinition = true;
|
||||
isStaticMethod = ancestor.static;
|
||||
isConstructor = ancestor.kind === 'constructor';
|
||||
isInstanceMethod = !ancestor.static && ancestor.kind !== 'constructor';
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Second pass: find function and class
|
||||
for (let i = ancestors.length - 1; i >= 0; i--) {
|
||||
const ancestor = ancestors[i];
|
||||
|
||||
@@ -168,7 +183,8 @@ function analyzeFile(filePath) {
|
||||
ancestor.type === 'FunctionDeclaration'
|
||||
)) {
|
||||
containingFunc = ancestor;
|
||||
isAnonymousFunc = ancestor.type === 'FunctionExpression';
|
||||
// Only mark as anonymous if NOT inside a MethodDefinition
|
||||
isAnonymousFunc = ancestor.type === 'FunctionExpression' && !hasMethodDefinition;
|
||||
}
|
||||
|
||||
if (!containingClass && (
|
||||
@@ -177,11 +193,6 @@ function analyzeFile(filePath) {
|
||||
)) {
|
||||
containingClass = ancestor;
|
||||
}
|
||||
|
||||
if (ancestor.type === 'MethodDefinition') {
|
||||
isStaticMethod = ancestor.static;
|
||||
isConstructor = ancestor.kind === 'constructor';
|
||||
}
|
||||
}
|
||||
|
||||
if (!containingFunc) {
|
||||
@@ -193,6 +204,12 @@ function analyzeFile(filePath) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip instance methods - 'this' is allowed directly in instance methods
|
||||
// Only enforce aliasing for anonymous functions and static methods
|
||||
if (isInstanceMethod) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check if this is part of the allowed first-line pattern with const
|
||||
const parent = ancestors[ancestors.length - 2];
|
||||
const firstStmt = containingFunc.body?.body?.[0];
|
||||
@@ -323,23 +340,9 @@ function analyzeFile(filePath) {
|
||||
if (isAnonymousFunc && !pattern) {
|
||||
remediation += `\nException: If this is a jQuery callback, add 'const $element = $(this);' as the first line.`;
|
||||
}
|
||||
} else {
|
||||
// Instance method
|
||||
if (!pattern) {
|
||||
message = `Instance method in '${className}' must alias 'this' for clarity.`;
|
||||
remediation = `Add 'const that = this;' as the first line of this method, then use 'that' instead of 'this'.\n` +
|
||||
`This applies even to ORM models and similar classes where direct property access is common.\n` +
|
||||
`Note: Constructors are exempt - 'this' is allowed directly in constructors for property assignment.\n` +
|
||||
`Example: Instead of 'return this.name;' use 'const that = this; return that.name;'`;
|
||||
} else if (pattern === 'that-pattern') {
|
||||
message = `'this' used after aliasing to 'that'. Use 'that' instead.`;
|
||||
remediation = `You already have 'const that = this'. Use 'that' consistently throughout the method.\n` +
|
||||
`All property access should use 'that.property' not 'this.property'.`;
|
||||
} else if (pattern === 'that-pattern-wrong-kind') {
|
||||
message = `Instance alias must use 'const', not 'let' or 'var'.`;
|
||||
remediation = `Change to 'const that = this;' - the instance reference should never be reassigned.`;
|
||||
}
|
||||
}
|
||||
// NOTE: Instance methods are exempt from this rule - they can use 'this' directly
|
||||
// The check returns early for instance methods, so this else block is unreachable for them
|
||||
|
||||
if (message) {
|
||||
violations.push({
|
||||
|
||||
@@ -99,7 +99,7 @@ class ModelEnumColumns_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$line_number,
|
||||
"Enum field '{$column}' does not exist as a column in table '{$table_name}'",
|
||||
"Enum field '{$column}' does not exist as a column in table '{$table_name}' (Have migrations been run yet?)",
|
||||
$lines[$line_number - 1] ?? '',
|
||||
"Remove the enum definition for '{$column}' or add the column to the database table",
|
||||
'high'
|
||||
|
||||
@@ -85,15 +85,15 @@ class ModelEnums_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
"Model {$class_name} is missing public static \$enums property",
|
||||
$lines[$class_line - 1] ?? '',
|
||||
"Add: public static \$enums = [];\n\n" .
|
||||
"For models with enum fields (fields ending in _id that reference lookup tables), use:\n" .
|
||||
"For models with enum fields (fields that reference lookup tables), use:\n" .
|
||||
"public static \$enums = [\n" .
|
||||
" 'role_id' => [\n" .
|
||||
" 1 => ['constant' => 'ROLE_OWNER', 'label' => 'Owner'],\n" .
|
||||
" 2 => ['constant' => 'ROLE_ADMIN', 'label' => 'Admin'],\n" .
|
||||
" 'status' => [\n" .
|
||||
" 1 => ['constant' => 'STATUS_ACTIVE', 'label' => 'Active'],\n" .
|
||||
" 2 => ['constant' => 'STATUS_INACTIVE', 'label' => 'Inactive'],\n" .
|
||||
" ]\n" .
|
||||
"];\n\n" .
|
||||
"Note: Top-level keys must match column names ending with '_id'. " .
|
||||
"Second-level keys are the integer values. Third-level arrays must have 'constant' and 'label' fields.",
|
||||
"Note: Top-level keys are column names. " .
|
||||
"Second-level keys must be integers. Third-level arrays must have 'constant' and 'label' fields.",
|
||||
'medium'
|
||||
);
|
||||
|
||||
@@ -107,9 +107,9 @@ class ModelEnums_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
if (trim($enums_content) !== '[]') {
|
||||
// Parse the enums array more carefully
|
||||
// We need to identify the structure:
|
||||
// 'field_id' => [ value_id => ['constant' => ..., 'label' => ...], ... ]
|
||||
// 'field_name' => [ integer_value => ['constant' => ..., 'label' => ...], ... ]
|
||||
|
||||
// First, find the top-level keys (should end with _id or start with is_)
|
||||
// First, find the top-level keys (any field name allowed, special handling for is_ prefix)
|
||||
// We'll look for patterns like 'key' => [ or "key" => [
|
||||
$pattern = '/[\'"]([^\'"]+)[\'\"]\s*=>\s*\[/';
|
||||
if (preg_match_all($pattern, $enums_content, $field_matches, PREG_OFFSET_CAPTURE)) {
|
||||
@@ -117,32 +117,9 @@ class ModelEnums_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
$field = $field_match[0];
|
||||
$offset = $field_match[1];
|
||||
|
||||
// Check that top-level field names end with _id or start with is_
|
||||
if (!str_ends_with($field, '_id') && !str_starts_with($field, 'is_')) {
|
||||
// Find the line number for this field
|
||||
$line_number = 1;
|
||||
$chars_before = substr($enums_content, 0, $offset);
|
||||
$line_number += substr_count($chars_before, "\n");
|
||||
|
||||
// Find actual line in original file
|
||||
foreach ($lines as $i => $line) {
|
||||
if ((str_contains($line, "'{$field}'") || str_contains($line, "\"{$field}\""))
|
||||
&& str_contains($line, '=>')) {
|
||||
$line_number = $i + 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$line_number,
|
||||
"Enum field '{$field}' must end with '_id' or start with 'is_'",
|
||||
$lines[$line_number - 1] ?? '',
|
||||
"Rename enum field to either '{$field}_id' or 'is_{$field}'. " .
|
||||
"Enum field names must end with '_id' for ID fields or start with 'is_' for boolean fields.",
|
||||
'medium'
|
||||
);
|
||||
}
|
||||
// No naming convention enforcement - any field name is allowed
|
||||
// Only requirement: integer keys for values (checked below)
|
||||
// Special handling for is_ prefix (boolean fields) is kept
|
||||
|
||||
// Now check the structure under this field
|
||||
// We need to find the content of this particular field's array
|
||||
|
||||
105
app/RSpade/CodeQuality/Rules/PHP/RealpathUsage_CodeQualityRule.php
Executable file
105
app/RSpade/CodeQuality/Rules/PHP/RealpathUsage_CodeQualityRule.php
Executable file
@@ -0,0 +1,105 @@
|
||||
<?php
|
||||
|
||||
namespace App\RSpade\CodeQuality\Rules\PHP;
|
||||
|
||||
use App\RSpade\CodeQuality\Rules\CodeQualityRule_Abstract;
|
||||
|
||||
class RealpathUsage_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
{
|
||||
public function get_id(): string
|
||||
{
|
||||
return 'PHP-REALPATH-01';
|
||||
}
|
||||
|
||||
public function get_name(): string
|
||||
{
|
||||
return 'Realpath Usage Check';
|
||||
}
|
||||
|
||||
public function get_description(): string
|
||||
{
|
||||
return 'Prohibits realpath() usage in app/RSpade - enforces rsxrealpath() for symlink-aware path normalization';
|
||||
}
|
||||
|
||||
public function get_file_patterns(): array
|
||||
{
|
||||
return ['*.php'];
|
||||
}
|
||||
|
||||
public function get_default_severity(): string
|
||||
{
|
||||
return 'high';
|
||||
}
|
||||
|
||||
/**
|
||||
* Check for realpath() usage in app/RSpade directory
|
||||
*/
|
||||
public function check(string $file_path, string $contents, array $metadata = []): void
|
||||
{
|
||||
// Only check files in app/RSpade directory
|
||||
if (!str_contains($file_path, 'app/RSpade/')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip vendor and node_modules
|
||||
if (str_contains($file_path, '/vendor/') || str_contains($file_path, '/node_modules/')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Skip this rule file itself
|
||||
if (str_contains($file_path, 'RealpathUsage_CodeQualityRule.php')) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Read raw file content for exception checking (not sanitized)
|
||||
$raw_contents = file_get_contents($file_path);
|
||||
|
||||
// If file has @REALPATH-EXCEPTION marker anywhere, skip entire file
|
||||
if (str_contains($raw_contents, '@REALPATH-EXCEPTION')) {
|
||||
return;
|
||||
}
|
||||
|
||||
$lines = explode("\n", $contents);
|
||||
|
||||
for ($i = 0; $i < count($lines); $i++) {
|
||||
$line = $lines[$i];
|
||||
$line_number = $i + 1;
|
||||
|
||||
// Skip comment lines first (but not inline realpath in code)
|
||||
$trimmed = trim($line);
|
||||
if (str_starts_with($trimmed, '//') || str_starts_with($trimmed, '*') || str_starts_with($trimmed, '/*')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Look for realpath( usage (function call, not in string)
|
||||
if (preg_match('/\brealpath\s*\(/', $line)) {
|
||||
// Get code snippet (current line and surrounding context)
|
||||
$snippet_start = max(0, $i - 2);
|
||||
$snippet_end = min(count($lines) - 1, $i + 2);
|
||||
$snippet_lines = array_slice($lines, $snippet_start, $snippet_end - $snippet_start + 1);
|
||||
$snippet = implode("\n", $snippet_lines);
|
||||
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$line_number,
|
||||
"Use rsxrealpath() instead of realpath()
|
||||
|
||||
RSpade uses symlinks for the /rsx/ directory mapping:
|
||||
- /var/www/html/system/rsx -> /var/www/html/rsx (symlink)
|
||||
|
||||
realpath() resolves symlinks to their physical paths, which breaks path-based comparisons and deduplication when files are accessed via different symlink paths.
|
||||
|
||||
rsxrealpath() normalizes paths without resolving symlinks, ensuring consistent path handling throughout the framework.",
|
||||
$snippet,
|
||||
"Replace: realpath(\$path)
|
||||
With: rsxrealpath(\$path)
|
||||
|
||||
If you need symlink resolution for security (path traversal prevention), add:
|
||||
// @REALPATH-EXCEPTION - Security: path traversal prevention
|
||||
\$real_path = realpath(\$path);",
|
||||
'high'
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user