Skip to content

Commit d12a13e

Browse files
committed
feat: add PreferRouteEventRule to PHPStan rules
1 parent f1d7540 commit d12a13e

File tree

6 files changed

+385
-1
lines changed

6 files changed

+385
-1
lines changed

composer.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@
2323
"Shopware\\PhpStan\\Tests\\": "tests/"
2424
},
2525
"classmap": [
26-
"tests/Rule/fixtures"
26+
"tests/Rule/fixtures",
27+
"tests/Rule/BestPractise/fixtures"
2728
]
2829
},
2930
"config": {

rules.neon

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ rules:
1515
- Shopware\PhpStan\Rule\SetForeignKeyRule
1616
- Shopware\PhpStan\Rule\NoEntityRepositoryInLoopRule
1717
- Shopware\PhpStan\Rule\NoSuperglobalsRule
18+
- Shopware\PhpStan\Rule\BestPractise\PreferRouteEventRule
1819

1920
services:
2021
-
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Shopware\PhpStan\Rule\BestPractise;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Arg;
9+
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
10+
use PhpParser\Node\Expr\MethodCall;
11+
use PhpParser\Node\Expr\PropertyFetch;
12+
use PhpParser\Node\Scalar\String_;
13+
use PhpParser\Node\Stmt\ClassLike;
14+
use PhpParser\Node\Stmt\ClassMethod;
15+
use PhpParser\Node\Stmt\If_;
16+
use PhpParser\Node\Stmt\Return_;
17+
use PhpParser\NodeFinder;
18+
use PHPStan\Analyser\Scope;
19+
use PHPStan\Node\InClassNode;
20+
use PHPStan\Reflection\ClassReflection;
21+
use PHPStan\Rules\Rule;
22+
use PHPStan\Rules\RuleErrorBuilder;
23+
use PHPStan\Type\Constant\ConstantArrayType;
24+
use PHPStan\Type\Constant\ConstantStringType;
25+
use PHPStan\Type\ObjectType;
26+
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
27+
use Symfony\Component\HttpFoundation\Request;
28+
29+
/**
30+
* @implements Rule<InClassNode>
31+
*/
32+
class PreferRouteEventRule implements Rule
33+
{
34+
public function getNodeType(): string
35+
{
36+
return InClassNode::class;
37+
}
38+
39+
public function processNode(Node $node, Scope $scope): array
40+
{
41+
$classReflection = $scope->getClassReflection();
42+
43+
if ($classReflection === null) {
44+
return [];
45+
}
46+
47+
$errors = [];
48+
49+
foreach ($node->getOriginalNode()->stmts as $stmt) {
50+
if ($stmt instanceof ClassMethod) {
51+
$events = $this->getEventsByAttributes($node->getOriginalNode(), $stmt, $classReflection, $scope);
52+
53+
foreach ($events as $event) {
54+
if (!in_array($event['eventName'], ['kernel.request', 'kernel.response', 'kernel.controller'], true)) {
55+
continue;
56+
}
57+
58+
if ($event['methodName'] !== (string) $stmt->name) {
59+
continue;
60+
}
61+
62+
$condition = $this->getRouteCondition($stmt, $scope);
63+
64+
if ($condition === null) {
65+
continue;
66+
}
67+
68+
$errors[] = RuleErrorBuilder::message(sprintf('Prefer listen on %s.%s event instead of %s. This improves the performance of the application as the event listener is only called if the route matches.', $condition, str_replace('kernel.', '', $event['eventName']), $event['eventName']))
69+
->line($stmt->getLine())
70+
->identifier('shopware.bestPractise.preferRouteEventListener')
71+
->build();
72+
}
73+
}
74+
}
75+
76+
return $errors;
77+
}
78+
79+
/**
80+
* @return array<array{eventName: string, methodName: string}>
81+
*/
82+
private function getEventsByAttributes(ClassLike $classNode, ClassMethod $node, ClassReflection $classReflection, Scope $scope): array
83+
{
84+
$events = [];
85+
86+
foreach ($classReflection->getAttributes() as $attribute) {
87+
if ($attribute->getName() !== AsEventListener::class) {
88+
continue;
89+
}
90+
91+
$arguments = $attribute->getArgumentTypes();
92+
93+
$eventName = '';
94+
$methodName = '';
95+
96+
// @phpstan-ignore-next-line phpstanApi.instanceofType
97+
if (isset($arguments['event']) && $arguments['event'] instanceof ConstantStringType) {
98+
$eventName = $arguments['event']->getValue();
99+
}
100+
101+
// @phpstan-ignore-next-line phpstanApi.instanceofType
102+
if (isset($arguments['method']) && $arguments['method'] instanceof ConstantStringType) {
103+
$methodName = $arguments['method']->getValue();
104+
} else {
105+
$methodName = 'on' . ucfirst($eventName);
106+
}
107+
108+
$events[] = [
109+
'eventName' => $eventName,
110+
'methodName' => $methodName,
111+
];
112+
}
113+
114+
foreach ($classReflection->getMethod($node->name->toString(), $scope)->getAttributes() as $attribute) {
115+
if ($attribute->getName() !== AsEventListener::class) {
116+
continue;
117+
}
118+
119+
$arguments = $attribute->getArgumentTypes();
120+
121+
// @phpstan-ignore-next-line phpstanApi.instanceofType
122+
if (isset($arguments['event']) && $arguments['event'] instanceof ConstantStringType) {
123+
$eventName = $arguments['event']->getValue();
124+
} else {
125+
$eventName = 'on' . ucfirst($node->name->toString());
126+
}
127+
128+
$events[] = [
129+
'eventName' => $eventName,
130+
'methodName' => $node->name->toString(),
131+
];
132+
}
133+
134+
if ($classReflection->hasMethod('getSubscribedEvents')) {
135+
foreach ($classNode->stmts as $stmt) {
136+
if ($stmt instanceof ClassMethod && $stmt->name->toString() === 'getSubscribedEvents') {
137+
$returns = (new NodeFinder())->findFirstInstanceOf($stmt, Return_::class);
138+
139+
if ($returns === null || $returns->expr === null) {
140+
continue;
141+
}
142+
143+
$returnType = $scope->getType($returns->expr);
144+
145+
// @phpstan-ignore-next-line phpstanApi.instanceofType
146+
if ($returnType instanceof ConstantArrayType) {
147+
foreach ($returnType->getKeyTypes() as $key => $keyType) {
148+
// @phpstan-ignore-next-line phpstanApi.instanceofType
149+
if ($keyType instanceof ConstantStringType) {
150+
$pair = $returnType->getValueTypes()[$key];
151+
152+
// @phpstan-ignore-next-line phpstanApi.instanceofType
153+
if ($pair instanceof ConstantStringType) {
154+
$events[] = [
155+
'eventName' => $keyType->getValue(),
156+
'methodName' => $pair->getValue(),
157+
];
158+
}
159+
160+
// @phpstan-ignore-next-line phpstanApi.instanceofType
161+
if ($pair instanceof ConstantArrayType) {
162+
foreach ($pair->getValueTypes() as $pairKey => $pairValue) {
163+
// @phpstan-ignore-next-line phpstanApi.instanceofType
164+
if ($pairValue instanceof ConstantStringType) {
165+
$events[] = ['eventName' => $keyType->getValue(), 'methodName' => $pairValue->getValue()];
166+
}
167+
168+
// @phpstan-ignore-next-line phpstanApi.instanceofType
169+
if ($pairValue instanceof ConstantArrayType && count($pairValue->getValueTypes()) > 0) {
170+
$firstValue = $pairValue->getValueTypes()[0];
171+
172+
// @phpstan-ignore-next-line phpstanApi.instanceofType
173+
if ($firstValue instanceof ConstantStringType) {
174+
$events[] = ['eventName' => $keyType->getValue(), 'methodName' => $firstValue->getValue()];
175+
} else {
176+
$events[] = ['eventName' => $keyType->getValue(), 'methodName' => ''];
177+
}
178+
}
179+
}
180+
}
181+
}
182+
}
183+
}
184+
}
185+
}
186+
}
187+
188+
return $events;
189+
}
190+
191+
private function getRouteCondition(Node $node, Scope $scope): ?string
192+
{
193+
$ifStatements = (new NodeFinder())->findInstanceOf($node, If_::class);
194+
195+
foreach ($ifStatements as $ifStatement) {
196+
if ($ifStatement->cond instanceof NotIdentical) {
197+
$left = $ifStatement->cond->left;
198+
$right = $ifStatement->cond->right;
199+
200+
if ($right instanceof String_ && $this->isRouteGetter($left, $scope)) {
201+
return $right->value;
202+
}
203+
}
204+
}
205+
206+
return null;
207+
}
208+
209+
private function isRouteGetter(Node $node, Scope $scope): bool
210+
{
211+
if (!($node instanceof MethodCall)) {
212+
return false;
213+
}
214+
215+
if (!$node->name instanceof Node\Identifier) {
216+
return false;
217+
}
218+
219+
if ($node->name->toString() !== 'get') {
220+
return false;
221+
}
222+
223+
if (count($node->args) !== 1) {
224+
return false;
225+
}
226+
227+
$arg = $node->args[0];
228+
229+
if (!($arg instanceof Arg)) {
230+
return false;
231+
}
232+
233+
if (!($arg->value instanceof String_)) {
234+
return false;
235+
}
236+
237+
if ($arg->value->value !== '_route') {
238+
return false;
239+
}
240+
241+
$propertyFetch = $node->var;
242+
243+
if (!$propertyFetch instanceof PropertyFetch) {
244+
return false;
245+
}
246+
247+
if (!$propertyFetch->name instanceof Node\Identifier) {
248+
return false;
249+
}
250+
251+
if ($propertyFetch->name->toString() !== 'attributes') {
252+
return false;
253+
}
254+
255+
$symfonyRequest = new ObjectType(Request::class);
256+
257+
return $scope->getType($propertyFetch->var)->isSuperTypeOf($symfonyRequest)->yes();
258+
}
259+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Shopware\PhpStan\Tests\Rule\BestPractise;
6+
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Testing\RuleTestCase;
9+
use Shopware\PhpStan\Rule\BestPractise\PreferRouteEventRule;
10+
11+
class PreferRouteEventListenerRuleTest extends RuleTestCase
12+
{
13+
protected function getRule(): Rule
14+
{
15+
return new PreferRouteEventRule();
16+
}
17+
18+
public function testPreferRouteEventListenerRule(): void
19+
{
20+
$this->analyse([__DIR__ . '/fixtures/PreferRouteEventRule/listener.php'], [
21+
[
22+
'Prefer listen on foo.request event instead of kernel.request. This improves the performance of the application as the event listener is only called if the route matches.',
23+
14,
24+
],
25+
[
26+
'Prefer listen on foo.request event instead of kernel.request. This improves the performance of the application as the event listener is only called if the route matches.',
27+
21,
28+
],
29+
]);
30+
}
31+
32+
public function testPreferRouteEventListenerRuleWithSubscriber(): void
33+
{
34+
$this->analyse([__DIR__ . '/fixtures/PreferRouteEventRule/subscriber.php'], [
35+
[
36+
'Prefer listen on foo.request event instead of kernel.request. This improves the performance of the application as the event listener is only called if the route matches.',
37+
26,
38+
],
39+
[
40+
'Prefer listen on foo.response event instead of kernel.response. This improves the performance of the application as the event listener is only called if the route matches.',
41+
33,
42+
],
43+
[
44+
'Prefer listen on foo.controller event instead of kernel.controller. This improves the performance of the application as the event listener is only called if the route matches.',
45+
40,
46+
],
47+
]);
48+
}
49+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Tests\PreferRouteEventListenerRule;
6+
7+
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
8+
use Symfony\Component\HttpKernel\Event\RequestEvent;
9+
use Symfony\Component\HttpKernel\KernelEvents;
10+
11+
#[AsEventListener(event: KernelEvents::REQUEST, method: 'onRequest')]
12+
class Listener
13+
{
14+
public function onRequest(RequestEvent $event): void
15+
{
16+
if ($event->getRequest()->attributes->get('_route') !== 'foo') {
17+
return;
18+
}
19+
}
20+
21+
#[AsEventListener(event: KernelEvents::REQUEST)]
22+
public function onFooRequest(RequestEvent $event): void
23+
{
24+
if ($event->getRequest()->attributes->get('_route') !== 'foo') {
25+
return;
26+
}
27+
}
28+
}

0 commit comments

Comments
 (0)