Skip to content

Commit

Permalink
[CodeQuality] Handle not identical return false then true on Simplify…
Browse files Browse the repository at this point in the history
…IfReturnBoolRector (#1969)

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
samsonasik and actions-user committed Mar 27, 2022
1 parent 6f61a09 commit 82656ad
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\SimplifyIfReturnBoolRector\Fixture;

use PhpCsFixer\Tokenizer\Token;

class NotIdenticalReturnFalseThenTrue
{
public function run()
{
$docToken = new Token([]);
if (strpos($docToken->getContent(), "\n") !== false) {
return false;
}

return true;
}
}

?>
-----
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\SimplifyIfReturnBoolRector\Fixture;

use PhpCsFixer\Tokenizer\Token;

class NotIdenticalReturnFalseThenTrue
{
public function run()
{
$docToken = new Token([]);
return strpos($docToken->getContent(), "\n") === false;
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\If_\SimplifyIfReturnBoolRector\Fixture;

class SkipNotIdenticalReturnFalseThenReturnInArray
{
private const POSSIBLE_DELIMITERS = ['#', '~', '/'];

public function run($firstChar, $lastChar)
{
if ($firstChar !== $lastChar) {
return false;
}

return in_array($firstChar, self::POSSIBLE_DELIMITERS, true);
}
}
13 changes: 10 additions & 3 deletions rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,14 @@ public function refactor(Node $node): ?Node
if ($this->valueResolver->isTrue($innerIfInnerNode)) {
$newReturnNode = $this->processReturnTrue($node, $nextNode);
} elseif ($this->valueResolver->isFalse($innerIfInnerNode)) {
$newReturnNode = $this->processReturnFalse($node, $nextNode);
/** @var Expr $expr */
$expr = $nextNode->expr;
if ($node->cond instanceof NotIdentical && $this->valueResolver->isTrue($expr)) {
$node->cond = new Identical($node->cond->left, $node->cond->right);
$newReturnNode = $this->processReturnTrue($node, $nextNode);
} else {
$newReturnNode = $this->processReturnFalse($node, $nextNode);
}
} else {
return null;
}
Expand Down Expand Up @@ -122,7 +129,7 @@ private function shouldSkip(If_ $if): bool
return true;
}

if ($nextNode->expr === null) {
if (! $nextNode->expr instanceof Expr) {
return true;
}

Expand All @@ -136,7 +143,7 @@ private function shouldSkip(If_ $if): bool
return ! $this->valueResolver->isTrueOrFalse($nextNode->expr);
}

return true;
return ! $if->cond instanceof NotIdentical;
}

private function processReturnTrue(If_ $if, Return_ $nextReturnNode): Return_
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ private function shouldRefactor(FuncCall $funcCall): bool
}

// If param is provided, do nothing
if ($funcCall->args !== []) {
return false;
}

return true;
return $funcCall->args === [];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,7 @@ private function shouldSkip(ClassMethod $classMethod): bool
return false;
}

if ($classLike->extends !== null) {
return false;
}

return true;
return $classLike->extends === null;
}

// skip interface without parents
Expand Down
6 changes: 1 addition & 5 deletions src/PhpParser/Comparing/ConditionSearcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ private function hasVariableDeclaration(Variable $variable, Stmt $stmt): bool
return false;
}

if ($variable->name !== $assignVar->name) {
return false;
}

return true;
return $variable->name === $assignVar->name;
}
}

0 comments on commit 82656ad

Please sign in to comment.