[perf] Add PHPStan rule to catch ScopeFetcher::fetch() before a scope-free guard

phpstan.neon

@@ -3,6 +3,7 @@ includes:
 
 rules:
     - Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule
+    - Rector\Utils\PHPStan\Rule\NoScopeFetchBeforeCheapGuardRule
 
 parameters:
     level: 8

rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php

@@ -147,7 +147,7 @@ public function refactor(Node $node): ?Node
 
     private function hasPrivateMethod(Class_ $class): bool
     {
-        return array_any($class->getMethods(), fn(ClassMethod $classMethod): bool => $classMethod->isPrivate());
+        return array_any($class->getMethods(), fn (ClassMethod $classMethod): bool => $classMethod->isPrivate());
     }
 
     private function shouldSkip(ClassMethod $classMethod, ClassReflection $classReflection): bool

rules/TypeDeclaration/Rector/BooleanAnd/BinaryOpNullableToInstanceofRector.php

@@ -82,6 +82,11 @@ public function refactor(Node $node): ?Node
         return $this->processNullableInstance($node);
     }
 
+    /**
+     * @template TBinaryOp of BooleanAnd|BooleanOr
+     * @param TBinaryOp $node
+     * @return TBinaryOp|null
+     */
     private function processNullableInstance(BooleanAnd|BooleanOr $node): null|BooleanAnd|BooleanOr
     {
         $nullableObjectType = $this->nullableTypeAnalyzer->resolveNullableObjectType($node->left);
@@ -134,9 +139,7 @@ private function processNegationBooleanOr(BooleanOr $booleanOr): ?BooleanOr
             return $booleanOr;
         }
 
-        /** @var BooleanOr|null $result */
-        $result = $this->processNullableInstance($booleanOr);
-        return $result;
+        return $this->processNullableInstance($booleanOr);
     }
 
     private function createExprInstanceof(Expr $expr, ObjectType $objectType): Instanceof_

utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php

