Skip to content

Commit 6de587c

Browse files
TomasVotrubaTomasVotruba
authored
[symfony] Add PreferAutowireAttributeOverConfigParamRule (#215)
1 parent d210333 commit 6de587c

File tree

12 files changed

+343
-4
lines changed

12 files changed

+343
-4
lines changed

‎README.md‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
[![Downloads](https://img.shields.io/packagist/dt/symplify/phpstan-rules.svg?style=flat-square)](https://packagist.org/packages/symplify/phpstan-rules/stats)
44

5-
Set of 65+ PHPStan fun and practical rules that check:
5+
Set of 70+ PHPStan fun and practical rules that check:
66

77
* clean architecture, logical errors,
88
* naming, class namespace locations
@@ -1528,6 +1528,46 @@ return static function (ContainerConfigurator $containerConfigurator): void {
15281528

15291529
<br>
15301530

1531+
### PreferAutowireAttributeOverConfigParamRule
1532+
1533+
Instead of parameter reference in config, add #[Autowire(param: ...)] in the "%s" class constructor
1534+
1535+
```yaml
1536+
rules:
1537+
- Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule
1538+
```
1539+
1540+
```php
1541+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
1542+
1543+
return static function (ContainerConfigurator $containerConfigurator): void {
1544+
$services = $containerConfigurator->services();
1545+
1546+
$services->set(SomeService::class)->args(['%some_param%']);
1547+
};
1548+
```
1549+
1550+
:x:
1551+
1552+
<br>
1553+
1554+
```php
1555+
use Symfony\Component\DependencyInjection\Attribute\Autowire;
1556+
1557+
final class SomeService
1558+
{
1559+
public function __construct(
1560+
#[Autowire(param: 'some_param')]
1561+
private string $someParam
1562+
) {
1563+
}
1564+
}
1565+
```
1566+
1567+
:+1:
1568+
1569+
<br>
1570+
15311571
### NoDuplicateArgsAutowireByTypeRule
15321572

15331573
Instead of passing "%s" to args(), remove the line and let autowiring handle it

‎config/symfony-config-rules.neon‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,8 @@ rules:
88
- Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoDuplicateArgsAutowireByTypeRule
99
- Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoDuplicateArgAutowireByTypeRule
1010

11+
# #[Autowire() in-class attribute over param() in config
12+
- Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule
13+
1114
# $services->set('X', 'X')
1215
- Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoServiceSameNameSetClassRule

‎src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,6 @@ final class SymfonyRuleIdentifier
5555
public const NO_SERVICE_SAME_NAME_SET_CLASS = 'symfony.noServiceSameNameSetClass';
5656

5757
public const REQUIRED_IS_GRANTED_ENUM = 'symfony.requiredIsGrantedEnum';
58+
59+
public const PREFER_AUTOWIRE_ATTRIBUTE_OVER_CONFIG_PARAM = 'symfony.preferAutowireAttributeOverConfigParam';
5860
}

‎src/Enum/SymfonyClass.php‎

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

1313
public const ROUTE_ATTRIBUTE = 'Symfony\Component\Routing\Attribute\Route';
1414

15-
/**
16-
* @api
17-
*/
1815
public const ROUTE_ANNOTATION = 'Symfony\Component\Routing\Annotation\Route';
1916

2017
public const CONTROLLER = 'Symfony\Bundle\FrameworkBundle\Controller\Controller';
@@ -44,4 +41,6 @@ final class SymfonyClass
4441
public const CONTAINER_CONFIGURATOR = 'Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator';
4542

4643
public const IS_GRANTED = 'Symfony\Component\Security\Http\Attribute\IsGranted';
44+
45+
public const ATTRIBUTE = 'Symfony\Component\DependencyInjection\Attribute\Autowire';
4746
}

‎src/Enum/SymfonyFunctionName.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,6 @@ final class SymfonyFunctionName
99
public const REF = 'Symfony\Component\DependencyInjection\Loader\Configurator\ref';
1010

1111
public const SERVICE = 'Symfony\Component\DependencyInjection\Loader\Configurator\service';
12+
13+
public const PARAM = 'Symfony\Component\DependencyInjection\Loader\Configurator\param';
1214
}

‎src/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule.php‎

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Symplify\PHPStanRules\Rules\Symfony\ConfigClosure;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr;
9+
use PhpParser\Node\Expr\Closure;
10+
use PhpParser\Node\Expr\FuncCall;
11+
use PhpParser\Node\Expr\MethodCall;
12+
use PhpParser\Node\Scalar\String_;
13+
use PhpParser\NodeFinder;
14+
use PHPStan\Analyser\Scope;
15+
use PHPStan\Reflection\ReflectionProvider;
16+
use PHPStan\Rules\Rule;
17+
use PHPStan\Rules\RuleErrorBuilder;
18+
use PHPStan\Type\Constant\ConstantStringType;
19+
use Symplify\PHPStanRules\Enum\RuleIdentifier\SymfonyRuleIdentifier;
20+
use Symplify\PHPStanRules\Enum\SymfonyClass;
21+
use Symplify\PHPStanRules\Enum\SymfonyFunctionName;
22+
use Symplify\PHPStanRules\Helper\NamingHelper;
23+
use Symplify\PHPStanRules\Symfony\NodeAnalyzer\SymfonyClosureDetector;
24+
25+
/**
26+
* @implements Rule<Closure>
27+
*
28+
* @see \Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule\PreferAutowireAttributeOverConfigParamRuleTest
29+
*/
30+
final readonly class PreferAutowireAttributeOverConfigParamRule implements Rule
31+
{
32+
/**
33+
* @api used in tests
34+
* @var string
35+
*/
36+
public const ERROR_MESSAGE = 'Instead of parameter reference in config, add #[Autowire(param: ...)] in the "%s" class constructor';
37+
38+
private NodeFinder $nodeFinder;
39+
40+
public function __construct(
41+
private ReflectionProvider $reflectionProvider,
42+
) {
43+
$this->nodeFinder = new NodeFinder();
44+
}
45+
46+
public function getNodeType(): string
47+
{
48+
return Closure::class;
49+
}
50+
51+
/**
52+
* @param Closure $node
53+
*/
54+
public function processNode(Node $node, Scope $scope): array
55+
{
56+
// enable this rule only if the autowire attribute is present
57+
if (! $this->reflectionProvider->hasClass(SymfonyClass::ATTRIBUTE)) {
58+
return [];
59+
}
60+
61+
if (! SymfonyClosureDetector::detect($node)) {
62+
return [];
63+
}
64+
65+
$methodCalls = $this->nodeFinder->findInstanceOf($node, MethodCall::class);
66+
67+
$ruleErrors = [];
68+
69+
foreach ($methodCalls as $methodCall) {
70+
if ($methodCall->isFirstClassCallable()) {
71+
continue;
72+
}
73+
74+
if (! NamingHelper::isNames($methodCall->name, ['arg', 'args'])) {
75+
continue;
76+
}
77+
78+
// find param() func call or string with '%' in it
79+
if (! $this->hasPossibleParameterInject($methodCall)) {
80+
continue;
81+
}
82+
83+
// find out parent class! if in /vendor, let's skip it
84+
$serviceClassName = $this->resolveRegisteredServiceClassName($methodCall, $scope);
85+
if (! is_string($serviceClassName)) {
86+
continue;
87+
}
88+
89+
// let's skip, as /vendor service that cannot be edited
90+
if ($this->isVendorClass($serviceClassName)) {
91+
continue;
92+
}
93+
94+
$errorMessage = sprintf(self::ERROR_MESSAGE, $serviceClassName);
95+
96+
$identifierRuleError = RuleErrorBuilder::message($errorMessage)
97+
->identifier(SymfonyRuleIdentifier::PREFER_AUTOWIRE_ATTRIBUTE_OVER_CONFIG_PARAM)
98+
->line($methodCall->getStartLine())
99+
->build();
100+
101+
$ruleErrors[] = $identifierRuleError;
102+
}
103+
104+
return $ruleErrors;
105+
}
106+
107+
private function hasPossibleParameterInject(MethodCall $methodCall): bool
108+
{
109+
foreach ($methodCall->getArgs() as $arg) {
110+
if ($this->isParamFuncOrString($arg->value)) {
111+
return true;
112+
}
113+
}
114+
115+
return false;
116+
}
117+
118+
private function isParamFuncOrString(Expr $expr): bool
119+
{
120+
$nodeFinder = new NodeFinder();
121+
122+
/** @var FuncCall[] $funcCalls */
123+
$funcCalls = $nodeFinder->findInstanceOf($expr, FuncCall::class);
124+
125+
foreach ($funcCalls as $funcCall) {
126+
if (NamingHelper::isName($funcCall->name, SymfonyFunctionName::PARAM)) {
127+
return true;
128+
}
129+
}
130+
131+
/** @var String_[] $strings */
132+
$strings = $nodeFinder->findInstanceOf($expr, String_::class);
133+
foreach ($strings as $string) {
134+
if (str_starts_with($string->value, '%')) {
135+
return true;
136+
}
137+
}
138+
139+
return false;
140+
}
141+
142+
private function isVendorClass(string $className): bool
143+
{
144+
if (! $this->reflectionProvider->hasClass($className)) {
145+
return false;
146+
}
147+
148+
$serviceClassReflection = $this->reflectionProvider->getClass($className);
149+
return str_contains((string) $serviceClassReflection->getFileName(), '/vendor/');
150+
}
151+
152+
private function resolveClassNameFromServiceSetMethodCall(MethodCall $setMethodCall, Scope $scope): ?string
153+
{
154+
$serviceSetMethodCallArgs = $setMethodCall->getArgs();
155+
156+
// two params? then service class is the 2nd arg
157+
$serviceArg = $serviceSetMethodCallArgs[1] ?? $serviceSetMethodCallArgs[0];
158+
$serviceClassNameType = $scope->getType($serviceArg->value);
159+
160+
if (! $serviceClassNameType instanceof ConstantStringType) {
161+
return null;
162+
}
163+
164+
$serviceClassName = $serviceClassNameType->getValue();
165+
166+
// probably only another service override
167+
if (! $this->reflectionProvider->hasClass($serviceClassName)) {
168+
return null;
169+
}
170+
171+
return $serviceClassName;
172+
}
173+
174+
private function resolveRegisteredServiceClassName(MethodCall $methodCall, Scope $scope): ?string
175+
{
176+
$currentMethodCall = $methodCall;
177+
178+
while ($currentMethodCall instanceof MethodCall) {
179+
if (NamingHelper::isName($currentMethodCall->name, 'set')) {
180+
return $this->resolveClassNameFromServiceSetMethodCall($currentMethodCall, $scope);
181+
}
182+
183+
$currentMethodCall = $currentMethodCall->var;
184+
}
185+
186+
return null;
187+
}
188+
}

‎tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/ParameterPercentReference.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
4+
5+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
6+
use Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule\Source\SomeSetServiceWithConstructor;
7+
8+
return function (ContainerConfigurator $container) {
9+
$services = $container->services();
10+
11+
$services->set(SomeSetServiceWithConstructor::class)
12+
->arg('$key', '%parameter_name%');
13+
};

‎tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SkipNoParameterReference.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
4+
5+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
6+
use Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule\Source\SomeSetServiceWithConstructor;
7+
8+
return function (ContainerConfigurator $container) {
9+
$services = $container->services();
10+
11+
$services->set(SomeSetServiceWithConstructor::class)
12+
->call('some', ['%parameter_name%']);
13+
};

‎tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SomeConfigWithInvalidSet.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
4+
5+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
6+
use Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule\Source\SomeSetServiceWithConstructor;
7+
use function Symfony\Component\DependencyInjection\Loader\Configurator\param;
8+
9+
return function (ContainerConfigurator $container) {
10+
$services = $container->services();
11+
12+
$services->set(SomeSetServiceWithConstructor::class)
13+
->arg('$key', param('parameter_name'));
14+
};

‎tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/PreferAutowireAttributeOverConfigParamRuleTest.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule;
6+
7+
use Iterator;
8+
use PHPStan\Rules\Rule;
9+
use PHPStan\Testing\RuleTestCase;
10+
use PHPUnit\Framework\Attributes\DataProvider;
11+
use Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule;
12+
use Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule\Source\SomeSetServiceWithConstructor;
13+
14+
final class PreferAutowireAttributeOverConfigParamRuleTest extends RuleTestCase
15+
{
16+
/**
17+
* @param mixed[] $expectedErrorMessagesWithLines
18+
*/
19+
#[DataProvider('provideData')]
20+
public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
21+
{
22+
$this->analyse([$filePath], $expectedErrorMessagesWithLines);
23+
}
24+
25+
public static function provideData(): Iterator
26+
{
27+
yield [__DIR__ . '/Fixture/SomeConfigWithInvalidSet.php', [
28+
[
29+
sprintf(PreferAutowireAttributeOverConfigParamRule::ERROR_MESSAGE, SomeSetServiceWithConstructor::class),
30+
12,
31+
],
32+
]];
33+
34+
yield [__DIR__ . '/Fixture/ParameterPercentReference.php', [
35+
[
36+
sprintf(PreferAutowireAttributeOverConfigParamRule::ERROR_MESSAGE, SomeSetServiceWithConstructor::class),
37+
11,
38+
],
39+
]];
40+
41+
yield [__DIR__ . '/Fixture/SkipNoParameterReference.php', []];
42+
}
43+
44+
public static function getAdditionalConfigFiles(): array
45+
{
46+
return [__DIR__ . '/config/configured_rule.neon'];
47+
}
48+
49+
protected function getRule(): Rule
50+
{
51+
return self::getContainer()->getByType(PreferAutowireAttributeOverConfigParamRule::class);
52+
}
53+
}

0 commit comments

Comments
 (0)