[Strict] Add support string|null|false on BooleanInBooleanNotRuleFixe…

…rRector (#2035) * [Strict] Add support string|null|false on BooleanInBooleanNotRuleFixerRector * fixed 🎉 * test in trat as non empty true as well * add test for string and null only, tested with both TREAT_AS_NON_EMPTY true and false * multi union types on DisallowedEmptyRuleFixerRector
rectorphp · Apr 10, 2022 · 9b46906 · 9b46906
1 parent 7666733
commit 9b46906
Show file tree

Hide file tree

Showing 8 changed files with 167 additions and 30 deletions.
diff --git a/build/target-repository/docs/rector_rules_overview.md b/build/target-repository/docs/rector_rules_overview.md
@@ -9811,8 +9811,7 @@ return static function (ContainerConfigurator $containerConfigurator): void {
 ```diff
  class SomeClass
  {
--    public function run(string|null $name)
-+    public function run(string $name)
+     public function run(string|null $name)
      {
 -        if (! $name) {
 +        if ($name === null) {

diff --git a/...ector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/string_null_and_false.php.inc b/...ector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/string_null_and_false.php.inc
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\Fixture;
+
+class StringNullAndFalse
+{
+    public function run(string|null|false $name)
+    {
+        if (! $name) {
+            return 'no name';
+        }
+
+        return 'name';
+    }
+}
+
+?>
+-----
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\Fixture;
+
+class StringNullAndFalse
+{
+    public function run(string|null|false $name)
+    {
+        if ($name === '' || $name === false || $name === null) {
+            return 'no name';
+        }
+
+        return 'name';
+    }
+}
+
+?>
diff --git a/...or/Fixture/union_with_string_null.php.inc → ...ixture/union_with_null_from_param.php.inc b/...or/Fixture/union_with_string_null.php.inc → ...ixture/union_with_null_from_param.php.inc
@@ -4,7 +4,7 @@ declare(strict_types=1);
 
 namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\Fixture;
 
-final class UnionWithStringNull
+final class UnionWithNullFromParam
 {
     public function run(string|null $value)
     {
@@ -24,7 +24,7 @@ declare(strict_types=1);
 
 namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\Fixture;
 
-final class UnionWithStringNull
+final class UnionWithNullFromParam
 {
     public function run(string|null $value)
     {

diff --git a/...t/BooleanInBooleanNotRuleFixerRector/FixtureTreatAsNonEmpty/string_null_and_false.php.inc b/...t/BooleanInBooleanNotRuleFixerRector/FixtureTreatAsNonEmpty/string_null_and_false.php.inc
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\FixtureTreatAsNonEmpty;
+
+class StringNullAndFalse
+{
+    public function run(string|null|false $name)
+    {
+        if (! $name) {
+            return 'no name';
+        }
+
+        return 'name';
+    }
+}
+
+?>
+-----
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\FixtureTreatAsNonEmpty;
+
+class StringNullAndFalse
+{
+    public function run(string|null|false $name)
+    {
+        if ($name === '' || $name === false || $name === null) {
+            return 'no name';
+        }
+
+        return 'name';
+    }
+}
+
+?>
diff --git a/...leanInBooleanNotRuleFixerRector/FixtureTreatAsNonEmpty/union_with_null_from_param.php.inc b/...leanInBooleanNotRuleFixerRector/FixtureTreatAsNonEmpty/union_with_null_from_param.php.inc
@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\FixtureTreatAsNonEmpty;
+
+class UnionWithNullFromParam
+{
+    public function run(string|null $name)
+    {
+        if (! $name) {
+            return 'no name';
+        }
+
+        return 'name';
+    }
+}
+
+?>
+-----
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Strict\Rector\BooleanNot\BooleanInBooleanNotRuleFixerRector\FixtureTreatAsNonEmpty;
+
+class UnionWithNullFromParam
+{
+    public function run(string|null $name)
+    {
+        if ($name === null) {
+            return 'no name';
+        }
+
+        return 'name';
+    }
+}
+
+?>
diff --git a/...or/Fixture/skip_multi_union_types.php.inc → ...rRector/Fixture/multi_union_types.php.inc b/...or/Fixture/skip_multi_union_types.php.inc → ...rRector/Fixture/multi_union_types.php.inc
@@ -26,7 +26,7 @@ final class MultiUnionTypes
      */
     public function get($id)
     {
-        return $id === [] || $id === 0 || $id === '';
+        return $id === [] || $id === 0 || $id === '' || $id === null;
     }
 }
 

diff --git a/rules/Strict/NodeFactory/ExactCompareFactory.php b/rules/Strict/NodeFactory/ExactCompareFactory.php
@@ -18,6 +18,7 @@
 use PHPStan\Type\ArrayType;
 use PHPStan\Type\BooleanType;
 use PHPStan\Type\IntegerType;
+use PHPStan\Type\NullType;
 use PHPStan\Type\StringType;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
@@ -50,6 +51,10 @@ public function createIdenticalFalsyCompare(Type $exprType, Expr $expr, bool $tr
             return new Identical($expr, new Array_([]));
         }
 
+        if ($exprType instanceof NullType) {
+            return new Identical($expr, $this->nodeFactory->createNull());
+        }
+
         if (! $exprType instanceof UnionType) {
             return null;
         }
@@ -113,8 +118,36 @@ private function resolveFromCleanedNullUnionType(UnionType $unionType, Expr $exp
             $compareExprs[] = $this->createNotIdenticalFalsyCompare($unionedType, $expr, $treatAsNotEmpty);
         }
 
-        /** @var ?Expr $truthyExpr */
+        return $this->resolveTruthyExpr($compareExprs);
+    }
+
+    /**
+     * @return array<Expr|null>
+     */
+    private function collectCompareExprs(UnionType $unionType, Expr $expr, bool $treatAsNonEmpty): array
+    {
+        $compareExprs = [];
+        foreach ($unionType->getTypes() as $unionedType) {
+            $compareExprs[] = $this->createIdenticalFalsyCompare($unionedType, $expr, $treatAsNonEmpty);
+        }
+
+        return $compareExprs;
+    }
+
+    private function cleanUpPossibleNullableUnionType(UnionType $unionType): Type
+    {
+        return count($unionType->getTypes()) === 2
+            ? TypeCombinator::removeNull($unionType)
+            : $unionType;
+    }
+
+    /**
+     * @param array<Expr|null> $compareExprs
+     */
+    private function resolveTruthyExpr(array $compareExprs): ?Expr
+    {
         $truthyExpr = array_shift($compareExprs);
+
         foreach ($compareExprs as $compareExpr) {
             if (! $compareExpr instanceof Expr) {
                 return null;
@@ -124,7 +157,6 @@ private function resolveFromCleanedNullUnionType(UnionType $unionType, Expr $exp
                 return null;
             }
 
-            /** @var Expr $compareExpr */
             $truthyExpr = new BooleanOr($truthyExpr, $compareExpr);
         }
 
@@ -133,22 +165,11 @@ private function resolveFromCleanedNullUnionType(UnionType $unionType, Expr $exp
 
     private function createTruthyFromUnionType(UnionType $unionType, Expr $expr, bool $treatAsNonEmpty): Expr|null
     {
-        $unionType = TypeCombinator::removeNull($unionType);
+        $unionType = $this->cleanUpPossibleNullableUnionType($unionType);
 
         if ($unionType instanceof UnionType) {
-            $compareExprs = [];
-            foreach ($unionType->getTypes() as $unionedType) {
-                $compareExprs[] = $this->createIdenticalFalsyCompare($unionedType, $expr, $treatAsNonEmpty);
-            }
-
-            /** @var Expr $truthyExpr */
-            $truthyExpr = array_shift($compareExprs);
-            foreach ($compareExprs as $compareExpr) {
-                /** @var Expr $compareExpr */
-                $truthyExpr = new BooleanOr($truthyExpr, $compareExpr);
-            }
-
-            return $truthyExpr;
+            $compareExprs = $this->collectCompareExprs($unionType, $expr, $treatAsNonEmpty);
+            return $this->resolveTruthyExpr($compareExprs);
         }
 
         if ($unionType instanceof BooleanType) {
@@ -162,16 +183,16 @@ private function createTruthyFromUnionType(UnionType $unionType, Expr $expr, boo
 
         $toNullIdentical = new Identical($expr, $this->nodeFactory->createNull());
 
-        // assume we have to check empty string, integer and bools
-        if (! $treatAsNonEmpty) {
-            $scalarFalsyIdentical = $this->createIdenticalFalsyCompare($unionType, $expr, $treatAsNonEmpty);
-            if (! $scalarFalsyIdentical instanceof Expr) {
-                return null;
-            }
+        if ($treatAsNonEmpty) {
+            return $toNullIdentical;
+        }
 
-            return new BooleanOr($toNullIdentical, $scalarFalsyIdentical);
+        // assume we have to check empty string, integer and bools
+        $scalarFalsyIdentical = $this->createIdenticalFalsyCompare($unionType, $expr, $treatAsNonEmpty);
+        if (! $scalarFalsyIdentical instanceof Expr) {
+            return null;
         }
 
-        return $toNullIdentical;
+        return new BooleanOr($toNullIdentical, $scalarFalsyIdentical);
     }
 }
diff --git a/rules/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector.php b/rules/Strict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector.php
@@ -52,7 +52,7 @@ public function run(string|null $name)
                 <<<'CODE_SAMPLE'
 class SomeClass
 {
-    public function run(string $name)
+    public function run(string|null $name)
     {
         if ($name === null) {
             return 'no name';