From f5c95f5f8d2476456409694aacad3ac1d78df3e1 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 11 Dec 2024 18:15:48 +0700 Subject: [PATCH] [CodeQuality] Handle crash on yield from on OptionalParametersAfterRequiredRector (#6545) * [CodeQuality] Handle crash on yield from on OptionalParametersAfterRequiredRector * fix * [ci-review] Rector Rectify * rename fixture * Fix phpstan * fix --------- Co-authored-by: GitHub Action --- ...first_class_callable_in_yield_from.php.inc | 15 ++++++ src/Configuration/ConfigurationRuleFilter.php | 15 +++--- src/Configuration/OnlyRuleResolver.php | 13 +++-- src/Console/Command/ListRulesCommand.php | 1 + src/Console/Command/ProcessCommand.php | 2 +- .../Scope/PHPStanNodeScopeResolver.php | 5 +- .../Command/WorkerCommandLineFactory.php | 2 +- tests/Configuration/OnlyRuleResolverTest.php | 48 ++++++++++--------- 8 files changed, 62 insertions(+), 39 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_first_class_callable_in_yield_from.php.inc diff --git a/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_first_class_callable_in_yield_from.php.inc b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_first_class_callable_in_yield_from.php.inc new file mode 100644 index 00000000000..df08ede20cc --- /dev/null +++ b/rules-tests/CodeQuality/Rector/ClassMethod/OptionalParametersAfterRequiredRector/Fixture/skip_first_class_callable_in_yield_from.php.inc @@ -0,0 +1,15 @@ +textElement(...); + } + + public function textElement() { return []; } +} diff --git a/src/Configuration/ConfigurationRuleFilter.php b/src/Configuration/ConfigurationRuleFilter.php index 266bab620f1..cf13c903b48 100644 --- a/src/Configuration/ConfigurationRuleFilter.php +++ b/src/Configuration/ConfigurationRuleFilter.php @@ -20,33 +20,32 @@ public function setConfiguration(Configuration $configuration): void } /** - * @param array $rectors - * @return array + * @param list $rectors + * @return list */ public function filter(array $rectors): array { - if ($this->configuration === null) { + if (!$this->configuration instanceof Configuration) { return $rectors; } $onlyRule = $this->configuration->getOnlyRule(); if ($onlyRule !== null) { - $rectors = $this->filterOnlyRule($rectors, $onlyRule); - return $rectors; + return $this->filterOnlyRule($rectors, $onlyRule); } return $rectors; } /** - * @param array $rectors - * @return array + * @param list $rectors + * @return list */ public function filterOnlyRule(array $rectors, string $onlyRule): array { $activeRectors = []; foreach ($rectors as $rector) { - if (is_a($rector, $onlyRule)) { + if ($rector instanceof $onlyRule) { $activeRectors[] = $rector; } } diff --git a/src/Configuration/OnlyRuleResolver.php b/src/Configuration/OnlyRuleResolver.php index a2da78b16af..ab1059d8549 100644 --- a/src/Configuration/OnlyRuleResolver.php +++ b/src/Configuration/OnlyRuleResolver.php @@ -11,13 +11,13 @@ /** * @see \Rector\Tests\Configuration\OnlyRuleResolverTest */ -final class OnlyRuleResolver +final readonly class OnlyRuleResolver { /** * @param RectorInterface[] $rectors */ public function __construct( - private readonly array $rectors + private array $rectors ) { } @@ -46,11 +46,13 @@ public function resolve(string $rule): string $matching[] = $rector::class; } } - $matching = array_unique($matching); + $matching = array_unique($matching); if (count($matching) == 1) { return $matching[0]; - } elseif (count($matching) > 1) { + } + + if (count($matching) > 1) { sort($matching); $message = sprintf( 'Short rule name "%s" is ambiguous. Specify the full rule name:' . PHP_EOL @@ -60,7 +62,7 @@ public function resolve(string $rule): string throw new RectorRuleNameAmbigiousException($message); } - if (strpos($rule, '\\') === false) { + if (in_array(str_contains($rule, '\\'), [0, false], true)) { $message = sprintf( 'Rule "%s" was not found.%sThe rule has no namespace. Make sure to escape the backslashes, and add quotes around the rule name: --only="My\\Rector\\Rule"', $rule, @@ -73,6 +75,7 @@ public function resolve(string $rule): string PHP_EOL ); } + throw new RectorRuleNotFoundException($message); } } diff --git a/src/Console/Command/ListRulesCommand.php b/src/Console/Command/ListRulesCommand.php index 0ea37930a5e..c6f8b37c54c 100644 --- a/src/Console/Command/ListRulesCommand.php +++ b/src/Console/Command/ListRulesCommand.php @@ -57,6 +57,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int if ($onlyRule !== null) { $onlyRule = $this->onlyRuleResolver->resolve($onlyRule); } + $rectorClasses = $this->resolveRectorClasses($onlyRule); $skippedClasses = $this->getSkippedCheckers(); diff --git a/src/Console/Command/ProcessCommand.php b/src/Console/Command/ProcessCommand.php index 9cfdd09a711..92ae7beece8 100644 --- a/src/Console/Command/ProcessCommand.php +++ b/src/Console/Command/ProcessCommand.php @@ -43,7 +43,7 @@ public function __construct( private readonly ConfigurationFactory $configurationFactory, private readonly DeprecatedRulesReporter $deprecatedRulesReporter, private readonly MissConfigurationReporter $missConfigurationReporter, - private ConfigurationRuleFilter $configurationRuleFilter, + private readonly ConfigurationRuleFilter $configurationRuleFilter, ) { parent::__construct(); } diff --git a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php index 3830647d344..5cc35621d4a 100644 --- a/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php +++ b/src/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php @@ -30,6 +30,7 @@ use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Expr\Yield_; +use PhpParser\Node\Expr\YieldFrom; use PhpParser\Node\Identifier; use PhpParser\Node\IntersectionType; use PhpParser\Node\Name; @@ -178,7 +179,8 @@ public function processNodes( $node instanceof Expression || $node instanceof Return_ || $node instanceof EnumCase || - $node instanceof Cast + $node instanceof Cast || + $node instanceof YieldFrom ) && $node->expr instanceof Expr) { $node->expr->setAttribute(AttributeKey::SCOPE, $mutatingScope); return; @@ -306,6 +308,7 @@ public function processNodes( if ($node instanceof Yield_) { $this->processYield($node, $mutatingScope); + return; } }; diff --git a/src/Parallel/Command/WorkerCommandLineFactory.php b/src/Parallel/Command/WorkerCommandLineFactory.php index a78444032e5..1641b1f81ed 100644 --- a/src/Parallel/Command/WorkerCommandLineFactory.php +++ b/src/Parallel/Command/WorkerCommandLineFactory.php @@ -116,7 +116,7 @@ public function create( if ($input->getOption(Option::ONLY) !== null) { $workerCommandArray[] = self::OPTION_DASHES . Option::ONLY; - $workerCommandArray[] = escapeshellarg($input->getOption(Option::ONLY)); + $workerCommandArray[] = escapeshellarg((string) $input->getOption(Option::ONLY)); } return implode(' ', $workerCommandArray); diff --git a/tests/Configuration/OnlyRuleResolverTest.php b/tests/Configuration/OnlyRuleResolverTest.php index 5c2640ee80f..135bc8a0752 100644 --- a/tests/Configuration/OnlyRuleResolverTest.php +++ b/tests/Configuration/OnlyRuleResolverTest.php @@ -4,6 +4,8 @@ namespace Rector\Tests\Configuration; +use Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector; +use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector; use Rector\Configuration\OnlyRuleResolver; use Rector\Contract\Rector\RectorInterface; use Rector\Exception\Configuration\RectorRuleNameAmbigiousException; @@ -12,46 +14,46 @@ final class OnlyRuleResolverTest extends AbstractLazyTestCase { - protected OnlyRuleResolver $resolver; + private OnlyRuleResolver $onlyRuleResolver; protected function setUp(): void { $this->bootFromConfigFiles([__DIR__ . '/config/only_rule_resolver_config.php']); $rectorConfig = self::getContainer(); - $this->resolver = new OnlyRuleResolver(iterator_to_array($rectorConfig->tagged(RectorInterface::class))); + $this->onlyRuleResolver = new OnlyRuleResolver(iterator_to_array($rectorConfig->tagged(RectorInterface::class))); } public function testResolveOk(): void { - $this->assertEquals( - \Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector::class, - $this->resolver->resolve('Rector\\DeadCode\\Rector\\Assign\\RemoveDoubleAssignRector') + $this->assertSame( + RemoveDoubleAssignRector::class, + $this->onlyRuleResolver->resolve('Rector\\DeadCode\\Rector\\Assign\\RemoveDoubleAssignRector') ); } public function testResolveOkLeadingBackslash(): void { - $this->assertEquals( - \Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector::class, - $this->resolver->resolve('\\Rector\\DeadCode\\Rector\\Assign\\RemoveDoubleAssignRector') + $this->assertSame( + RemoveDoubleAssignRector::class, + $this->onlyRuleResolver->resolve('\\Rector\\DeadCode\\Rector\\Assign\\RemoveDoubleAssignRector') ); } public function testResolveOkDoubleBackslashes(): void { - $this->assertEquals( - \Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector::class, - $this->resolver->resolve('\\\\Rector\\\\DeadCode\\\\Rector\\\\Assign\\\\RemoveDoubleAssignRector'), + $this->assertSame( + RemoveDoubleAssignRector::class, + $this->onlyRuleResolver->resolve('\\\\Rector\\\\DeadCode\\\\Rector\\\\Assign\\\\RemoveDoubleAssignRector'), 'We want to fix wrongly double-quoted backslashes automatically' ); } public function testResolveOkSingleQuotes(): void { - $this->assertEquals( - \Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector::class, - $this->resolver->resolve("'Rector\\DeadCode\\Rector\\Assign\\RemoveDoubleAssignRector'"), + $this->assertSame( + RemoveDoubleAssignRector::class, + $this->onlyRuleResolver->resolve("'Rector\\DeadCode\\Rector\\Assign\\RemoveDoubleAssignRector'"), 'Remove stray single quotes on Windows systems' ); } @@ -64,7 +66,7 @@ public function testResolveMissingBackslash(): void ); $this->expectException(RectorRuleNotFoundException::class); - $this->resolver->resolve('RectorDeadCodeRectorAssignRemoveDoubleAssignRector'); + $this->onlyRuleResolver->resolve('RectorDeadCodeRectorAssignRemoveDoubleAssignRector'); } public function testResolveNotFound(): void @@ -75,22 +77,22 @@ public function testResolveNotFound(): void ); $this->expectException(RectorRuleNotFoundException::class); - $this->resolver->resolve('This\\Rule\\Does\\Not\\Exist'); + $this->onlyRuleResolver->resolve('This\\Rule\\Does\\Not\\Exist'); } public function testResolveShortOk(): void { - $this->assertEquals( - \Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector::class, - $this->resolver->resolve('RemoveUnusedPrivateMethodRector'), + $this->assertSame( + RemoveUnusedPrivateMethodRector::class, + $this->onlyRuleResolver->resolve('RemoveUnusedPrivateMethodRector'), ); } public function testResolveShortOkTwoLevels(): void { - $this->assertEquals( - \Rector\DeadCode\Rector\Assign\RemoveDoubleAssignRector::class, - $this->resolver->resolve('Assign\\RemoveDoubleAssignRector'), + $this->assertSame( + RemoveDoubleAssignRector::class, + $this->onlyRuleResolver->resolve('Assign\\RemoveDoubleAssignRector'), ); } @@ -103,6 +105,6 @@ public function testResolveShortAmbiguous(): void ); $this->expectException(RectorRuleNameAmbigiousException::class); - $this->resolver->resolve('RemoveDoubleAssignRector'); + $this->onlyRuleResolver->resolve('RemoveDoubleAssignRector'); } }