Skip to content

Commit 95ba7c9

Browse files
TomasVotrubaTomasVotruba
committed
[symfony] Add PreferAutowireAttributeOverConfigParamRule
1 parent d210333 commit 95ba7c9

File tree

12 files changed

+302
-4
lines changed

12 files changed

+302
-4
lines changed

‎README.md‎

Lines changed: 1 addition & 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

‎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: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
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 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 readonly 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+
->build();
99+
100+
$ruleErrors[] = $identifierRuleError;
101+
}
102+
103+
return $ruleErrors;
104+
}
105+
106+
private function hasPossibleParameterInject(MethodCall $methodCall): bool
107+
{
108+
foreach ($methodCall->getArgs() as $arg) {
109+
if ($this->isParamFuncOrString($arg->value)) {
110+
return true;
111+
}
112+
}
113+
114+
return false;
115+
}
116+
117+
private function isParamFuncOrString(Expr $expr): bool
118+
{
119+
$nodeFinder = new NodeFinder();
120+
121+
/** @var FuncCall[] $funcCalls */
122+
$funcCalls = $nodeFinder->findInstanceOf($expr, FuncCall::class);
123+
124+
foreach ($funcCalls as $funcCall) {
125+
if (NamingHelper::isName($funcCall->name, SymfonyFunctionName::PARAM)) {
126+
return true;
127+
}
128+
}
129+
130+
/** @var String_[] $strings */
131+
$strings = $nodeFinder->findInstanceOf($expr, String_::class);
132+
foreach ($strings as $string) {
133+
if (str_starts_with($string->value, '%')) {
134+
return true;
135+
}
136+
}
137+
138+
return false;
139+
}
140+
141+
private function isVendorClass(string $className): bool
142+
{
143+
if (! $this->reflectionProvider->hasClass($className)) {
144+
return false;
145+
}
146+
147+
$serviceClassReflection = $this->reflectionProvider->getClass($className);
148+
return str_contains((string) $serviceClassReflection->getFileName(), '/vendor/');
149+
}
150+
151+
private function resolveClassNameFromServiceSetMethodCall(MethodCall $setMethodCall, Scope $scope): ?string
152+
{
153+
$serviceSetMethodCallArgs = $setMethodCall->getArgs();
154+
155+
// two params? then service class is the 2nd arg
156+
$serviceArg = $serviceSetMethodCallArgs[1] ?? $serviceSetMethodCallArgs[0];
157+
$serviceClassNameType = $scope->getType($serviceArg->value);
158+
159+
if (! $serviceClassNameType instanceof ConstantStringType) {
160+
return null;
161+
}
162+
163+
$serviceClassName = $serviceClassNameType->getValue();
164+
165+
// probably only another service override
166+
if (! $this->reflectionProvider->hasClass($serviceClassName)) {
167+
return null;
168+
}
169+
170+
return $serviceClassName;
171+
}
172+
173+
private function resolveRegisteredServiceClassName(MethodCall $methodCall, Scope $scope): ?string
174+
{
175+
$currentMethodCall = $methodCall;
176+
177+
while ($currentMethodCall instanceof MethodCall) {
178+
if (NamingHelper::isName($currentMethodCall->name, 'set')) {
179+
return $this->resolveClassNameFromServiceSetMethodCall($currentMethodCall, $scope);
180+
}
181+
182+
$currentMethodCall = $currentMethodCall->var;
183+
}
184+
185+
return null;
186+
}
187+
}

‎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)