Skip to content

Commit 5bd887d

Browse files
committed
Add SingleRequiredMethodRule
1 parent 0b2f3c7 commit 5bd887d

File tree

19 files changed

+155
-73
lines changed

19 files changed

+155
-73
lines changed

README.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,17 @@ class SomeClass extends SomeParentClass
794794

795795
<br>
796796

797+
### SingleRequiredMethodRule
798+
799+
There must be maximum 1 @required method in the class. Merge it to one to avoid possible injection collision or duplicated injects.
800+
801+
```yaml
802+
rules:
803+
- Symplify\PHPStanRules\Rules\Symfony\SingleRequiredMethodRule
804+
```
805+
806+
<br>
807+
797808
### RequireAttributeNameRule
798809

799810
Attribute must have all names explicitly defined

src/Enum/ClassName.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,9 @@ final class ClassName
8080
* @var string
8181
*/
8282
public const MOCK_OBJECT_CLASS = 'PHPUnit\Framework\MockObject\MockObject';
83+
84+
/**
85+
* @var string
86+
*/
87+
public const REQUIRED_ATTRIBUTE = 'Symfony\Contracts\Service\Attribute\Required';
8388
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Symplify\PHPStanRules\NodeAnalyzer;
6+
7+
use PhpParser\Comment\Doc;
8+
use PhpParser\Node\Stmt\ClassMethod;
9+
use Symplify\PHPStanRules\Enum\ClassName;
10+
11+
final class SymfonyRequiredMethodAnalyzer
12+
{
13+
public static function detect(ClassMethod $classMethod): bool
14+
{
15+
// speed up
16+
if (! $classMethod->isPublic()) {
17+
return false;
18+
}
19+
20+
if ($classMethod->isMagic()) {
21+
return false;
22+
}
23+
24+
foreach ($classMethod->getAttrGroups() as $attributeGroup) {
25+
foreach ($attributeGroup->attrs as $attr) {
26+
if ($attr->name->toString() === ClassName::REQUIRED_ATTRIBUTE) {
27+
return true;
28+
}
29+
}
30+
}
31+
32+
$docComment = $classMethod->getDocComment();
33+
if (! $docComment instanceof Doc) {
34+
return false;
35+
}
36+
37+
return str_contains($docComment->getText(), '@required');
38+
}
39+
}

src/Rules/Symfony/NoRequiredOutsideClassRule.php

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,13 @@
44

55
namespace Symplify\PHPStanRules\Rules\Symfony;
66

7-
use PhpParser\Comment\Doc;
87
use PhpParser\Node;
9-
use PhpParser\Node\Stmt\ClassMethod;
108
use PhpParser\Node\Stmt\Trait_;
119
use PHPStan\Analyser\Scope;
1210
use PHPStan\Rules\Rule;
1311
use PHPStan\Rules\RuleErrorBuilder;
1412
use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier;
13+
use Symplify\PHPStanRules\NodeAnalyzer\SymfonyRequiredMethodAnalyzer;
1514

1615
/**
1716
* @see \Symplify\PHPStanRules\Tests\Rules\Symfony\NoRequiredOutsideClassRule\NoRequiredOutsideClassRuleTest
@@ -25,11 +24,6 @@ final class NoRequiredOutsideClassRule implements Rule
2524
*/
2625
public const ERROR_MESSAGE = 'Symfony #[Require]/@required should be used only in classes to avoid misuse';
2726

28-
/**
29-
* @var string
30-
*/
31-
private const REQUIRED_ATTRIBUTE = 'Symfony\Contracts\Service\Attribute\Required';
32-
3327
public function getNodeType(): string
3428
{
3529
return Trait_::class;
@@ -43,29 +37,17 @@ public function processNode(Node $node, Scope $scope): array
4337
$ruleErrors = [];
4438

4539
foreach ($node->getMethods() as $classMethod) {
46-
if ($this->isAutowiredClassMethod($classMethod)) {
47-
$ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE)
48-
->file($scope->getFile())
49-
->identifier(SymfonyRuleIdentifier::SYMFONY_NO_REQUIRED_OUTSIDE_CLASS)
50-
->line($classMethod->getLine())
51-
->build();
40+
if (! SymfonyRequiredMethodAnalyzer::detect($classMethod)) {
41+
continue;
5242
}
53-
}
5443

55-
return $ruleErrors;
56-
}
57-
58-
private function isAutowiredClassMethod(ClassMethod $classMethod): bool
59-
{
60-
foreach ($classMethod->getAttrGroups() as $attributeGroup) {
61-
foreach ($attributeGroup->attrs as $attr) {
62-
if ($attr->name->toString() === self::REQUIRED_ATTRIBUTE) {
63-
return true;
64-
}
65-
}
44+
$ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE)
45+
->file($scope->getFile())
46+
->identifier(SymfonyRuleIdentifier::SYMFONY_NO_REQUIRED_OUTSIDE_CLASS)
47+
->line($classMethod->getLine())
48+
->build();
6649
}
6750

