Fixing rules return values (take 2) (#1880)

* Fixing rules return values (take 2) * fix on ReturnNeverTypeRector * fix on PropertyAssignToMethodCallRector * fix on AddInterfaceByParentRector * fix on AddInterfaceByTraitRector * fix on MergeInterfacesRector * fix on CompleteImportForPartialAnnotationRector * fix on RenameAnnotationRector * phpstan * [ci-review] Rector Rectify * fix on RemoveFuncCallRector Co-authored-by: GitHub Action <action@github.com>
rectorphp · Feb 28, 2022 · 2604c76 · 2604c76
1 parent d93639c
commit 2604c76
Show file tree

Hide file tree

Showing 10 changed files with 69 additions and 14 deletions.
diff --git a/rules/CodingStyle/Rector/ClassConst/VarConstantCommentRector.php b/rules/CodingStyle/Rector/ClassConst/VarConstantCommentRector.php
@@ -98,6 +98,10 @@ public function refactor(Node $node): ?Node
 
         $this->phpDocTypeChanger->changeVarType($phpDocInfo, $constType);
 
+        if (! $phpDocInfo->hasChanged()) {
+            return null;
+        }
+
         return $node;
     }
 

diff --git a/rules/Removing/Rector/FuncCall/RemoveFuncCallArgRector.php b/rules/Removing/Rector/FuncCall/RemoveFuncCallArgRector.php
@@ -58,6 +58,7 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
+        $hasChanged = false;
         foreach ($this->removedFunctionArguments as $removedFunctionArgument) {
             if (! $this->isName($node->name, $removedFunctionArgument->getFunction())) {
                 continue;
@@ -69,9 +70,14 @@ public function refactor(Node $node): ?Node
                 }
 
                 $this->nodeRemover->removeArg($node, $position);
+                $hasChanged = true;
             }
         }
 
+        if (! $hasChanged) {
+            return null;
+        }
+
         return $node;
     }
 

diff --git a/rules/Removing/Rector/FuncCall/RemoveFuncCallRector.php b/rules/Removing/Rector/FuncCall/RemoveFuncCallRector.php
@@ -74,10 +74,13 @@ public function refactor(Node $node): ?Node
 
             if ($removeFuncCall->getArgumentPositionAndValues() === []) {
                 $this->removeNode($node);
-                return null;
+                return $node;
             }
 
-            $this->refactorFuncCallsWithPositions($node, $removeFuncCall);
+            $removedFuncCall = $this->refactorFuncCallsWithPositions($node, $removeFuncCall);
+            if ($removedFuncCall instanceof FuncCall) {
+                return $node;
+            }
         }
 
         return null;
@@ -92,7 +95,7 @@ public function configure(array $configuration): void
         $this->removeFuncCalls = $configuration;
     }
 