@@ -0,0 +1,148 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Rule;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use PhpParser\Node\Stmt\If_;
+use PhpParser\Node\Stmt\Return_;
+use PhpParser\NodeFinder;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Rector\PHPStan\ScopeFetcher;
+
+/**
+ * Flags ScopeFetcher::fetch() called right before a cheap guard that does not use the scope.
+ * The guard should run first, so most nodes bail out before scope resolution.
+ *
+ * @implements Rule<ClassMethod>
+ * @see \Rector\Utils\PHPStan\Tests\Rule\NoScopeFetchBeforeCheapGuardRule\NoScopeFetchBeforeCheapGuardRuleTest
+ */
+final class NoScopeFetchBeforeCheapGuardRule implements Rule
+{
+    private const string ERROR_MESSAGE = 'Move the "return null" guard above ScopeFetcher::fetch(), as it does not use the scope - most nodes then bail out before scope resolution.';
+
+    public function getNodeType(): string
+    {
+        return ClassMethod::class;
+    }
+
+    /**
+     * @param ClassMethod $node
+     * @return list<IdentifierRuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if ($node->name->toString() !== 'refactor') {
+            return [];
+        }
+
+        $stmts = $node->stmts;
+        if ($stmts === null) {
+            return [];
+        }
+
+        foreach ($stmts as $key => $stmt) {
+            $scopeVariableName = $this->matchScopeFetchVariableName($stmt);
+            if ($scopeVariableName === null) {
+                continue;
+            }
+
+            $nextStmt = $stmts[$key + 1] ?? null;
+            if (! $nextStmt instanceof If_) {
+                continue;
+            }
+
+            if (! $this->isReturnNullGuard($nextStmt)) {
+                continue;
+            }
+
+            if ($this->usesVariable($nextStmt->cond, $scopeVariableName)) {
+                continue;
+            }
+
+            return [
+                RuleErrorBuilder::message(self::ERROR_MESSAGE)
+                    ->identifier('rector.scopeFetchBeforeCheapGuard')
+                    ->line($stmt->getStartLine())
+                    ->build(),
+            ];
+        }
+
+        return [];
+    }
+
+    private function matchScopeFetchVariableName(Stmt $stmt): ?string
+    {
+        if (! $stmt instanceof Expression) {
+            return null;
+        }
+
+        if (! $stmt->expr instanceof Assign) {
+            return null;
+        }
+
+        $assign = $stmt->expr;
+        if (! $assign->var instanceof Variable || ! is_string($assign->var->name)) {
+            return null;
+        }
+
+        if (! $assign->expr instanceof StaticCall) {
+            return null;
+        }
+
+        $staticCall = $assign->expr;
+        if (! $staticCall->class instanceof Node\Name) {
+            return null;
+        }
+
+        if ($staticCall->class->toString() !== ScopeFetcher::class) {
+            return null;
+        }
+
+        if (! $staticCall->name instanceof Node\Identifier || $staticCall->name->toString() !== 'fetch') {
+            return null;
+        }
+
+        return $assign->var->name;
+    }
+
+    private function isReturnNullGuard(If_ $if): bool
+    {
+        if (count($if->stmts) !== 1) {
+            return false;
+        }
+
+        $onlyStmt = $if->stmts[0];
+        if (! $onlyStmt instanceof Return_) {
+            return false;
+        }
+
+        return $onlyStmt->expr === null || $this->isNullConstant($onlyStmt->expr);
+    }
+
+    private function isNullConstant(Expr $expr): bool
+    {
+        return $expr instanceof Node\Expr\ConstFetch && $expr->name->toLowerString() === 'null';
+    }
+
+    private function usesVariable(Expr $expr, string $variableName): bool
+    {
+        $foundVariable = (new NodeFinder())->findFirst(
+            [$expr],
+            static fn (Node $subNode): bool => $subNode instanceof Variable && $subNode->name === $variableName
+        );
+
+        return $foundVariable instanceof Variable;
+    }
+}

utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoScopeFetchBeforeCheapGuardRule;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Rector\Utils\PHPStan\Rule\NoScopeFetchBeforeCheapGuardRule;
+
+/**
+ * @extends RuleTestCase<NoScopeFetchBeforeCheapGuardRule>
+ */
+final class NoScopeFetchBeforeCheapGuardRuleTest extends RuleTestCase
+{
+    public function testScopeFetchBeforeGuard(): void
+    {
+        $expectedErrorMessage = 'Move the "return null" guard above ScopeFetcher::fetch(), as it does not use the scope - most nodes then bail out before scope resolution.';
+
+        $this->analyse([__DIR__ . '/Source/ScopeFetchBeforeGuardRector.php'], [[$expectedErrorMessage, 15]]);
+    }
+
+    public function testSkip(): void
+    {
+        $this->analyse([__DIR__ . '/Source/SkipScopeFetchRector.php'], []);
+    }
+
+    protected function getRule(): Rule
+    {
+        return new NoScopeFetchBeforeCheapGuardRule();
+    }
+}

utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php

@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoScopeFetchBeforeCheapGuardRule\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\ClassMethod;
+use Rector\PHPStan\ScopeFetcher;
+
+final class ScopeFetchBeforeGuardRector
+{
+    public function refactor(ClassMethod $node): ?Node
+    {
+        $scope = ScopeFetcher::fetch($node);
+        if ($node->returnType instanceof Node) {
+            return null;
+        }
+
+        return $node;
+    }
+}

utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoScopeFetchBeforeCheapGuardRule\Source;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\ClassMethod;
+use Rector\PHPStan\ScopeFetcher;
+
+final class SkipScopeFetchRector
+{
+    public function refactor(ClassMethod $node): ?Node
+    {
+        // cheap guard already runs first
+        if ($node->returnType instanceof Node) {
+            return null;
+        }
+
+        $scope = ScopeFetcher::fetch($node);
+        if (! $scope->isInClass()) {
+            return null;
+        }
+
+        return $node;
+    }
+}