68-
$docComment = $classMethod->getDocComment();
69-
return $docComment instanceof Doc && str_contains($docComment->getText(), '@required');
51+
return $ruleErrors;
7052
}
7153
}

src/Rules/Symfony/SingleRequiredMethodRule.php

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,22 @@
22

33
namespace Symplify\PHPStanRules\Rules\Symfony;
44

5-
use PhpParser\Comment\Doc;
65
use PhpParser\Node;
76
use PhpParser\Node\Stmt\Class_;
87
use PHPStan\Analyser\Scope;
98
use PHPStan\Rules\Rule;
109
use PHPStan\Rules\RuleError;
1110
use PHPStan\Rules\RuleErrorBuilder;
1211
use Symplify\PHPStanRules\Enum\SymfonyRuleIdentifier;
12+
use Symplify\PHPStanRules\NodeAnalyzer\SymfonyRequiredMethodAnalyzer;
1313

1414
/**
1515
* @implements Rule<Class_>
1616
*/
1717
final class SingleRequiredMethodRule implements Rule
1818
{
19+
public const ERROR_MESSAGE = 'Found %d @required methods. Use only one method to avoid unexpected behavior.';
20+
1921
public function getNodeType(): string
2022
{
2123
return Class_::class;
@@ -30,31 +32,18 @@ public function processNode(Node $node, Scope $scope): array
3032
$requiredClassMethodCount = 0;
3133

3234
foreach ($node->getMethods() as $classMethod) {
33-
if (! $classMethod->getDocComment() instanceof Doc) {
34-
continue;
35-
}
36-
37-
$doc = $classMethod->getDocComment();
38-
39-
if (! str_contains($doc->getText(), '@required')) {
35+
if (! SymfonyRequiredMethodAnalyzer::detect($classMethod)) {
4036
continue;
4137
}
4238

4339
++$requiredClassMethodCount;
4440
}
4541

46-
if ($requiredClassMethodCount === 0) {
47-
return [];
48-
}
49-
5042
if ($requiredClassMethodCount < 2) {
5143
return [];
5244
}
5345

54-
$errorMessage = sprintf(
55-
'Found %d @required methods. Use only one method to avoid unexpected behavior.',
56-
$requiredClassMethodCount
57-
);
46+
$errorMessage = sprintf(self::ERROR_MESSAGE, $requiredClassMethodCount);
5847

5948
$ruleError = RuleErrorBuilder::message($errorMessage)
6049
->identifier(SymfonyRuleIdentifier::SINGLE_REQUIRED_METHOD)

tests/Rules/CheckRequiredInterfaceInContractNamespaceRule/CheckRequiredInterfaceInContractNamespaceRuleTest.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212

1313
final class CheckRequiredInterfaceInContractNamespaceRuleTest extends RuleTestCase
1414
{
15-
/**
16-
* @param mixed[] $expectedErrorsWithLines
17-
*/
1815
#[DataProvider('provideData')]
1916
public function testRule(string $filePath, array $expectedErrorsWithLines): void
2017
{

tests/Rules/Explicit/ExplicitClassPrefixSuffixRule/ExplicitClassPrefixSuffixRuleTest.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212

1313
final class ExplicitClassPrefixSuffixRuleTest extends RuleTestCase
1414
{
15-
/**
16-
* @param mixed[] $expectedErrorsWithLines
17-
*/
1815
#[DataProvider('provideData')]
1916
public function testRule(string $filePath, array $expectedErrorsWithLines): void
2017
{

tests/Rules/NoReturnSetterMethodRule/NoReturnSetterMethodRuleTest.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212

1313
final class NoReturnSetterMethodRuleTest extends RuleTestCase
1414
{
15-
/**
16-
* @param mixed[] $expectedErrorsWithLines
17-
*/
1815
#[DataProvider('provideData')]
1916
public function testRule(string $filePath, array $expectedErrorsWithLines): void
2017
{

tests/Rules/Rector/NoClassReflectionStaticReflectionRule/NoClassReflectionStaticReflectionRuleTest.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212

1313
final class NoClassReflectionStaticReflectionRuleTest extends RuleTestCase
1414
{
15-
/**
16-
* @param mixed[] $expectedErrorsWithLines
17-
*/
1815
#[DataProvider('provideData')]
1916
public function testRule(string $filePath, array $expectedErrorsWithLines): void
2017
{

tests/Rules/Rector/NoInstanceOfStaticReflectionRule/NoInstanceOfStaticReflectionRuleTest.php

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212

1313
final class NoInstanceOfStaticReflectionRuleTest extends RuleTestCase
1414
{
15-
/**
16-
* @param mixed[] $expectedErrorsWithLines
17-
*/
1815
#[DataProvider('provideData')]
1916
public function testRule(string $filePath, array $expectedErrorsWithLines): void
2017
{

0 commit comments

Comments
 (0)