Remove parent node from StringClassNameToClassConstantRector (#4145)

rectorphp · Jun 10, 2023 · bc61ac2 · bc61ac2
1 parent b92e378
commit bc61ac2
Show file tree

Hide file tree

Showing 11 changed files with 109 additions and 101 deletions.
diff --git a/...-tests/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector/Fixture/skip_in_arg.php.inc b/...-tests/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector/Fixture/skip_in_arg.php.inc
@@ -0,0 +1,32 @@
+<?php
+
+namespace Rector\Tests\DeadCode\Rector\MethodCall\RemoveEmptyMethodCallRector\Fixture;
+
+use Rector\Tests\DeadCode\Rector\MethodCall\RemoveEmptyMethodCallRector\Source\EmptyMethod;
+
+final class SkipInArg
+{
+    /**
+     * @var EmptyMethod
+     */
+    private $emptyMethod;
+
+    public function __construct(EmptyMethod $emptyMethod)
+    {
+        $this->emptyMethod = $emptyMethod;
+    }
+
+    public function run()
+    {
+        if ($this->process($this->emptyMethod->run())) {
+            return true;
+        }
+
+        return false;
+    }
+
+    private function process()
+    {
+        return true;
+    }
+}
diff --git a/...ctor/String_/StringClassNameToClassConstantRector/Fixture/skip_non_existing_class.php.inc b/...ctor/String_/StringClassNameToClassConstantRector/Fixture/skip_non_existing_class.php.inc
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Php55\Rector\String_\StringClassNameToClassConstantRector\Fixture;
+
+final class SkipNonExistingClass
+{
+    public function run()
+    {
+        return 'Not\Existing\Everything';
+    }
+}
diff --git a/...enameEverywhere/rename_everywhere.php.inc → ...nRector/Fixture/rename_everywhere.php.inc b/...enameEverywhere/rename_everywhere.php.inc → ...nRector/Fixture/rename_everywhere.php.inc
@@ -1,6 +1,6 @@
 <?php
 
-namespace Rector\Tests\Renaming\Rector\ClassMethod\RenameAnnotationRector\FixtureRenameEverywhere;
+namespace Rector\Tests\Renaming\Rector\ClassMethod\RenameAnnotationRector\Fixture;
 
 final class RenameEveryWhere
 {
@@ -18,7 +18,7 @@ final class RenameEveryWhere
 -----
 <?php
 
-namespace Rector\Tests\Renaming\Rector\ClassMethod\RenameAnnotationRector\FixtureRenameEverywhere;
+namespace Rector\Tests\Renaming\Rector\ClassMethod\RenameAnnotationRector\Fixture;
 
 final class RenameEveryWhere
 {

diff --git a/...naming/Rector/ClassMethod/RenameAnnotationRector/RenameAnnotationEverywhereRectorTest.php b/...naming/Rector/ClassMethod/RenameAnnotationRector/RenameAnnotationEverywhereRectorTest.php
diff --git a/rules-tests/Renaming/Rector/ClassMethod/RenameAnnotationRector/config/configured_rule.php b/rules-tests/Renaming/Rector/ClassMethod/RenameAnnotationRector/config/configured_rule.php
@@ -4,12 +4,12 @@
 
 use Rector\Config\RectorConfig;
 use Rector\Renaming\Rector\ClassMethod\RenameAnnotationRector;
+use Rector\Renaming\ValueObject\RenameAnnotation;
 use Rector\Renaming\ValueObject\RenameAnnotationByType;
 
 return static function (RectorConfig $rectorConfig): void {
-    $rectorConfig
-        ->ruleWithConfiguration(
-            RenameAnnotationRector::class,
-            [new RenameAnnotationByType('PHPUnit\Framework\TestCase', 'scenario', 'test')]
-        );
+    $rectorConfig->ruleWithConfiguration(RenameAnnotationRector::class, [
+        new RenameAnnotationByType('PHPUnit\Framework\TestCase', 'scenario', 'test'),
+        new RenameAnnotation('psalm-ignore', 'phpstan-ignore'),
+    ]);
 };
diff --git a/rules-tests/Renaming/Rector/ClassMethod/RenameAnnotationRector/config/rename_everywhere.php b/rules-tests/Renaming/Rector/ClassMethod/RenameAnnotationRector/config/rename_everywhere.php
diff --git a/rules/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector.php b/rules/DeadCode/Rector/MethodCall/RemoveEmptyMethodCallRector.php
@@ -5,7 +5,6 @@
 namespace Rector\DeadCode\Rector\MethodCall;
 
 use PhpParser\Node;
-use PhpParser\Node\Arg;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Stmt\Class_;
@@ -140,16 +139,6 @@ private function resolveClassLike(MethodCall $methodCall): ?ClassLike
         return $this->reflectionAstResolver->resolveClassFromName($classReflection->getName());
     }
 
-    private function shouldSkipMethodCall(MethodCall $methodCall): bool
-    {
-        if ($this->callAnalyzer->isObjectCall($methodCall->var)) {
-            return true;
-        }
-
-        $parentArg = $this->betterNodeFinder->findParentType($methodCall, Arg::class);
-        return $parentArg instanceof Arg;
-    }
-
     private function shouldSkipClassMethod(
         Class_ | Trait_ | Interface_ | Enum_ $classLike,
         MethodCall $methodCall,
@@ -195,7 +184,7 @@ private function shouldSkipClassMethod(
 
     private function shouldRemoveMethodCall(MethodCall $methodCall): bool
     {
-        if ($this->shouldSkipMethodCall($methodCall)) {
+        if ($this->callAnalyzer->isObjectCall($methodCall->var)) {
             return false;
         }
 

diff --git a/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php b/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php
@@ -81,14 +81,19 @@ public function run()
      */
     public function getNodeTypes(): array
     {
-        return [String_::class, FuncCall::class];
+        return [String_::class, FuncCall::class, ClassConst::class];
     }
 
     /**
-     * @param String_|FuncCall $node
+     * @param String_|FuncCall|ClassConst $node
      */
     public function refactorWithScope(Node $node, Scope $scope)
     {
+        // allow class strings to be part of class const arrays, as probably on purpose
+        if ($node instanceof ClassConst) {
+            return NodeTraverser::STOP_TRAVERSAL;
+        }
+
         // keep allowed string as condition
         if ($node instanceof FuncCall) {
             if ($this->isName($node, 'is_a')) {
@@ -106,7 +111,7 @@ public function refactorWithScope(Node $node, Scope $scope)
             return null;
         }
 
-        if ($this->shouldSkip($classLikeName, $node)) {
+        if ($this->shouldSkip($classLikeName)) {
             return null;
         }
 
@@ -139,17 +144,12 @@ public function provideMinPhpVersion(): int
         return PhpVersionFeature::CLASSNAME_CONSTANT;
     }
 
-    private function shouldSkip(string $classLikeName, String_ $string): bool
+    private function shouldSkip(string $classLikeName): bool
     {
         if (! $this->reflectionProvider->hasClass($classLikeName)) {
             return true;
         }
 
-        $classReflection = $this->reflectionProvider->getClass($classLikeName);
-        if ($classReflection->getName() !== $classLikeName) {
-            return true;
-        }
-
         // skip short class names, mostly invalid use of strings
         if (! str_contains($classLikeName, '\\')) {
             return true;
@@ -166,8 +166,6 @@ private function shouldSkip(string $classLikeName, String_ $string): bool
             }
         }
 
-        // allow class strings to be part of class const arrays, as probably on purpose
-        $parentClassConst = $this->betterNodeFinder->findParentType($string, ClassConst::class);
-        return $parentClassConst instanceof ClassConst;
+        return false;
     }
 }
diff --git a/rules/Renaming/Rector/ClassMethod/RenameAnnotationRector.php b/rules/Renaming/Rector/ClassMethod/RenameAnnotationRector.php
@@ -78,38 +78,62 @@ public function someMethod()
      */
     public function getNodeTypes(): array
     {
-        return [ClassMethod::class, Property::class, Expression::class];
+        return [Class_::class, Expression::class];
     }
 
     /**
-     * @param ClassMethod|Property $node
+     * @param Class_|Expression $node
      */
     public function refactor(Node $node): ?Node
     {
-        $classLike = $this->betterNodeFinder->findParentType($node, Class_::class);
-        if (! $classLike instanceof Class_) {
+        $hasChanged = false;
+
+        if ($node instanceof Expression) {
+            $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
+
+            foreach ($this->renameAnnotations as $renameAnnotation) {
+                $hasDocBlockChanged = $this->docBlockTagReplacer->replaceTagByAnother(
+                    $phpDocInfo,
+                    $renameAnnotation->getOldAnnotation(),
+                    $renameAnnotation->getNewAnnotation()
+                );
+
+                if ($hasDocBlockChanged) {
+                    $hasChanged = true;
+                }
+            }
+
+            if ($hasChanged) {
+                return $node;
+            }
+
             return null;
         }
 
-        $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
-        $hasChanged = false;
-
-        foreach ($this->renameAnnotations as $renameAnnotation) {
-            if ($renameAnnotation instanceof RenameAnnotationByType && ! $this->isObjectType(
-                $classLike,
-                $renameAnnotation->getObjectType()
-            )) {
+        foreach ($node->stmts as $stmt) {
+            if (! $stmt instanceof ClassMethod && ! $stmt instanceof Property) {
                 continue;
             }
 
-            $hasDocBlockChanged = $this->docBlockTagReplacer->replaceTagByAnother(
-                $phpDocInfo,
-                $renameAnnotation->getOldAnnotation(),
-                $renameAnnotation->getNewAnnotation()
-            );
-
-            if ($hasDocBlockChanged) {
-                $hasChanged = true;
+            $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($stmt);
+
+            foreach ($this->renameAnnotations as $renameAnnotation) {
+                if ($renameAnnotation instanceof RenameAnnotationByType && ! $this->isObjectType(
+                    $node,
+                    $renameAnnotation->getObjectType()
+                )) {
+                    continue;
+                }
+
+                $hasDocBlockChanged = $this->docBlockTagReplacer->replaceTagByAnother(
+                    $phpDocInfo,
+                    $renameAnnotation->getOldAnnotation(),
+                    $renameAnnotation->getNewAnnotation()
+                );
+
+                if ($hasDocBlockChanged) {
+                    $hasChanged = true;
+                }
             }
         }
 
@@ -126,7 +150,6 @@ public function refactor(Node $node): ?Node
     public function configure(array $configuration): void
     {
         Assert::allIsAOf($configuration, RenameAnnotationInterface::class);
-
         $this->renameAnnotations = $configuration;
     }
 }
diff --git a/src/Kernel/RectorKernel.php b/src/Kernel/RectorKernel.php
@@ -17,7 +17,7 @@ final class RectorKernel
     /**
      * @var string
      */
-    private const CACHE_KEY = 'v84';
+    private const CACHE_KEY = 'v85';
 
     private ContainerInterface|null $container = null;
 

diff --git a/src/NodeAnalyzer/ArgsAnalyzer.php b/src/NodeAnalyzer/ArgsAnalyzer.php
@@ -10,15 +10,11 @@
 final class ArgsAnalyzer
 {
     /**
-     * @param mixed[]|Arg[] $args
+     * @param Arg[] $args
      */
     public function hasNamedArg(array $args): bool
     {
         foreach ($args as $arg) {
-            if (! $arg instanceof Arg) {
-                continue;
-            }
-
             if ($arg->name instanceof Identifier) {
                 return true;
             }