[Core] Merge BetterNodeFinder findFirstPreviousOfNode() with findFirs…

…tPrevious() (#2213) * [Core] Merge BetterNodeFinder findFirstPreviousOfNode() with findFirstPrevious() * resolve find inside ClassConst * [ci-review] Rector Rectify * [ci-review] Rector Rectify * [ci-review] Rector Rectify Co-authored-by: GitHub Action <action@github.com>
rectorphp · May 2, 2022 · 464b74c · 464b74c
1 parent 992d1ef
commit 464b74c
Show file tree

Hide file tree

Showing 19 changed files with 36 additions and 77 deletions.
diff --git a/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php b/rules/CodeQuality/Rector/FuncCall/ArrayKeysAndInArrayToArrayKeyExistsRector.php
@@ -120,7 +120,7 @@ private function createArrayKeyExists(FuncCall $inArrayFuncCall, FuncCall $array
 
     private function findPreviousAssignToArrayKeys(FuncCall $funcCall, Expr $expr): null|Node|FunctionLike
     {
-        return $this->betterNodeFinder->findFirstPreviousOfNode($funcCall, function (Node $node) use ($expr): bool {
+        return $this->betterNodeFinder->findFirstPrevious($funcCall, function (Node $node) use ($expr): bool {
             // breaking out of scope
             if ($node instanceof FunctionLike) {
                 return true;

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

diff --git a/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php b/rules/DeadCode/Rector/Assign/RemoveUnusedVariableAssignRector.php
@@ -185,7 +185,7 @@ private function isUsed(Assign $assign, Variable $variable): bool
 
     private function isUsedInPreviousNode(Variable $variable): bool
     {
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode(
+        return (bool) $this->betterNodeFinder->findFirstPrevious(
             $variable,
             fn (Node $node): bool => $this->usedVariableNameAnalyzer->isVariableNamed($node, $variable)
         );
@@ -214,7 +214,7 @@ private function isUsedInPreviousAssign(Assign $assign, Expr $expr): bool
             return false;
         }
 
-        $previousAssign = $this->betterNodeFinder->findFirstPreviousOfNode(
+        $previousAssign = $this->betterNodeFinder->findFirstPrevious(
             $assign,
             fn (Node $node): bool => $node instanceof Assign && $this->usedVariableNameAnalyzer->isVariableNamed(
                 $node->var,

diff --git a/rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php b/rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php
@@ -159,7 +159,7 @@ private function shouldSkipFromNotTypedParam(Instanceof_ $instanceof): bool
         }
 
         $variable = $instanceof->expr;
-        $isReassign = (bool) $this->betterNodeFinder->findFirstPreviousOfNode(
+        $isReassign = (bool) $this->betterNodeFinder->findFirstPrevious(
             $instanceof,
             fn (Node $subNode): bool => $subNode instanceof Assign && $this->nodeComparator->areNodesEqual(
                 $subNode->var,

diff --git a/rules/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector.php b/rules/DeadCode/Rector/Node/RemoveNonExistingVarAnnotationRector.php
@@ -141,7 +141,7 @@ private function isUsedInNextNodeWithExtractPreviouslyCalled(Node $node, string
             return false;
         }
 
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($node, function (Node $subNode): bool {
+        return (bool) $this->betterNodeFinder->findFirstPrevious($node, function (Node $subNode): bool {
             if (! $subNode instanceof FuncCall) {
                 return false;
             }

diff --git a/rules/DeadCode/UselessIfCondBeforeForeachDetector.php b/rules/DeadCode/UselessIfCondBeforeForeachDetector.php
@@ -91,7 +91,7 @@ public function isMatchingNotIdenticalEmptyArray(If_ $if, Expr $foreachExpr): bo
 
     private function fromPreviousParam(Expr $expr): ?Node
     {
-        return $this->betterNodeFinder->findFirstPreviousOfNode($expr, function (Node $node) use ($expr): bool {
+        return $this->betterNodeFinder->findFirstPrevious($expr, function (Node $node) use ($expr): bool {
             if (! $node instanceof Param) {
                 return false;
             }

diff --git a/rules/DogFood/Rector/Closure/UpgradeRectorConfigRector.php b/rules/DogFood/Rector/Closure/UpgradeRectorConfigRector.php
@@ -147,7 +147,7 @@ public function refactor(Node $node): ?Node
                         return null;
                     }
 
-                    $isPossiblyServiceDefinition = (bool) $this->betterNodeFinder->findFirstPreviousOfNode(
+                    $isPossiblyServiceDefinition = (bool) $this->betterNodeFinder->findFirstPrevious(
                         $node,
                         fn (Node $node): bool => $this->isFoundFluentServiceCall($node)
                     );

diff --git a/rules/DowngradePhp72/NodeAnalyzer/RegexFuncAnalyzer.php b/rules/DowngradePhp72/NodeAnalyzer/RegexFuncAnalyzer.php
@@ -42,7 +42,7 @@ public function isRegexFunctionNames(FuncCall $funcCall): bool
         }
 
         /** @var Assign|null $assignExprVariable */
-        $assignExprVariable = $this->betterNodeFinder->findFirstPreviousOfNode($funcCall, function (Node $node) use (
+        $assignExprVariable = $this->betterNodeFinder->findFirstPrevious($funcCall, function (Node $node) use (
             $variable
         ): bool {
             if (! $node instanceof Assign) {

diff --git a/rules/DowngradePhp72/NodeManipulator/JsonConstCleaner.php b/rules/DowngradePhp72/NodeManipulator/JsonConstCleaner.php
@@ -40,7 +40,7 @@ public function clean(ConstFetch|BitwiseOr $node, array $constants): ConstFetch|
      */
     private function hasDefinedCheck(ConstFetch|BitwiseOr $node, array $constants): bool
     {
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode(
+        return (bool) $this->betterNodeFinder->findFirstPrevious(
             $node,
             function (Node $subNode) use ($constants): bool {
                 if (! $subNode instanceof FuncCall) {

diff --git a/rules/EarlyReturn/Rector/Foreach_/ReturnAfterToEarlyOnBreakRector.php b/rules/EarlyReturn/Rector/Foreach_/ReturnAfterToEarlyOnBreakRector.php
@@ -99,7 +99,7 @@ public function refactor(Node $node): ?Node
 
         $assignVariable = $assign->var;
         /** @var Expr $variablePrevious */
-        $variablePrevious = $this->betterNodeFinder->findFirstPreviousOfNode($node, function (Node $node) use (
+        $variablePrevious = $this->betterNodeFinder->findFirstPrevious($node, function (Node $node) use (
             $assignVariable
         ): bool {
             $parent = $node->getAttribute(AttributeKey::PARENT_NODE);

diff --git a/rules/PSR4/Rector/FileWithoutNamespace/NormalizeNamespaceByPSR4ComposerAutoloadRector.php b/rules/PSR4/Rector/FileWithoutNamespace/NormalizeNamespaceByPSR4ComposerAutoloadRector.php
@@ -117,7 +117,7 @@ public function refactor(Node $node): ?Node
 
     private function hasNamespaceInPreviousNamespace(Namespace_ $namespace): bool
     {
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode(
+        return (bool) $this->betterNodeFinder->findFirstPrevious(
             $namespace,
             fn (Node $node): bool => $node instanceof Namespace_
         );

diff --git a/rules/Php56/NodeAnalyzer/UndefinedVariableResolver.php b/rules/Php56/NodeAnalyzer/UndefinedVariableResolver.php
@@ -171,7 +171,7 @@ private function shouldSkipVariable(Variable $variable, Node $parentNode): bool
 
     private function isAfterSwitchCaseWithParentCase(Variable $variable): bool
     {
-        $previousSwitch = $this->betterNodeFinder->findFirstPreviousOfNode(
+        $previousSwitch = $this->betterNodeFinder->findFirstPrevious(
             $variable,
             fn (Node $subNode): bool => $subNode instanceof Switch_
         );
@@ -197,7 +197,7 @@ private function isDifferentWithOriginalNodeOrNoScope(Variable $variable): bool
 
     private function hasPreviousCheckedWithIsset(Variable $variable): bool
     {
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($variable, function (Node $subNode) use (
+        return (bool) $this->betterNodeFinder->findFirstPrevious($variable, function (Node $subNode) use (
             $variable
         ): bool {
             if (! $subNode instanceof Isset_) {
@@ -217,7 +217,7 @@ private function hasPreviousCheckedWithIsset(Variable $variable): bool
 
     private function hasPreviousCheckedWithEmpty(Variable $variable): bool
     {
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($variable, function (Node $subNode) use (
+        return (bool) $this->betterNodeFinder->findFirstPrevious($variable, function (Node $subNode) use (
             $variable
         ): bool {
             if (! $subNode instanceof Empty_) {

diff --git a/rules/Php80/NodeManipulator/ResourceReturnToObject.php b/rules/Php80/NodeManipulator/ResourceReturnToObject.php
@@ -121,7 +121,7 @@ private function resolveArgValueTypeFromPreviousAssign(
         array $collectionFunctionToReturnObject
     ): ?FullyQualifiedObjectType {
         $objectInstanceCheck = null;
-        $assign = $this->betterNodeFinder->findFirstPreviousOfNode($funcCall, function (Node $subNode) use (
+        $assign = $this->betterNodeFinder->findFirstPrevious($funcCall, function (Node $subNode) use (
             &$objectInstanceCheck,
             $expr,
             $collectionFunctionToReturnObject

diff --git a/rules/Php80/Rector/Switch_/ChangeSwitchToMatchRector.php b/rules/Php80/Rector/Switch_/ChangeSwitchToMatchRector.php
@@ -143,7 +143,7 @@ private function processReturn(Match_ $match): ?Return_
 
     private function changeToAssign(Switch_ $switch, Match_ $match, Expr $assignExpr): Assign
     {
-        $prevInitializedAssign = $this->betterNodeFinder->findFirstPreviousOfNode(
+        $prevInitializedAssign = $this->betterNodeFinder->findFirstPrevious(
             $switch,
             fn (Node $node): bool => $node instanceof Assign && $this->nodeComparator->areNodesEqual(
                 $node->var,

diff --git a/rules/Php81/Rector/FuncCall/NullToStrictStringFuncCallArgRector.php b/rules/Php81/Rector/FuncCall/NullToStrictStringFuncCallArgRector.php
@@ -202,9 +202,7 @@ private function processNullToStrictStringOnNodePosition(
 
     private function isCastedReassign(Expr $expr): bool
     {
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($expr, function (Node $subNode) use (
-            $expr
-        ): bool {
+        return (bool) $this->betterNodeFinder->findFirstPrevious($expr, function (Node $subNode) use ($expr): bool {
             if (! $subNode instanceof Assign) {
                 return false;
             }

diff --git a/rules/Privatization/Rector/Class_/RepeatedLiteralToClassConstantRector.php b/rules/Privatization/Rector/Class_/RepeatedLiteralToClassConstantRector.php
@@ -15,7 +15,6 @@
 use Rector\Core\Php\ReservedKeywordAnalyzer;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\Util\StaticRectorStrings;
-use Rector\NodeNestingScope\NodeFinder\ScopeAwareNodeFinder;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -52,8 +51,7 @@ final class RepeatedLiteralToClassConstantRector extends AbstractRector
 
     public function __construct(
         private readonly ClassInsertManipulator $classInsertManipulator,
-        private readonly ReservedKeywordAnalyzer $reservedKeywordAnalyzer,
-        private readonly ScopeAwareNodeFinder $scopeAwareNodeFinder
+        private readonly ReservedKeywordAnalyzer $reservedKeywordAnalyzer
     ) {
     }
 
@@ -253,7 +251,7 @@ private function shouldSkipString(String_ $string): bool
         }
 
         // skip values in another constants
-        $parentConst = $this->scopeAwareNodeFinder->findParentType($string, [ClassConst::class]);
+        $parentConst = $this->betterNodeFinder->findParentType($string, ClassConst::class);
         if ($parentConst !== null) {
             return true;
         }

diff --git a/src/NodeAnalyzer/CallAnalyzer.php b/src/NodeAnalyzer/CallAnalyzer.php
@@ -81,7 +81,7 @@ public function isNewInstance(Expr $expr): bool
             return true;
         }
 
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($expr, function (Node $node) use ($expr): bool {
+        return (bool) $this->betterNodeFinder->findFirstPrevious($expr, function (Node $node) use ($expr): bool {
             if (! $node instanceof Assign) {
                 return false;
             }

diff --git a/src/NodeAnalyzer/VariableAnalyzer.php b/src/NodeAnalyzer/VariableAnalyzer.php
@@ -28,7 +28,7 @@ public function isStaticOrGlobal(Variable $variable): bool
             return true;
         }
 
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($variable, function (Node $node) use (
+        return (bool) $this->betterNodeFinder->findFirstPrevious($variable, function (Node $node) use (
             $variable
         ): bool {
             if (! in_array($node::class, [Static_::class, Global_::class], true)) {
@@ -56,7 +56,7 @@ public function isStaticOrGlobal(Variable $variable): bool
 
     public function isUsedByReference(Variable $variable): bool
     {
-        return (bool) $this->betterNodeFinder->findFirstPreviousOfNode($variable, function (Node $subNode) use (
+        return (bool) $this->betterNodeFinder->findFirstPrevious($variable, function (Node $subNode) use (
             $variable
         ): bool {
             if (! $subNode instanceof Variable) {

diff --git a/src/PhpParser/Node/BetterNodeFinder.php b/src/PhpParser/Node/BetterNodeFinder.php
@@ -272,73 +272,36 @@ public function findPreviousAssignToExpr(Expr $expr): ?Node
     /**
      * @param callable(Node $node): bool $filter
      */
-    public function findFirstPreviousOfNode(Node $node, callable $filter, bool $lookupParent = true): ?Node
-    {
-        // move to previous expression
+    public function findFirstPrevious(
+        Node $node,
+        callable $filter,
+        bool $lookupParent = true,
+        bool $stopOnFunctionLike = true
+    ): ?Node {
+        // move to previous Node
         $previousStatement = $node->getAttribute(AttributeKey::PREVIOUS_NODE);
-        if ($previousStatement !== null) {
+        if ($previousStatement instanceof Node) {
             $foundNode = $this->findFirst([$previousStatement], $filter);
+
             // we found what we need
-            if ($foundNode !== null) {
+            if ($foundNode instanceof Node) {
                 return $foundNode;
             }
 
-            return $this->findFirstPreviousOfNode($previousStatement, $filter);
+            return $this->findFirstPrevious($previousStatement, $filter);
         }
 
         if (! $lookupParent) {
             return null;
         }
 
         $parent = $node->getAttribute(AttributeKey::PARENT_NODE);
-        if ($parent instanceof FunctionLike) {
+        if ($stopOnFunctionLike && $parent instanceof FunctionLike) {
             return null;
         }
 
         if ($parent instanceof Node) {
-            return $this->findFirstPreviousOfNode($parent, $filter);
-        }
-
-        return null;
-    }
-
-    /**
-     * @param callable(Node $node): bool $filter
-     */
-    public function findFirstPrevious(Node $node, callable $filter): ?Node
-    {
-        $currentStmt = $this->resolveCurrentStatement($node);
-
-        // current Stmt not an Stmt may caused by Node already removed
-        if (! $currentStmt instanceof Stmt) {
-            return null;
-        }
-
-        $foundInCurrentStmt = $this->findFirst($currentStmt, $filter);
-
-        if ($foundInCurrentStmt instanceof Node) {
-            return $foundInCurrentStmt;
-        }
-
-        // previous Stmt of Stmt must be Stmt if found
-        $previousStatement = $currentStmt->getAttribute(AttributeKey::PREVIOUS_NODE);
-
-        if ($previousStatement instanceof Stmt) {
-            return $this->findFirstPrevious($previousStatement, $filter);
-        }
-
-        $parent = $currentStmt->getAttribute(AttributeKey::PARENT_NODE);
-
-        // Last Node? Node not found
-        if (! $parent instanceof Stmt) {
-            return null;
-        }
-
-        // previous Stmt of Stmt must be Stmt if found
-        $previousStatement = $parent->getAttribute(AttributeKey::PREVIOUS_NODE);
-
-        if ($previousStatement instanceof Stmt) {
-            return $this->findFirstPrevious($previousStatement, $filter);
+            return $this->findFirstPrevious($parent, $filter);
         }
 
         return null;