Skip to content

Commit ff69001

Browse files
TomasVotrubaTomasVotruba
committed
[Utils] Add PHPStan rule to spot Rector rules not registered in any set
Flags any Rector rule whose ::class is in no set/level config - it can never run otherwise. Scans rector-src config + src/Config, sibling rector-* configs, and rector-laravel (dev dep) so downstream-configured rules count. Deprecated rules and test doubles are skipped. Null-safe extension-config includes lookup so the laravel dev dep does not flip the ExtensionConfigResolver ignore.
1 parent 9bbce0c commit ff69001

12 files changed

Lines changed: 365 additions & 8 deletions

‎composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
"webmozart/assert": "^2.4"
4141
},
4242
"require-dev": {
43+
"driftingly/rector-laravel": "^2.6",
4344
"nette/robot-loader": "^4.1",
4445
"php-parallel-lint/php-parallel-lint": "^1.4",
4546
"phpstan/extension-installer": "^1.4",

‎phpstan.neon‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ includes:
55
rules:
66
- Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule
77
- Rector\Utils\PHPStan\Rule\NoResolveClassReflectionInForeachRule
8+
- Rector\Utils\PHPStan\Rule\RegisterRectorRule
89

910
parameters:
1011
level: 8
@@ -174,12 +175,6 @@ parameters:
174175
identifier: missingType.generics
175176
message: '#(.*?) with generic interface (.*?)Interface does not specify its types#'
176177

177-
# generated class in /vendor
178-
-
179-
identifier: nullCoalesce.offset
180-
message: '#Offset (.*?) on null on left side of \?\? does not exist#'
181-
path: src/Bootstrap/ExtensionConfigResolver.php
182-
183178
# false positive
184179
-
185180
identifier: offsetAccess.invalidOffset
@@ -228,6 +223,11 @@ parameters:
228223
- rules/Php81/Rector/FuncCall/NullToStrictStringFuncCallArgRector.php
229224
- rules/Php80/Rector/NotIdentical/MbStrContainsRector.php
230225

226+
# standalone rule, opt-in via explicit withRules() only, kept out of any set on purpose, as risky
227+
-
228+
identifier: rector.registerRector
229+
path: rules/Php81/Rector/FuncCall/NullToStrictStringFuncCallArgRector.php
230+
231231
# lack of generic array in nikic/php-parser
232232
-
233233
identifier: return.type

‎src/Bootstrap/ExtensionConfigResolver.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ public function provide(): array
2828

2929
$generatedConfigDirectory = dirname($generatedConfigReflectionClass->getFileName());
3030
foreach (GeneratedConfig::EXTENSIONS as $extensionConfig) {
31-
/** @var string[] $includedFiles */
32-
$includedFiles = $extensionConfig['extra']['includes'] ?? [];
31+
$includedFiles = ($extensionConfig['extra'] ?? [])['includes'] ?? [];
3332

3433
foreach ($includedFiles as $includedFile) {
3534
$includedFilePath = $this->resolveIncludeFilePath(

‎utils/phpstan/src/Rule/RegisterRectorRule.php‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Utils\PHPStan\Rule;
6+
7+
use PhpParser\Node;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Node\InClassNode;
10+
use PHPStan\Rules\IdentifierRuleError;
11+
use PHPStan\Rules\Rule;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
use Rector\Configuration\Deprecation\Contract\DeprecatedInterface;
14+
use Rector\Contract\Rector\RectorInterface;
15+
use RecursiveDirectoryIterator;
16+
use RecursiveIteratorIterator;
17+
use SplFileInfo;
18+
19+
/**
20+
* Every Rector rule must be registered in a set or level config, otherwise it can
21+
* never run and becomes dead code. Registrations in sibling extension packages
22+
* (rector-doctrine, rector-symfony, ...) count too, and in third-party extensions
23+
* (rector-laravel, drupal-rector) when they are installed. Deprecated rules are
24+
* skipped, as they are intentionally unregistered.
25+
*
26+
* @implements Rule<InClassNode>
27+
* @see \Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\RegisterRectorRuleTest
28+
*/
29+
final class RegisterRectorRule implements Rule
30+
{
31+
private const string ERROR_MESSAGE = 'Rector rule "%s" is not registered in any set or level config, so it can never run. Register it in a set/level, or deprecate it if it is no longer wanted.';
32+
33+
/**
34+
* @var string[]
35+
*/
36+
private readonly array $registrationDirectories;
37+
38+
private ?string $registrationContents = null;
39+
40+
/**
41+
* @param string[]|null $registrationDirectories
42+
*/
43+
public function __construct(?array $registrationDirectories = null)
44+
{
45+
$root = __DIR__ . '/../../../../';
46+
47+
$this->registrationDirectories = $registrationDirectories ?? array_merge(
48+
[$root . 'config', $root . 'src/Config'],
49+
// sibling extension packages register rules in their own config sets
50+
glob($root . 'vendor/rector/*/config') ?: [],
51+
// third-party extension packages, e.g. rector-laravel and drupal-rector, when installed
52+
glob($root . 'vendor/driftingly/rector-laravel/config') ?: [],
53+
glob($root . 'vendor/palantirnet/drupal-rector/config') ?: []
54+
);
55+
}
56+
57+
public function getNodeType(): string
58+
{
59+
return InClassNode::class;
60+
}
61+
62+
/**
63+
* @param InClassNode $node
64+
* @return list<IdentifierRuleError>
65+
*/
66+
public function processNode(Node $node, Scope $scope): array
67+
{
68+
$classReflection = $node->getClassReflection();
69+
70+
if ($classReflection->isAbstract()) {
71+
return [];
72+
}
73+
74+
if (! $classReflection->implementsInterface(RectorInterface::class)) {
75+
return [];
76+
}
77+
78+
// test doubles are wired directly in their test, not through a set
79+
if (str_starts_with($classReflection->getName(), 'Rector\\Tests\\')) {
80+
return [];
81+
}
82+
83+
// intentionally unregistered
84+
if ($classReflection->implementsInterface(DeprecatedInterface::class)) {
85+
return [];
86+
}
87+
88+
$className = $classReflection->getName();
89+
if ($this->isRegistered($className)) {
90+
return [];
91+
}
92+
93+
return [
94+
RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $className))
95+
->identifier('rector.registerRector')
96+
->build(),
97+
];
98+
}
99+
100+
private function isRegistered(string $className): bool
101+
{
102+
$shortClassName = substr((string) strrchr($className, '\\'), 1);
103+
104+
// match "ShortName::class", also as a fully qualified "\Some\Ns\ShortName::class",
105+
// but not a longer class name ending with it, e.g. "SubShortName::class"
106+
$pattern = '#(?<![a-zA-Z0-9_])' . preg_quote($shortClassName, '#') . '::class#';
107+
108+
return preg_match($pattern, $this->getRegistrationContents()) === 1;
109+
}
110+
111+
private function getRegistrationContents(): string
112+
{
113+
if ($this->registrationContents !== null) {
114+
return $this->registrationContents;
115+
}
116+
117+
$contents = '';
118+
foreach ($this->registrationDirectories as $registrationDirectory) {
119+
if (! is_dir($registrationDirectory)) {
120+
continue;
121+
}
122+
123+
$recursiveIterator = new RecursiveIteratorIterator(
124+
new RecursiveDirectoryIterator($registrationDirectory, RecursiveDirectoryIterator::SKIP_DOTS)
125+
);
126+
127+
/** @var SplFileInfo $fileInfo */
128+
foreach ($recursiveIterator as $fileInfo) {
129+
if ($fileInfo->getExtension() !== 'php') {
130+
continue;
131+
}
132+
133+
$fileContents = file_get_contents($fileInfo->getRealPath());
134+
if ($fileContents === false) {
135+
continue;
136+
}
137+
138+
$contents .= $fileContents;
139+
}
140+
}
141+
142+
$this->registrationContents = $contents;
143+
144+
return $contents;
145+
}
146+
}

‎utils/phpstan/tests/Rule/RegisterRectorRule/RegisterRectorRuleTest.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule;
6+
7+
use PHPStan\Rules\Rule;
8+
use PHPStan\Testing\RuleTestCase;
9+
use Rector\Utils\PHPStan\Rule\RegisterRectorRule;
10+
11+
/**
12+
* @extends RuleTestCase<RegisterRectorRule>
13+
*/
14+
final class RegisterRectorRuleTest extends RuleTestCase
15+
{
16+
public function testUnregistered(): void
17+
{
18+
$expectedErrorMessage = 'Rector rule "Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\Source\UnregisteredRector" is not registered in any set or level config, so it can never run. Register it in a set/level, or deprecate it if it is no longer wanted.';
19+
20+
$this->analyse([__DIR__ . '/Source/UnregisteredRector.php'], [[$expectedErrorMessage, 12]]);
21+
}
22+
23+
public function testUnregisteredConfigurable(): void
24+
{
25+
$expectedErrorMessage = 'Rector rule "Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\Source\UnregisteredConfigurableRector" is not registered in any set or level config, so it can never run. Register it in a set/level, or deprecate it if it is no longer wanted.';
26+
27+
$this->analyse([__DIR__ . '/Source/UnregisteredConfigurableRector.php'], [[$expectedErrorMessage, 13]]);
28+
}
29+
30+
public function testRegistered(): void
31+
{
32+
$this->analyse([__DIR__ . '/Source/RegisteredRector.php'], []);
33+
}
34+
35+
public function testRegisteredInSiblingConfig(): void
36+
{
37+
$this->analyse([__DIR__ . '/Source/SiblingRegisteredRector.php'], []);
38+
}
39+
40+
public function testSkipDeprecated(): void
41+
{
42+
$this->analyse([__DIR__ . '/Source/SkipDeprecatedRector.php'], []);
43+
}
44+
45+
protected function getRule(): Rule
46+
{
47+
return new RegisterRectorRule([__DIR__ . '/config', __DIR__ . '/config-sibling']);
48+
}
49+
}

‎utils/phpstan/tests/Rule/RegisterRectorRule/Source/RegisteredRector.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\Source;
6+
7+
use PhpParser\Node;
8+
use Rector\Rector\AbstractRector;
9+
use Rector\RuleDoc\CodeSample\CodeSample;
10+
use Rector\RuleDoc\RuleDefinition;
11+
12+
final class RegisteredRector extends AbstractRector
13+
{
14+
public function getRuleDefinition(): RuleDefinition
15+
{
16+
return new RuleDefinition('Registered rule', [new CodeSample('before', 'after')]);
17+
}
18+
19+
public function getNodeTypes(): array
20+
{
21+
return [Node::class];
22+
}
23+
24+
public function refactor(Node $node): ?Node
25+
{
26+
return null;
27+
}
28+
}

‎utils/phpstan/tests/Rule/RegisterRectorRule/Source/SiblingRegisteredRector.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\Source;
6+
7+
use PhpParser\Node;
8+
use Rector\Rector\AbstractRector;
9+
use Rector\RuleDoc\CodeSample\CodeSample;
10+
use Rector\RuleDoc\RuleDefinition;
11+
12+
final class SiblingRegisteredRector extends AbstractRector
13+
{
14+
public function getRuleDefinition(): RuleDefinition
15+
{
16+
return new RuleDefinition('Rule registered in a sibling package config', [new CodeSample('before', 'after')]);
17+
}
18+
19+
public function getNodeTypes(): array
20+
{
21+
return [Node::class];
22+
}
23+
24+
public function refactor(Node $node): ?Node
25+
{
26+
return null;
27+
}
28+
}

‎utils/phpstan/tests/Rule/RegisterRectorRule/Source/SkipDeprecatedRector.php‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\Source;
6+
7+
use PhpParser\Node;
8+
use Rector\Configuration\Deprecation\Contract\DeprecatedInterface;
9+
use Rector\Rector\AbstractRector;
10+
use Rector\RuleDoc\CodeSample\CodeSample;
11+
use Rector\RuleDoc\RuleDefinition;
12+
13+
final class SkipDeprecatedRector extends AbstractRector implements DeprecatedInterface
14+
{
15+
public function getRuleDefinition(): RuleDefinition
16+
{
17+
return new RuleDefinition('Deprecated rule', [new CodeSample('before', 'after')]);
18+
}
19+
20+
public function getNodeTypes(): array
21+
{
22+
return [Node::class];
23+
}
24+
25+
public function refactor(Node $node): ?Node
26+
{
27+
return null;
28+
}
29+
}

‎utils/phpstan/tests/Rule/RegisterRectorRule/Source/UnregisteredConfigurableRector.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\Source;
6+
7+
use PhpParser\Node;
8+
use Rector\Contract\Rector\ConfigurableRectorInterface;
9+
use Rector\Rector\AbstractRector;
10+
use Rector\RuleDoc\CodeSample\CodeSample;
11+
use Rector\RuleDoc\RuleDefinition;
12+
13+
final class UnregisteredConfigurableRector extends AbstractRector implements ConfigurableRectorInterface
14+
{
15+
public function getRuleDefinition(): RuleDefinition
16+
{
17+
return new RuleDefinition('Configurable rule', [new CodeSample('before', 'after')]);
18+
}
19+
20+
public function getNodeTypes(): array
21+
{
22+
return [Node::class];
23+
}
24+
25+
public function refactor(Node $node): ?Node
26+
{
27+
return null;
28+
}
29+
30+
public function configure(array $configuration): void
31+
{
32+
}
33+
}

‎utils/phpstan/tests/Rule/RegisterRectorRule/Source/UnregisteredRector.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Utils\PHPStan\Tests\Rule\RegisterRectorRule\Source;
6+
7+
use PhpParser\Node;
8+
use Rector\Rector\AbstractRector;
9+
use Rector\RuleDoc\CodeSample\CodeSample;
10+
use Rector\RuleDoc\RuleDefinition;
11+
12+
final class UnregisteredRector extends AbstractRector
13+
{
14+
public function getRuleDefinition(): RuleDefinition
15+
{
16+
return new RuleDefinition('Unregistered rule', [new CodeSample('before', 'after')]);
17+
}
18+
19+
public function getNodeTypes(): array
20+
{
21+
return [Node::class];
22+
}
23+
24+
public function refactor(Node $node): ?Node
25+
{
26+
return null;
27+
}
28+
}

0 commit comments

Comments
 (0)