Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -716,6 +718,33 @@ final class Alert

<br>

```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:

<br>

### 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`.
Expand Down
101 changes: 98 additions & 3 deletions src/Rules/TwigComponent/MethodsShouldBePublicOrPrivateRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -18,6 +20,11 @@
*/
final class MethodsShouldBePublicOrPrivateRule implements Rule
{
public function __construct(
private ReflectionProvider $reflectionProvider,
) {
}

public function getNodeType(): string
{
return Class_::class;
Expand All @@ -29,22 +36,110 @@ 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)
Copy link

Copilot AI Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment "already handled above" is slightly misleading. Abstract methods are not "handled" in the earlier loop - they are intentionally allowed and skipped. Consider rephrasing to something like "Skip if method is abstract (abstract methods are allowed to be protected)" to better reflect the actual behavior.

Suggested change
// Skip if method is abstract (already handled above)
// Skip if method is abstract (abstract methods are allowed to be protected)

Copilot uses AI. Check for mistakes.
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();
}
}

return $errors;
}

/**
* Get the list of abstract method names defined in traits used by the class.
*
* @return array<string, true>
*/
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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

use Symfony\UX\TwigComponent\Attribute\AsTwigComponent;

#[AsTwigComponent]
class ComponentWithDuplicateTraitMethods
{
use TraitA;
use TraitB;

public string $name = '';

protected function duplicateAbstractMethod(): void
{
// Implementation of abstract method from both traits - should NOT trigger an error
}

// duplicateConcreteMethod() comes from both traits - should trigger only ONE error
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

use Symfony\UX\TwigComponent\Attribute\AsTwigComponent;

#[AsTwigComponent]
class ComponentWithProtectedConcreteMethodFromTrait
{
use TestTraitWithConcreteMethod;

public string $name = '';

// protectedConcreteTraitMethod() comes from the trait and is NOT abstract - SHOULD trigger an error
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

use Symfony\UX\TwigComponent\Attribute\AsTwigComponent;

#[AsTwigComponent]
class ComponentWithProtectedMethodFromTrait
{
use TestTraitWithAbstractMethod;

public string $name = '';

protected function protectedAbstractTraitMethod(): void
{
// Implementation of abstract method from trait - should NOT trigger an error
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;

#[AsLiveComponent]
class LiveComponentWithProtectedConcreteMethodFromTrait
{
use TestTraitWithConcreteMethod;

public string $name = '';

// protectedConcreteTraitMethod() comes from the trait and is NOT abstract - SHOULD trigger an error
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;

#[AsLiveComponent]
class LiveComponentWithProtectedMethodFromTrait
{
use TestTraitWithAbstractMethod;

public string $name = '';

protected function protectedAbstractTraitMethod(): void
{
// Implementation of abstract method from trait - should NOT trigger an error
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

trait TestTraitWithAbstractMethod
{
abstract protected function protectedAbstractTraitMethod(): void;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

trait TestTraitWithConcreteMethod
{
protected function protectedConcreteTraitMethod(): void
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

trait TraitA
{
abstract protected function duplicateAbstractMethod(): void;

protected function duplicateConcreteMethod(): void
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Kocal\PHPStanSymfonyUX\Tests\Rules\TwigComponent\MethodsShouldBePublicOrPrivateRule\Fixture;

trait TraitB
{
abstract protected function duplicateAbstractMethod(): void;

protected function duplicateConcreteMethod(): void
{
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,41 @@ public function testViolations(): void
],
]
);

// Protected concrete (non-abstract) methods from traits should trigger an error
$this->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
Expand All @@ -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
Expand Down