diff --git a/README.md b/README.md index 981eee1..0bbfa70 100644 --- a/README.md +++ b/README.md @@ -662,6 +662,8 @@ final class Alert Enforces that all methods in Twig Components are either public or private, but not protected. Since Twig Components must be final classes and inheritance is forbidden (see `ForbiddenInheritanceRule`), protected methods serve no purpose and should be avoided. +**Exception:** Protected methods are allowed when they implement abstract methods defined in traits (e.g., `instantiateForm()` from `ComponentWithFormTrait`). + ```yaml rules: - Kocal\PHPStanSymfonyUX\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule @@ -716,6 +718,33 @@ final class Alert
+```php +// src/Twig/Components/PostForm.php +namespace App\Twig\Components; + +use Symfony\Component\Form\FormInterface; +use Symfony\UX\LiveComponent\Attribute\AsLiveComponent; +use Symfony\UX\LiveComponent\ComponentWithFormTrait; +use Symfony\UX\LiveComponent\DefaultActionTrait; + +#[AsLiveComponent] +final class PostForm +{ + use DefaultActionTrait; + use ComponentWithFormTrait; + + // Implementing abstract method from ComponentWithFormTrait is allowed + protected function instantiateForm(): FormInterface + { + return $this->createForm(PostType::class); + } +} +``` + +:+1: + +
+ ### PostMountMethodSignatureRule Enforces that methods with the `#[PostMount]` attribute have the correct signature: they must be public with an optional parameter of type `array`, and a return type of `array`, `void`, or `array|void`. diff --git a/src/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule.php b/src/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule.php index 4a808c1..9d6811f 100644 --- a/src/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule.php +++ b/src/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule.php @@ -8,6 +8,8 @@ use PhpParser\Node; use PhpParser\Node\Stmt\Class_; use PHPStan\Analyser\Scope; +use PHPStan\Reflection\ClassReflection; +use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use Symfony\UX\LiveComponent\Attribute\AsLiveComponent; @@ -18,6 +20,11 @@ */ final class MethodsShouldBePublicOrPrivateRule implements Rule { + public function __construct( + private ReflectionProvider $reflectionProvider, + ) { + } + public function getNodeType(): string { return Class_::class; @@ -29,17 +36,85 @@ public function processNode(Node $node, Scope $scope): array return []; } + if ($node->namespacedName === null) { + return []; + } + + $className = $node->namespacedName->toString(); + if (! $this->reflectionProvider->hasClass($className)) { + return []; + } + $errors = []; + $classReflection = $this->reflectionProvider->getClass($className); + $abstractTraitMethods = $this->getAbstractTraitMethods($classReflection); foreach ($node->getMethods() as $method) { - if ($method->isProtected()) { - $methodName = $method->name->toString(); + if (! $method->isProtected()) { + continue; + } + + $methodName = $method->name->toString(); + + // Skip if the method implements an abstract method from a trait + if (isset($abstractTraitMethods[$methodName])) { + continue; + } + + $errors[] = RuleErrorBuilder::message( + sprintf('Method "%s()" in a Twig component should not be protected.', $methodName) + ) + ->identifier('symfonyUX.twigComponent.methodsShouldBePublicOrPrivate') + ->line($method->getLine()) + ->tip('Twig component methods should be either public or private, not protected.') + ->build(); + } + + // Check for protected concrete methods from traits that are not overridden in the class + $classDefinedMethods = []; + foreach ($node->getMethods() as $method) { + $classDefinedMethods[$method->name->toString()] = true; + } + + $reportedTraitMethods = []; + foreach ($classReflection->getTraits() as $traitReflection) { + // Use native reflection to get trait methods + foreach ($traitReflection->getNativeReflection()->getMethods() as $traitMethod) { + if (! $traitMethod->isProtected()) { + continue; + } + + $methodName = $traitMethod->getName(); + + // Skip if method is abstract (already handled above) + if (isset($abstractTraitMethods[$methodName])) { + continue; + } + + // Skip if method is overridden in the class (already checked above) + if (isset($classDefinedMethods[$methodName])) { + continue; + } + + // Skip if we already reported an error for this method name + if (isset($reportedTraitMethods[$methodName])) { + continue; + } + + $reportedTraitMethods[$methodName] = true; + + // Find the line number from the trait use statement + $lineNumber = $node->getLine(); + foreach ($node->getTraitUses() as $traitUse) { + $lineNumber = $traitUse->getLine(); + break; + } $errors[] = RuleErrorBuilder::message( sprintf('Method "%s()" in a Twig component should not be protected.', $methodName) ) ->identifier('symfonyUX.twigComponent.methodsShouldBePublicOrPrivate') - ->line($method->getLine()) + ->line($lineNumber) ->tip('Twig component methods should be either public or private, not protected.') ->build(); } @@ -47,4 +122,24 @@ public function processNode(Node $node, Scope $scope): array return $errors; } + + /** + * Get the list of abstract method names defined in traits used by the class. + * + * @return array + */ + private function getAbstractTraitMethods(ClassReflection $classReflection): array + { + $abstractTraitMethods = []; + + foreach ($classReflection->getTraits() as $traitReflection) { + foreach ($traitReflection->getNativeReflection()->getMethods() as $method) { + if ($method->isAbstract()) { + $abstractTraitMethods[$method->getName()] = true; + } + } + } + + return $abstractTraitMethods; + } } diff --git a/tests/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule/Fixture/ComponentWithDuplicateTraitMethods.php b/tests/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule/Fixture/ComponentWithDuplicateTraitMethods.php new file mode 100644 index 0000000..b254ec2 --- /dev/null +++ b/tests/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule/Fixture/ComponentWithDuplicateTraitMethods.php @@ -0,0 +1,23 @@ +analyse( + [__DIR__ . '/Fixture/ComponentWithProtectedConcreteMethodFromTrait.php'], + [ + [ + 'Method "protectedConcreteTraitMethod()" in a Twig component should not be protected.', + 12, + 'Twig component methods should be either public or private, not protected.', + ], + ] + ); + + $this->analyse( + [__DIR__ . '/Fixture/LiveComponentWithProtectedConcreteMethodFromTrait.php'], + [ + [ + 'Method "protectedConcreteTraitMethod()" in a Twig component should not be protected.', + 12, + 'Twig component methods should be either public or private, not protected.', + ], + ] + ); + + // When two traits define the same method, should report only one error + $this->analyse( + [__DIR__ . '/Fixture/ComponentWithDuplicateTraitMethods.php'], + [ + [ + 'Method "duplicateConcreteMethod()" in a Twig component should not be protected.', + 12, + 'Twig component methods should be either public or private, not protected.', + ], + ] + ); } public function testNoViolations(): void @@ -54,6 +89,17 @@ public function testNoViolations(): void [__DIR__ . '/Fixture/LiveComponentWithPublicAndPrivateMethods.php'], [] ); + + // Protected methods implementing abstract trait methods should not trigger an error + $this->analyse( + [__DIR__ . '/Fixture/ComponentWithProtectedMethodFromTrait.php'], + [] + ); + + $this->analyse( + [__DIR__ . '/Fixture/LiveComponentWithProtectedMethodFromTrait.php'], + [] + ); } public static function getAdditionalConfigFiles(): array