[CodeQuality] Handle crash on yield from on OptionalParametersAfterRe…

…quiredRector (#6545) * [CodeQuality] Handle crash on yield from on OptionalParametersAfterRequiredRector * fix * [ci-review] Rector Rectify * rename fixture * Fix phpstan * fix --------- Co-authored-by: GitHub Action <[email protected]>
rectorphp · Dec 11, 2024 · f5c95f5 · f5c95f5
1 parent ebbada3
commit f5c95f5
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 39 deletions.
diff --git a/...onalParametersAfterRequiredRector/Fixture/skip_first_class_callable_in_yield_from.php.inc b/...onalParametersAfterRequiredRector/Fixture/skip_first_class_callable_in_yield_from.php.inc
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\CodeQuality\Rector\ClassMethod\OptionalParametersAfterRequiredRector\Fixture;
+
+final class SkipFirstClassCallableInYield2
+{
+    public function getSubscribedEvents(string $nodePath): iterable
+    {
+        yield from $this->textElement(...);
+    }
+
+    public function textElement() { return []; }
+}
diff --git a/src/Configuration/ConfigurationRuleFilter.php b/src/Configuration/ConfigurationRuleFilter.php
@@ -20,33 +20,32 @@ public function setConfiguration(Configuration $configuration): void
     }
 
     /**
-     * @param array<RectorInterface> $rectors
-     * @return array<RectorInterface>
+     * @param list<RectorInterface> $rectors
+     * @return list<RectorInterface>
      */
     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<RectorInterface> $rectors
-     * @return array<RectorInterface>
+     * @param list<RectorInterface> $rectors
+     * @return list<RectorInterface>
      */
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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');
     }
 }