diff --git a/phpstan.neon b/phpstan.neon index 6c42f1c6617..ded2a47bc19 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -400,3 +400,11 @@ parameters: - rules/DeadCode/Rector/If_/RemoveDeadInstanceOfRector.php - rules/DeadCode/Rector/ConstFetch/RemovePhpVersionIdCheckRector.php - rules/DeadCode/Rector/If_/UnwrapFutureCompatibleIfPhpVersionRector.php + + # condition check, just to be sure + - '#Method Rector\\Rector\\AbstractRector\:\:enterNode\(\) never returns 3 so it can be removed from the return type#' + + # special case, working on a file-level + - + identifier: rector.noOnlyNullReturnInRefactor + path: rules/TypeDeclaration/Rector/StmtsAwareInterface/DeclareStrictTypesRector.php diff --git a/src/Contract/Rector/RectorInterface.php b/src/Contract/Rector/RectorInterface.php index b2960111770..2602bd2a67e 100644 --- a/src/Contract/Rector/RectorInterface.php +++ b/src/Contract/Rector/RectorInterface.php @@ -20,7 +20,7 @@ public function getNodeTypes(): array; /** * Process Node of matched type - * @return Node|Node[]|null|NodeVisitor::* + * @return Node|Node[]|null|NodeVisitor::REMOVE_NODE */ public function refactor(Node $node); } diff --git a/src/NodeTypeResolver/Node/AttributeKey.php b/src/NodeTypeResolver/Node/AttributeKey.php index 3c390d82ca9..ba48287c8cf 100644 --- a/src/NodeTypeResolver/Node/AttributeKey.php +++ b/src/NodeTypeResolver/Node/AttributeKey.php @@ -92,12 +92,6 @@ final class AttributeKey */ public const CREATED_BY_RULE = 'created_by_rule'; - /** - * Helps with skipped below node - * @var string - */ - public const SKIPPED_BY_RECTOR_RULE = 'skipped_rector_rule'; - /** * @var string */ diff --git a/src/ProcessAnalyzer/RectifiedAnalyzer.php b/src/ProcessAnalyzer/RectifiedAnalyzer.php index a21b9aca5bb..6dc304ee8ae 100644 --- a/src/ProcessAnalyzer/RectifiedAnalyzer.php +++ b/src/ProcessAnalyzer/RectifiedAnalyzer.php @@ -29,16 +29,11 @@ public function __construct( public function hasRectified(string $rectorClass, Node $node): bool { $originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE); - if ($this->hasConsecutiveCreatedByRule($rectorClass, $node, $originalNode)) { return true; } - if ($this->isJustReprintedOverlappedTokenStart($node, $originalNode)) { - return true; - } - - return $node->getAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE) === $rectorClass; + return $this->isJustReprintedOverlappedTokenStart($node, $originalNode); } /** diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index d8110732a47..abcc15b6662 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -12,6 +12,7 @@ use PhpParser\Node\Stmt\Interface_; use PhpParser\Node\Stmt\Property; use PhpParser\Node\Stmt\Trait_; +use PhpParser\NodeTraverser; use PhpParser\NodeVisitor; use PhpParser\NodeVisitorAbstract; use PHPStan\Analyser\MutatingScope; @@ -122,6 +123,9 @@ public function beforeTraverse(array $nodes): ?array return null; } + /** + * @return NodeTraverser::REMOVE_NODE|Node|null + */ final public function enterNode(Node $node): int|Node|null { if (! $this->isMatchingNodeType($node)) { @@ -140,52 +144,41 @@ final public function enterNode(Node $node): int|Node|null // ensure origNode pulled before refactor to avoid changed during refactor, ref https://3v4l.org/YMEGN $originalNode = $node->getAttribute(AttributeKey::ORIGINAL_NODE) ?? $node; - $refactoredNode = $this->refactor($node); + $refactoredNodeOrState = $this->refactor($node); // nothing to change → continue - if ($refactoredNode === null) { + if ($refactoredNodeOrState === null) { return null; } - if ($refactoredNode === []) { + if ($refactoredNodeOrState === []) { $errorMessage = sprintf(self::EMPTY_NODE_ARRAY_MESSAGE, static::class); throw new ShouldNotHappenException($errorMessage); } - $isIntRefactoredNode = is_int($refactoredNode); + $isState = is_int($refactoredNodeOrState); - /** - * If below node and/or its children not traversed on current rule - * early return null with decorate current and children node with skipped by "only" current rule - */ - if ($isIntRefactoredNode) { + if ($isState) { $this->createdByRuleDecorator->decorate($node, $originalNode, static::class); - if (in_array( - $refactoredNode, - [NodeVisitor::DONT_TRAVERSE_CHILDREN, NodeVisitor::DONT_TRAVERSE_CURRENT_AND_CHILDREN], - true - )) { - $this->decorateCurrentAndChildren($node); + // only remove node is supported + if ($refactoredNodeOrState !== NodeVisitor::REMOVE_NODE) { + // @todo warn about unsupported state in the future return null; } - // @see NodeVisitor::* codes, e.g. removal of node of stopping the traversing - if ($refactoredNode === NodeVisitor::REMOVE_NODE) { - // log here, so we can remove the node in leaveNode() method - $this->toBeRemovedNodeId = spl_object_id($originalNode); - } + // log here, so we can remove the node in leaveNode() method + $this->toBeRemovedNodeId = spl_object_id($originalNode); - // notify this rule changing code + // notify this rule changed code $rectorWithLineChange = new RectorWithLineChange(static::class, $originalNode->getStartLine()); $this->file->addRectorClassWithLine($rectorWithLineChange); - return $refactoredNode === NodeVisitor::REMOVE_NODE - ? $originalNode - : $refactoredNode; + // keep original node as node will be removed in leaveNode() + return $originalNode; } - return $this->postRefactorProcess($originalNode, $node, $refactoredNode, $filePath); + return $this->postRefactorProcess($originalNode, $node, $refactoredNodeOrState, $filePath); } /** @@ -275,35 +268,6 @@ protected function mirrorComments(Node $newNode, Node $oldNode): void $this->commentsMerger->mirrorComments($newNode, $oldNode); } - private function decorateCurrentAndChildren(Node $node): void - { - // skip sole type, as no other nodes to filter out - if (count($this->getNodeTypes()) === 1) { - return; - } - - // filter only types that - // 1. registered in getNodesTypes() method - // 2. different with current node type, as already decorated above - // - $otherTypes = array_filter( - $this->getNodeTypes(), - static fn (string $nodeType): bool => $nodeType !== $node::class - ); - - if ($otherTypes === []) { - return; - } - - $this->traverseNodesWithCallable($node, static function (Node $subNode) use ($otherTypes): null { - if (in_array($subNode::class, $otherTypes, true)) { - $subNode->setAttribute(AttributeKey::SKIPPED_BY_RECTOR_RULE, static::class); - } - - return null; - }); - } - /** * @param Node|Node[] $refactoredNode */