[DX] Add strict PHPStan rules - step #3 (#1331)

rectorphp · Nov 28, 2021 · 6340d26 · 6340d26
1 parent fa9b4ef
commit 6340d26
Show file tree

Hide file tree

Showing 29 changed files with 83 additions and 99 deletions.
diff --git a/packages/StaticTypeMapper/PhpParser/IntersectionTypeNodeMapper.php b/packages/StaticTypeMapper/PhpParser/IntersectionTypeNodeMapper.php
@@ -24,9 +24,6 @@ public function autowireUnionTypeNodeMapper(PhpParserNodeMapper $phpParserNodeMa
         $this->phpParserNodeMapper = $phpParserNodeMapper;
     }
 
-    /**
-     * @return class-string<Node>
-     */
     public function getNodeType(): string
     {
         return Node\IntersectionType::class;

diff --git a/packages/StaticTypeMapper/PhpParser/NullableTypeNodeMapper.php b/packages/StaticTypeMapper/PhpParser/NullableTypeNodeMapper.php
@@ -31,9 +31,6 @@ public function autowireNullableTypeNodeMapper(PhpParserNodeMapper $phpParserNod
         $this->phpParserNodeMapper = $phpParserNodeMapper;
     }
 
-    /**
-     * @return class-string<Node>
-     */
     public function getNodeType(): string
     {
         return NullableType::class;

diff --git a/packages/StaticTypeMapper/PhpParser/UnionTypeNodeMapper.php b/packages/StaticTypeMapper/PhpParser/UnionTypeNodeMapper.php
@@ -30,9 +30,6 @@ public function autowireUnionTypeNodeMapper(PhpParserNodeMapper $phpParserNodeMa
         $this->phpParserNodeMapper = $phpParserNodeMapper;
     }
 
-    /**
-     * @return class-string<Node>
-     */
     public function getNodeType(): string
     {
         return UnionType::class;

diff --git a/packages/VendorLocker/NodeVendorLocker/PropertyTypeVendorLockResolver.php b/packages/VendorLocker/NodeVendorLocker/PropertyTypeVendorLockResolver.php
@@ -57,7 +57,7 @@ private function isParentClassLocked(ClassReflection $classReflection, string $p
                 continue;
             }
 
-            if ($parentClassReflection->getfileName() === $fileName) {
+            if ($parentClassReflection->getFileName() === $fileName) {
                 continue;
             }
 

diff --git a/rules/Arguments/ArgumentDefaultValueReplacer.php b/rules/Arguments/ArgumentDefaultValueReplacer.php
@@ -102,7 +102,7 @@ private function processArgs(
         } elseif (is_array($replaceArgumentDefaultValue->getValueBefore())) {
             $newArgs = $this->processArrayReplacement($expr->args, $replaceArgumentDefaultValue);
 
-            if ($newArgs) {
+            if (is_array($newArgs)) {
                 $expr->args = $newArgs;
             }
         }

diff --git a/rules/CodeQuality/Rector/Foreach_/SimplifyForeachToCoalescingRector.php b/rules/CodeQuality/Rector/Foreach_/SimplifyForeachToCoalescingRector.php
@@ -129,7 +129,7 @@ private function matchReturnOrAssignNode(Foreach_ $foreach): ?Node
         });
     }
 
-    private function processForeachNodeWithReturnInside(Foreach_ $foreach, Return_ $return): ?Node
+    private function processForeachNodeWithReturnInside(Foreach_ $foreach, Return_ $return): Return_|null
     {
         if (! $this->nodeComparator->areNodesEqual($foreach->valueVar, $return->expr)) {
             return null;
@@ -160,7 +160,7 @@ private function processForeachNodeWithReturnInside(Foreach_ $foreach, Return_ $
         $coalesce = new Coalesce(new ArrayDimFetch(
             $foreach->expr,
             $checkedNode
-        ), $this->return && $this->return->expr !== null ? $this->return->expr : $checkedNode);
+        ), $this->return instanceof Return_ && $this->return->expr !== null ? $this->return->expr : $checkedNode);
 
         if ($this->return !== null) {
             return new Return_($coalesce);

diff --git a/rules/CodingStyle/Naming/ClassNaming.php b/rules/CodingStyle/Naming/ClassNaming.php
@@ -50,7 +50,7 @@ public function getNamespace(string $fullyQualifiedName): ?string
     {
         $fullyQualifiedName = trim($fullyQualifiedName, '\\');
 
-        return Strings::before($fullyQualifiedName, '\\', -1) ?: null;
+        return Strings::before($fullyQualifiedName, '\\', -1);
     }
 
     public function getNameFromFileInfo(SmartFileInfo $smartFileInfo): string

diff --git a/rules/CodingStyle/NodeFactory/JsonArrayFactory.php b/rules/CodingStyle/NodeFactory/JsonArrayFactory.php
@@ -60,7 +60,7 @@ private function replaceNodeObjectHashPlaceholdersWithNodes(Array_ $array, array
 
                 $placeholderNode = $this->matchPlaceholderNode($onlyItem->value, $placeholderNodes);
 
-                if ($placeholderNode && $this->implodeAnalyzer->isImplodeToJson($placeholderNode)) {
+                if ($placeholderNode instanceof Expr && $this->implodeAnalyzer->isImplodeToJson($placeholderNode)) {
                     /**
                      * @var FuncCall $placeholderNode
                      * @var Arg $firstArg

diff --git a/rules/CodingStyle/Rector/Catch_/CatchExceptionNameMatchingTypeRector.php b/rules/CodingStyle/Rector/Catch_/CatchExceptionNameMatchingTypeRector.php
@@ -87,7 +87,7 @@ public function refactor(Node $node): ?Node
         }
 
         $oldVariableName = $this->getName($node->var);
-        if (! $oldVariableName) {
+        if (! is_string($oldVariableName)) {
             return null;
         }
 

diff --git a/rules/DeadCode/PhpDoc/DeadParamTagValueNodeAnalyzer.php b/rules/DeadCode/PhpDoc/DeadParamTagValueNodeAnalyzer.php
@@ -20,14 +20,16 @@
 use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey;
 use Rector\BetterPhpDocParser\ValueObject\Type\BracketsAwareUnionTypeNode;
 use Rector\BetterPhpDocParser\ValueObject\Type\SpacingAwareCallableTypeNode;
+use Rector\DeadCode\TypeNodeAnalyzer\GenericTypeNodeAnalyzer;
 use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
 
 final class DeadParamTagValueNodeAnalyzer
 {
     public function __construct(
         private NodeNameResolver $nodeNameResolver,
-        private TypeComparator $typeComparator
+        private TypeComparator $typeComparator,
+        private GenericTypeNodeAnalyzer $genericTypeNodeAnalyzer,
     ) {
     }
 
@@ -65,7 +67,7 @@ public function isDead(ParamTagValueNode $paramTagValueNode, FunctionLike $funct
             return $this->isEmptyDescription($paramTagValueNode, $param->type);
         }
 
-        if (! $this->hasGenericType($paramTagValueNode->type)) {
+        if (! $this->genericTypeNodeAnalyzer->hasGenericType($paramTagValueNode->type)) {
             return $this->isEmptyDescription($paramTagValueNode, $param->type);
         }
 
@@ -136,23 +138,6 @@ private function isUnionIdentifier(PhpDocTagNode $phpDocTagNode): bool
         return true;
     }
 
-    private function hasGenericType(BracketsAwareUnionTypeNode $bracketsAwareUnionTypeNode): bool
-    {
-        $types = $bracketsAwareUnionTypeNode->types;
-
-        foreach ($types as $type) {
-            if ($type instanceof GenericTypeNode) {
-                if ($type->type instanceof IdentifierTypeNode && $type->type->name === 'array') {
-                    continue;
-                }
-
-                return true;
-            }
-        }
-
-        return false;
-    }
-
     private function matchParamByName(string $desiredParamName, FunctionLike $functionLike): ?Param
     {
         foreach ($functionLike->getParams() as $param) {

diff --git a/rules/DeadCode/PhpDoc/DeadReturnTagValueNodeAnalyzer.php b/rules/DeadCode/PhpDoc/DeadReturnTagValueNodeAnalyzer.php
@@ -9,18 +9,19 @@
 use PhpParser\Node\Stmt\Trait_;
 use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode;
 use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
-use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
 use PHPStan\PhpDocParser\Ast\Type\ThisTypeNode;
 use Rector\BetterPhpDocParser\ValueObject\Type\BracketsAwareUnionTypeNode;
 use Rector\BetterPhpDocParser\ValueObject\Type\SpacingAwareCallableTypeNode;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\DeadCode\TypeNodeAnalyzer\GenericTypeNodeAnalyzer;
 use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
 
 final class DeadReturnTagValueNodeAnalyzer
 {
     public function __construct(
         private TypeComparator $typeComparator,
         private BetterNodeFinder $betterNodeFinder,
+        private GenericTypeNodeAnalyzer $genericTypeNodeAnalyzer,
     ) {
     }
 
@@ -55,27 +56,10 @@ public function isDead(ReturnTagValueNode $returnTagValueNode, FunctionLike $fun
             return $returnTagValueNode->description === '';
         }
 
-        if (! $this->hasGenericType($returnTagValueNode->type)) {
+        if (! $this->genericTypeNodeAnalyzer->hasGenericType($returnTagValueNode->type)) {
             return $returnTagValueNode->description === '';
         }
 
         return false;
     }
-
-    private function hasGenericType(BracketsAwareUnionTypeNode $bracketsAwareUnionTypeNode): bool
-    {
-        $types = $bracketsAwareUnionTypeNode->types;
-
-        foreach ($types as $type) {
-            if ($type instanceof GenericTypeNode) {
-                if ($type->type instanceof IdentifierTypeNode && $type->type->name === 'array') {
-                    continue;
-                }
-
-                return true;
-            }
-        }
-
-        return false;
-    }
 }
diff --git a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php
@@ -125,7 +125,11 @@ private function shouldSkip(Assign $assign): bool
             return true;
         }
 
-        return $variable->name instanceof Variable && $this->betterNodeFinder->findFirstNext(
+        if (! $variable->name instanceof Variable) {
+            return false;
+        }
+
+        return (bool) $this->betterNodeFinder->findFirstNext(
             $assign,
             fn (Node $node): bool => $node instanceof Variable
         );

diff --git a/rules/DeadCode/Rector/ClassMethod/RemoveDelegatingParentCallRector.php b/rules/DeadCode/Rector/ClassMethod/RemoveDelegatingParentCallRector.php
@@ -163,16 +163,17 @@ private function shouldSkipWithAnnotationsOrAttributes(ClassMethod $classMethod)
 
     private function matchClassMethodOnlyStmt(ClassMethod $classMethod): null | Stmt | Expr
     {
-        if ($classMethod->stmts === null) {
+        $classMethodStmts = $classMethod->stmts;
+        if ($classMethodStmts === null) {
             return null;
         }
 
-        if (count((array) $classMethod->stmts) !== 1) {
+        if (count($classMethodStmts) !== 1) {
             return null;
         }
 
         // recount empty notes
-        $stmtsValues = array_values($classMethod->stmts);
+        $stmtsValues = array_values($classMethodStmts);
 
         return $this->unwrapExpression($stmtsValues[0]);
     }

diff --git a/rules/DeadCode/Rector/FunctionLike/RemoveOverriddenValuesRector.php b/rules/DeadCode/Rector/FunctionLike/RemoveOverriddenValuesRector.php
@@ -151,7 +151,7 @@ private function getNodeNames(array $nodes): array
         $nodeNames = [];
         foreach ($nodes as $node) {
             $nodeName = $this->getName($node);
-            if ($nodeName) {
+            if (is_string($nodeName)) {
                 $nodeNames[] = $nodeName;
             }
         }

diff --git a/rules/DeadCode/Rector/If_/UnwrapFutureCompatibleIfPhpVersionRector.php b/rules/DeadCode/Rector/If_/UnwrapFutureCompatibleIfPhpVersionRector.php
@@ -65,7 +65,7 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?array
     {
-        if ($node->elseifs) {
+        if ($node->elseifs !== []) {
             return null;
         }
 
@@ -92,7 +92,7 @@ public function refactor(Node $node): ?array
      */
     private function refactorIsMatch(If_ $if): ?array
     {
-        if ($if->elseifs) {
+        if ($if->elseifs !== []) {
             return null;
         }
 

diff --git a/rules/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector.php b/rules/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector.php
@@ -4,7 +4,6 @@
 
 namespace Rector\DeadCode\Rector\Node;
 
-use PhpParser\Comment;
 use PhpParser\Node;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\AssignRef;
@@ -121,7 +120,7 @@ public function refactor(Node $node): ?Node
         }
 
         $comments = $node->getComments();
-        if (isset($comments[1]) && $comments[1] instanceof Comment) {
+        if (isset($comments[1])) {
             $this->commentRemover->rollbackComments($node, $comments[1]);
             return $node;
         }

diff --git a/rules/DeadCode/Rector/PropertyProperty/RemoveNullPropertyInitializationRector.php b/rules/DeadCode/Rector/PropertyProperty/RemoveNullPropertyInitializationRector.php
@@ -61,7 +61,7 @@ public function refactor(Node $node): ?Node
         $parent = $node->getAttribute(AttributeKey::PARENT_NODE);
 
         // skip typed properties
-        if ($parent instanceof Property && $parent->type) {
+        if ($parent instanceof Property && $parent->type !== null) {
             return null;
         }
 

diff --git a/rules/DeadCode/Rector/Switch_/RemoveDuplicatedCaseInSwitchRector.php b/rules/DeadCode/Rector/Switch_/RemoveDuplicatedCaseInSwitchRector.php
@@ -85,7 +85,7 @@ public function refactor(Node $node): ?Node
         /** @var Case_|null $previousCase */
         $previousCase = null;
         foreach ($node->cases as $case) {
-            if ($previousCase && $this->areSwitchStmtsEqualsAndWithBreak($case, $previousCase)) {
+            if ($previousCase instanceof Case_ && $this->areSwitchStmtsEqualsAndWithBreak($case, $previousCase)) {
                 $previousCase->stmts = [];
             }
 

diff --git a/rules/DeadCode/TypeNodeAnalyzer/GenericTypeNodeAnalyzer.php b/rules/DeadCode/TypeNodeAnalyzer/GenericTypeNodeAnalyzer.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\DeadCode\TypeNodeAnalyzer;
+
+use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
+use PHPStan\PhpDocParser\Ast\Type\UnionTypeNode;
+
+final class GenericTypeNodeAnalyzer
+{
+    public function hasGenericType(UnionTypeNode $unionTypeNode): bool
+    {
+        $types = $unionTypeNode->types;
+
+        foreach ($types as $type) {
+            if ($type instanceof GenericTypeNode) {
+                if ($type->type->name === 'array') {
+                    continue;
+                }
+
+                return true;
+            }
+        }
+
+        return false;
+    }
+}
diff --git a/rules/DowngradePhp72/Rector/FuncCall/DowngradePregUnmatchedAsNullConstantRector.php b/rules/DowngradePhp72/Rector/FuncCall/DowngradePregUnmatchedAsNullConstantRector.php
@@ -232,17 +232,9 @@ private function cleanBitwiseOrFlags(FuncCall $funcCall, BitwiseOr $bitwiseOr, E
 
     private function assignThirdArgsValue(FuncCall $funcCall, BitwiseOr $bitwiseOr): void
     {
-        if ($bitwiseOr instanceof BitwiseOr && $bitwiseOr->right instanceof ConstFetch && $this->isName(
-            $bitwiseOr->right,
-            self::FLAG
-        )) {
+        if ($bitwiseOr->right instanceof ConstFetch && $this->isName($bitwiseOr->right, self::FLAG)) {
             $bitwiseOr = $bitwiseOr->left;
-        }
-
-        if ($bitwiseOr instanceof BitwiseOr && $bitwiseOr->left instanceof ConstFetch && $this->isName(
-            $bitwiseOr->left,
-            self::FLAG
-        )) {
+        } elseif ($bitwiseOr->left instanceof ConstFetch && $this->isName($bitwiseOr->left, self::FLAG)) {
             $bitwiseOr = $bitwiseOr->right;
         }
 

diff --git a/rules/DowngradePhp74/Rector/Array_/DowngradeArraySpreadRector.php b/rules/DowngradePhp74/Rector/Array_/DowngradeArraySpreadRector.php
@@ -216,11 +216,8 @@ private function createArrayMerge(Array_ $array, array $items): FuncCall
      * as to invoke it only once and avoid potential bugs,
      * such as a method executing some side-effect
      */
-    private function createVariableFromNonVariable(
-        Array_ $array,
-        ArrayItem $arrayItem,
-        int | string $position
-    ): Variable {
+    private function createVariableFromNonVariable(Array_ $array, ArrayItem $arrayItem, int $position): Variable
+    {
         /** @var Scope $nodeScope */
         $nodeScope = $array->getAttribute(AttributeKey::SCOPE);
         // The variable name will be item0Unpacked, item1Unpacked, etc,
@@ -277,18 +274,16 @@ private function createArgFromSpreadArrayItem(Scope $nodeScope, ArrayItem $array
 
         $iteratorToArrayFuncCall = new FuncCall(new Name('iterator_to_array'), [new Arg($arrayItem)]);
 
-        if ($type !== null) {
-            // If we know it is an array, then print it directly
-            // Otherwise PHPStan throws an error:
-            // "Else branch is unreachable because ternary operator condition is always true."
-            if ($type instanceof ArrayType) {
-                return new Arg($arrayItem);
-            }
+        // If we know it is an array, then print it directly
+        // Otherwise PHPStan throws an error:
+        // "Else branch is unreachable because ternary operator condition is always true."
+        if ($type instanceof ArrayType) {
+            return new Arg($arrayItem);
+        }
 
-            // If it is iterable, then directly return `iterator_to_array`
-            if ($this->isIterableType($type)) {
-                return new Arg($iteratorToArrayFuncCall);
-            }
+        // If it is iterable, then directly return `iterator_to_array`
+        if ($this->isIterableType($type)) {
+            return new Arg($iteratorToArrayFuncCall);
         }
 
         // Print a ternary, handling either an array or an iterator

diff --git a/rules/DowngradePhp74/Rector/ClassMethod/DowngradeCovariantReturnTypeRector.php b/rules/DowngradePhp74/Rector/ClassMethod/DowngradeCovariantReturnTypeRector.php
@@ -112,7 +112,7 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        $isAlreadyDowngraded = $node->getAttribute(self::ALREADY_DOWNGRADED);
+        $isAlreadyDowngraded = (bool) $node->getAttribute(self::ALREADY_DOWNGRADED, false);
         if ($isAlreadyDowngraded) {
             return null;
         }

diff --git a/rules/EarlyReturn/NodeFactory/InvertedIfFactory.php b/rules/EarlyReturn/NodeFactory/InvertedIfFactory.php
@@ -31,7 +31,8 @@ public function createFromConditions(If_ $if, array $conditions, Return_ $return
     {
         $ifs = [];
         $ifNextReturn = $this->getIfNextReturn($if);
-        $stmt = $this->contextAnalyzer->isInLoop($if) && ! $ifNextReturn
+
+        $stmt = $this->contextAnalyzer->isInLoop($if) && ! $ifNextReturn instanceof Return_
             ? [new Continue_()]
             : [$return];