diff --git a/src/Rules/Rector/NoIntegerRefactorReturnRule.php b/src/Rules/Rector/NoIntegerRefactorReturnRule.php index e1d9f5da..49a0fb2b 100644 --- a/src/Rules/Rector/NoIntegerRefactorReturnRule.php +++ b/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,7 +23,7 @@ /** * @see \Symplify\PHPStanRules\Tests\Rules\Rector\NoIntegerRefactorReturnRule\NoIntegerRefactorReturnRuleTest * - * @implements Rule + * @implements Rule */ final class NoIntegerRefactorReturnRule implements Rule { @@ -30,35 +31,38 @@ final class NoIntegerRefactorReturnRule implements Rule 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; diff --git a/tests/Rules/Rector/NoIntegerRefactorReturnRule/Fixture/SkipDelegatedRemoveNode.php b/tests/Rules/Rector/NoIntegerRefactorReturnRule/Fixture/SkipDelegatedRemoveNode.php new file mode 100644 index 00000000..83e4690b --- /dev/null +++ b/tests/Rules/Rector/NoIntegerRefactorReturnRule/Fixture/SkipDelegatedRemoveNode.php @@ -0,0 +1,32 @@ +refactorClass($node); + } + + private function refactorClass(Node $node): null|int + { + if ($node instanceof Class_) { + return NodeVisitor::REMOVE_NODE; + } + + return null; + } +} diff --git a/tests/Rules/Rector/NoIntegerRefactorReturnRule/NoIntegerRefactorReturnRuleTest.php b/tests/Rules/Rector/NoIntegerRefactorReturnRule/NoIntegerRefactorReturnRuleTest.php index 65e29ef9..2ec8242e 100644 --- a/tests/Rules/Rector/NoIntegerRefactorReturnRule/NoIntegerRefactorReturnRuleTest.php +++ b/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