[DeadCode] Fix remove dead stmt Rector (#65)

Co-authored-by: Sebastian Schreiber <me@schreibersebastian.de> Co-authored-by: kaizen-ci <info@kaizen-ci.org>
rectorphp · May 17, 2021 · 92358f7 · 92358f7
1 parent ee07086
commit 92358f7
Show file tree

Hide file tree

Showing 49 changed files with 266 additions and 228 deletions.
diff --git a/packages/BetterPhpDocParser/Printer/PhpDocInfoPrinter.php b/packages/BetterPhpDocParser/Printer/PhpDocInfoPrinter.php
@@ -20,6 +20,7 @@
 use Rector\BetterPhpDocParser\PhpDocNodeVisitor\ChangedPhpDocNodeVisitor;
 use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey;
 use Rector\BetterPhpDocParser\ValueObject\StartAndEnd;
+use Rector\Core\Exception\ShouldNotHappenException;
 use Symplify\SimplePhpDocParser\PhpDocNodeTraverser;
 
 /**
@@ -62,35 +63,18 @@ final class PhpDocInfoPrinter
      */
     private const TAG_AND_SPACE_REGEX = '#(@.*?) \(#';
 
-    /**
-     * @var int
-     */
-    private $tokenCount;
+    private int $tokenCount = 0;
 
-    /**
-     * @var int
-     */
-    private $currentTokenPosition;
+    private int $currentTokenPosition = 0;
 
     /**
      * @var mixed[]
      */
-    private $tokens = [];
+    private array $tokens = [];
 
-    /**
-     * @var PhpDocNode
-     */
-    private $phpDocNode;
-
-    /**
-     * @var PhpDocInfo
-     */
-    private $phpDocInfo;
+    private ?PhpDocInfo $phpDocInfo = null;
 
-    /**
-     * @var PhpDocNodeTraverser
-     */
-    private $changedPhpDocNodeTraverser;
+    private PhpDocNodeTraverser $changedPhpDocNodeTraverser;
 
     public function __construct(
         private EmptyPhpDocDetector $emptyPhpDocDetector,
@@ -133,20 +117,29 @@ public function printFormatPreserving(PhpDocInfo $phpDocInfo): string
             return (string) $phpDocInfo->getPhpDocNode();
         }
 
-        $this->phpDocNode = $phpDocInfo->getPhpDocNode();
+        $phpDocNode = $phpDocInfo->getPhpDocNode();
         $this->tokens = $phpDocInfo->getTokens();
 
         $this->tokenCount = $phpDocInfo->getTokenCount();
         $this->phpDocInfo = $phpDocInfo;
 
         $this->currentTokenPosition = 0;
 
-        $phpDocString = $this->printPhpDocNode($this->phpDocNode);
+        $phpDocString = $this->printPhpDocNode($phpDocNode);
 
         // hotfix of extra space with callable ()
         return Strings::replace($phpDocString, self::CALLABLE_REGEX, 'callable(');
     }
 
