From 15b3ea616acaa95ea6d5f102ef980111b00ce3a8 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 3 Sep 2023 18:32:11 +0200 Subject: [PATCH 1/2] Skip identical to false in SimplifyBoolIdenticalTrueRector, as exact comparison is stronger than negated expression --- .../Fixture/negate.php.inc | 2 -- .../skip_function_equals_false.php.inc | 11 ++++++ .../SimplifyBoolIdenticalTrueRector.php | 35 ++++++++++--------- 3 files changed, 29 insertions(+), 19 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/skip_function_equals_false.php.inc diff --git a/rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/negate.php.inc b/rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/negate.php.inc index 4530137e010..aebefd0ffc2 100644 --- a/rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/negate.php.inc +++ b/rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/negate.php.inc @@ -7,7 +7,6 @@ final class Negate public function run($value, array $items) { $isMatch = in_array($value, $items, TRUE) !== TRUE; - $isMatch = in_array($value, $items, TRUE) === FALSE; $isMatch = true !== in_array($value, $items, TRUE); } @@ -24,7 +23,6 @@ final class Negate public function run($value, array $items) { $isMatch = !in_array($value, $items, TRUE); - $isMatch = !in_array($value, $items, TRUE); $isMatch = !in_array($value, $items, TRUE); } diff --git a/rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/skip_function_equals_false.php.inc b/rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/skip_function_equals_false.php.inc new file mode 100644 index 00000000000..34ab2112126 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector/Fixture/skip_function_equals_false.php.inc @@ -0,0 +1,11 @@ +valueResolver->isTrueOrFalse($node->left) && $this->getType($node->left)->isBoolean()->yes()) { + if ($this->isBooleanButNotTrueAndFalse($node->left)) { return $this->processBoolTypeToNotBool($node, $node->left, $node->right); } - if ($this->valueResolver->isTrueOrFalse($node->right)) { - return null; - } - - if (! $this->getType($node->right)->isBoolean()->yes()) { - return null; + if ($this->isBooleanButNotTrueAndFalse($node->right)) { + return $this->processBoolTypeToNotBool($node, $node->right, $node->left); } - return $this->processBoolTypeToNotBool($node, $node->right, $node->left); + return null; } private function processBoolTypeToNotBool(Node $node, Expr $leftExpr, Expr $rightExpr): ?Expr @@ -99,17 +96,10 @@ private function refactorIdentical(Expr $leftExpr, Expr $rightExpr): ?Expr } if ($this->valueResolver->isFalse($rightExpr)) { - // prevent !! + // prevent double negation !! if ($leftExpr instanceof BooleanNot) { return $leftExpr->expr; } - - // keep as it is, readable enough - if ($leftExpr instanceof Variable && $this->getType($leftExpr)->isBoolean()->yes()) { - return null; - } - - return new BooleanNot($leftExpr); } return null; @@ -127,4 +117,15 @@ private function refactorNotIdentical(Expr $leftExpr, Expr $rightExpr): ?Expr return null; } + + private function isBooleanButNotTrueAndFalse(Expr $expr): bool + { + if ($this->valueResolver->isTrueOrFalse($expr)) { + return false; + } + + return $this->getType($expr) + ->isBoolean() + ->yes(); + } } From 88709b01db873b0bd3be499f8ad3cf9bda35a9df Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Sun, 3 Sep 2023 16:33:54 +0000 Subject: [PATCH 2/2] [ci-review] Rector Rectify --- .../Identical/SimplifyBoolIdenticalTrueRector.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/rules/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector.php b/rules/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector.php index 78ab10e6d1b..3b2dde06d81 100644 --- a/rules/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector.php +++ b/rules/CodeQuality/Rector/Identical/SimplifyBoolIdenticalTrueRector.php @@ -94,15 +94,14 @@ private function refactorIdentical(Expr $leftExpr, Expr $rightExpr): ?Expr if ($this->valueResolver->isTrue($rightExpr)) { return $leftExpr; } - - if ($this->valueResolver->isFalse($rightExpr)) { - // prevent double negation !! - if ($leftExpr instanceof BooleanNot) { - return $leftExpr->expr; - } + // prevent double negation !! + if (!$this->valueResolver->isFalse($rightExpr)) { + return null; } - - return null; + if (!$leftExpr instanceof BooleanNot) { + return null; + } + return $leftExpr->expr; } private function refactorNotIdentical(Expr $leftExpr, Expr $rightExpr): ?Expr