Skip to content

Commit d614f11

Browse files
authored
Merge pull request #19 from Kocal/MethodsShouldBePublicOrPrivateRule-protected
2 parents 9e2d22a + 933956e commit d614f11

12 files changed

+320
-3
lines changed

README.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,8 @@ final class Alert
662662
Enforces that all methods in Twig Components are either public or private, but not protected.
663663
Since Twig Components must be final classes and inheritance is forbidden (see `ForbiddenInheritanceRule`), protected methods serve no purpose and should be avoided.
664664

665+
**Exception:** Protected methods are allowed when they implement abstract methods defined in traits (e.g., `instantiateForm()` from `ComponentWithFormTrait`).
666+
665667
```yaml
666668
rules:
667669
- Kocal\PHPStanSymfonyUX\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule
@@ -716,6 +718,33 @@ final class Alert
716718

717719
<br>
718720

721+
```php
722+
// src/Twig/Components/PostForm.php
723+
namespace App\Twig\Components;
724+
725+
use Symfony\Component\Form\FormInterface;
726+
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
727+
use Symfony\UX\LiveComponent\ComponentWithFormTrait;
728+
use Symfony\UX\LiveComponent\DefaultActionTrait;
729+
730+
#[AsLiveComponent]
731+
final class PostForm
732+
{
733+
use DefaultActionTrait;
734+
use ComponentWithFormTrait;
735+
736+
// Implementing abstract method from ComponentWithFormTrait is allowed
737+
protected function instantiateForm(): FormInterface
738+
{
739+
return $this->createForm(PostType::class);
740+
}
741+
}
742+
```
743+
744+
:+1:
745+
746+
<br>
747+
719748
### PostMountMethodSignatureRule
720749

721750
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`.

src/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule.php

