From 52120f9d63491dac3d2dfcc16c8f5953ccc99ff8 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 11:12:04 +0200 Subject: [PATCH 1/2] [perf] Add NoScopeFetchBeforeCheapGuardRule to spot scope fetched before a scope-free return-null guard --- phpstan.neon | 1 + .../Rule/NoScopeFetchBeforeCheapGuardRule.php | 148 ++++++++++++++++++ .../NoScopeFetchBeforeCheapGuardRuleTest.php | 32 ++++ .../Source/ScopeFetchBeforeGuardRector.php | 22 +++ .../Source/SkipScopeFetchRector.php | 27 ++++ 5 files changed, 230 insertions(+) create mode 100644 utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php create mode 100644 utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php create mode 100644 utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php create mode 100644 utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php diff --git a/phpstan.neon b/phpstan.neon index 99ab47116e8..4d738e20e15 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -3,6 +3,7 @@ includes: rules: - Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule + - Rector\Utils\PHPStan\Rule\NoScopeFetchBeforeCheapGuardRule parameters: level: 8 diff --git a/utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php b/utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php new file mode 100644 index 00000000000..b891ccca98a --- /dev/null +++ b/utils/phpstan/src/Rule/NoScopeFetchBeforeCheapGuardRule.php @@ -0,0 +1,148 @@ + + * @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 + */ + 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; + } +} diff --git a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php new file mode 100644 index 00000000000..8e381f7c14b --- /dev/null +++ b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/NoScopeFetchBeforeCheapGuardRuleTest.php @@ -0,0 +1,32 @@ + + */ +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(); + } +} diff --git a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php new file mode 100644 index 00000000000..10441b9ebf0 --- /dev/null +++ b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/ScopeFetchBeforeGuardRector.php @@ -0,0 +1,22 @@ +returnType instanceof Node) { + return null; + } + + return $node; + } +} diff --git a/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php new file mode 100644 index 00000000000..d8d5ee9d41f --- /dev/null +++ b/utils/phpstan/tests/Rule/NoScopeFetchBeforeCheapGuardRule/Source/SkipScopeFetchRector.php @@ -0,0 +1,27 @@ +returnType instanceof Node) { + return null; + } + + $scope = ScopeFetcher::fetch($node); + if (! $scope->isInClass()) { + return null; + } + + return $node; + } +} From 1de7b212857fc16549846b851b4d57173c3cede3 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 10 Oct 2026 11:14:55 +0200 Subject: [PATCH 2/2] tidy up --- .../ClassMethod/RemoveUnusedPrivateMethodRector.php | 2 +- .../BooleanAnd/BinaryOpNullableToInstanceofRector.php | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php index fe03b9af361..13b2178c04b 100644 --- a/rules/DeadCode/Rector/ClassMethod/RemoveUnusedPrivateMethodRector.php +++ b/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 diff --git a/rules/TypeDeclaration/Rector/BooleanAnd/BinaryOpNullableToInstanceofRector.php b/rules/TypeDeclaration/Rector/BooleanAnd/BinaryOpNullableToInstanceofRector.php index 9778e9ed261..e5fca4be50f 100644 --- a/rules/TypeDeclaration/Rector/BooleanAnd/BinaryOpNullableToInstanceofRector.php +++ b/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_