From 20ec11e73d8998f9ad7533c8f179ee7be8e2abdf Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Wed, 14 Jun 2023 11:35:12 +0700 Subject: [PATCH] [Docblock] Move DocBlockUpdater service usage from AbstractRector to PhpDocTypeChanger (#4215) * [Docblock] Move DocBlockUpdater service usage on PhpDocTypeChanger * [ci-review] Rector Rectify * Fix test * fix phpstan * update changeVarTypeNode as well * Fix phpstan * fix --------- Co-authored-by: GitHub Action --- .../PhpDocManipulator/PhpDocTypeChanger.php | 28 +++++++++++++++---- ...er_rename_class_without_namespace.php.inc} | 0 .../NodeFactory/PropertyTypeDecorator.php | 2 +- .../ClassConst/VarConstantCommentRector.php | 2 +- ...ertyAssignToConstructorPromotionRector.php | 2 +- .../PropertyTypeDecorator.php | 4 +-- .../AddVoidReturnTypeWhereNoReturnRector.php | 4 +-- ...rrayShapeFromConstantArrayReturnRector.php | 2 +- ...eturnAnnotationIncorrectNullableRector.php | 2 +- .../ReturnTypeFromStrictNewArrayRector.php | 4 +-- .../AddParamTypeSplFixedArrayRector.php | 2 +- ...pedPropertyFromStrictConstructorRector.php | 2 +- .../VarAnnotationIncorrectNullableRector.php | 2 +- src/NodeDecorator/PropertyTypeDecorator.php | 4 +-- src/Rector/AbstractRector.php | 11 -------- 15 files changed, 38 insertions(+), 33 deletions(-) rename rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/{rename_class_without_namespace2.php.inc => another_rename_class_without_namespace.php.inc} (100%) diff --git a/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php b/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php index 34787725722..11432c0b0f0 100644 --- a/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php +++ b/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTypeChanger.php @@ -4,7 +4,9 @@ namespace Rector\BetterPhpDocParser\PhpDocManipulator; +use PhpParser\Node\FunctionLike; use PhpParser\Node\Param; +use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Property; use PHPStan\PhpDocParser\Ast\ConstExpr\ConstFetchNode; @@ -29,6 +31,7 @@ use Rector\BetterPhpDocParser\ValueObject\Type\BracketsAwareUnionTypeNode; use Rector\BetterPhpDocParser\ValueObject\Type\SpacingAwareArrayTypeNode; use Rector\BetterPhpDocParser\ValueObject\Type\SpacingAwareCallableTypeNode; +use Rector\Comments\NodeDocBlock\DocBlockUpdater; use Rector\NodeNameResolver\NodeNameResolver; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\NodeTypeResolver\TypeComparator\TypeComparator; @@ -60,11 +63,12 @@ public function __construct( private readonly NodeNameResolver $nodeNameResolver, private readonly CommentsMerger $commentsMerger, private readonly PhpDocInfoFactory $phpDocInfoFactory, - private readonly NewPhpDocFromPHPStanTypeGuard $newPhpDocFromPHPStanTypeGuard + private readonly NewPhpDocFromPHPStanTypeGuard $newPhpDocFromPHPStanTypeGuard, + private readonly DocBlockUpdater $docBlockUpdater ) { } - public function changeVarType(PhpDocInfo $phpDocInfo, Type $newType): void + public function changeVarType(Stmt $stmt, PhpDocInfo $phpDocInfo, Type $newType): void { // better skip, could crash hard if ($phpDocInfo->hasInvalidTag('@var')) { @@ -102,9 +106,11 @@ public function changeVarType(PhpDocInfo $phpDocInfo, Type $newType): void $phpDocInfo->addTagValueNode($varTagValueNode); } + + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($stmt); } - public function changeReturnType(PhpDocInfo $phpDocInfo, Type $newType): bool + public function changeReturnType(FunctionLike $functionLike, PhpDocInfo $phpDocInfo, Type $newType): bool { // better not touch this, can crash if ($phpDocInfo->hasInvalidTag('@return')) { @@ -138,10 +144,12 @@ public function changeReturnType(PhpDocInfo $phpDocInfo, Type $newType): bool $phpDocInfo->addTagValueNode($returnTagValueNode); } + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($functionLike); + return true; } - public function changeParamType(PhpDocInfo $phpDocInfo, Type $newType, Param $param, string $paramName): void + public function changeParamType(FunctionLike $functionLike, PhpDocInfo $phpDocInfo, Type $newType, Param $param, string $paramName): void { // better skip, could crash hard if ($phpDocInfo->hasInvalidTag('@param')) { @@ -178,6 +186,8 @@ public function changeParamType(PhpDocInfo $phpDocInfo, Type $newType, Param $pa $paramTagValueNode = $this->paramPhpDocNodeFactory->create($phpDocTypeNode, $param); $phpDocInfo->addTagValueNode($paramTagValueNode); } + + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($functionLike); } public function isAllowed(TypeNode $typeNode): bool @@ -237,18 +247,20 @@ public function copyPropertyDocToParam(ClassMethod $classMethod, Property $prope $phpDocInfo = $classMethod->getAttribute(AttributeKey::PHP_DOC_INFO); $paramType = $this->staticTypeMapper->mapPHPStanPhpDocTypeToPHPStanType($varTagValueNode, $property); - $this->changeParamType($phpDocInfo, $paramType, $param, $paramVarName); + $this->changeParamType($classMethod, $phpDocInfo, $paramType, $param, $paramVarName); $this->processKeepComments($property, $param); } /** * @api doctrine */ - public function changeVarTypeNode(PhpDocInfo $phpDocInfo, TypeNode $typeNode): void + public function changeVarTypeNode(Stmt $stmt, PhpDocInfo $phpDocInfo, TypeNode $typeNode): void { // add completely new one $varTagValueNode = new VarTagValueNode($typeNode, '', ''); $phpDocInfo->addTagValueNode($varTagValueNode); + + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($stmt); } private function processKeepComments(Property $property, Param $param): void @@ -259,6 +271,8 @@ private function processKeepComments(Property $property, Param $param): void $toBeRemoved = ! $varTagValueNode instanceof VarTagValueNode; $this->commentsMerger->keepComments($param, [$property]); + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($property); + $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($param); $varTagValueNode = $phpDocInfo->getVarTagValueNode(); if (! $toBeRemoved) { @@ -274,5 +288,7 @@ private function processKeepComments(Property $property, Param $param): void } $phpDocInfo->removeByType(VarTagValueNode::class); + + $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($property); } } diff --git a/rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/rename_class_without_namespace2.php.inc b/rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/another_rename_class_without_namespace.php.inc similarity index 100% rename from rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/rename_class_without_namespace2.php.inc rename to rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/another_rename_class_without_namespace.php.inc diff --git a/rules/CodeQuality/NodeFactory/PropertyTypeDecorator.php b/rules/CodeQuality/NodeFactory/PropertyTypeDecorator.php index 57b790d9077..1008257c2aa 100644 --- a/rules/CodeQuality/NodeFactory/PropertyTypeDecorator.php +++ b/rules/CodeQuality/NodeFactory/PropertyTypeDecorator.php @@ -27,6 +27,6 @@ public function decorateProperty(Property $property, Type $propertyType): void $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($property); $phpDocInfo->makeMultiLined(); - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $propertyType); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $propertyType); } } diff --git a/rules/CodingStyle/Rector/ClassConst/VarConstantCommentRector.php b/rules/CodingStyle/Rector/ClassConst/VarConstantCommentRector.php index 734e6ebe945..ad3ed13b1d7 100644 --- a/rules/CodingStyle/Rector/ClassConst/VarConstantCommentRector.php +++ b/rules/CodingStyle/Rector/ClassConst/VarConstantCommentRector.php @@ -96,7 +96,7 @@ public function refactor(Node $node): ?Node return null; } - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $constType); + $this->phpDocTypeChanger->changeVarType($node, $phpDocInfo, $constType); if (! $phpDocInfo->hasChanged()) { return null; } diff --git a/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php b/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php index 7424647817d..2518338c994 100644 --- a/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php +++ b/rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php @@ -251,7 +251,7 @@ private function decorateParamWithPropertyPhpDocInfo( $paramType = $this->staticTypeMapper->mapPHPStanPhpDocTypeToPHPStanType($varTagValueNode, $property); $classMethodPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod); - $this->phpDocTypeChanger->changeParamType($classMethodPhpDocInfo, $paramType, $param, $paramName); + $this->phpDocTypeChanger->changeParamType($classMethod, $classMethodPhpDocInfo, $paramType, $param, $paramName); } else { $paramType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($param->type); } diff --git a/rules/TypeDeclaration/NodeTypeAnalyzer/PropertyTypeDecorator.php b/rules/TypeDeclaration/NodeTypeAnalyzer/PropertyTypeDecorator.php index 1e83186488b..44eff9e0637 100644 --- a/rules/TypeDeclaration/NodeTypeAnalyzer/PropertyTypeDecorator.php +++ b/rules/TypeDeclaration/NodeTypeAnalyzer/PropertyTypeDecorator.php @@ -42,7 +42,7 @@ public function decoratePropertyUnionType( } if ($changeVarTypeFallback) { - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $unionType); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $unionType); } return; @@ -66,7 +66,7 @@ public function decoratePropertyUnionType( return; } - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $unionType); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $unionType); } private function isDocBlockRequired(UnionType $unionType): bool diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddVoidReturnTypeWhereNoReturnRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddVoidReturnTypeWhereNoReturnRector.php index fe5079cb985..5fcb7ed642e 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddVoidReturnTypeWhereNoReturnRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddVoidReturnTypeWhereNoReturnRector.php @@ -137,7 +137,7 @@ public function configure(array $configuration): void $this->usePhpdoc = $usePhpdoc; } - private function changePhpDocToVoidIfNotNever(ClassMethod|Function_|Closure|Node $node): bool + private function changePhpDocToVoidIfNotNever(ClassMethod|Function_|Closure $node): bool { $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); @@ -145,7 +145,7 @@ private function changePhpDocToVoidIfNotNever(ClassMethod|Function_|Closure|Node return false; } - return $this->phpDocTypeChanger->changeReturnType($phpDocInfo, new VoidType()); + return $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, new VoidType()); } private function shouldSkipClassMethod(ClassMethod|Function_|Closure $functionLike): bool diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ArrayShapeFromConstantArrayReturnRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ArrayShapeFromConstantArrayReturnRector.php index 32078041e7a..7d3bfc51945 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ArrayShapeFromConstantArrayReturnRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ArrayShapeFromConstantArrayReturnRector.php @@ -129,7 +129,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node return null; } - $hasChanged = $this->phpDocTypeChanger->changeReturnType($phpDocInfo, $returnExprType); + $hasChanged = $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $returnExprType); if (! $hasChanged) { return null; } diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnAnnotationIncorrectNullableRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnAnnotationIncorrectNullableRector.php index 5645a0e03ed..3c0a72c9850 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnAnnotationIncorrectNullableRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnAnnotationIncorrectNullableRector.php @@ -117,7 +117,7 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node return null; } - $hasReturnTypeChanged = $this->phpDocTypeChanger->changeReturnType($phpDocInfo, $updatedPhpDocType); + $hasReturnTypeChanged = $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $updatedPhpDocType); if ($hasReturnTypeChanged) { return $node; diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php index e2733b95dbf..646a2e9f0e6 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictNewArrayRector.php @@ -165,14 +165,14 @@ private function shouldSkip(ClassMethod|Function_|Closure $node, Scope $scope): ); } - private function changeReturnType(Node $node, Type $exprType): void + private function changeReturnType(ClassMethod|Function_|Closure $node, Type $exprType): void { $phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node); $exprType = $this->narrowConstantArrayType($exprType); if (! $this->typeComparator->isSubtype($phpDocInfo->getReturnType(), $exprType)) { - $this->phpDocTypeChanger->changeReturnType($phpDocInfo, $exprType); + $this->phpDocTypeChanger->changeReturnType($node, $phpDocInfo, $exprType); } } diff --git a/rules/TypeDeclaration/Rector/FunctionLike/AddParamTypeSplFixedArrayRector.php b/rules/TypeDeclaration/Rector/FunctionLike/AddParamTypeSplFixedArrayRector.php index da0c843e526..6d6c28ff755 100644 --- a/rules/TypeDeclaration/Rector/FunctionLike/AddParamTypeSplFixedArrayRector.php +++ b/rules/TypeDeclaration/Rector/FunctionLike/AddParamTypeSplFixedArrayRector.php @@ -114,7 +114,7 @@ public function refactor(Node $node): ?Node } $paramName = $this->getName($param); - $this->phpDocTypeChanger->changeParamType($functionLikePhpDocInfo, $genericParamType, $param, $paramName); + $this->phpDocTypeChanger->changeParamType($node, $functionLikePhpDocInfo, $genericParamType, $param, $paramName); } if ($functionLikePhpDocInfo->hasChanged()) { diff --git a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php index 94ba5fa6ecb..b32bc0679ff 100644 --- a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php +++ b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromStrictConstructorRector.php @@ -110,7 +110,7 @@ public function refactor(Node $node): ?Node // public property can be anything if ($property->isPublic()) { - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $propertyType); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $propertyType); $hasChanged = true; continue; } diff --git a/rules/TypeDeclaration/Rector/Property/VarAnnotationIncorrectNullableRector.php b/rules/TypeDeclaration/Rector/Property/VarAnnotationIncorrectNullableRector.php index 4510d6ba08a..ad53e3d9766 100755 --- a/rules/TypeDeclaration/Rector/Property/VarAnnotationIncorrectNullableRector.php +++ b/rules/TypeDeclaration/Rector/Property/VarAnnotationIncorrectNullableRector.php @@ -115,7 +115,7 @@ public function refactor(Node $node): ?Node return null; } - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $updatedPhpDocType); + $this->phpDocTypeChanger->changeVarType($node, $phpDocInfo, $updatedPhpDocType); if (! $phpDocInfo->hasChanged()) { return null; diff --git a/src/NodeDecorator/PropertyTypeDecorator.php b/src/NodeDecorator/PropertyTypeDecorator.php index baf50effbbc..4b98ac1f381 100644 --- a/src/NodeDecorator/PropertyTypeDecorator.php +++ b/src/NodeDecorator/PropertyTypeDecorator.php @@ -39,13 +39,13 @@ public function decorate(Property $property, ?Type $type): void $property->type = $phpParserType; if ($type instanceof GenericObjectType) { - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $type); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $type); } return; } } - $this->phpDocTypeChanger->changeVarType($phpDocInfo, $type); + $this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $type); } } diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index fcb630b6948..d12373b2312 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -19,7 +19,6 @@ use PHPStan\Type\Type; use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory; use Rector\ChangesReporting\ValueObject\RectorWithLineChange; -use Rector\Comments\NodeDocBlock\DocBlockUpdater; use Rector\Core\Application\ChangedNodeScopeRefresher; use Rector\Core\Configuration\CurrentNodeProvider; use Rector\Core\Console\Output\RectorOutputStyle; @@ -104,8 +103,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn private FilePathHelper $filePathHelper; - private DocBlockUpdater $docBlockUpdater; - private NodeConnectingTraverser $nodeConnectingTraverser; private ?string $toBeRemovedNodeHash = null; @@ -130,7 +127,6 @@ public function autowire( ChangedNodeScopeRefresher $changedNodeScopeRefresher, RectorOutputStyle $rectorOutputStyle, FilePathHelper $filePathHelper, - DocBlockUpdater $docBlockUpdater, NodeConnectingTraverser $nodeConnectingTraverser ): void { $this->nodeNameResolver = $nodeNameResolver; @@ -151,7 +147,6 @@ public function autowire( $this->changedNodeScopeRefresher = $changedNodeScopeRefresher; $this->rectorOutputStyle = $rectorOutputStyle; $this->filePathHelper = $filePathHelper; - $this->docBlockUpdater = $docBlockUpdater; $this->nodeConnectingTraverser = $nodeConnectingTraverser; } @@ -392,12 +387,6 @@ private function refreshScopeNodes(array | Node $node, string $filePath, ?Mutati $nodes = $node instanceof Node ? [$node] : $node; foreach ($nodes as $node) { - /** - * Early refresh Doc Comment of Node before refresh Scope to ensure doc node is latest update - * to make PHPStan type can be correctly detected - */ - $this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node); - $this->changedNodeScopeRefresher->refresh($node, $mutatingScope, $filePath); } }