From c3840325600449b562691e34612279dcd4316043 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 29 Nov 2023 00:39:41 +0700 Subject: [PATCH] [Strict] Fix DisallowedEmptyRuleFixerRector empty() allow string '0' check (#5296) * DisallowedEmptyRuleFixerRector: fix test expectations * fix * fix * [ci-review] Rector Rectify * more fixture * also negation * run * run --------- Co-authored-by: Markus Staab Co-authored-by: GitHub Action --- .../Fixture/union_with_null.php.inc | 2 +- .../union_with_null_from_param.php.inc | 2 +- .../Fixture/empty_string_nullable.php.inc | 2 +- .../negation_empty_string_nullable.php.inc | 27 +++++++++++++++++++ .../Fixture/nullable_string.php.inc | 2 +- .../NodeFactory/ExactCompareFactory.php | 16 +++++++++++ .../SimplifyEmpty/Fixture/fixture.php.inc | 2 +- 7 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/negation_empty_string_nullable.php.inc diff --git a/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null.php.inc b/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null.php.inc index fa2bc5001f3..64a4ed221fe 100644 --- a/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null.php.inc +++ b/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null.php.inc @@ -32,7 +32,7 @@ final class UnionWithNull public function run() { - if ($this->value === null || $this->value === '') { + if ($this->value === null || $this->value === '' || $this->value === '0') { return 'empty'; } diff --git a/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null_from_param.php.inc b/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null_from_param.php.inc index e96f8bb9dad..8fa49eef456 100644 --- a/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null_from_param.php.inc +++ b/rules-tests/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/union_with_null_from_param.php.inc @@ -28,7 +28,7 @@ final class UnionWithNullFromParam { public function run(string|null $value) { - if ($value === null || $value === '') { + if ($value === null || $value === '' || $value === '0') { return 'empty'; } diff --git a/rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/empty_string_nullable.php.inc b/rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/empty_string_nullable.php.inc index 6bc55496977..91cf79b8dd0 100644 --- a/rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/empty_string_nullable.php.inc +++ b/rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/empty_string_nullable.php.inc @@ -20,7 +20,7 @@ final class EmptyStringNullable { public function run(string|null $value) { - return $value === null || $value === ''; + return $value === null || $value === '' || $value === '0'; } } diff --git a/rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/negation_empty_string_nullable.php.inc b/rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/negation_empty_string_nullable.php.inc new file mode 100644 index 00000000000..81e067fc40e --- /dev/null +++ b/rules-tests/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/negation_empty_string_nullable.php.inc @@ -0,0 +1,27 @@ + +----- + diff --git a/rules-tests/Strict/Rector/Ternary/BooleanInTernaryOperatorRuleFixerRector/Fixture/nullable_string.php.inc b/rules-tests/Strict/Rector/Ternary/BooleanInTernaryOperatorRuleFixerRector/Fixture/nullable_string.php.inc index b3e2b9ca1a7..482d52d578b 100644 --- a/rules-tests/Strict/Rector/Ternary/BooleanInTernaryOperatorRuleFixerRector/Fixture/nullable_string.php.inc +++ b/rules-tests/Strict/Rector/Ternary/BooleanInTernaryOperatorRuleFixerRector/Fixture/nullable_string.php.inc @@ -24,7 +24,7 @@ final class NullableString { public function run(string|null $value) { - return $value !== null && $value !== '' ? 1 : 2; + return $value !== null && $value !== '' && $value !== '0' ? 1 : 2; } } diff --git a/rules/Strict/NodeFactory/ExactCompareFactory.php b/rules/Strict/NodeFactory/ExactCompareFactory.php index 6e4ba113995..896a660770d 100644 --- a/rules/Strict/NodeFactory/ExactCompareFactory.php +++ b/rules/Strict/NodeFactory/ExactCompareFactory.php @@ -112,6 +112,16 @@ private function createFromUnionType( return null; } + if ($treatAsNotEmpty) { + return new BooleanAnd($toNullNotIdentical, $compareExpr); + } + + if ($unionType->isString()->yes()) { + $booleanAnd = new BooleanAnd($toNullNotIdentical, $compareExpr); + + return new BooleanAnd($booleanAnd, new NotIdentical($expr, new String_('0'))); + } + return new BooleanAnd($toNullNotIdentical, $compareExpr); } @@ -227,6 +237,12 @@ private function createTruthyFromUnionType( return null; } + if ($unionType->isString()->yes()) { + $booleanOr = new BooleanOr($toNullIdentical, $scalarFalsyIdentical); + + return new BooleanOr($booleanOr, new Identical($expr, new String_('0'))); + } + return new BooleanOr($toNullIdentical, $scalarFalsyIdentical); } } diff --git a/tests/Issues/SimplifyEmpty/Fixture/fixture.php.inc b/tests/Issues/SimplifyEmpty/Fixture/fixture.php.inc index 891518c9cb6..5a6e9800070 100644 --- a/tests/Issues/SimplifyEmpty/Fixture/fixture.php.inc +++ b/tests/Issues/SimplifyEmpty/Fixture/fixture.php.inc @@ -54,7 +54,7 @@ class Fixture { public function check(): bool { - return $this->getString() !== null && $this->getString() !== '' && !(($this->getString2() === null || $this->getString2() === '') && !is_numeric($this->getString2())); + return $this->getString() !== null && $this->getString() !== '' && $this->getString() !== '0' && !(($this->getString2() === null || $this->getString2() === '' || $this->getString2() === '0') && !is_numeric($this->getString2())); } }