-
-
Notifications
You must be signed in to change notification settings - Fork 0
Add ClassMustBeFinalRule and remove ForbiddenInheritanceRule
#8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR replaces the ForbiddenInheritanceRule with a new ClassMustBeFinalRule for Twig Components. The new rule enforces that all Twig Component and Live Component classes must be declared as final, which is a stricter and more direct approach than the previous rule that only forbade class inheritance. This change promotes better code maintainability by preventing inheritance and encouraging composition via traits.
Key Changes:
- Replaced
ForbiddenInheritanceRule(which checked forextendsclauses) withClassMustBeFinalRule(which enforces thefinalkeyword) - Updated error messages and identifiers to reflect the new rule's focus on requiring final classes
- Provided separate error messages for abstract classes vs. non-final classes for better clarity
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Rules/TwigComponent/ClassMustBeFinalRule.php |
New rule implementation that enforces final classes for Twig/Live components, checking for both abstract and non-final classes |
tests/Rules/TwigComponent/ClassMustBeFinalRule/ClassMustBeFinalRuleTest.php |
Comprehensive test suite covering violations (non-final and abstract) and valid cases for both TwigComponent and LiveComponent |
tests/Rules/TwigComponent/ClassMustBeFinalRule/Fixture/*.php |
Test fixtures including invalid cases (non-final, abstract) and valid cases (final classes, non-components) |
tests/Rules/TwigComponent/ClassMustBeFinalRule/config/configured_rule.neon |
Test configuration for the new rule |
tests/Rules/TwigComponent/ForbiddenInheritanceRule/**/* |
Removed old rule test files and fixtures |
README.md |
Updated documentation to replace ForbiddenInheritanceRule with ClassMustBeFinalRule, including usage examples |
Comments suppressed due to low confidence (2)
src/Rules/TwigComponent/ClassMustBeFinalRule.php:38
- Missing
->line()call in error builder. The test expects the error to be reported on line 9 (where the attribute is), but without specifying the line number explicitly, PHPStan will default to the class declaration line. Consider adding->line($node->getLine())or storing the attribute fromfindAnyAttribute()and using->line($attribute->getLine())to match the test expectations and be consistent with other rules likeExposePublicPropsShouldBeFalseRule.
src/Rules/TwigComponent/ClassMustBeFinalRule.php:47 - Missing
->line()call in error builder. The test expects the error to be reported on line 9 (where the attribute is), but without specifying the line number explicitly, PHPStan will default to the class declaration line. Consider adding->line($node->getLine())or storing the attribute fromfindAnyAttribute()and using->line($attribute->getLine())to match the test expectations and be consistent with other rules likeExposePublicPropsShouldBeFalseRule.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ```php | ||
| // src/Twig/Components/Alert.php | ||
| namespace App\Twig\Components; | ||
|
|
||
| use Symfony\UX\TwigComponent\Attribute\AsTwigComponent; | ||
|
|
||
| #[AsTwigComponent] | ||
| class Alert | ||
| { | ||
| public string $message; | ||
| } | ||
| ``` | ||
|
|
||
| ```php | ||
| // src/Twig/Components/Alert.php | ||
| namespace App\Twig\Components; | ||
|
|
||
| use Symfony\UX\TwigComponent\Attribute\AsTwigComponent; | ||
|
|
||
| #[AsTwigComponent] | ||
| abstract class Alert | ||
| { | ||
| public string $message; | ||
| } | ||
| ``` | ||
|
|
||
| :x: | ||
|
|
||
| <br> | ||
|
|
||
| ```php | ||
| // src/Twig/Components/Alert.php | ||
| namespace App\Twig\Components; | ||
|
|
||
| use Symfony\UX\TwigComponent\Attribute\AsTwigComponent; | ||
|
|
||
| #[AsTwigComponent] | ||
| final class Alert | ||
| { | ||
| public string $message; | ||
| } | ||
| ``` | ||
|
|
||
| :+1: |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The documentation structure is ambiguous with two invalid examples followed by a single :x: marker. Consider adding clearer labels or separating the examples to make it obvious that both the non-final class (lines 34-45) and the abstract class (lines 47-58) are invalid. For example:
**Invalid: Non-final class**
```php
#[AsTwigComponent]
class Alert { }❌
Invalid: Abstract class
#[AsTwigComponent]
abstract class Alert { }❌
Valid: Final class
#[AsTwigComponent]
final class Alert { }👍
No description provided.