Skip to content

Commit

Permalink
[EarlyReturn] Skip left or right is BooleanAnd on ChangeOrIfReturnToE…
Browse files Browse the repository at this point in the history
…arlyReturnRector (#799)

* [EarlyReturn] Handle left or right is BooleanAnd on ChangeOrIfReturnToEarlyReturnRector

* skip

* remove tweak in DateTimeToDateTimeInterfaceRector

* naming

* remove tweak in RemoveAlwaysElseRector

* rename fixture
  • Loading branch information
samsonasik committed Aug 31, 2021
1 parent c42b196 commit 2e18cab
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 138 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,7 @@ public function refactor(Node $node): ?Node
{
if ($node instanceof ClassMethod) {
$this->refactorClassMethod($node);

// @see https://github.com/rectorphp/rector-src/pull/794
// avoid duplicated ifs and returns when combined with ChangeOrIfReturnToEarlyReturnRector and ChangeAndIfToEarlyReturnRector
return null;
return $node;
}

return $this->refactorProperty($node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
use PhpParser\Node\Expr\Instanceof_;
use PhpParser\Node\Stmt\If_;
Expand Down Expand Up @@ -86,7 +87,7 @@ public function refactor(Node $node): ?Node
return null;
}

if ($this->isInstanceofCondOnly($node->cond)) {
if ($this->isInstanceofCondOnlyOrHasBooleanAnd($node->cond)) {
return null;
}

Expand Down Expand Up @@ -166,16 +167,20 @@ private function createIf(Expr $expr, Return_ $return): If_
);
}

private function isInstanceofCondOnly(BooleanOr $booleanOr): bool
private function isInstanceofCondOnlyOrHasBooleanAnd(BooleanOr $booleanOr): bool
{
$currentNode = $booleanOr;

if ($currentNode->left instanceof BooleanAnd || $currentNode->right instanceof BooleanAnd) {
return true;
}

if ($currentNode->left instanceof BooleanOr) {
return $this->isInstanceofCondOnly($currentNode->left);
return $this->isInstanceofCondOnlyOrHasBooleanAnd($currentNode->left);
}

if ($currentNode->right instanceof BooleanOr) {
return $this->isInstanceofCondOnly($currentNode->right);
return $this->isInstanceofCondOnlyOrHasBooleanAnd($currentNode->right);
}

if (! $currentNode->right instanceof Instanceof_) {
Expand Down
6 changes: 0 additions & 6 deletions rules/EarlyReturn/Rector/If_/RemoveAlwaysElseRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,6 @@ public function refactor(Node $node): ?Node

private function shouldSkip(If_ $if): bool
{
// to avoid repetitive If_ creation when used along with ChangeOrIfReturnToEarlyReturnRector
// @see https://github.com/rectorphp/rector-src/pull/651
if ($if->cond instanceof BooleanOr && $if->elseifs !== []) {
return true;
}

// to avoid repetitive flipped elseif above return when used along with ChangeAndIfReturnToEarlyReturnRector
// @see https://github.com/rectorphp/rector-src/pull/654
return $if->cond instanceof BooleanAnd && count($if->elseifs) > 1;
Expand Down
41 changes: 0 additions & 41 deletions tests/Issues/IssueEarlyReturnAndOr/Fixture/fixture.php.inc

This file was deleted.

19 changes: 19 additions & 0 deletions tests/Issues/IssueEarlyReturnAndOr/Fixture/skip_next_or.php.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

namespace Rector\Core\Tests\Issues\IssueEarlyReturnAndOr\Fixture;

class SkipAndNextOr
{
public function run($a, $b, $c, $d)
{
if (($a && false === $b) || ! $c) {
return '';
}

return $d;
}
}

?>

This file was deleted.

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

declare(strict_types=1);

namespace Rector\Core\Tests\Issues\IssueEarlyReturnAndOrDatetime\Fixture;

class SkipAndNextOr
{
public function run($a, $b, $c, $d)
{
if ($a && $b || $c) {
return null;
}

$a = 1;
$b = 2;

if ($b > $a) {
return null;
}

$d = 1;

return;
}
}

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

declare(strict_types=1);

namespace Rector\Core\Tests\Issues\IssueReturnBeforeElseIf\Fixture;

class ComplexIfCondOr
{
public function run($a, $b, $c, $d, $e)
{
if (($a && false === $b) || ! $c) {
return '';
} elseif ($d) {
return '';
}
return $e;
}
}

?>
-----
<?php

declare(strict_types=1);

namespace Rector\Core\Tests\Issues\IssueReturnBeforeElseIf\Fixture;

class ComplexIfCondOr
{
public function run($a, $b, $c, $d, $e)
{
if (($a && false === $b) || ! $c) {
return '';
}
if ($d) {
return '';
}
return $e;
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ class ComplexIfCondOrWithoutElseIf
{
public function run($a, $b, $c)
{
if ($a && false === $b) {
return 'a';
}
if (! $c) {
if (($a && false === $b) || ! $c) {
return 'a';
}
return 'b';
Expand Down

This file was deleted.

0 comments on commit 2e18cab

Please sign in to comment.