[Rector] Skip refactor() delegating REMOVE_NODE to helper methods

src/Rules/Rector/NoIntegerRefactorReturnRule.php

@@ -9,6 +9,7 @@
 use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Identifier;
 use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\UnionType;
 use PhpParser\NodeTraverser;
@@ -22,43 +23,46 @@
 /**
  * @see \Symplify\PHPStanRules\Tests\Rules\Rector\NoIntegerRefactorReturnRule\NoIntegerRefactorReturnRuleTest
  *
- * @implements Rule<ClassMethod>
+ * @implements Rule<Class_>
  */
 final class NoIntegerRefactorReturnRule implements Rule
 {
     public const string ERROR_MESSAGE = 'Instead of using DONT_TRAVERSE_CHILDREN* or STOP_TRAVERSAL in refactor() method, make use of attributes. Return always node, null or REMOVE_NODE. Using traverser enums might lead to unexpected results';
 
     public function getNodeType(): string
     {
-        return ClassMethod::class;
+        return Class_::class;
     }
 
     /**
-     * @param ClassMethod $node
+     * @param Class_ $node
      */
     public function processNode(Node $node, Scope $scope): array
     {
-        if (! $node->isPublic()) {
+        $refactorClassMethod = $node->getMethod('refactor');
+        if (! $refactorClassMethod instanceof ClassMethod) {
             return [];
         }
 
-        if ($node->name->toString() !== 'refactor') {
+        if (! $refactorClassMethod->isPublic()) {
             return [];
         }
 
-        if (! $this->hasIntReturnType($node->returnType)) {
+        if (! $this->hasIntReturnType($refactorClassMethod->returnType)) {
             return [];
         }
 
+        // scan the whole class, as refactor() often delegates the int return to private helper methods
         $constantNames = $this->findUsedNodeVisitorConstantNames($node);
 
         $undesiredConstantNames = array_diff($constantNames, ['REMOVE_NODE']);
-        if ($constantNames !== [] && $undesiredConstantNames === []) {
+        if ($undesiredConstantNames === []) {
             return [];
         }
 
         $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
             ->identifier(RectorRuleIdentifier::NO_INTEGER_REFACTOR_RETURN)
+            ->line($refactorClassMethod->getStartLine())
             ->build();
 
         return [$identifierRuleError];
@@ -86,12 +90,12 @@ private function hasIntReturnType(?Node $node): bool
     /**
      * @return string[]
      */
-    private function findUsedNodeVisitorConstantNames(ClassMethod $classMethod): array
+    private function findUsedNodeVisitorConstantNames(Class_ $class): array
     {
         $constantNames = [];
 
         $simpleCallableNodeTraverser = new SimpleCallableNodeTraverser();
-        $simpleCallableNodeTraverser->traverseNodesWithCallable($classMethod, function (Node $subNode) use (&$constantNames): int|null {
+        $simpleCallableNodeTraverser->traverseNodesWithCallable($class, function (Node $subNode) use (&$constantNames): int|null {
             // skip closure nodes as they have their own scope
             if ($subNode instanceof Closure) {
                 return NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN;

tests/Rules/Rector/NoIntegerRefactorReturnRule/Fixture/SkipDelegatedRemoveNode.php

@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Rector\NoIntegerRefactorReturnRule\Fixture;
+
+use PhpParser\Node;
+use PhpParser\Node\Stmt\Class_;
+use PhpParser\NodeVisitor;
+use Rector\Rector\AbstractRector;
+
+final class SkipDelegatedRemoveNode extends AbstractRector
+{
+    public function getNodeTypes(): array
+    {
+        return [Class_::class];
+    }
+
+    public function refactor(Node $node): null|int
+    {
+        return $this->refactorClass($node);
+    }
+
+    private function refactorClass(Node $node): null|int
+    {
+        if ($node instanceof Class_) {
+            return NodeVisitor::REMOVE_NODE;
+        }
+
+        return null;
+    }
+}

tests/Rules/Rector/NoIntegerRefactorReturnRule/NoIntegerRefactorReturnRuleTest.php

@@ -38,6 +38,7 @@ public static function provideData(): Iterator
         yield [__DIR__ . '/Fixture/AllowRemoveNode.php', []];
         yield [__DIR__ . '/Fixture/AllowBareIntRemoveNode.php', []];
         yield [__DIR__ . '/Fixture/AllowNestedClosure.php', []];
+        yield [__DIR__ . '/Fixture/SkipDelegatedRemoveNode.php', []];
     }
 
     protected function getRule(): Rule