perf: skip scope lookup for unsupported empty checks (#8795)

Co-authored-by: Alfred Bez <1001186+alfredbez@users.noreply.github.com>

Author: alfredbez

rules-tests/CodeQuality/Rector/Empty_/SimplifyEmptyCheckOnEmptyArrayRector/SimplifyEmptyCheckOnEmptyArrayRectorTest.php

@@ -5,7 +5,14 @@
 namespace Rector\Tests\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRector;
 
 use Iterator;
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\BooleanNot;
+use PhpParser\Node\Expr\Empty_;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Name;
 use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRector;
 use Rector\Testing\PHPUnit\AbstractRectorTestCase;
 
 final class SimplifyEmptyCheckOnEmptyArrayRectorTest extends AbstractRectorTestCase
@@ -21,6 +28,26 @@ public static function provideData(): Iterator
         return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
     }
 
+    #[DataProvider('provideUnsupportedNodes')]
+    public function testSkipsUnsupportedNodesWithoutScope(BooleanNot|Empty_ $expr): void
+    {
+        $rector = self::getContainer()->make(SimplifyEmptyCheckOnEmptyArrayRector::class);
+
+        $this->assertNull($rector->refactor($expr));
+    }
+
+    /**
+     * @return Iterator<string, array{BooleanNot|Empty_}>
+     */
+    public static function provideUnsupportedNodes(): Iterator
+    {
+        yield 'plain negation' => [new BooleanNot(new Variable('values'))];
+        yield 'empty array literal' => [new Empty_(new Array_())];
+        yield 'negated empty function call' => [new BooleanNot(new Empty_(new FuncCall(new Name('getValues'))))];
+        yield 'superglobal get' => [new Empty_(new Variable('_GET'))];
+        yield 'negated superglobal post' => [new BooleanNot(new Empty_(new Variable('_POST')))];
+    }
+
     public function provideConfigFilePath(): string
     {
         return __DIR__ . '/config/configured_rule.php';

rules/CodeQuality/Rector/Empty_/SimplifyEmptyCheckOnEmptyArrayRector.php

@@ -5,7 +5,6 @@
 namespace Rector\CodeQuality\Rector\Empty_;
 
 use PhpParser\Node;
-use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\Array_;
 use PhpParser\Node\Expr\BinaryOp\Identical;
 use PhpParser\Node\Expr\BinaryOp\NotIdentical;
@@ -79,43 +78,36 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        $scope = ScopeFetcher::fetch($node);
-        if ($node instanceof BooleanNot) {
-            if ($node->expr instanceof Empty_ && $this->isAllowedExpr($node->expr->expr, $scope)) {
-                return new NotIdentical($node->expr->expr, new Array_());
-            }
-
+        $empty = $node instanceof BooleanNot ? $node->expr : $node;
+        if (! $empty instanceof Empty_) {
             return null;
         }
 
-        if (! $this->isAllowedExpr($node->expr, $scope)) {
+        $expr = $empty->expr;
+        if (! $expr instanceof Variable && ! $expr instanceof PropertyFetch && ! $expr instanceof StaticPropertyFetch) {
             return null;
         }
 
-        return new Identical($node->expr, new Array_());
-    }
+        if ($expr instanceof Variable && is_string($expr->name) && $this->reservedKeywordAnalyzer->isNativeVariable($expr->name)) {
+            return null;
+        }
 
-    private function isAllowedVariable(Variable $variable): bool
-    {
-        if (is_string($variable->name) && $this->reservedKeywordAnalyzer->isNativeVariable($variable->name)) {
-            return false;
+        $scope = ScopeFetcher::fetch($node);
+        if (! $this->isAllowedExpr($expr, $scope)) {
+            return null;
         }
 
-        return ! $this->exprAnalyzer->isNonTypedFromParam($variable);
+        return $node instanceof BooleanNot ? new NotIdentical($expr, new Array_()) : new Identical($expr, new Array_());
     }
 
-    private function isAllowedExpr(Expr $expr, Scope $scope): bool
+    private function isAllowedExpr(Variable|PropertyFetch|StaticPropertyFetch $expr, Scope $scope): bool
     {
-        if (! $expr instanceof Variable && ! $expr instanceof PropertyFetch && ! $expr instanceof StaticPropertyFetch) {
-            return false;
-        }
-
         if (! $scope->getType($expr)->isArray()->yes()) {
             return false;
         }
 
         if ($expr instanceof Variable) {
-            return $this->isAllowedVariable($expr);
+            return ! $this->exprAnalyzer->isNonTypedFromParam($expr);
         }
 
         if (! $expr->name instanceof Identifier) {