Skip to content

Commit 08246ce

Browse files
TomasVotrubaTomasVotruba
authored
bump symplify phpstan rules to spot int return in refactor() (#7707)
* bump symplify phpstan rules to spot int return in refactor() * skip remove node, only allowed * [cleanup] handle stop traverse in ClassConstFetchNodeVisitor
1 parent 3072141 commit 08246ce

File tree

10 files changed

+96
-41
lines changed

10 files changed

+96
-41
lines changed

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
"rector/type-perfect": "^2.1",
5757
"shipmonk/composer-dependency-analyser": "^1.8",
5858
"symplify/phpstan-extensions": "^12.0.2",
59-
"symplify/phpstan-rules": "dev-main",
59+
"symplify/phpstan-rules": "^14.9.3",
6060
"symplify/vendor-patches": "^11.5",
6161
"tomasvotruba/class-leak": "^2.1",
6262
"tracy/tracy": "^2.11"

‎phpstan.neon‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,3 +387,14 @@ parameters:
387387
-
388388
path: rules/TypeDeclaration/Rector/StmtsAwareInterface/IncreaseDeclareStrictTypesRector.php
389389
identifier: rector.noOnlyNullReturnInRefactor
390+
391+
-
392+
identifier: rector.noIntegerRefactorReturn
393+
paths:
394+
- rules/Transform/Rector/ArrayDimFetch/ArrayDimFetchToMethodCallRector.php
395+
396+
# valid, as use REMOVE_NODE
397+
- rules/DeadCode/Rector/Expression/RemoveDeadStmtRector.php
398+
- rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php
399+
- rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php
400+
- rules/DeadCode/Rector/If_/UnwrapFutureCompatibleIfPhpVersionRector.php

‎rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public function getNodeTypes(): array
8080

8181
/**
8282
* @param If_ $node
83-
* @return null|int|Stmt[]
83+
* @return null|NodeVisitor::REMOVE_NODE|Stmt[]
8484
*/
8585
public function refactor(Node $node): null|array|int
8686
{
@@ -112,7 +112,7 @@ public function refactor(Node $node): null|array|int
112112
}
113113

114114
/**
115-
* @return null|Stmt[]|int
115+
* @return null|Stmt[]|NodeVisitor::REMOVE_NODE
116116
*/
117117
private function refactorSmaller(ConstFetch $constFetch, Smaller $smaller, If_ $if): null|array|int
118118
{
@@ -128,7 +128,7 @@ private function refactorSmaller(ConstFetch $constFetch, Smaller $smaller, If_ $
128128
}
129129

130130
/**
131-
* @return null|int|Stmt[]
131+
* @return null|NodeVisitor::REMOVE_NODE|Stmt[]
132132
*/
133133
private function processGreaterOrEqual(
134134
ConstFetch $constFetch,
@@ -146,6 +146,9 @@ private function processGreaterOrEqual(
146146
return null;
147147
}
148148

149+
/**
150+
* @return null|NodeVisitor::REMOVE_NODE
151+
*/
149152
private function refactorSmallerLeft(Smaller $smaller): ?int
150153
{
151154
$value = $smaller->right;
@@ -161,7 +164,7 @@ private function refactorSmallerLeft(Smaller $smaller): ?int
161164
}
162165

163166
/**
164-
* @return null|Stmt[]|int
167+
* @return null|Stmt[]|NodeVisitor::REMOVE_NODE
165168
*/
166169
private function refactorSmallerRight(Smaller $smaller, If_ $if): null|array|int
167170
{
@@ -182,7 +185,7 @@ private function refactorSmallerRight(Smaller $smaller, If_ $if): null|array|int
182185
}
183186

184187
/**
185-
* @return null|Stmt[]|int
188+
* @return null|Stmt[]|NodeVisitor::REMOVE_NODE
186189
*/
187190
private function refactorGreaterOrEqualLeft(GreaterOrEqual $greaterOrEqual, If_ $if): null|array|int
188191
{
@@ -202,6 +205,9 @@ private function refactorGreaterOrEqualLeft(GreaterOrEqual $greaterOrEqual, If_
202205
return $if->stmts;
203206
}
204207

208+
/**
209+
* @return NodeVisitor::REMOVE_NODE|null
210+
*/
205211
private function refactorGreaterOrEqualRight(GreaterOrEqual $greaterOrEqual): ?int
206212
{
207213
$value = $greaterOrEqual->left;
@@ -217,7 +223,7 @@ private function refactorGreaterOrEqualRight(GreaterOrEqual $greaterOrEqual): ?i
217223
}
218224

219225
/**
220-
* @return null|Stmt[]|int
226+
* @return null|Stmt[]|NodeVisitor::REMOVE_NODE
221227
*/
222228
private function refactorGreater(ConstFetch $constFetch, Greater $greater, If_ $if): null|array|int
223229
{
@@ -233,7 +239,7 @@ private function refactorGreater(ConstFetch $constFetch, Greater $greater, If_ $
233239
}
234240

235241
/**
236-
* @return null|Stmt[]|int
242+
* @return null|Stmt[]|NodeVisitor::REMOVE_NODE
237243
*/
238244
private function refactorGreaterLeft(Greater $greater, If_ $if): null|array|int
239245
{
@@ -253,6 +259,9 @@ private function refactorGreaterLeft(Greater $greater, If_ $if): null|array|int
253259
return $if->stmts;
254260
}
255261

262+
/**
263+
* @return NodeVisitor::REMOVE_NODE|null
264+
*/
256265
private function refactorGreaterRight(Greater $greater): ?int
257266
{
258267
$value = $greater->left;
@@ -268,7 +277,7 @@ private function refactorGreaterRight(Greater $greater): ?int
268277
}
269278

270279
/**
271-
* @return null|Stmt[]|int
280+
* @return null|Stmt[]|NodeVisitor::REMOVE_NODE
272281
*/
273282
private function refactorConstFetch(ConstFetch $constFetch, If_ $if, BinaryOp $binaryOp): null|array|int
274283
{

‎rules/DeadCode/Rector/Expression/RemoveDeadStmtRector.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function getNodeTypes(): array
5959

6060
/**
6161
* @param Expression $node
62-
* @return Node[]|Node|null|int
62+
* @return Node[]|Node|null|NodeVisitor::REMOVE_NODE
6363
*/
6464
public function refactor(Node $node): array|Node|null|int
6565
{
@@ -106,6 +106,9 @@ private function hasGetMagic(Expression $expression): bool
106106
return ! $phpPropertyReflection instanceof PhpPropertyReflection;
107107
}
108108

109+
/**
110+
* @return NodeVisitor::REMOVE_NODE|Node
111+
*/
109112
private function removeNodeAndKeepComments(Expression $expression): int|Node
110113
{
111114
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($expression);

‎rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public function getNodeTypes(): array
7474

7575
/**
7676
* @param If_ $node
77-
* @return Stmt[]|null|int|If_
77+
* @return Stmt[]|null|NodeVisitor::REMOVE_NODE|If_
7878
*/
7979
public function refactor(Node $node): array|null|int|If_
8080
{
@@ -98,7 +98,7 @@ public function refactor(Node $node): array|null|int|If_
9898
}
9999

100100
/**
101-
* @return null|Stmt[]|int
101+
* @return null|Stmt[]|NodeVisitor::REMOVE_NODE
102102
*/
103103
private function refactorStmtAndInstanceof(If_ $if, Instanceof_ $instanceof): null|array|int
104104
{

‎rules/DeadCode/Rector/If_/UnwrapFutureCompatibleIfPhpVersionRector.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public function getNodeTypes(): array
6161

6262
/**
6363
* @param If_ $node
64-
* @return Stmt[]|null|int
64+
* @return Stmt[]|null|NodeVisitor::REMOVE_NODE
6565
*/
6666
public function refactor(Node $node): array|int|null
6767
{
@@ -104,7 +104,7 @@ private function refactorIsMatch(If_ $if): ?array
104104
}
105105

106106
/**
107-
* @return Stmt[]|int
107+
* @return Stmt[]|NodeVisitor::REMOVE_NODE
108108
*/
109109
private function refactorIsNotMatch(If_ $if): array|int
110110
{

‎rules/Renaming/Rector/Name/RenameClassRector.php‎

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,13 @@
55
namespace Rector\Renaming\Rector\Name;
66

77
use PhpParser\Node;
8-
use PhpParser\Node\Expr\ClassConstFetch;
98
use PhpParser\Node\FunctionLike;
10-
use PhpParser\Node\Identifier;
119
use PhpParser\Node\Name;
1210
use PhpParser\Node\Name\FullyQualified;
1311
use PhpParser\Node\Stmt\ClassLike;
1412
use PhpParser\Node\Stmt\Expression;
1513
use PhpParser\Node\Stmt\If_;
1614
use PhpParser\Node\Stmt\Property;
17-
use PhpParser\NodeVisitor;
1815
use PHPStan\Reflection\ReflectionProvider;
1916
use Rector\Configuration\RenamedClassesDataCollector;
2017
use Rector\Contract\Rector\ConfigurableRectorInterface;
@@ -80,7 +77,6 @@ function someFunction(SomeNewClass $someOldClass): SomeNewClass
8077
public function getNodeTypes(): array
8178
{
8279
return [
83-
ClassConstFetch::class,
8480
// place FullyQualified before Name on purpose executed early before the Name as parent
8581
FullyQualified::class,
8682
// Name as parent of FullyQualified executed later for fallback annotation to attribute rename to Name
@@ -94,19 +90,21 @@ public function getNodeTypes(): array
9490
}
9591

9692
/**
97-
* @param ClassConstFetch|FunctionLike|FullyQualified|Name|ClassLike|Expression|Property|If_ $node
98-
* @return null|NodeVisitor::DONT_TRAVERSE_CHILDREN|Node
93+
* @param FunctionLike|FullyQualified|Name|ClassLike|Expression|Property|If_ $node
9994
*/
100-
public function refactor(Node $node): int|null|Node
95+
public function refactor(Node $node): ?Node
10196
{
10297
$oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
10398

10499
if ($oldToNewClasses === []) {
105100
return null;
106101
}
107102

108-
if ($node instanceof ClassConstFetch) {
109-
return $this->processClassConstFetch($node, $oldToNewClasses);
103+
if ($node instanceof FullyQualified && $this->shouldSkipClassConstFetchForMissingConstantName(
104+
$node,
105+
$oldToNewClasses
106+
)) {
107+
return null;
110108
}
111109

112110
$scope = $node->getAttribute(AttributeKey::SCOPE);
@@ -126,18 +124,23 @@ public function configure(array $configuration): void
126124

127125
/**
128126
* @param array<string, string> $oldToNewClasses
129-
* @return null|NodeVisitor::DONT_TRAVERSE_CHILDREN
130127
*/
131-
private function processClassConstFetch(ClassConstFetch $classConstFetch, array $oldToNewClasses): int|null
132-
{
133-
if (! $classConstFetch->class instanceof FullyQualified
134-
|| ! $classConstFetch->name instanceof Identifier
135-
|| ! $this->reflectionProvider->hasClass($classConstFetch->class->toString())) {
136-
return null;
128+
private function shouldSkipClassConstFetchForMissingConstantName(
129+
FullyQualified $fullyQualified,
130+
array $oldToNewClasses
131+
): bool {
132+
if (! $this->reflectionProvider->hasClass($fullyQualified->toString())) {
133+
return false;
134+
}
135+
136+
// not part of class const fetch (e.g. SomeClass::SOME_VALUE)
137+
$constFetchName = $fullyQualified->getAttribute(AttributeKey::CLASS_CONST_FETCH_NAME);
138+
if (! is_string($constFetchName)) {
139+
return false;
137140
}
138141

139142
foreach ($oldToNewClasses as $oldClass => $newClass) {
140-
if (! $this->isName($classConstFetch->class, $oldClass)) {
143+
if (! $this->isName($fullyQualified, $oldClass)) {
141144
continue;
142145
}
143146

@@ -146,20 +149,14 @@ private function processClassConstFetch(ClassConstFetch $classConstFetch, array
146149
}
147150

148151
$classReflection = $this->reflectionProvider->getClass($newClass);
149-
if (! $classReflection->isInterface()) {
150-
continue;
151-
}
152-
153152
$oldClassReflection = $this->reflectionProvider->getClass($oldClass);
154153

155-
if ($oldClassReflection->hasConstant($classConstFetch->name->toString())
156-
&& ! $classReflection->hasConstant($classConstFetch->name->toString())) {
157-
// no constant found on new interface? skip node below ClassConstFetch on this rule
158-
return NodeVisitor::DONT_TRAVERSE_CHILDREN;
154+
if ($oldClassReflection->hasConstant($constFetchName) && ! $classReflection->hasConstant($constFetchName)) {
155+
// should be skipped as new class does not have access to the constant
156+
return true;
159157
}
160158
}
161159

162-
// continue to next Name usage
163-
return null;
160+
return false;
164161
}
165162
}

‎src/DependencyInjection/LazyContainerFactory.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@
9999
use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\AssignedToNodeVisitor;
100100
use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ByRefReturnNodeVisitor;
101101
use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ByRefVariableNodeVisitor;
102+
use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ClassConstFetchNodeVisitor;
102103
use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ContextNodeVisitor;
103104
use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\GlobalVariableNodeVisitor;
104105
use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\NameNodeVisitor;
@@ -247,6 +248,7 @@ final class LazyContainerFactory
247248
NameNodeVisitor::class,
248249
StaticVariableNodeVisitor::class,
249250
PropertyOrClassConstDefaultNodeVisitor::class,
251+
ClassConstFetchNodeVisitor::class,
250252
];
251253

252254
/**

‎src/NodeTypeResolver/Node/AttributeKey.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,4 +294,6 @@ final class AttributeKey
294294
public const IS_INSIDE_SYMFONY_PHP_CLOSURE = 'is_inside_symfony_php_closure';
295295

296296
public const IS_INSIDE_BYREF_FUNCTION_LIKE = 'is_inside_byref_function_like';
297+
298+
public const CLASS_CONST_FETCH_NAME = 'class_const_fetch_name';
297299
}

‎src/NodeTypeResolver/PHPStan/Scope/NodeVisitor/ClassConstFetchNodeVisitor.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\ClassConstFetch;
9+
use PhpParser\Node\Identifier;
10+
use PhpParser\NodeVisitorAbstract;
11+
use Rector\Contract\PhpParser\DecoratingNodeVisitorInterface;
12+
use Rector\NodeTypeResolver\Node\AttributeKey;
13+
14+
final class ClassConstFetchNodeVisitor extends NodeVisitorAbstract implements DecoratingNodeVisitorInterface
15+
{
16+
public function enterNode(Node $node): ?Node
17+
{
18+
if (! $node instanceof ClassConstFetch) {
19+
return null;
20+
}
21+
22+
// pass value metadata to class node
23+
if (! $node->name instanceof Identifier) {
24+
return null;
25+
}
26+
27+
$node->class->setAttribute(AttributeKey::CLASS_CONST_FETCH_NAME, $node->name->toString());
28+
29+
return null;
30+
}
31+
}

0 commit comments

Comments
 (0)