Skip to content

Commit

Permalink
[CodeQuality] Skip while(true) always returned on ExplicitReturnNullR…
Browse files Browse the repository at this point in the history
…ector (#5810)

* [CodeQuality] Skip while(true) always returned on ExplicitReturnNullRector

* [ci-review] Rector Rectify

* [ci-review] Rector Rectify

* more fixture

* fix

---------

Co-authored-by: GitHub Action <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Apr 8, 2024
1 parent b292010 commit 54e2e11
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;

final class SkipDoWhileTrueAlwaysReturned
{
public function run()
{
do {
if (mt_rand() === 0) {
return 'yay';
}
} while (true);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector\Fixture;

final class SkipWhileTrueAlwaysReturned
{
public function run()
{
while (true) {
if (mt_rand() === 0) {
return 'yay';
}
}
}
}
52 changes: 43 additions & 9 deletions rules/TypeDeclaration/TypeInferer/SilentVoidResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Break_;
use PhpParser\Node\Stmt\Case_;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\Do_;
Expand All @@ -28,8 +29,12 @@
use PhpParser\Node\Stmt\Switch_;
use PhpParser\Node\Stmt\Throw_;
use PhpParser\Node\Stmt\TryCatch;
use PhpParser\Node\Stmt\While_;
use PhpParser\NodeTraverser;
use PHPStan\Reflection\ClassReflection;
use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
use Rector\PhpParser\Node\BetterNodeFinder;
use Rector\PhpParser\Node\Value\ValueResolver;
use Rector\Reflection\ReflectionResolver;
use Rector\TypeDeclaration\NodeAnalyzer\NeverFuncCallAnalyzer;

Expand All @@ -38,7 +43,9 @@
public function __construct(
private BetterNodeFinder $betterNodeFinder,
private ReflectionResolver $reflectionResolver,
private NeverFuncCallAnalyzer $neverFuncCallAnalyzer
private NeverFuncCallAnalyzer $neverFuncCallAnalyzer,
private ValueResolver $valueResolver,
private SimpleCallableNodeTraverser $simpleCallableNodeTraverser
) {
}

Expand Down Expand Up @@ -100,24 +107,51 @@ private function hasStmtsAlwaysReturnOrExit(array $stmts): bool
return true;
}

if ($stmt instanceof Do_ && $this->isDoWithAlwaysReturnOrExit($stmt)) {
return true;
if (!$this->isDoOrWhileWithAlwaysReturnOrExit($stmt)) {
continue;
}

return true;
}

return false;
}

private function isDoWithAlwaysReturnOrExit(Do_ $do): bool
private function isFoundLoopControl(Do_|While_ $node): bool
{
$isFoundLoopControl = false;
$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
$node->stmts,
static function (Node $subNode) use (&$isFoundLoopControl) {
if ($subNode instanceof Class_ || $subNode instanceof Function_ || $subNode instanceof Closure) {
return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
}

if ($subNode instanceof Break_ || $subNode instanceof Continue_ || $subNode instanceof Goto_) {
$isFoundLoopControl = true;
return NodeTraverser::STOP_TRAVERSAL;
}
}
);

return $isFoundLoopControl;
}

private function isDoOrWhileWithAlwaysReturnOrExit(Stmt $stmt): bool
{
if (! $this->hasStmtsAlwaysReturnOrExit($do->stmts)) {
if (! $stmt instanceof Do_ && ! $stmt instanceof While_) {
return false;
}

return ! (bool) $this->betterNodeFinder->findFirst(
$do->stmts,
static fn (Node $node): bool => $node instanceof Break_ || $node instanceof Continue_ || $node instanceof Goto_
);
if ($this->valueResolver->isTrue($stmt->cond)) {
return ! $this->isFoundLoopControl($stmt);
}

if (! $this->hasStmtsAlwaysReturnOrExit($stmt->stmts)) {
return false;
}

return ! $this->isFoundLoopControl($stmt);;
}

private function isIfReturn(Stmt|Expr $stmt): bool
Expand Down

0 comments on commit 54e2e11

Please sign in to comment.