-    private function refactorFuncCallsWithPositions(FuncCall $funcCall, RemoveFuncCall $removeFuncCall): void
+    private function refactorFuncCallsWithPositions(FuncCall $funcCall, RemoveFuncCall $removeFuncCall): ?FuncCall
     {
         foreach ($removeFuncCall->getArgumentPositionAndValues() as $argumentPosition => $values) {
             if (! $this->isArgumentPositionValueMatch($funcCall, $argumentPosition, $values)) {
@@ -101,8 +104,11 @@ private function refactorFuncCallsWithPositions(FuncCall $funcCall, RemoveFuncCa
 
             if ($this->breakingRemovalGuard->isLegalNodeRemoval($funcCall)) {
                 $this->removeNode($funcCall);
+                return $funcCall;
             }
         }
+
+        return null;
     }
 
     /**

diff --git a/rules/Renaming/Rector/ClassMethod/RenameAnnotationRector.php b/rules/Renaming/Rector/ClassMethod/RenameAnnotationRector.php
@@ -92,6 +92,7 @@ public function refactor(Node $node): ?Node
         }
 
         $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
+        $hasChanged = false;
 
         foreach ($this->renameAnnotations as $renameAnnotation) {
             if ($renameAnnotation instanceof RenameAnnotationByType && ! $this->isObjectType(
@@ -106,6 +107,11 @@ public function refactor(Node $node): ?Node
                 $renameAnnotation->getOldAnnotation(),
                 $renameAnnotation->getNewAnnotation()
             );
+            $hasChanged = true;
+        }
+
+        if (! $hasChanged) {
+            return null;
         }
 
         return $node;

diff --git a/rules/Restoration/Rector/Namespace_/CompleteImportForPartialAnnotationRector.php b/rules/Restoration/Rector/Namespace_/CompleteImportForPartialAnnotationRector.php
@@ -82,14 +82,19 @@ public function refactor(Node $node): ?Node
         }
 
         $printedClass = $this->print($class);
-
+        $hasChanged = false;
         foreach ($this->useImportsToRestore as $useImportToRestore) {
             $annotationToSeek = '#\*\s+\@' . $useImportToRestore->getAlias() . '#';
             if (! StringUtils::isMatch($printedClass, $annotationToSeek)) {
                 continue;
             }
 
             $node = $this->addImportToNamespaceIfMissing($node, $useImportToRestore);
+            $hasChanged = true;
+        }
+
+        if (! $hasChanged) {
+            return null;
         }
 
         return $node;

diff --git a/rules/Transform/Rector/Assign/PropertyAssignToMethodCallRector.php b/rules/Transform/Rector/Assign/PropertyAssignToMethodCallRector.php
@@ -82,7 +82,7 @@ public function refactor(Node $node): ?Node
             );
         }
 
-        return $node;
+        return null;
     }
 
     /**

diff --git a/rules/Transform/Rector/Class_/AddInterfaceByParentRector.php b/rules/Transform/Rector/Class_/AddInterfaceByParentRector.php
@@ -64,19 +64,13 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        /** @var Scope $scope */
-        $scope = $node->getAttribute(AttributeKey::SCOPE);
-
-        $classReflection = $scope->getClassReflection();
-        if (! $classReflection instanceof ClassReflection) {
-            return null;
-        }
+        $parentClassReflection = $this->resolveParentClassReflection($node);
 
-        $parentClassReflection = $classReflection->getParentClass();
         if (! $parentClassReflection instanceof ClassReflection) {
             return null;
         }
 
+        $hasChanged = false;
         foreach ($this->interfaceByParent as $parentName => $interfaceName) {
             if ($parentName !== $parentClassReflection->getName()) {
                 continue;
@@ -89,6 +83,11 @@ public function refactor(Node $node): ?Node
             }
 
             $node->implements[] = new FullyQualified($interfaceName);
+            $hasChanged = true;
+        }
+
+        if (! $hasChanged) {
+            return null;
         }
 
         return $node;
@@ -104,4 +103,17 @@ public function configure(array $configuration): void
 
         $this->interfaceByParent = $configuration;
     }
+
+    private function resolveParentClassReflection(Class_ $class): ?ClassReflection
+    {
+        /** @var Scope $scope */
+        $scope = $class->getAttribute(AttributeKey::SCOPE);
+
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return null;
+        }
+
+        return $classReflection->getParentClass();
+    }
 }
diff --git a/rules/Transform/Rector/Class_/AddInterfaceByTraitRector.php b/rules/Transform/Rector/Class_/AddInterfaceByTraitRector.php
@@ -71,6 +71,7 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
+        $hasChanged = false;
         foreach ($this->interfaceByTrait as $traitName => $interfaceName) {
             if (! $classReflection->hasTraitUse($traitName)) {
                 continue;
@@ -83,6 +84,11 @@ public function refactor(Node $node): ?Node
             }
 
             $node->implements[] = new FullyQualified($interfaceName);
+            $hasChanged = true;
+        }
+
+        if (! $hasChanged) {
+            return null;
         }
 
         return $node;

diff --git a/rules/Transform/Rector/Class_/MergeInterfacesRector.php b/rules/Transform/Rector/Class_/MergeInterfacesRector.php
@@ -67,6 +67,7 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
+        $hasChanged = false;
         foreach ($node->implements as $key => $implement) {
             $oldInterfaces = array_keys($this->oldToNewInterfaces);
             if (! $this->isNames($implement, $oldInterfaces)) {
@@ -75,6 +76,11 @@ public function refactor(Node $node): ?Node
 
             $interface = $this->getName($implement);
             $node->implements[$key] = new Name($this->oldToNewInterfaces[$interface]);
+            $hasChanged = true;
+        }
+
+        if (! $hasChanged) {
+            return null;
         }
 
         $this->makeImplementsUnique($node);

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector.php
@@ -90,7 +90,11 @@ public function refactor(Node $node): ?Node
         } else {
             // static anlysis based never type
             $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
-            $this->phpDocTypeChanger->changeReturnType($phpDocInfo, new NeverType());
+            $hasChanged = $this->phpDocTypeChanger->changeReturnType($phpDocInfo, new NeverType());
+
+            if (! $hasChanged) {
+                return null;
+            }
         }
 
         return $node;