From 4f9036e208fe7b7ab983abee04f845f308ce6e92 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 3 Apr 2019 20:29:46 +0200 Subject: [PATCH 1/3] [CodeQuality] prevent double negative in SimplifyBoolIdenticalTrueRector --- docs/AllRectorsOverview.md | 2 +- docs/NodesOverview.md | 2 +- .../SimplifyBoolIdenticalTrueRector.php | 58 +++++++++---------- .../Fixture/double_negate.php.inc | 31 ++++++++++ .../SimplifyBoolIdenticalTrueRectorTest.php | 6 +- 5 files changed, 64 insertions(+), 35 deletions(-) create mode 100644 packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/double_negate.php.inc diff --git a/docs/AllRectorsOverview.md b/docs/AllRectorsOverview.md index fd2d60b9f4f..43f80f88041 100644 --- a/docs/AllRectorsOverview.md +++ b/docs/AllRectorsOverview.md @@ -2714,7 +2714,7 @@ Changes rand, srand and getrandmax by new md_* alternatives. - class: `Rector\Php\Rector\FuncCall\PregReplaceEModifierRector` -The /e modifier is no longer supported, use preg_replace_callback instead +The /e modifier is no longer supported, use preg_replace_callback instead ```diff class SomeClass diff --git a/docs/NodesOverview.md b/docs/NodesOverview.md index 81aef55e937..134b5d8a865 100644 --- a/docs/NodesOverview.md +++ b/docs/NodesOverview.md @@ -910,7 +910,7 @@ if (true) { ```php ?> -feelfeel diff --git a/packages/CodeQuality/src/Rector/Identical/SimplifyBoolIdenticalTrueRector.php b/packages/CodeQuality/src/Rector/Identical/SimplifyBoolIdenticalTrueRector.php index 6599512a8e9..f077891eeb7 100644 --- a/packages/CodeQuality/src/Rector/Identical/SimplifyBoolIdenticalTrueRector.php +++ b/packages/CodeQuality/src/Rector/Identical/SimplifyBoolIdenticalTrueRector.php @@ -55,46 +55,40 @@ CODE_SAMPLE public function refactor(Node $node): ?Node { if ($this->isBoolType($node->left) && ! $this->isBool($node->left)) { - if ($node instanceof Identical) { - if ($this->isTrue($node->right)) { - return $node->left; - } - - if ($this->isFalse($node->right)) { - return new BooleanNot($node->left); - } - } - - if ($node instanceof NotIdentical) { - if ($this->isFalse($node->right)) { - return $node->left; - } - - if ($this->isTrue($node->right)) { - return new BooleanNot($node->left); - } - } + return $this->processBoolTypeToNotBool($node, $node->left, $node->right); } if ($this->isBoolType($node->right) && ! $this->isBool($node->right)) { - if ($node instanceof Identical) { - if ($this->isTrue($node->left)) { - return $node->right; - } + return $this->processBoolTypeToNotBool($node, $node->right, $node->left); + } - if ($this->isFalse($node->left)) { - return new BooleanNot($node->right); - } + return null; + } + + private function processBoolTypeToNotBool(Node $node, Node\Expr $leftExpr, Node\Expr $rightExpr): ?Node\Expr + { + if ($node instanceof Identical) { + if ($this->isTrue($rightExpr)) { + return $leftExpr; } - if ($node instanceof NotIdentical) { - if ($this->isFalse($node->left)) { - return $node->right; + if ($this->isFalse($rightExpr)) { + // prevent !! + if ($leftExpr instanceof BooleanNot) { + return $leftExpr->expr; } - if ($this->isTrue($node->left)) { - return new BooleanNot($node->right); - } + return new BooleanNot($leftExpr); + } + } + + if ($node instanceof NotIdentical) { + if ($this->isFalse($rightExpr)) { + return $leftExpr; + } + + if ($this->isTrue($rightExpr)) { + return new BooleanNot($leftExpr); } } diff --git a/packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/double_negate.php.inc b/packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/double_negate.php.inc new file mode 100644 index 00000000000..61d9b1f7e91 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/double_negate.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/SimplifyBoolIdenticalTrueRectorTest.php b/packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/SimplifyBoolIdenticalTrueRectorTest.php index bf72fe5ec5e..ef5891e319c 100644 --- a/packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/SimplifyBoolIdenticalTrueRectorTest.php +++ b/packages/CodeQuality/tests/Rector/Identical/SimplifyBoolIdenticalTrueRector/SimplifyBoolIdenticalTrueRectorTest.php @@ -9,7 +9,11 @@ final class SimplifyBoolIdenticalTrueRectorTest extends AbstractRectorTestCase { public function test(): void { - $this->doTestFiles([__DIR__ . '/Fixture/directly.php.inc', __DIR__ . '/Fixture/negate.php.inc']); + $this->doTestFiles([ + __DIR__ . '/Fixture/directly.php.inc', + __DIR__ . '/Fixture/negate.php.inc', + __DIR__ . '/Fixture/double_negate.php.inc', + ]); } protected function getRectorClass(): string From a0d132912488e70b7b9045a1ab807e6b7807f459 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 3 Apr 2019 21:26:53 +0200 Subject: [PATCH 2/3] composer: add --ansi --- composer.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index 82bf08a7a6a..f103b8d46db 100644 --- a/composer.json +++ b/composer.json @@ -116,12 +116,12 @@ "@phpstan", "@update-docs" ], - "check-cs": "vendor/bin/ecs check bin packages src tests utils", + "check-cs": "vendor/bin/ecs check bin packages src tests utils --ansi", "fix-cs": [ - "vendor/bin/ecs check bin packages src tests utils --fix", + "vendor/bin/ecs check bin packages src tests utils --fix --ansi", "bin/clean_trailing_spaces.sh" ], - "phpstan": "vendor/bin/phpstan analyse packages src tests --error-format symplify", + "phpstan": "vendor/bin/phpstan analyse packages src tests --error-format symplify --ansi", "docs": [ "bin/rector dump-rectors -o markdown > docs/AllRectorsOverview.md", "bin/rector dump-nodes -o markdown > docs/NodesOverview.md" From 9768ffc0330afab7cd8d3dfa900c9f80a8094239 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 3 Apr 2019 21:33:48 +0200 Subject: [PATCH 3/3] [CodeQuality] Add BooleanNotIdenticalToNotIdenticalRector --- .../level/symfony/symfony-code-quality.yaml | 1 + ...ooleanNotIdenticalToNotIdenticalRector.php | 105 ++++++++++++++++++ ...anNotIdenticalToNotIdenticalRectorTest.php | 23 ++++ .../Fixture/fixture.php.inc | 37 ++++++ .../Fixture/just_first.php.inc | 37 ++++++ .../Fixture/keep.php.inc | 25 +++++ 6 files changed, 228 insertions(+) create mode 100644 packages/CodeQuality/src/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector.php create mode 100644 packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/BooleanNotIdenticalToNotIdenticalRectorTest.php create mode 100644 packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/fixture.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/just_first.php.inc create mode 100644 packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/keep.php.inc diff --git a/config/level/symfony/symfony-code-quality.yaml b/config/level/symfony/symfony-code-quality.yaml index 20c89e6a70d..96dbfee69f3 100644 --- a/config/level/symfony/symfony-code-quality.yaml +++ b/config/level/symfony/symfony-code-quality.yaml @@ -1,2 +1,3 @@ services: Rector\Symfony\Rector\BinaryOp\ResponseStatusCodeRector: ~ + Rector\CodeQuality\Rector\Identical\BooleanNotIdenticalToNotIdenticalRector: ~ diff --git a/packages/CodeQuality/src/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector.php b/packages/CodeQuality/src/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector.php new file mode 100644 index 00000000000..3ac37944bd3 --- /dev/null +++ b/packages/CodeQuality/src/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector.php @@ -0,0 +1,105 @@ +processIdentical($node); + } + + if ($node->expr instanceof Identical) { + $identical = $node->expr; + if (! $this->isBoolType($identical->left)) { + return null; + } + + if (! $this->isBoolType($identical->right)) { + return null; + } + + return new Node\Expr\BinaryOp\NotIdentical($identical->left, $identical->right); + } + + return null; + } + + private function processIdentical(Identical $identical): ?Node\Expr\BinaryOp\NotIdentical + { + if (! $this->isBoolType($identical->left)) { + return null; + } + + if (! $this->isBoolType($identical->right)) { + return null; + } + + if ($identical->left instanceof Node\Expr\BooleanNot) { + return new Node\Expr\BinaryOp\NotIdentical($identical->left->expr, $identical->right); + } + + return null; + } +} diff --git a/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/BooleanNotIdenticalToNotIdenticalRectorTest.php b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/BooleanNotIdenticalToNotIdenticalRectorTest.php new file mode 100644 index 00000000000..e6e02e5f5ce --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/BooleanNotIdenticalToNotIdenticalRectorTest.php @@ -0,0 +1,23 @@ +doTestFiles([ + __DIR__ . '/Fixture/fixture.php.inc', + __DIR__ . '/Fixture/just_first.php.inc', + __DIR__ . '/Fixture/keep.php.inc', + ]); + } + + protected function getRectorClass(): string + { + return BooleanNotIdenticalToNotIdenticalRector::class; + } +} diff --git a/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/fixture.php.inc b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/fixture.php.inc new file mode 100644 index 00000000000..ca11ab41ee7 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/fixture.php.inc @@ -0,0 +1,37 @@ + +----- + diff --git a/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/just_first.php.inc b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/just_first.php.inc new file mode 100644 index 00000000000..5cd56ff27d8 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/just_first.php.inc @@ -0,0 +1,37 @@ + +----- + diff --git a/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/keep.php.inc b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/keep.php.inc new file mode 100644 index 00000000000..3c9b6f3f3d5 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Identical/BooleanNotIdenticalToNotIdenticalRector/Fixture/keep.php.inc @@ -0,0 +1,25 @@ +