[DeadCode] Skip standalone @return false or true on RemoveUselessRetu…

…rnTagRector (#3202) * [DeadCode] Skip standalone @return false or true on RemoveUselessReturnTagRector * Fixed 🎉 * [ci-review] Rector Rectify * [ci-review] Rector Rectify * Fix phpstan Co-authored-by: GitHub Action <action@github.com>
rectorphp · Dec 15, 2022 · 0d6234b · 0d6234b
1 parent 40f6d19
commit 0d6234b
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 15 deletions.
diff --git a/...veUselessReturnTagRector/Fixture/skip_return_standalone_true_false_by_return_bool.php.inc b/...veUselessReturnTagRector/Fixture/skip_return_standalone_true_false_by_return_bool.php.inc
@@ -0,0 +1,25 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveUselessReturnTagRector\Fixture;
+
+/**
+ * @see https://phpstan.org/r/6fbad07c-4b5e-4d89-8970-03df898abc6e
+ */
+final class SkipReturnStandaloneTrueFalseByReturnBool
+{
+    /**
+     * @return true
+     */
+    function someTrue(): bool
+    {
+        return true;
+    }
+
+    /**
+     * @return false
+     */
+    function someFalse(): bool
+    {
+        return false;
+    }
+}
diff --git a/rules/DeadCode/PhpDoc/DeadReturnTagValueNodeAnalyzer.php b/rules/DeadCode/PhpDoc/DeadReturnTagValueNodeAnalyzer.php
@@ -4,6 +4,7 @@
 
 namespace Rector\DeadCode\PhpDoc;
 
+use PhpParser\Node;
 use PhpParser\Node\FunctionLike;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\Trait_;
@@ -13,6 +14,7 @@
 use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
 use Rector\BetterPhpDocParser\ValueObject\Type\BracketsAwareUnionTypeNode;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\DeadCode\PhpDoc\Guard\StandaloneTypeRemovalGuard;
 use Rector\DeadCode\TypeNodeAnalyzer\GenericTypeNodeAnalyzer;
 use Rector\DeadCode\TypeNodeAnalyzer\MixedArrayTypeNodeAnalyzer;
 use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
@@ -24,6 +26,7 @@ public function __construct(
         private readonly BetterNodeFinder $betterNodeFinder,
         private readonly GenericTypeNodeAnalyzer $genericTypeNodeAnalyzer,
         private readonly MixedArrayTypeNodeAnalyzer $mixedArrayTypeNodeAnalyzer,
+        private readonly StandaloneTypeRemovalGuard $standaloneTypeRemovalGuard
     ) {
     }
 
@@ -52,7 +55,7 @@ public function isDead(ReturnTagValueNode $returnTagValueNode, FunctionLike $fun
         }
 
         if (! $returnTagValueNode->type instanceof BracketsAwareUnionTypeNode) {
-            return $returnTagValueNode->description === '';
+            return $this->isIdentiferRemovalAllowed($returnTagValueNode, $returnType);
         }
 
         if ($this->genericTypeNodeAnalyzer->hasGenericType($returnTagValueNode->type)) {
@@ -70,6 +73,15 @@ public function isDead(ReturnTagValueNode $returnTagValueNode, FunctionLike $fun
         return $returnTagValueNode->description === '';
     }
 
+    private function isIdentiferRemovalAllowed(ReturnTagValueNode $returnTagValueNode, Node $node): bool
+    {
+        if ($returnTagValueNode->description === '') {
+            return $this->standaloneTypeRemovalGuard->isLegal($returnTagValueNode->type, $node);
+        }
+
+        return false;
+    }
+
     private function hasTruePseudoType(BracketsAwareUnionTypeNode $bracketsAwareUnionTypeNode): bool
     {
         $unionTypes = $bracketsAwareUnionTypeNode->types;

diff --git a/rules/DeadCode/PhpDoc/Guard/StandaloneTypeRemovalGuard.php b/rules/DeadCode/PhpDoc/Guard/StandaloneTypeRemovalGuard.php
@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\DeadCode\PhpDoc\Guard;
+
+use PhpParser\Node;
+use PhpParser\Node\Identifier;
+use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
+use PHPStan\PhpDocParser\Ast\Type\TypeNode;
+
+final class StandaloneTypeRemovalGuard
+{
+    /**
+     * @var string[]
+     */
+    private const ALLOWED_TYPES = ['false', 'true'];
+
+    public function isLegal(TypeNode $typeNode, Node $node): bool
+    {
+        if (! $typeNode instanceof IdentifierTypeNode) {
+            return true;
+        }
+
+        if (! $node instanceof Identifier) {
+            return true;
+        }
+
+        if ($node->toString() !== 'bool') {
+            return true;
+        }
+
+        return ! in_array($typeNode->name, self::ALLOWED_TYPES, true);
+    }
+}
diff --git a/rules/Php81/NodeFactory/EnumFactory.php b/rules/Php81/NodeFactory/EnumFactory.php
@@ -5,12 +5,14 @@
 namespace Rector\Php81\NodeFactory;
 
 use PhpParser\BuilderFactory;
-use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\ArrayItem;
 use PhpParser\Node\Identifier;
 use PhpParser\Node\Scalar\LNumber;
 use PhpParser\Node\Scalar\String_;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassConst;
+use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Enum_;
 use PhpParser\Node\Stmt\EnumCase;
 use PhpParser\Node\Stmt\Return_;
@@ -119,24 +121,46 @@ private function createEnumCaseFromDocComment(PhpDocTagNode $phpDocTagNode, arra
     private function generateMappingFromClass(Class_ $class): array
     {
         $classMethod = $class->getMethod('values');
-        if ($classMethod === null) {
+        if (! $classMethod instanceof ClassMethod) {
             return [];
         }
 
+        /** @var Return_[] $returns */
         $returns = $this->betterNodeFinder->findInstancesOfInFunctionLikeScoped($classMethod, Return_::class);
+        /** @var array<int|string, mixed> $mapping */
         $mapping = [];
-        foreach ($returns as $returnStmt) {
-            if (! ($returnStmt->expr instanceof Expr\Array_)) {
+        foreach ($returns as $return) {
+            if (! ($return->expr instanceof Array_)) {
                 continue;
             }
-            foreach ($returnStmt->expr->items as $item) {
-                if ($item instanceof Expr\ArrayItem
-                    && ($item->key instanceof LNumber || $item->key instanceof String_)
-                    && ($item->value instanceof LNumber || $item->value instanceof String_)
-                ) {
-                    $mapping[$item->key->value] = $item->value->value;
-                }
+
+            $mapping = $this->collectMappings($return->expr->items, $mapping);
+        }
+
+        return $mapping;
+    }
+
+    /**
+     * @param null[]|ArrayItem[] $items
+     * @param array<int|string, mixed> $mapping
+     * @return array<int|string, mixed>
+     */
+    private function collectMappings(array $items, array $mapping): array
+    {
+        foreach ($items as $item) {
+            if (! $item instanceof ArrayItem) {
+                continue;
+            }
+
+            if (! $item->key instanceof LNumber && ! $item->key instanceof String_) {
+                continue;
             }
+
+            if (! $item->value instanceof LNumber && ! $item->value instanceof String_) {
+                continue;
+            }
+
+            $mapping[$item->key->value] = $item->value->value;
         }
 
         return $mapping;
@@ -147,9 +171,8 @@ private function generateMappingFromClass(Class_ $class): array
      */
     private function getIdentifierTypeFromMappings(array $mapping): string
     {
-        $valueTypes = array_map(static function ($value): string {
-            return gettype($value);
-        }, $mapping);
+        $callableGetType = static fn ($value): string => gettype($value);
+        $valueTypes = array_map($callableGetType, $mapping);
         $uniqueValueTypes = array_unique($valueTypes);
         if (count($uniqueValueTypes) === 1) {
             $identifierType = reset($uniqueValueTypes);