Add nested component selector check, fix animation rule false positives
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
root
@@ -21,6 +21,9 @@ use App\RSpade\Core\Manifest\Manifest;
|
||||
* large stylesheets (e.g., by breakpoint or feature).
|
||||
* - Other SCSS files are not validated by this rule
|
||||
*
|
||||
* Additionally checks for nested component selectors - styling another component from within
|
||||
* this component's SCSS creates hidden coupling and scatters styles across files.
|
||||
*
|
||||
* NO EXEMPTIONS: Files in these paths MUST follow the convention. If a file truly
|
||||
* cannot follow the pattern, it must be moved outside these directories (e.g., to
|
||||
* rsx/theme/base/) but this requires explicit developer approval.
|
||||
@@ -192,12 +195,19 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
$wrapper_class = $file_metadata['scss_wrapper_class'] ?? null;
|
||||
$scss_filename = pathinfo($file, PATHINFO_FILENAME);
|
||||
|
||||
$this->validate_scss_file($file, $wrapper_class, $scss_filename, $components, $blade_ids, $wrapper_classes_with_primary_scss);
|
||||
$is_valid = $this->validate_scss_file($file, $wrapper_class, $scss_filename, $components, $blade_ids, $wrapper_classes_with_primary_scss);
|
||||
|
||||
// If file passed basic validation, check for nested component selectors
|
||||
if ($is_valid && $wrapper_class !== null) {
|
||||
$this->check_nested_component_selectors($file, $wrapper_class, $components);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate SCSS file against Component classes and Blade @rsx_id values
|
||||
*
|
||||
* @return bool True if file passed validation, false if violations were found
|
||||
*/
|
||||
private function validate_scss_file(
|
||||
string $file,
|
||||
@@ -206,7 +216,7 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
array $components,
|
||||
array $blade_ids,
|
||||
array $wrapper_classes_with_primary_scss
|
||||
): void {
|
||||
): bool {
|
||||
// Check if file has a wrapper class
|
||||
if ($wrapper_class === null) {
|
||||
$this->add_violation(
|
||||
@@ -217,7 +227,7 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
$this->build_no_wrapper_suggestion($file),
|
||||
'critical'
|
||||
);
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check if wrapper class matches a Component or Blade @rsx_id
|
||||
@@ -241,7 +251,7 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
$this->build_no_match_suggestion($file, $wrapper_class),
|
||||
'critical'
|
||||
);
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check if filename matches
|
||||
@@ -249,7 +259,7 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
// Allow supplemental SCSS files if a primary file already exists for this wrapper class
|
||||
if (isset($wrapper_classes_with_primary_scss[$wrapper_class])) {
|
||||
// This is a supplemental file - filename mismatch is allowed
|
||||
return;
|
||||
return true;
|
||||
}
|
||||
|
||||
$this->add_violation(
|
||||
@@ -262,7 +272,10 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
$this->build_filename_mismatch_suggestion($file, $scss_filename, $matched_filename, $match_type),
|
||||
'critical'
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -355,4 +368,173 @@ class ScssClassScope_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
|
||||
return implode("\n", $lines);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check for nested component selectors within the wrapper class
|
||||
*
|
||||
* Detects patterns like .Some_Component { .Another_Component { ... } }
|
||||
* which indicates styling another component from within this component's SCSS.
|
||||
*/
|
||||
private function check_nested_component_selectors(string $file, string $wrapper_class, array $components): void
|
||||
{
|
||||
// Read file contents
|
||||
$full_path = base_path() . '/' . $file;
|
||||
if (!file_exists($full_path)) {
|
||||
return;
|
||||
}
|
||||
|
||||
$contents = file_get_contents($full_path);
|
||||
if ($contents === false) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Strip comments to avoid false positives
|
||||
$contents = $this->strip_scss_comments($contents);
|
||||
|
||||
// Find the wrapper class block and extract its contents
|
||||
$wrapper_content = $this->extract_wrapper_block_content($contents, $wrapper_class);
|
||||
if ($wrapper_content === null) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Find all class selectors within the wrapper block
|
||||
// Pattern: . followed by uppercase letter, then word characters (letters, numbers, underscores)
|
||||
// Must NOT contain __ or -- (BEM elements/modifiers are allowed)
|
||||
preg_match_all('/\.([A-Z][A-Za-z0-9_]*)\s*\{/', $wrapper_content, $matches, PREG_OFFSET_CAPTURE);
|
||||
|
||||
foreach ($matches[1] as $match) {
|
||||
$nested_class = $match[0];
|
||||
|
||||
// Skip if it's the wrapper class itself (shouldn't happen but be safe)
|
||||
if ($nested_class === $wrapper_class) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Skip BEM elements and modifiers (contain __ or --)
|
||||
if (strpos($nested_class, '__') !== false || strpos($nested_class, '--') !== false) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Skip if this class is not a known component (could be a utility class like .Container)
|
||||
// Only flag if it matches a known component
|
||||
if (!isset($components[$nested_class])) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Find the line number for this match
|
||||
$position = $match[1];
|
||||
$line_number = $this->find_line_number_in_wrapper($contents, $wrapper_class, $nested_class);
|
||||
|
||||
$this->add_violation(
|
||||
$file,
|
||||
$line_number,
|
||||
"Nested selector '.{$nested_class}' styles a different component. " .
|
||||
"Each component should define its own styles in its own SCSS file.",
|
||||
".{$nested_class} { ... }",
|
||||
$this->build_nested_component_suggestion($wrapper_class, $nested_class),
|
||||
'high'
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Strip SCSS comments from content
|
||||
*/
|
||||
private function strip_scss_comments(string $contents): string
|
||||
{
|
||||
// Remove single-line comments
|
||||
$contents = preg_replace('/\/\/.*$/m', '', $contents);
|
||||
|
||||
// Remove multi-line comments
|
||||
$contents = preg_replace('/\/\*.*?\*\//s', '', $contents);
|
||||
|
||||
return $contents;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the content inside a wrapper class block
|
||||
*
|
||||
* Given ".Wrapper { content }", returns "content"
|
||||
*/
|
||||
private function extract_wrapper_block_content(string $contents, string $wrapper_class): ?string
|
||||
{
|
||||
// Find the wrapper class opening
|
||||
$pattern = '/\.' . preg_quote($wrapper_class, '/') . '\s*\{/';
|
||||
if (!preg_match($pattern, $contents, $matches, PREG_OFFSET_CAPTURE)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$start_pos = $matches[0][1] + strlen($matches[0][0]);
|
||||
|
||||
// Find the matching closing brace
|
||||
$brace_count = 1;
|
||||
$length = strlen($contents);
|
||||
$pos = $start_pos;
|
||||
|
||||
while ($pos < $length && $brace_count > 0) {
|
||||
$char = $contents[$pos];
|
||||
if ($char === '{') {
|
||||
$brace_count++;
|
||||
} elseif ($char === '}') {
|
||||
$brace_count--;
|
||||
}
|
||||
$pos++;
|
||||
}
|
||||
|
||||
if ($brace_count !== 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Return content between braces (excluding the final closing brace)
|
||||
return substr($contents, $start_pos, $pos - $start_pos - 1);
|
||||
}
|
||||
|
||||
/**
|
||||
* Find line number of a nested class within the wrapper block
|
||||
*/
|
||||
private function find_line_number_in_wrapper(string $contents, string $wrapper_class, string $nested_class): int
|
||||
{
|
||||
// Find the nested class after the wrapper class
|
||||
$pattern = '/\.' . preg_quote($wrapper_class, '/') . '\s*\{.*?\.' . preg_quote($nested_class, '/') . '\s*\{/s';
|
||||
if (preg_match($pattern, $contents, $matches, PREG_OFFSET_CAPTURE)) {
|
||||
$position = $matches[0][1];
|
||||
// Count newlines up to this position
|
||||
$before = substr($contents, 0, $position + strlen($matches[0][0]));
|
||||
return substr_count($before, "\n") + 1;
|
||||
}
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build suggestion for nested component selector
|
||||
*/
|
||||
private function build_nested_component_suggestion(string $wrapper_class, string $nested_class): string
|
||||
{
|
||||
$wrapper_lower = strtolower(str_replace('_', '_', $wrapper_class));
|
||||
$nested_lower = strtolower(str_replace('_', '-', $nested_class));
|
||||
|
||||
$lines = [];
|
||||
$lines[] = "Styling '.{$nested_class}' from within '.{$wrapper_class}' creates hidden coupling";
|
||||
$lines[] = "and scatters {$nested_class}'s styles across multiple files.";
|
||||
$lines[] = "";
|
||||
$lines[] = "Each component should own its own styles. If {$wrapper_class} needs to";
|
||||
$lines[] = "customize {$nested_class}'s appearance in this context, add a contextual";
|
||||
$lines[] = "class to the component in your template:";
|
||||
$lines[] = "";
|
||||
$lines[] = " <{$nested_class} class=\"{$wrapper_class}__{$nested_lower}\" />";
|
||||
$lines[] = "";
|
||||
$lines[] = "Then style that class instead:";
|
||||
$lines[] = "";
|
||||
$lines[] = " .{$wrapper_class} {";
|
||||
$lines[] = " &__{$nested_lower} {";
|
||||
$lines[] = " // custom styles for {$nested_class} in this context";
|
||||
$lines[] = " }";
|
||||
$lines[] = " }";
|
||||
$lines[] = "";
|
||||
$lines[] = "This keeps all contextual styling within {$wrapper_class}'s scope while";
|
||||
$lines[] = "allowing {$nested_class} to maintain its own base styles.";
|
||||
|
||||
return implode("\n", $lines);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,17 +120,29 @@ class NoAnimations_CodeQualityRule extends CodeQualityRule_Abstract
|
||||
$is_allowed_element = false;
|
||||
}
|
||||
|
||||
// If hover effects on non-clickable elements, flag it
|
||||
// If hover effects on non-clickable elements contain position/size changes, flag it
|
||||
// Note: Color/background/opacity changes are allowed on any element (visual feedback)
|
||||
// Only position-altering properties are prohibited
|
||||
if (!$is_allowed_element && !empty($properties)) {
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$line_num,
|
||||
"Hover effects on non-clickable elements are PROHIBITED",
|
||||
$selector,
|
||||
"Professional business applications must remain static like a PDF. Only buttons, links, form fields, images, and table rows may have hover effects. Remove hover effects from static elements.",
|
||||
'critical'
|
||||
);
|
||||
continue; // Skip further checks for this context
|
||||
$has_position_properties = false;
|
||||
foreach (array_keys($properties) as $property) {
|
||||
if (ScssContextParser::is_position_property($property) || str_starts_with($property, 'transform')) {
|
||||
$has_position_properties = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if ($has_position_properties) {
|
||||
$this->add_violation(
|
||||
$file_path,
|
||||
$line_num,
|
||||
"Position/size changes on hover for non-clickable elements are PROHIBITED",
|
||||
$selector,
|
||||
"Professional business applications must remain static. Remove position/size changes from hover states. Color, background, opacity, and other visual changes are allowed.",
|
||||
'critical'
|
||||
);
|
||||
continue; // Skip further checks for this context
|
||||
}
|
||||
}
|
||||
// Get base properties for comparison
|
||||
$base_selector = ScssContextParser::get_base_selector($selector);
|
||||
|
||||
Reference in New Issue
Block a user