Lines changed: 98 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use PhpParser\Node;
99
use PhpParser\Node\Stmt\Class_;
1010
use PHPStan\Analyser\Scope;
11+
use PHPStan\Reflection\ClassReflection;
12+
use PHPStan\Reflection\ReflectionProvider;
1113
use PHPStan\Rules\Rule;
1214
use PHPStan\Rules\RuleErrorBuilder;
1315
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
@@ -18,6 +20,11 @@
1820
*/
1921
final class MethodsShouldBePublicOrPrivateRule implements Rule
2022
{
23+
public function __construct(
24+
private ReflectionProvider $reflectionProvider,
25+
) {
26+
}
27+
2128
public function getNodeType(): string
2229
{
2330
return Class_::class;
@@ -29,22 +36,110 @@ public function processNode(Node $node, Scope $scope): array
2936
return [];
3037
}
3138

39+
if ($node->namespacedName === null) {
40+
return [];
41+
}
42+
43+
$className = $node->namespacedName->toString();
44+
if (! $this->reflectionProvider->hasClass($className)) {
45+
return [];
46+
}
47+
3248
$errors = [];
49+
$classReflection = $this->reflectionProvider->getClass($className);
50+
$abstractTraitMethods = $this->getAbstractTraitMethods($classReflection);
3351

3452
foreach ($node->getMethods() as $method) {
35-
if ($method->isProtected()) {
36-
$methodName = $method->name->toString();
53+
if (! $method->isProtected()) {
54+
continue;
55+
}
56+
57+
$methodName = $method->name->toString();
58+
59+
// Skip if the method implements an abstract method from a trait
60+
if (isset($abstractTraitMethods[$methodName])) {
61+
continue;
62+
}
63+
64+
$errors[] = RuleErrorBuilder::message(
65+
sprintf('Method "%s()" in a Twig component should not be protected.', $methodName)
66+
)
67+
->identifier('symfonyUX.twigComponent.methodsShouldBePublicOrPrivate')
68+
->line($method->getLine())
69+
->tip('Twig component methods should be either public or private, not protected.')
70+
->build();
71+
}
72+
73+
// Check for protected concrete methods from traits that are not overridden in the class
74+
$classDefinedMethods = [];
75+
foreach ($node->getMethods() as $method) {
76+
$classDefinedMethods[$method->name->toString()] = true;
77+
}
78+
79+
$reportedTraitMethods = [];
80+
foreach ($classReflection->getTraits() as $traitReflection) {
81+
// Use native reflection to get trait methods
82+
foreach ($traitReflection->getNativeReflection()->getMethods() as $traitMethod) {
83+
if (! $traitMethod->isProtected()) {
84+
continue;
85+
}
86+
87+
$methodName = $traitMethod->getName();
88+
89+
// Skip if method is abstract (already handled above)
90+
if (isset($abstractTraitMethods[$methodName])) {
91+
continue;
92+
}
93+
94+
// Skip if method is overridden in the class (already checked above)
95+
if (isset($classDefinedMethods[$methodName])) {
96+
continue;
97+
}
98+
99+
// Skip if we already reported an error for this method name
100+
if (isset($reportedTraitMethods[$methodName])) {
101+
continue;
102+
}
103+
104+
$reportedTraitMethods[$methodName] = true;
105+
106+
// Find the line number from the trait use statement
107+
$lineNumber = $node->getLine();
108+
foreach ($node->getTraitUses() as $traitUse) {
109+
$lineNumber = $traitUse->getLine();
110+
break;
111+
}
37112

38113
$errors[] = RuleErrorBuilder::message(
39114
sprintf('Method "%s()" in a Twig component should not be protected.', $methodName)
40115
)
41116
->identifier('symfonyUX.twigComponent.methodsShouldBePublicOrPrivate')
42-
->line($method->getLine())
117+
->line($lineNumber)
43118
->tip('Twig component methods should be either public or private, not protected.')
44119
->build();
45120
}
46121
}
47122

48123
return $errors;
49124
}
125+
126+
/**
127+
* Get the list of abstract method names defined in traits used by the class.
128+
*
129+
* @return array<string, true>
130+
*/
131+
private function getAbstractTraitMethods(ClassReflection $classReflection): array
132+
{
133+
$abstractTraitMethods = [];
134+
135+
foreach ($classReflection->getTraits() as $traitReflection) {
136+
foreach ($traitReflection->getNativeReflection()->getMethods() as $method) {
137+
if ($method->isAbstract()) {
138+
$abstractTraitMethods[$method->getName()] = true;
139+
}
140+
}
141+
}
142+
143+
return $abstractTraitMethods;
144+
}
50145
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
use Symfony\UX\TwigComponent\Attribute\AsTwigComponent;
8+
9+
#[AsTwigComponent]
10+
class ComponentWithDuplicateTraitMethods
11+
{
12+
use TraitA;
13+
use TraitB;
14+
15+
public string $name = '';
16+
17+
protected function duplicateAbstractMethod(): void
18+
{
19+
// Implementation of abstract method from both traits - should NOT trigger an error
20+
}
21+
22+
// duplicateConcreteMethod() comes from both traits - should trigger only ONE error
23+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
use Symfony\UX\TwigComponent\Attribute\AsTwigComponent;
8+
9+
#[AsTwigComponent]
10+
class ComponentWithProtectedConcreteMethodFromTrait
11+
{
12+
use TestTraitWithConcreteMethod;
13+
14+
public string $name = '';
15+
16+
// protectedConcreteTraitMethod() comes from the trait and is NOT abstract - SHOULD trigger an error
17+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
use Symfony\UX\TwigComponent\Attribute\AsTwigComponent;
8+
9+
#[AsTwigComponent]
10+
class ComponentWithProtectedMethodFromTrait
11+
{
12+
use TestTraitWithAbstractMethod;
13+
14+
public string $name = '';
15+
16+
protected function protectedAbstractTraitMethod(): void
17+
{
18+
// Implementation of abstract method from trait - should NOT trigger an error
19+
}
20+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
8+
9+
#[AsLiveComponent]
10+
class LiveComponentWithProtectedConcreteMethodFromTrait
11+
{
12+
use TestTraitWithConcreteMethod;
13+
14+
public string $name = '';
15+
16+
// protectedConcreteTraitMethod() comes from the trait and is NOT abstract - SHOULD trigger an error
17+
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
8+
9+
#[AsLiveComponent]
10+
class LiveComponentWithProtectedMethodFromTrait
11+
{
12+
use TestTraitWithAbstractMethod;
13+
14+
public string $name = '';
15+
16+
protected function protectedAbstractTraitMethod(): void
17+
{
18+
// Implementation of abstract method from trait - should NOT trigger an error
19+
}
20+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
trait TestTraitWithAbstractMethod
8+
{
9+
abstract protected function protectedAbstractTraitMethod(): void;
10+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
trait TestTraitWithConcreteMethod
8+
{
9+
protected function protectedConcreteTraitMethod(): void
10+
{
11+
}
12+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;
6+
7+
trait TraitA
8+
{
9+
abstract protected function duplicateAbstractMethod(): void;
10+
11+
protected function duplicateConcreteMethod(): void
12+
{
13+
}
14+
}

0 commit comments

Comments
 (0)