From 95c5990f699fc6fb5fb7f8d1c3e97b82edc737fc Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 14 Oct 2026 23:47:56 +0200 Subject: [PATCH] [CodeQuality] Speed up ExplicitReturnNullRector with single scoped walk and cheap bail before return type inference --- .../ClassMethod/ExplicitReturnNullRector.php | 66 ++++++++++++------- 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php b/rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php index c063ed28060..70f84d9c341 100644 --- a/rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php +++ b/rules/CodeQuality/Rector/ClassMethod/ExplicitReturnNullRector.php @@ -20,7 +20,6 @@ use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger; use Rector\NodeTypeResolver\PHPStan\Type\TypeFactory; -use Rector\PhpParser\Node\BetterNodeFinder; use Rector\Rector\AbstractRector; use Rector\RuleDoc\CodeSample\CodeSample; use Rector\RuleDoc\RuleDefinition; @@ -39,7 +38,6 @@ public function __construct( private readonly TypeFactory $typeFactory, private readonly PhpDocTypeChanger $phpDocTypeChanger, private readonly ReturnTypeInferer $returnTypeInferer, - private readonly BetterNodeFinder $betterNodeFinder, ) { } @@ -105,41 +103,65 @@ public function refactor(Node $node): ?Node return null; } - if ($node instanceof ClassMethod && $this->isName($node, MethodName::CONSTRUCT)) { + // bodyless method (abstract/interface), nothing to improve + if ($node->stmts === null) { return null; } - $returnType = $this->returnTypeInferer->inferFunctionLike($node); - if (! $returnType instanceof UnionType) { + if ($node instanceof ClassMethod && $this->isName($node, MethodName::CONSTRUCT)) { return null; } - $hasGoto = (bool) $this->betterNodeFinder->findFirstInFunctionLikeScoped( - $node, - fn (Node $node): bool => $node instanceof Goto_ - ); + $hasGoto = false; + $hasValueReturn = false; + $bareReturns = []; - if ($hasGoto) { - return null; - } - - $hasChanged = false; - $this->traverseNodesWithCallable((array) $node->stmts, static function (Node $node) use ( - &$hasChanged - ): int|null|Return_ { - if ($node instanceof Class_ || $node instanceof Function_ || $node instanceof Closure) { + $this->traverseNodesWithCallable($node->stmts, static function (Node $subNode) use ( + &$hasGoto, + &$hasValueReturn, + &$bareReturns + ): ?int { + if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) { return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN; } - if ($node instanceof Return_ && ! $node->expr instanceof Expr) { - $hasChanged = true; - $node->expr = new ConstFetch(new Name('null')); - return $node; + if ($subNode instanceof Goto_) { + $hasGoto = true; + return null; + } + + if ($subNode instanceof Return_) { + if ($subNode->expr instanceof Expr) { + $hasValueReturn = true; + } else { + $bareReturns[] = $subNode; + } } return null; }); + // goto makes control flow analysis unreliable + if ($hasGoto) { + return null; + } + + // no returned value means the inferred type can never be a value|void union + if (! $hasValueReturn) { + return null; + } + + $returnType = $this->returnTypeInferer->inferFunctionLike($node); + if (! $returnType instanceof UnionType) { + return null; + } + + $hasChanged = false; + foreach ($bareReturns as $bareReturn) { + $bareReturn->expr = new ConstFetch(new Name('null')); + $hasChanged = true; + } + // allow non native @return never type use // to avoid noise for addition returns if (! $this->silentVoidResolver->hasSilentVoid($node, false)) {