Skip to content

Commit 0c1f9ef

Browse files
committed
Tweak MethodsShouldBePublicOrPrivateRule to accept abstract protected methods
1 parent 9e2d22a commit 0c1f9ef

9 files changed

+249
-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+
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: 90 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,102 @@ 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+
foreach ($classReflection->getTraits() as $traitReflection) {
80+
// Use native reflection to get trait methods
81+
foreach ($traitReflection->getNativeReflection()->getMethods() as $traitMethod) {
82+
if (! $traitMethod->isProtected()) {
83+
continue;
84+
}
85+
86+
$methodName = $traitMethod->getName();
87+
88+
// Skip if method is abstract (already handled above)
89+
if (isset($abstractTraitMethods[$methodName])) {
90+
continue;
91+
}
92+
93+
// Skip if method is overridden in the class (already checked above)
94+
if (isset($classDefinedMethods[$methodName])) {
95+
continue;
96+
}
97+
98+
// Find the line number from the trait use statement
99+
$lineNumber = $node->getLine();
100+
foreach ($node->getTraitUses() as $traitUse) {
101+
$lineNumber = $traitUse->getLine();
102+
break;
103+
}
37104

38105
$errors[] = RuleErrorBuilder::message(
39106
sprintf('Method "%s()" in a Twig component should not be protected.', $methodName)
40107
)
41108
->identifier('symfonyUX.twigComponent.methodsShouldBePublicOrPrivate')
42-
->line($method->getLine())
109+
->line($lineNumber)
43110
->tip('Twig component methods should be either public or private, not protected.')
44111
->build();
45112
}
46113
}
47114

48115
return $errors;
49116
}
117+
118+
/**
119+
* Get the list of abstract method names defined in traits used by the class.
120+
*
121+
* @return array<string, true>
122+
*/
123+
private function getAbstractTraitMethods(ClassReflection $classReflection): array
124+
{
125+
$abstractTraitMethods = [];
126+
127+
foreach ($classReflection->getTraits() as $traitReflection) {
128+
foreach ($traitReflection->getNativeReflection()->getMethods() as $method) {
129+
if ($method->isAbstract()) {
130+
$abstractTraitMethods[$method->getName()] = true;
131+
}
132+
}
133+
}
134+
135+
return $abstractTraitMethods;
136+
}
50137
}
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+
}

tests/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule/MethodsShouldBePublicOrPrivateRuleTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,29 @@ public function testViolations(): void
3636
],
3737
]
3838
);
39+
40+
// Protected concrete (non-abstract) methods from traits should trigger an error
41+
$this->analyse(
42+
[__DIR__ . '/Fixture/ComponentWithProtectedConcreteMethodFromTrait.php'],
43+
[
44+
[
45+
'Method "protectedConcreteTraitMethod()" in a Twig component should not be protected.',
46+
12,
47+
'Twig component methods should be either public or private, not protected.',
48+
],
49+
]
50+
);
51+
52+
$this->analyse(
53+
[__DIR__ . '/Fixture/LiveComponentWithProtectedConcreteMethodFromTrait.php'],
54+
[
55+
[
56+
'Method "protectedConcreteTraitMethod()" in a Twig component should not be protected.',
57+
12,
58+
'Twig component methods should be either public or private, not protected.',
59+
],
60+
]
61+
);
3962
}
4063

4164
public function testNoViolations(): void
@@ -54,6 +77,17 @@ public function testNoViolations(): void
5477
[__DIR__ . '/Fixture/LiveComponentWithPublicAndPrivateMethods.php'],
5578
[]
5679
);
80+
81+
// Protected methods implementing abstract trait methods should not trigger an error
82+
$this->analyse(
83+
[__DIR__ . '/Fixture/ComponentWithProtectedMethodFromTrait.php'],
84+
[]
85+
);
86+
87+
$this->analyse(
88+
[__DIR__ . '/Fixture/LiveComponentWithProtectedMethodFromTrait.php'],
89+
[]
90+
);
5791
}
5892

5993
public static function getAdditionalConfigFiles(): array

0 commit comments

Comments
 (0)