[Strict] Allow check '0' empty zero on BooleanInTernaryOperatorRuleFi…

…xerRector on string type (#5297) * [Strict] Allow check 0 empty zero on BooleanInTernaryOperatorRuleFixerRector on string type * fix * [ci-review] Rector Rectify * [ci-review] Rector Rectify * [ci-review] Rector Rectify * Fix * Fix * fix --------- Co-authored-by: GitHub Action <actions@github.com>
rectorphp · Dec 3, 2023 · d9c5d8f · d9c5d8f
1 parent e3f909c
commit d9c5d8f
Show file tree

Hide file tree

Showing 17 changed files with 93 additions and 55 deletions.
diff --git a/config/set/php83.php b/config/set/php83.php
@@ -1,12 +1,14 @@
 <?php
 
 declare(strict_types=1);
+use Rector\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector;
+use Rector\Php83\Rector\ClassConst\AddTypeToConstRector;
 
 use Rector\Config\RectorConfig;
 
 return static function (RectorConfig $rectorConfig): void {
     $rectorConfig->rules([
-        \Rector\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector::class,
-        \Rector\Php83\Rector\ClassConst\AddTypeToConstRector::class,
+        AddOverrideAttributeToOverriddenMethodsRector::class,
+        AddTypeToConstRector::class,
     ]);
 };
diff --git a/rules-tests/Php83/Rector/ClassConst/AddTypeToConstRector/config/configured_rule.php b/rules-tests/Php83/Rector/ClassConst/AddTypeToConstRector/config/configured_rule.php
@@ -1,12 +1,13 @@
 <?php
 
 declare(strict_types=1);
+use Rector\Php83\Rector\ClassConst\AddTypeToConstRector;
 
 use Rector\Config\RectorConfig;
 use Rector\Core\ValueObject\PhpVersion;
 
 return static function (RectorConfig $rectorConfig): void {
-    $rectorConfig->rule(\Rector\Php83\Rector\ClassConst\AddTypeToConstRector::class);
+    $rectorConfig->rule(AddTypeToConstRector::class);
 
     $rectorConfig->phpVersion(PhpVersion::PHP_83);
 };
diff --git a/...trict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/negated_string.php.inc b/...trict/Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/negated_string.php.inc
@@ -32,7 +32,7 @@ final class NegatedString
 
     public function run()
     {
-        if ($this->name === '') {
+        if ($this->name === '' || $this->name === '0') {
             return 'name';
         }
 

diff --git a/...ector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/string_null_and_false.php.inc b/...ector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/string_null_and_false.php.inc
@@ -28,7 +28,7 @@ class StringNullAndFalse
 {
     public function run(string|null|false $name)
     {
-        if ($name === '' || $name === null || $name === false) {
+        if ($name === '' || $name === '0' || $name === null || $name === false) {
             return 'no name';
         }
 

diff --git a/...Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/string_union_integer.php.inc b/...Rector/BooleanNot/BooleanInBooleanNotRuleFixerRector/Fixture/string_union_integer.php.inc
@@ -28,7 +28,7 @@ final class StringUnionInteger
 {
     public function run(string|int $maye)
     {
-        if ($maye === '' || $maye === 0) {
+        if ($maye === '' || $maye === '0' || $maye === 0) {
             return true;
         }
 

diff --git a/...ts/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/from_return_method.php.inc b/...ts/Strict/Rector/Empty_/DisallowedEmptyRuleFixerRector/Fixture/from_return_method.php.inc
@@ -30,12 +30,12 @@ final class FromReturnCall
 {
     public function run()
     {
-        return $this->getProperty() === '';
+        return $this->getProperty() === '' || $this->getProperty() === '0';
     }
 
     public function run2()
     {
-        return $this->getProperty() !== '';
+        return $this->getProperty() !== '' && $this->getProperty() !== '0';
     }
 
     public function getProperty(): string

diff --git a/...rict/Rector/If_/BooleanInIfConditionRuleFixerRector/Fixture/elseif_negated_string.php.inc b/...rict/Rector/If_/BooleanInIfConditionRuleFixerRector/Fixture/elseif_negated_string.php.inc
@@ -36,7 +36,7 @@ final class ElseIfNegatedString
     {
         if ($boolValue) {
             return 'bool';
-        } elseif ($this->name !== '') {
+        } elseif ($this->name !== '' && $this->name !== '0') {
             return 'name';
         }
 

diff --git a/...trict/Rector/If_/BooleanInIfConditionRuleFixerRector/Fixture/integer_union_string.php.inc b/...trict/Rector/If_/BooleanInIfConditionRuleFixerRector/Fixture/integer_union_string.php.inc
@@ -28,7 +28,7 @@ final class IntegerUnionString
 {
     public function run(int|string $value)
     {
-        if ($value !== 0 && $value !== '') {
+        if ($value !== 0 && ($value !== '' && $value !== '0')) {
             return 'name';
         }
 

diff --git a/...ests/Strict/Rector/If_/BooleanInIfConditionRuleFixerRector/Fixture/negated_string.php.inc b/...ests/Strict/Rector/If_/BooleanInIfConditionRuleFixerRector/Fixture/negated_string.php.inc
@@ -32,7 +32,7 @@ final class NegatedString
 
     public function run()
     {
-        if ($this->name !== '') {
+        if ($this->name !== '' && $this->name !== '0') {
             return 'name';
         }
 

diff --git a/...tor/Ternary/BooleanInTernaryOperatorRuleFixerRector/Fixture/string_silent_compare.php.inc b/...tor/Ternary/BooleanInTernaryOperatorRuleFixerRector/Fixture/string_silent_compare.php.inc
@@ -24,7 +24,7 @@ final class StringSilentCompare
 {
     public function run(string $string)
     {
-        return $string !== '' ? 1 : 2;
+        return $string !== '' && $string !== '0' ? 1 : 2;
     }
 }
 

diff --git a/...Rector/Ternary/DisallowedShortTernaryRuleFixerRector/Fixture/short_ternary_string.php.inc b/...Rector/Ternary/DisallowedShortTernaryRuleFixerRector/Fixture/short_ternary_string.php.inc
@@ -24,7 +24,7 @@ final class ShortTernaryString
 {
     public function run(string $string)
     {
-        return $string !== '' ? $string : 2;
+        return $string !== '' && $string !== '0' ? $string : 2;
     }
 }
 

diff --git a/rules/Php83/Rector/ClassConst/AddTypeToConstRector.php b/rules/Php83/Rector/ClassConst/AddTypeToConstRector.php
@@ -4,6 +4,16 @@
 
 namespace Rector\Php83\Rector\ClassConst;
 
+use PhpParser\Node\Stmt\ClassConst;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Const_;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\Node\Scalar\LNumber;
+use PhpParser\Node\Scalar\DNumber;
+use PhpParser\Node\Expr\ConstFetch;
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Name;
 use PhpParser\Node;
 use PhpParser\Node\Name\FullyQualified;
 use PhpParser\Node\Stmt\Class_;
@@ -20,7 +30,7 @@
 /**
  * @see \Rector\Tests\Php83\Rector\ClassConst\AddTypeToConstRector\AddTypeToConstRectorTest
  */
-class AddTypeToConstRector extends AbstractRector implements MinPhpVersionInterface
+final class AddTypeToConstRector extends AbstractRector implements MinPhpVersionInterface
 {
     public function __construct(
         private readonly ReflectionProvider $reflectionProvider,
@@ -63,9 +73,7 @@ public function refactor(Node $node): Class_|null
             return null;
         }
 
-        $consts = array_filter($node->stmts, function (Node $stmt) {
-            return $stmt instanceof Node\Stmt\ClassConst;
-        });
+        $consts = array_filter($node->stmts, static fn(Node $node): bool => $node instanceof ClassConst);
 
         if ($consts === []) {
             return null;
@@ -91,13 +99,15 @@ public function refactor(Node $node): Class_|null
                 if ($this->shouldSkipDueToInheritance($constNode, $parents, $implementations, $traits)) {
                     continue;
                 }
+
                 if ($this->canBeInheritied($const, $node)) {
                     continue;
                 }
+
                 $valueType = $this->findValueType($constNode->value);
             }
 
-            if (($valueType ?? null) === null) {
+            if (!($valueType ?? null) instanceof Identifier) {
                 continue;
             }
 
@@ -124,45 +134,52 @@ public function provideMinPhpVersion(): int
      * @param ClassReflection[] $traits
      */
     public function shouldSkipDueToInheritance(
-        Node\Const_ $constNode,
+        Const_ $const,
         array $parents,
         array $implementations,
         array $traits,
     ): bool {
         foreach ([$parents, $implementations, $traits] as $inheritance) {
             foreach ($inheritance as $inheritanceItem) {
-                if ($constNode->name->name === '') {
+                if ($const->name->name === '') {
                     continue;
                 }
+
                 try {
-                    $inheritanceItem->getConstant($constNode->name->name);
+                    $inheritanceItem->getConstant($const->name->name);
                     return true;
                 } catch (MissingConstantFromReflectionException) {
                 }
             }
         }
+
         return false;
     }
 
-    private function findValueType(Node\Expr $value): ?Node\Identifier
+    private function findValueType(Expr $expr): ?Identifier
     {
-        if ($value instanceof Node\Scalar\String_) {
-            return new Node\Identifier('string');
+        if ($expr instanceof String_) {
+            return new Identifier('string');
         }
-        if ($value instanceof Node\Scalar\LNumber) {
-            return new Node\Identifier('int');
+
+        if ($expr instanceof LNumber) {
+            return new Identifier('int');
         }
-        if ($value instanceof Node\Scalar\DNumber) {
-            return new Node\Identifier('float');
+
+        if ($expr instanceof DNumber) {
+            return new Identifier('float');
         }
-        if ($value instanceof Node\Expr\ConstFetch && $value->name->toLowerString() !== 'null') {
-            return new Node\Identifier('bool');
+
+        if ($expr instanceof ConstFetch && $expr->name->toLowerString() !== 'null') {
+            return new Identifier('bool');
         }
-        if ($value instanceof Node\Expr\ConstFetch && $value->name->toLowerString() === 'null') {
-            return new Node\Identifier('null');
+
+        if ($expr instanceof ConstFetch && $expr->name->toLowerString() === 'null') {
+            return new Identifier('null');
         }
-        if ($value instanceof Node\Expr\Array_) {
-            return new Node\Identifier('array');
+
+        if ($expr instanceof Array_) {
+            return new Identifier('array');
         }
 
         return null;
@@ -175,7 +192,7 @@ private function getParents(Class_ $class): array
     {
         $parents = array_filter([$class->extends]);
 
-        return array_map(function (Node\Name $name): ClassReflection {
+        return array_map(function (Name $name): ClassReflection {
             if (! $name instanceof FullyQualified) {
                 throw new FullyQualifiedNameNotAutoloadedException($name);
             }
@@ -193,7 +210,7 @@ private function getParents(Class_ $class): array
      */
     private function getImplementations(Class_ $class): array
     {
-        return array_map(function (Node\Name $name): ClassReflection {
+        return array_map(function (Name $name): ClassReflection {
             if (! $name instanceof FullyQualified) {
                 throw new FullyQualifiedNameNotAutoloadedException($name);
             }
@@ -209,14 +226,14 @@ private function getImplementations(Class_ $class): array
     /**
      * @return ClassReflection[]
      */
-    private function getTraits(Class_ $node): array
+    private function getTraits(Class_ $class): array
     {
         $traits = [];
-        foreach ($node->getTraitUses() as $traitUse) {
+        foreach ($class->getTraitUses() as $traitUse) {
             $traits = [...$traits, ...$traitUse->traits];
         }
 
-        return array_map(function (Node\Name $name): ClassReflection {
+        return array_map(function (Name $name): ClassReflection {
             if (! $name instanceof FullyQualified) {
                 throw new FullyQualifiedNameNotAutoloadedException($name);
             }
@@ -229,8 +246,8 @@ private function getTraits(Class_ $node): array
         }, $traits);
     }
 
-    private function canBeInheritied(Node\Stmt\ClassConst $constNode, Class_ $node): bool
+    private function canBeInheritied(ClassConst $classConst, Class_ $class): bool
     {
-        return ! $node->isFinal() && ! $constNode->isPrivate();
+        return ! $class->isFinal() && ! $classConst->isPrivate();
     }
 }
diff --git a/rules/Strict/NodeFactory/ExactCompareFactory.php b/rules/Strict/NodeFactory/ExactCompareFactory.php
@@ -32,10 +32,18 @@ public function __construct(
     public function createIdenticalFalsyCompare(
         Type $exprType,
         Expr $expr,
-        bool $treatAsNonEmpty
+        bool $treatAsNonEmpty,
+        bool $isOnlyString = true
     ): Identical|BooleanOr|NotIdentical|BooleanNot|Instanceof_|BooleanAnd|null {
         if ($exprType->isString()->yes()) {
-            return new Identical($expr, new String_(''));
+            if ($treatAsNonEmpty || ! $isOnlyString) {
+                return new Identical($expr, new String_(''));
+            }
+
+            return new BooleanOr(
+                new Identical($expr, new String_('')),
+                new Identical($expr, new String_('0'))
+            );
         }
 
         if ($exprType->isInteger()->yes()) {
@@ -58,16 +66,24 @@ public function createIdenticalFalsyCompare(
             return null;
         }
 
-        return $this->createTruthyFromUnionType($exprType, $expr, $treatAsNonEmpty);
+        return $this->createTruthyFromUnionType($exprType, $expr, $treatAsNonEmpty, false);
     }
 
     public function createNotIdenticalFalsyCompare(
         Type $exprType,
         Expr $expr,
-        bool $treatAsNotEmpty
+        bool $treatAsNotEmpty,
+        bool $isOnlyString = true
     ): Identical|Instanceof_|BooleanOr|NotIdentical|BooleanAnd|BooleanNot|null {
         if ($exprType->isString()->yes()) {
-            return new NotIdentical($expr, new String_(''));
+            if ($treatAsNotEmpty || ! $isOnlyString) {
+                return new NotIdentical($expr, new String_(''));
+            }
+
+            return new BooleanAnd(
+                new NotIdentical($expr, new String_('')),
+                new NotIdentical($expr, new String_('0'))
+            );
         }
 
         if ($exprType->isInteger()->yes()) {
@@ -82,13 +98,14 @@ public function createNotIdenticalFalsyCompare(
             return null;
         }
 
-        return $this->createFromUnionType($exprType, $expr, $treatAsNotEmpty);
+        return $this->createFromUnionType($exprType, $expr, $treatAsNotEmpty, false);
     }
 
     private function createFromUnionType(
         UnionType $unionType,
         Expr $expr,
-        bool $treatAsNotEmpty
+        bool $treatAsNotEmpty,
+        bool $isOnlyString
     ): Identical|Instanceof_|BooleanOr|NotIdentical|BooleanAnd|BooleanNot|null {
         $unionType = TypeCombinator::removeNull($unionType);
 
@@ -107,7 +124,7 @@ private function createFromUnionType(
             return $this->resolveFromCleanedNullUnionType($unionType, $expr, $treatAsNotEmpty);
         }
 
-        $compareExpr = $this->createNotIdenticalFalsyCompare($unionType, $expr, $treatAsNotEmpty);
+        $compareExpr = $this->createNotIdenticalFalsyCompare($unionType, $expr, $treatAsNotEmpty, $isOnlyString);
         if (! $compareExpr instanceof Expr) {
             return null;
         }
@@ -208,7 +225,8 @@ private function createBooleanAnd(
     private function createTruthyFromUnionType(
         UnionType $unionType,
         Expr $expr,
-        bool $treatAsNonEmpty
+        bool $treatAsNonEmpty,
+        bool $isOnlyString
     ): BooleanOr|NotIdentical|Identical|BooleanNot|Instanceof_|BooleanAnd|null {
         $unionType = $this->cleanUpPossibleNullableUnionType($unionType);
 
@@ -232,7 +250,7 @@ private function createTruthyFromUnionType(
         }
 
         // assume we have to check empty string, integer and bools
-        $scalarFalsyIdentical = $this->createIdenticalFalsyCompare($unionType, $expr, $treatAsNonEmpty);
+        $scalarFalsyIdentical = $this->createIdenticalFalsyCompare($unionType, $expr, $treatAsNonEmpty, $isOnlyString);
         if (! $scalarFalsyIdentical instanceof Expr) {
             return null;
         }

diff --git a/src/Exception/FullyQualifiedNameNotAutoloadedException.php b/src/Exception/FullyQualifiedNameNotAutoloadedException.php
@@ -7,7 +7,7 @@
 use PhpParser\Node\Name;
 use RuntimeException;
 
-class FullyQualifiedNameNotAutoloadedException extends RuntimeException
+final class FullyQualifiedNameNotAutoloadedException extends RuntimeException
 {
     public function __construct(
         protected Name $name