+    public function getCurrentPhpDocInfo(): PhpDocInfo
+    {
+        if ($this->phpDocInfo === null) {
+            throw new ShouldNotHappenException();
+        }
+
+        return $this->phpDocInfo;
+    }
+
     private function printPhpDocNode(PhpDocNode $phpDocNode): string
     {
         // no nodes were, so empty doc
@@ -242,9 +235,9 @@ private function printDocChildNode(
 
     private function printEnd(string $output): string
     {
-        $lastTokenPosition = $this->phpDocNode->getAttribute(
-            PhpDocAttributeKey::LAST_PHP_DOC_TOKEN_POSITION
-        ) ?: $this->currentTokenPosition;
+        $lastTokenPosition = $this->getCurrentPhpDocInfo()
+            ->getPhpDocNode()
+            ->getAttribute(PhpDocAttributeKey::LAST_PHP_DOC_TOKEN_POSITION) ?: $this->currentTokenPosition;
         if ($lastTokenPosition === 0) {
             $lastTokenPosition = 1;
         }
@@ -258,8 +251,10 @@ private function addTokensFromTo(string $output, int $from, int $to, bool $shoul
         $positionJumpSet = [];
 
         $removedStartAndEnds = $this->removeNodesStartAndEndResolver->resolve(
-            $this->phpDocInfo->getOriginalPhpDocNode(),
-            $this->phpDocNode,
+            $this->getCurrentPhpDocInfo()
+                ->getOriginalPhpDocNode(),
+            $this->getCurrentPhpDocInfo()
+                ->getPhpDocNode(),
             $this->tokens
         );
 
@@ -312,7 +307,8 @@ private function correctPreviouslyReprintedFirstNode(int $key, StartAndEnd $star
 
         $startTokenPosition = $startAndEnd->getStart();
 
-        $tokens = $this->phpDocInfo->getTokens();
+        $tokens = $this->getCurrentPhpDocInfo()
+            ->getTokens();
         if (! isset($tokens[$startTokenPosition - 1])) {
             return;
         }
@@ -333,7 +329,7 @@ private function shouldReprint(PhpDocChildNode $phpDocChildNode): bool
 
     private function standardPrintPhpDocChildNode(PhpDocChildNode $phpDocChildNode): string
     {
-        if ($this->phpDocInfo->isSingleLine()) {
+        if ($this->getCurrentPhpDocInfo()->isSingleLine()) {
             return ' ' . $phpDocChildNode;
         }
 

diff --git a/packages/FamilyTree/NodeAnalyzer/PropertyUsageAnalyzer.php b/packages/FamilyTree/NodeAnalyzer/PropertyUsageAnalyzer.php
@@ -59,9 +59,7 @@ public function isPropertyFetchedInChildClass(Property $property): bool
 
             $isPropertyFetched = (bool) $this->betterNodeFinder->findFirst(
                 $childClass->stmts,
-                function (Node $node) use ($propertyName): bool {
-                    return $this->nodeNameResolver->isLocalPropertyFetchNamed($node, $propertyName);
-                }
+                fn (Node $node): bool => $this->nodeNameResolver->isLocalPropertyFetchNamed($node, $propertyName)
             );
 
             if ($isPropertyFetched) {

diff --git a/packages/NodeNestingScope/ScopeNestingComparator.php b/packages/NodeNestingScope/ScopeNestingComparator.php
@@ -79,19 +79,19 @@ public function isInBothIfElseBranch(Node $foundParentNode, Expr $seekedExpr): b
             return false;
         }
 
-        $foundIfNode = $this->betterNodeFinder->find($foundParentNode->stmts, function ($node) use ($seekedExpr): bool {
-            return $this->nodeComparator->areNodesEqual($node, $seekedExpr);
-        });
+        $foundIfNode = $this->betterNodeFinder->find(
+            $foundParentNode->stmts,
+            fn ($node): bool => $this->nodeComparator->areNodesEqual($node, $seekedExpr)
+        );
 
         if ($foundParentNode->else === null) {
             return false;
         }
 
-        $foundElseNode = $this->betterNodeFinder->find($foundParentNode->else, function ($node) use (
-            $seekedExpr
-        ): bool {
-            return $this->nodeComparator->areNodesEqual($node, $seekedExpr);
-        });
+        $foundElseNode = $this->betterNodeFinder->find(
+            $foundParentNode->else,
+            fn ($node): bool => $this->nodeComparator->areNodesEqual($node, $seekedExpr)
+        );
 
         if ($foundIfNode && $foundElseNode) {
             $this->doubleIfBranchExprs[] = $seekedExpr;

diff --git a/phpstan.neon b/phpstan.neon
@@ -472,3 +472,8 @@ parameters:
 
         # fixed in dev-main
         - '#Class like namespace "Rector\\(.*?)" does not follow PSR\-4 configuration in composer\.json#'
+
+        # deserves better architecture
+        -
+            message: '#Do not use chained method calls#'
+            path: packages/BetterPhpDocParser/Printer/PhpDocInfoPrinter.php
diff --git a/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php b/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php
@@ -78,9 +78,10 @@ public function refactor(Node $node): ?Node
             }
 
             if (! $node instanceof Assign) {
-                return ! (bool) $this->betterNodeFinder->find($node, function (Node $n) use ($arrayVariable) {
-                    return $this->nodeComparator->areNodesEqual($arrayVariable, $n);
-                });
+                return ! (bool) $this->betterNodeFinder->find(
+                    $node,
+                    fn (Node $n): bool => $this->nodeComparator->areNodesEqual($arrayVariable, $n)
+                );
             }
 
             if (! $this->nodeComparator->areNodesEqual($arrayVariable, $node->var)) {

diff --git a/rules/CodeQuality/Rector/Identical/GetClassToInstanceOfRector.php b/rules/CodeQuality/Rector/Identical/GetClassToInstanceOfRector.php
@@ -63,12 +63,8 @@ public function refactor(Node $node): ?Node
     {
         $twoNodeMatch = $this->binaryOpManipulator->matchFirstAndSecondConditionNode(
             $node,
-            function (Node $node): bool {
-                return $this->isClassReference($node);
-            },
-            function (Node $node): bool {
-                return $this->isGetClassFuncCallNode($node);
-            }
+            fn (Node $node): bool => $this->isClassReference($node),
+            fn (Node $node): bool => $this->isGetClassFuncCallNode($node)
         );
 
         if (! $twoNodeMatch instanceof TwoNodeMatch) {

diff --git a/rules/CodeQuality/Rector/Identical/SimplifyArraySearchRector.php b/rules/CodeQuality/Rector/Identical/SimplifyArraySearchRector.php
@@ -61,9 +61,7 @@ function (Node $node): bool {
 
                 return $this->nodeNameResolver->isName($node, 'array_search');
             },
-            function (Node $node): bool {
-                return $this->valueResolver->isFalse($node);
-            }
+            fn (Node $node): bool => $this->valueResolver->isFalse($node)
         );
 
         if (! $twoNodeMatch instanceof TwoNodeMatch) {

diff --git a/rules/CodeQuality/Rector/Identical/SimplifyConditionsRector.php b/rules/CodeQuality/Rector/Identical/SimplifyConditionsRector.php
@@ -73,12 +73,8 @@ private function processIdenticalAndNotIdentical(Identical $identical): ?Node
     {
         $twoNodeMatch = $this->binaryOpManipulator->matchFirstAndSecondConditionNode(
             $identical,
-            function (Node $binaryOp): bool {
-                return $binaryOp instanceof Identical || $binaryOp instanceof NotIdentical;
-            },
-            function (Node $binaryOp): bool {
-                return $this->valueResolver->isTrueOrFalse($binaryOp);
-            }
+            fn (Node $binaryOp): bool => $binaryOp instanceof Identical || $binaryOp instanceof NotIdentical,
+            fn (Node $binaryOp): bool => $this->valueResolver->isTrueOrFalse($binaryOp)
         );
 
         if (! $twoNodeMatch instanceof TwoNodeMatch) {

diff --git a/rules/CodeQuality/Rector/Ternary/SimplifyTautologyTernaryRector.php b/rules/CodeQuality/Rector/Ternary/SimplifyTautologyTernaryRector.php
@@ -53,12 +53,8 @@ public function refactor(Node $node): ?Node
 
         $twoNodeMatch = $this->binaryOpManipulator->matchFirstAndSecondConditionNode(
             $node->cond,
-            function (Node $leftNode) use ($node): bool {
-                return $this->nodeComparator->areNodesEqual($leftNode, $node->if);
-            },
-            function (Node $leftNode) use ($node): bool {
-                return $this->nodeComparator->areNodesEqual($leftNode, $node->else);
-            }
+            fn (Node $leftNode): bool => $this->nodeComparator->areNodesEqual($leftNode, $node->if),
+            fn (Node $leftNode): bool => $this->nodeComparator->areNodesEqual($leftNode, $node->else)
         );
 
         if (! $twoNodeMatch instanceof TwoNodeMatch) {

diff --git a/rules/CodeQualityStrict/Rector/Variable/MoveVariableDeclarationNearReferenceRector.php b/rules/CodeQualityStrict/Rector/Variable/MoveVariableDeclarationNearReferenceRector.php
@@ -136,9 +136,10 @@ private function isUsedAsArraykeyOrInsideIfCondition(Expression $expression, Var
 
     private function hasPropertyInExpr(Expression $expression, Expr $expr): bool
     {
-        return (bool) $this->betterNodeFinder->findFirst($expr, function (Node $node): bool {
-            return $node instanceof PropertyFetch || $node instanceof StaticPropertyFetch;
-        });
+        return (bool) $this->betterNodeFinder->findFirst(
+            $expr,
+            fn (Node $node): bool => $node instanceof PropertyFetch || $node instanceof StaticPropertyFetch
+        );
     }
 
     private function shouldSkipReAssign(Expression $expression, Assign $assign): bool
@@ -204,11 +205,10 @@ private function isUsedAsArrayKey(?Node $node, Variable $variable): bool
                 continue;
             }
 
-            $isFoundInKey = (bool) $this->betterNodeFinder->findFirst($dim, function (Node $node) use (
-                $variable
-            ): bool {
-                return $this->nodeComparator->areNodesEqual($node, $variable);
-            });
+            $isFoundInKey = (bool) $this->betterNodeFinder->findFirst(
+                $dim,
+                fn (Node $node): bool => $this->nodeComparator->areNodesEqual($node, $variable)
+            );
             if ($isFoundInKey) {
                 return true;
             }
@@ -228,9 +228,7 @@ private function isInsideCondition(Expression $expression): bool
     private function hasReAssign(Expression $expression, Expr $expr): bool
     {
         $next = $expression->getAttribute(AttributeKey::NEXT_NODE);
-        $exprValues = $this->betterNodeFinder->find($expr, function (Node $node): bool {
-            return $node instanceof Variable;
-        });
+        $exprValues = $this->betterNodeFinder->find($expr, fn (Node $node): bool => $node instanceof Variable);
 
         if ($exprValues === []) {
             return false;

diff --git a/rules/CodingStyle/Rector/Catch_/CatchExceptionNameMatchingTypeRector.php b/rules/CodingStyle/Rector/Catch_/CatchExceptionNameMatchingTypeRector.php
@@ -109,11 +109,10 @@ function (array $matches): string {
         }
 
         $newVariable = new Variable($newVariableName);
-        $isFoundInPrevious = (bool) $this->betterNodeFinder->findFirstPrevious($node, function (Node $n) use (
-            $newVariable
-        ): bool {
-            return $this->nodeComparator->areNodesEqual($n, $newVariable);
-        });
+        $isFoundInPrevious = (bool) $this->betterNodeFinder->findFirstPrevious(
+            $node,
+            fn (Node $n): bool => $this->nodeComparator->areNodesEqual($n, $newVariable)
+        );
 
         if ($isFoundInPrevious) {
             return null;

diff --git a/rules/DeadCode/NodeCollector/NodeByTypeAndPositionCollector.php b/rules/DeadCode/NodeCollector/NodeByTypeAndPositionCollector.php
@@ -80,9 +80,7 @@ private function sortByStart(array $nodesByTypeAndPosition): array
     {
         usort(
             $nodesByTypeAndPosition,
-            function (VariableNodeUse $firstVariableNodeUse, VariableNodeUse $secondVariableNodeUse): int {
-                return $firstVariableNodeUse->getStartTokenPosition() <=> $secondVariableNodeUse->getStartTokenPosition();
-            }
+            fn (VariableNodeUse $firstVariableNodeUse, VariableNodeUse $secondVariableNodeUse): int => $firstVariableNodeUse->getStartTokenPosition() <=> $secondVariableNodeUse->getStartTokenPosition()
         );
         return $nodesByTypeAndPosition;
     }

diff --git a/rules/DeadCode/Rector/Array_/RemoveDuplicatedArrayKeyRector.php b/rules/DeadCode/Rector/Array_/RemoveDuplicatedArrayKeyRector.php
@@ -94,12 +94,8 @@ private function getArrayItemsWithDuplicatedKey(Array_ $array): array
     private function filterItemsWithSameKey(array $arrayItemsByKeys): array
     {
         /** @var ArrayItem[][] $arrayItemsByKeys */
-        $arrayItemsByKeys = array_filter($arrayItemsByKeys, function (array $arrayItems): bool {
-            return count($arrayItems) > 1;
-        });
+        $arrayItemsByKeys = array_filter($arrayItemsByKeys, fn (array $arrayItems): bool => count($arrayItems) > 1);
 
-        return array_filter($arrayItemsByKeys, function (array $arrayItems): bool {
-            return count($arrayItems) > 1;
-        });
+        return array_filter($arrayItemsByKeys, fn (array $arrayItems): bool => count($arrayItems) > 1);
     }
 }
diff --git a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php
@@ -125,19 +125,16 @@ private function shouldSkip(Assign $assign): bool
 
         return $variable->name instanceof Variable && $this->betterNodeFinder->findFirstNext(
             $assign,
-            function (Node $node): bool {
-                return $node instanceof Variable;
-            }
+            fn (Node $node): bool => $node instanceof Variable
         );
     }
 
     private function isUsed(Assign $assign, Variable $variable): bool
     {
-        $isUsedPrev = (bool) $this->betterNodeFinder->findFirstPreviousOfNode($variable, function (Node $node) use (
-            $variable
-        ): bool {
-            return $this->usedVariableNameAnalyzer->isVariableNamed($node, $variable);
-        });
+        $isUsedPrev = (bool) $this->betterNodeFinder->findFirstPreviousOfNode(
+            $variable,
+            fn (Node $node): bool => $this->usedVariableNameAnalyzer->isVariableNamed($node, $variable)
+        );
 
         if ($isUsedPrev) {
             return true;
@@ -184,14 +181,13 @@ private function isUsedInAssignExpr(Expr $expr, Assign $assign): bool
                 continue;
             }
 
-            $previousAssign = $this->betterNodeFinder->findFirstPreviousOfNode($assign, function (Node $node) use (
+            $previousAssign = $this->betterNodeFinder->findFirstPreviousOfNode(
+                $assign,
+                fn (Node $node): bool => $node instanceof Assign && $this->usedVariableNameAnalyzer->isVariableNamed(
+                $node->var,
                 $variable
-            ): bool {
-                return $node instanceof Assign && $this->usedVariableNameAnalyzer->isVariableNamed(
-                    $node->var,
-                    $variable
-                );
-            });
+            )
+            );
 
             if ($previousAssign instanceof Assign) {
                 return $this->isUsed($assign, $variable);

diff --git a/rules/DeadCode/Rector/Expression/RemoveDeadStmtRector.php b/rules/DeadCode/Rector/Expression/RemoveDeadStmtRector.php
@@ -58,6 +58,10 @@ public function refactor(Node $node)
             return $this->removeNodeAndKeepComments($node);
         }
 
+        if ($livingCode === [$node->expr]) {
+            return null;
+        }
+
         $firstExpr = array_shift($livingCode);
         $node->expr = $firstExpr;
 

diff --git a/rules/DeadCode/Rector/Foreach_/RemoveUnusedForeachKeyRector.php b/rules/DeadCode/Rector/Foreach_/RemoveUnusedForeachKeyRector.php
@@ -55,11 +55,10 @@ public function refactor(Node $node): ?Node
 
         $keyVar = $node->keyVar;
 
-        $isNodeUsed = (bool) $this->betterNodeFinder->findFirst($node->stmts, function (Node $node) use (
-            $keyVar
-        ): bool {
-            return $this->nodeComparator->areNodesEqual($node, $keyVar);
-        });
+        $isNodeUsed = (bool) $this->betterNodeFinder->findFirst(
+            $node->stmts,
+            fn (Node $node): bool => $this->nodeComparator->areNodesEqual($node, $keyVar)
+        );
 
         if ($isNodeUsed) {
             return null;