From a5a34fceac59b92996fecc3297e602c8ed87c33a Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 22:24:30 +0200 Subject: [PATCH 1/3] merge SkipSkipper into Skipper --- src/Skipper/Skipper/SkipSkipper.php | 42 ----------------------------- src/Skipper/Skipper/Skipper.php | 23 ++++++++++++++-- 2 files changed, 21 insertions(+), 44 deletions(-) delete mode 100644 src/Skipper/Skipper/SkipSkipper.php diff --git a/src/Skipper/Skipper/SkipSkipper.php b/src/Skipper/Skipper/SkipSkipper.php deleted file mode 100644 index 6df76ea3c8a..00000000000 --- a/src/Skipper/Skipper/SkipSkipper.php +++ /dev/null @@ -1,42 +0,0 @@ - $skippedClasses - */ - public function match(object|string $checker, string $filePath, array $skippedClasses): ?SkipMatch - { - foreach ($skippedClasses as $skippedClass => $skippedFiles) { - if (! is_a($checker, $skippedClass, true)) { - continue; - } - - // skip everywhere - if (! is_array($skippedFiles)) { - return new SkipMatch($skippedClass, null); - } - - // the same path can be skipped under multiple rules, so the matched path is reported - // scoped to its rule, not tracked on its own - $matchedPath = $this->fileInfoMatcher->matchPattern($filePath, $skippedFiles); - if ($matchedPath !== null) { - return new SkipMatch($skippedClass, $matchedPath); - } - } - - return null; - } -} diff --git a/src/Skipper/Skipper/Skipper.php b/src/Skipper/Skipper/Skipper.php index 9b144c793a3..d404cbcf483 100644 --- a/src/Skipper/Skipper/Skipper.php +++ b/src/Skipper/Skipper/Skipper.php @@ -8,6 +8,7 @@ use PHPStan\Reflection\ReflectionProvider; use Rector\Contract\Rector\RectorInterface; use Rector\ProcessAnalyzer\RectifiedAnalyzer; +use Rector\Skipper\Matcher\FileInfoMatcher; use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; use Rector\Skipper\ValueObject\SkipMatch; @@ -20,7 +21,7 @@ public function __construct( private RectifiedAnalyzer $rectifiedAnalyzer, private PathSkipper $pathSkipper, - private SkipSkipper $skipSkipper, + private FileInfoMatcher $fileInfoMatcher, private SkippedClassResolver $skippedClassResolver, private ReflectionProvider $reflectionProvider, private UsedSkipCollector $usedSkipCollector, @@ -53,7 +54,25 @@ public function matchSkip(string|object $element, string $filePath): ?SkipMatch return null; } - return $this->skipSkipper->match($element, $filePath, $this->skippedClassResolver->resolve()); + foreach ($this->skippedClassResolver->resolve() as $skippedClass => $skippedFiles) { + if (! is_a($element, $skippedClass, true)) { + continue; + } + + // skip everywhere + if (! is_array($skippedFiles)) { + return new SkipMatch($skippedClass, null); + } + + // the same path can be skipped under multiple rules, so the matched path is reported + // scoped to its rule, not tracked on its own + $matchedPath = $this->fileInfoMatcher->matchPattern($filePath, $skippedFiles); + if ($matchedPath !== null) { + return new SkipMatch($skippedClass, $matchedPath); + } + } + + return null; } public function markSkipUsed(SkipMatch $skipMatch): void From b0fa3b74afce1808c06b7194d1ab9be549e1a5a7 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 22:26:54 +0200 Subject: [PATCH 2/3] simplify --- src/Application/RectorRegistry.php | 10 ++++++++-- tests/Application/RectorRegistryTest.php | 12 ++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/Application/RectorRegistry.php b/src/Application/RectorRegistry.php index 691f8c03b8f..31dba5a7fc3 100644 --- a/src/Application/RectorRegistry.php +++ b/src/Application/RectorRegistry.php @@ -4,8 +4,10 @@ namespace Rector\Application; +use Rector\Contract\Rector\HTMLAverseRectorInterface; use Rector\Contract\Rector\RectorInterface; use Rector\Skipper\Skipper\Skipper; +use Rector\ValueObject\Application\File; /** * @see \Rector\Tests\Application\RectorRegistryTest @@ -35,11 +37,15 @@ public function refreshRectors(array $rectors): void /** * @return array */ - public function forPath(string $filePath): array + public function forFile(File $file): array { $rectorsForPath = []; foreach ($this->rectors as $rector) { - if ($this->skipper->shouldSkipRectorAndFile($rector, $filePath)) { + if ($this->skipper->shouldSkipRectorAndFile($rector, $file->getFilePath())) { + continue; + } + + if ($rector instanceof HTMLAverseRectorInterface && $file->containsHTML()) { continue; } diff --git a/tests/Application/RectorRegistryTest.php b/tests/Application/RectorRegistryTest.php index bff14754b54..4c97439ecbf 100644 --- a/tests/Application/RectorRegistryTest.php +++ b/tests/Application/RectorRegistryTest.php @@ -9,6 +9,7 @@ use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\Testing\PHPUnit\AbstractLazyTestCase; use Rector\Tests\Skipper\Skipper\Fixture\Element\FifthElement; +use Rector\ValueObject\Application\File; final class RectorRegistryTest extends AbstractLazyTestCase { @@ -31,16 +32,15 @@ protected function tearDown(): void SimpleParameterProvider::setParameter(Option::SKIP, []); } - public function testForPathSkipsRectorForSkippedPath(): void + public function testForFile(): void { - $rectorsForPath = $this->rectorRegistry->forPath(__DIR__ . '/skipped_directory/some_file.php'); + $file = new File(__DIR__ . '/skipped_directory/some_file.php', ''); + $rectorsForPath = $this->rectorRegistry->forFile($file); $this->assertSame([], $rectorsForPath); - } - public function testForPathKeepsRectorForOtherPath(): void - { - $rectorsForPath = $this->rectorRegistry->forPath(__DIR__ . '/some_file.php'); + $file = new File(__DIR__ . '/some_file.php', ''); + $rectorsForPath = $this->rectorRegistry->forFile($file); $this->assertCount(1, $rectorsForPath); $this->assertInstanceOf(FifthElement::class, $rectorsForPath[0]); From a1e3c93fb9e3c4497d3d832c109f2a6edc74db5f Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 22:32:37 +0200 Subject: [PATCH 3/3] merge SkippedClassResolver into Skipper --- phpstan.neon | 2 +- src/Config/RectorConfig.php | 6 +- src/Console/Command/ListRulesCommand.php | 6 +- src/Console/Command/ProcessCommand.php | 8 +- src/Console/Command/ValidateConfigCommand.php | 6 +- src/Reporting/UnusedSkipResolver.php | 8 +- .../SkippedClassResolver.php | 74 ------------------ src/Skipper/Skipper/Skipper.php | 78 ++++++++++++++++--- .../Command/ValidateConfigCommandTest.php | 4 +- .../Skipper/SkippedClassResolverTest.php | 18 ----- tests/Skipper/Skipper/SkipperTest.php | 5 ++ 11 files changed, 94 insertions(+), 121 deletions(-) delete mode 100644 src/Skipper/SkipCriteriaResolver/SkippedClassResolver.php delete mode 100644 tests/Skipper/Skipper/SkippedClassResolverTest.php diff --git a/phpstan.neon b/phpstan.neon index 1141b6310a5..32cfedd41df 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -339,7 +339,7 @@ parameters: identifier: symplify.forbiddenFuncCall message: '#Function "(class_exists|interface_exists)\(\)" cannot be used/left in the code\: use ReflectionProvider\->has\*\(\) instead#' paths: - - src/Skipper/SkipCriteriaResolver/SkippedClassResolver.php + - src/Skipper/Skipper/Skipper.php - identifier: symplify.noConstructorOverride diff --git a/src/Config/RectorConfig.php b/src/Config/RectorConfig.php index 3480247e6fb..a43d1b245a8 100644 --- a/src/Config/RectorConfig.php +++ b/src/Config/RectorConfig.php @@ -17,7 +17,7 @@ use Rector\Enum\Config\Defaults; use Rector\Exception\ShouldNotHappenException; use Rector\Php\PhpVersionResolver\ComposerJsonPhpVersionResolver; -use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; +use Rector\Skipper\Skipper\Skipper; use Rector\Validation\RectorConfigValidator; use Rector\ValueObject\Configuration\LevelOverflow; use Rector\ValueObject\PhpVersion; @@ -510,8 +510,8 @@ public function resetRuleConfigurations(): void */ public function boot(): void { - $skippedClassResolver = new SkippedClassResolver(); - $skippedElements = $skippedClassResolver->resolve(); + $skippedElements = $this->make(Skipper::class) + ->resolveSkippedClasses(); foreach ($skippedElements as $skippedClass => $path) { if ($path !== null) { diff --git a/src/Console/Command/ListRulesCommand.php b/src/Console/Command/ListRulesCommand.php index 965e21de21a..931f88592a2 100644 --- a/src/Console/Command/ListRulesCommand.php +++ b/src/Console/Command/ListRulesCommand.php @@ -9,7 +9,7 @@ use Rector\Configuration\Option; use Rector\Contract\Rector\RectorInterface; use Rector\PostRector\Contract\Rector\PostRectorInterface; -use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; +use Rector\Skipper\Skipper\Skipper; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -23,7 +23,7 @@ final class ListRulesCommand extends Command */ public function __construct( private readonly SymfonyStyle $symfonyStyle, - private readonly SkippedClassResolver $skippedClassResolver, + private readonly Skipper $skipper, private readonly array $rectors ) { parent::__construct(); @@ -101,7 +101,7 @@ private function getSkippedRectorClasses(): array { $skippedRectorClasses = []; - foreach ($this->skippedClassResolver->resolve() as $rectorClass => $fileList) { + foreach ($this->skipper->resolveSkippedClasses() as $rectorClass => $fileList) { // ignore specific skips if ($fileList !== null) { continue; diff --git a/src/Console/Command/ProcessCommand.php b/src/Console/Command/ProcessCommand.php index be4efbd8f58..c8202761d3a 100644 --- a/src/Console/Command/ProcessCommand.php +++ b/src/Console/Command/ProcessCommand.php @@ -18,7 +18,7 @@ use Rector\Exception\ShouldNotHappenException; use Rector\Reporting\DeprecatedRulesReporter; use Rector\Reporting\MissConfigurationReporter; -use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; +use Rector\Skipper\Skipper\Skipper; use Rector\StaticReflection\DynamicSourceLocatorDecorator; use Rector\Util\MemoryLimiter; use Rector\ValueObject\Configuration; @@ -45,7 +45,7 @@ public function __construct( private readonly DeprecatedRulesReporter $deprecatedRulesReporter, private readonly MissConfigurationReporter $missConfigurationReporter, private readonly ConfigurationRuleFilter $configurationRuleFilter, - private readonly SkippedClassResolver $skippedClassResolver, + private readonly Skipper $skipper, ) { parent::__construct(); } @@ -116,11 +116,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int } // 0. warn about skipped rules that are deprecated - if ($this->skippedClassResolver->resolveDeprecatedSkippedClasses() !== []) { + if ($this->skipper->resolveDeprecatedSkippedClasses() !== []) { $this->symfonyStyle->warning(sprintf( 'These rules are skipped, but are deprecated. Most likely you do not need to skip them anymore as not part of any set and remove them: %s%s', "\n\n", - '* ' . implode("\n* ", $this->skippedClassResolver->resolveDeprecatedSkippedClasses()) . "\n" + '* ' . implode("\n* ", $this->skipper->resolveDeprecatedSkippedClasses()) . "\n" )); } diff --git a/src/Console/Command/ValidateConfigCommand.php b/src/Console/Command/ValidateConfigCommand.php index 61bcee9af64..064cdab6ed2 100644 --- a/src/Console/Command/ValidateConfigCommand.php +++ b/src/Console/Command/ValidateConfigCommand.php @@ -11,7 +11,7 @@ use Rector\Console\ExitCode; use Rector\Reporting\DeprecatedRulesReporter; use Rector\Reporting\MissConfigurationReporter; -use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; +use Rector\Skipper\Skipper\Skipper; use Rector\ValueObject\Configuration; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -28,7 +28,7 @@ public function __construct( private readonly SymfonyStyle $symfonyStyle, private readonly DeprecatedRulesReporter $deprecatedRulesReporter, private readonly MissConfigurationReporter $missConfigurationReporter, - private readonly SkippedClassResolver $skippedClassResolver, + private readonly Skipper $skipper, ) { parent::__construct(); } @@ -94,7 +94,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function reportDeprecatedSkippedClasses(): int { - $deprecatedSkippedClasses = $this->skippedClassResolver->resolveDeprecatedSkippedClasses(); + $deprecatedSkippedClasses = $this->skipper->resolveDeprecatedSkippedClasses(); if ($deprecatedSkippedClasses === []) { return 0; } diff --git a/src/Reporting/UnusedSkipResolver.php b/src/Reporting/UnusedSkipResolver.php index 90469847990..81fe5515b25 100644 --- a/src/Reporting/UnusedSkipResolver.php +++ b/src/Reporting/UnusedSkipResolver.php @@ -7,7 +7,7 @@ use Rector\Configuration\Option; use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\FileSystem\FilePathHelper; -use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; +use Rector\Skipper\Skipper\Skipper; use Rector\Skipper\SkipCriteriaResolver\SkippedPathsResolver; use Rector\ValueObject\ProcessResult; @@ -17,7 +17,7 @@ final readonly class UnusedSkipResolver { public function __construct( - private SkippedClassResolver $skippedClassResolver, + private Skipper $skipper, private SkippedPathsResolver $skippedPathsResolver, private FilePathHelper $filePathHelper, ) { @@ -73,7 +73,7 @@ private function shouldSkipReporting(): bool private function resolveRelativePathsByClass(): array { $relativePathsByClass = []; - foreach ($this->skippedClassResolver->resolve() as $rectorClass => $paths) { + foreach ($this->skipper->resolveSkippedClasses() as $rectorClass => $paths) { if ($paths === null) { continue; } @@ -118,7 +118,7 @@ private function resolveUnusedRuleScopedSkips(array $usedSkips): array $unusedRelativePaths = []; foreach ($relativePaths as $path => $relativePath) { // used skips are tracked scoped to their rule (class => [path]), so the same path - // skipped under another rule does not mark this one used (see SkipSkipper) + // skipped under another rule does not mark this one used (see Skipper::matchSkip()) if (! in_array($path, $usedSkips[$rectorClass] ?? [], true)) { $unusedRelativePaths[] = $relativePath; } diff --git a/src/Skipper/SkipCriteriaResolver/SkippedClassResolver.php b/src/Skipper/SkipCriteriaResolver/SkippedClassResolver.php deleted file mode 100644 index 8f3f947a9e7..00000000000 --- a/src/Skipper/SkipCriteriaResolver/SkippedClassResolver.php +++ /dev/null @@ -1,74 +0,0 @@ - - */ - private null|array $skippedClassesToFiles = null; - - /** - * @return array> - */ - public function resolveDeprecatedSkippedClasses(): array - { - $skippedClassNames = array_keys($this->resolve()); - - return array_filter( - $skippedClassNames, - fn (string $class): bool => is_a($class, DeprecatedInterface::class, true) - ); - } - - /** - * @return array - */ - public function resolve(): array - { - // disable cache in tests - if (StaticPHPUnitEnvironment::isPHPUnitRun()) { - $this->skippedClassesToFiles = null; - } - - // already cached, even only empty array - if ($this->skippedClassesToFiles !== null) { - return $this->skippedClassesToFiles; - } - - $skip = SimpleParameterProvider::provideArrayParameter(Option::SKIP); - $this->skippedClassesToFiles = []; - - foreach ($skip as $key => $value) { - // e.g. [SomeClass::class] → shift values to [SomeClass::class => null] - if (is_int($key)) { - $key = $value; - $value = null; - } - - if (! is_string($key)) { - continue; - } - - // this only checks for Rector rules, that are always autoloaded - if (! class_exists($key) && ! interface_exists($key)) { - continue; - } - - $this->skippedClassesToFiles[$key] = $value; - } - - return $this->skippedClassesToFiles; - } -} diff --git a/src/Skipper/Skipper/Skipper.php b/src/Skipper/Skipper/Skipper.php index d404cbcf483..884be924823 100644 --- a/src/Skipper/Skipper/Skipper.php +++ b/src/Skipper/Skipper/Skipper.php @@ -6,25 +6,32 @@ use PhpParser\Node; use PHPStan\Reflection\ReflectionProvider; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Configuration\Option; +use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\Contract\Rector\RectorInterface; use Rector\ProcessAnalyzer\RectifiedAnalyzer; use Rector\Skipper\Matcher\FileInfoMatcher; -use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; use Rector\Skipper\ValueObject\SkipMatch; +use Rector\Testing\PHPUnit\StaticPHPUnitEnvironment; /** * @api * @see \Rector\Tests\Skipper\Skipper\SkipperTest */ -final readonly class Skipper +final class Skipper { + /** + * @var null|array + */ + private null|array $skippedClassesToFiles = null; + public function __construct( - private RectifiedAnalyzer $rectifiedAnalyzer, - private PathSkipper $pathSkipper, - private FileInfoMatcher $fileInfoMatcher, - private SkippedClassResolver $skippedClassResolver, - private ReflectionProvider $reflectionProvider, - private UsedSkipCollector $usedSkipCollector, + private readonly RectifiedAnalyzer $rectifiedAnalyzer, + private readonly PathSkipper $pathSkipper, + private readonly FileInfoMatcher $fileInfoMatcher, + private readonly ReflectionProvider $reflectionProvider, + private readonly UsedSkipCollector $usedSkipCollector, ) { } @@ -54,7 +61,7 @@ public function matchSkip(string|object $element, string $filePath): ?SkipMatch return null; } - foreach ($this->skippedClassResolver->resolve() as $skippedClass => $skippedFiles) { + foreach ($this->resolveSkippedClasses() as $skippedClass => $skippedFiles) { if (! is_a($element, $skippedClass, true)) { continue; } @@ -80,6 +87,59 @@ public function markSkipUsed(SkipMatch $skipMatch): void $this->usedSkipCollector->markUsed($skipMatch->getSkippedClass(), $skipMatch->getMatchedPath()); } + /** + * @return array + */ + public function resolveSkippedClasses(): array + { + // disable cache in tests + if (StaticPHPUnitEnvironment::isPHPUnitRun()) { + $this->skippedClassesToFiles = null; + } + + // already cached, even only empty array + if ($this->skippedClassesToFiles !== null) { + return $this->skippedClassesToFiles; + } + + $skip = SimpleParameterProvider::provideArrayParameter(Option::SKIP); + $this->skippedClassesToFiles = []; + + foreach ($skip as $key => $value) { + // e.g. [SomeClass::class] → shift values to [SomeClass::class => null] + if (is_int($key)) { + $key = $value; + $value = null; + } + + if (! is_string($key)) { + continue; + } + + // this only checks for Rector rules, that are always autoloaded + if (! class_exists($key) && ! interface_exists($key)) { + continue; + } + + $this->skippedClassesToFiles[$key] = $value; + } + + return $this->skippedClassesToFiles; + } + + /** + * @return array> + */ + public function resolveDeprecatedSkippedClasses(): array + { + $skippedClassNames = array_keys($this->resolveSkippedClasses()); + + return array_filter( + $skippedClassNames, + fn (string $class): bool => is_a($class, DeprecatedInterface::class, true) + ); + } + /** * @param class-string $rectorClass */ diff --git a/tests/Console/Command/ValidateConfigCommandTest.php b/tests/Console/Command/ValidateConfigCommandTest.php index 115ec0e432d..c2b2cf61550 100644 --- a/tests/Console/Command/ValidateConfigCommandTest.php +++ b/tests/Console/Command/ValidateConfigCommandTest.php @@ -14,7 +14,7 @@ use Rector\Reporting\DeprecatedRulesReporter; use Rector\Reporting\MissConfigurationReporter; use Rector\Reporting\UnusedSkipResolver; -use Rector\Skipper\SkipCriteriaResolver\SkippedClassResolver; +use Rector\Skipper\Skipper\Skipper; use Rector\Testing\PHPUnit\AbstractLazyTestCase; use Rector\Tests\Console\Command\Source\DeprecatedFixtureRule; use Symfony\Component\Console\Input\ArrayInput; @@ -56,7 +56,7 @@ protected function setUp(): void new VendorMissAnalyseGuard(), $this->make(UnusedSkipResolver::class), ), - $this->make(SkippedClassResolver::class), + $this->make(Skipper::class), ); $this->commandTester = new CommandTester($validateConfigCommand); diff --git a/tests/Skipper/Skipper/SkippedClassResolverTest.php b/tests/Skipper/Skipper/SkippedClassResolverTest.php deleted file mode 100644 index c8980975d00..00000000000 --- a/tests/Skipper/Skipper/SkippedClassResolverTest.php +++ /dev/null @@ -1,18 +0,0 @@ -make(SkippedClassResolver::class); - - $this->assertSame([], $skippedClassResolver->resolveDeprecatedSkippedClasses()); - } -} diff --git a/tests/Skipper/Skipper/SkipperTest.php b/tests/Skipper/Skipper/SkipperTest.php index 7c43afa3888..39182dbd001 100644 --- a/tests/Skipper/Skipper/SkipperTest.php +++ b/tests/Skipper/Skipper/SkipperTest.php @@ -55,6 +55,11 @@ public function testSkipFilePath(string $filePath, bool $expectedSkip): void $this->assertSame($expectedSkip, $filePathResultSkip); } + public function testResolveDeprecatedSkippedClasses(): void + { + $this->assertSame([], $this->skipper->resolveDeprecatedSkippedClasses()); + } + /** * @return Iterator */