Make RemoveNonExistingVarAnnotationRector simpler, skip compact() che…

…ck as not generic (#4439)
rectorphp · Jul 8, 2023 · 2f1d4d3 · 2f1d4d3
1 parent ac63f2a
commit 2f1d4d3
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 106 deletions.
diff --git a/...s/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector/Fixture/skip_property.php.inc b/...s/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector/Fixture/skip_property.php.inc
@@ -0,0 +1,11 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\Node\RemoveNonExistingVarAnnotationRector\Fixture;
+
+final class skipProperty
+{
+    /**
+     * @var string
+     */
+    public $skipProperty;
+}
diff --git a/.../Node/RemoveNonExistingVarAnnotationRector/Fixture/skip_used_after_next_statement.php.inc b/.../Node/RemoveNonExistingVarAnnotationRector/Fixture/skip_used_after_next_statement.php.inc
diff --git a/rules/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector.php b/rules/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector.php
@@ -6,23 +6,15 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\CallLike;
-use PhpParser\Node\Expr\FuncCall;
 use PhpParser\Node\Expr\New_;
 use PhpParser\Node\Expr\Variable;
-use PhpParser\Node\Stmt\Echo_;
-use PhpParser\Node\Stmt\Expression;
-use PhpParser\Node\Stmt\Foreach_;
-use PhpParser\Node\Stmt\If_;
-use PhpParser\Node\Stmt\Nop;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\ClassConst;
+use PhpParser\Node\Stmt\Property;
 use PhpParser\Node\Stmt\Return_;
-use PhpParser\Node\Stmt\Static_;
-use PhpParser\Node\Stmt\Switch_;
-use PhpParser\Node\Stmt\Throw_;
-use PhpParser\Node\Stmt\While_;
 use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode;
 use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
 use Rector\Core\Rector\AbstractRector;
-use Rector\DeadCode\NodeAnalyzer\ExprUsedInNodeAnalyzer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -33,11 +25,6 @@
  */
 final class RemoveNonExistingVarAnnotationRector extends AbstractRector
 {
-    public function __construct(
-        private readonly ExprUsedInNodeAnalyzer $exprUsedInNodeAnalyzer
-    ) {
-    }
-
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition(
@@ -74,27 +61,20 @@ public function get()
      */
     public function getNodeTypes(): array
     {
-        return [
-            Foreach_::class,
-            Static_::class,
-            Echo_::class,
-            Return_::class,
-            Expression::class,
-            Throw_::class,
-            If_::class,
-            While_::class,
-            Switch_::class,
-            Nop::class,
-        ];
+        return [Stmt::class];
     }
 
+    /**
+     * @param Stmt $node
+     */
     public function refactor(Node $node): ?Node
     {
         if ($this->shouldSkip($node)) {
             return null;
         }
 
         $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
+
         $varTagValueNode = $phpDocInfo->getVarTagValueNode();
         if (! $varTagValueNode instanceof VarTagValueNode) {
             return null;
@@ -114,10 +94,6 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        if ($this->isUsedInNextNodeWithExtractPreviouslyCalled($node, $variableName)) {
-            return null;
-        }
-
         $comments = $node->getComments();
         if (isset($comments[1])) {
             // skip edge case with double comment, as impossible to resolve by PHPStan doc parser
@@ -128,35 +104,18 @@ public function refactor(Node $node): ?Node
         return $node;
     }
 
-    private function isUsedInNextNodeWithExtractPreviouslyCalled(Node $node, string $variableName): bool
+    private function shouldSkip(Stmt $stmt): bool
     {
-        $variable = new Variable($variableName);
-        $isUsedInNextNode = (bool) $this->betterNodeFinder->findFirstNext(
-            $node,
-            fn (Node $node): bool => $this->exprUsedInNodeAnalyzer->isUsed($node, $variable)
-        );
-
-        if (! $isUsedInNextNode) {
-            return false;
+        if ($stmt instanceof ClassConst || $stmt instanceof Property) {
+            return true;
         }
 
-        return (bool) $this->betterNodeFinder->findFirstPrevious($node, function (Node $subNode): bool {
-            if (! $subNode instanceof FuncCall) {
-                return false;
-            }
-
-            return $this->nodeNameResolver->isName($subNode, 'extract');
-        });
-    }
-
-    private function shouldSkip(Node $node): bool
-    {
-        return count($node->getComments()) !== 1;
+        return count($stmt->getComments()) !== 1;
     }
 
-    private function hasVariableName(Node $node, string $variableName): bool
+    private function hasVariableName(Stmt $stmt, string $variableName): bool
     {
-        return (bool) $this->betterNodeFinder->findFirst($node, function (Node $node) use ($variableName): bool {
+        return (bool) $this->betterNodeFinder->findFirst($stmt, function (Node $node) use ($variableName): bool {
             if (! $node instanceof Variable) {
                 return false;
             }
@@ -186,10 +145,8 @@ private function isObjectShapePseudoType(VarTagValueNode $varTagValueNode): bool
         return str_contains($varTagValueNode->description, '}');
     }
 
-    private function isAnnotatableReturn(Node $node): bool
+    private function isAnnotatableReturn(Stmt $stmt): bool
     {
-        return $node instanceof Return_
-            && $node->expr instanceof CallLike
-            && ! $node->expr instanceof New_;
+        return $stmt instanceof Return_ && $stmt->expr instanceof CallLike && ! $stmt->expr instanceof New_;
     }
 }
diff --git a/src/PhpParser/Node/BetterNodeFinder.php b/src/PhpParser/Node/BetterNodeFinder.php
@@ -6,7 +6,6 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\FunctionLike;
@@ -31,7 +30,6 @@
 use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\Exception\StopSearchException;
 use Rector\Core\NodeAnalyzer\ClassAnalyzer;
-use Rector\Core\PhpParser\Comparing\NodeComparator;
 use Rector\Core\PhpParser\Node\CustomNode\FileWithoutNamespace;
 use Rector\Core\Provider\CurrentFileProvider;
 use Rector\Core\Util\MultiInstanceofChecker;
@@ -49,7 +47,6 @@ final class BetterNodeFinder
     public function __construct(
         private readonly NodeFinder $nodeFinder,
         private readonly NodeNameResolver $nodeNameResolver,
-        private readonly NodeComparator $nodeComparator,
         private readonly ClassAnalyzer $classAnalyzer,
         private readonly MultiInstanceofChecker $multiInstanceofChecker,
         private readonly CurrentFileProvider $currentFileProvider,
@@ -177,21 +174,6 @@ public function findFirst(Node | array $nodes, callable $filter): ?Node
         return $this->nodeFinder->findFirst($nodes, $filter);
     }
 
-    /**
-     * @api symfony
-     * @return Assign|null
-     */
-    public function findPreviousAssignToExpr(Expr $expr): ?Node
-    {
-        return $this->findFirstPrevious($expr, function (Node $node) use ($expr): bool {
-            if (! $node instanceof Assign) {
-                return false;
-            }
-
-            return $this->nodeComparator->areNodesEqual($node->var, $expr);
-        });
-    }
-
     /**
      * @deprecated Use nodes directly
      *