Skip to content

Commit

Permalink
[Docblock] Move DocBlockUpdater service usage from AbstractRector to …
Browse files Browse the repository at this point in the history
…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 <actions@github.com>
  • Loading branch information
samsonasik and actions-user committed Jun 14, 2023
1 parent 2551618 commit 20ec11e
Show file tree
Hide file tree
Showing 15 changed files with 38 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -274,5 +288,7 @@ private function processKeepComments(Property $property, Param $param): void
}

$phpDocInfo->removeByType(VarTagValueNode::class);

$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($property);
}
}
2 changes: 1 addition & 1 deletion rules/CodeQuality/NodeFactory/PropertyTypeDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function decoratePropertyUnionType(
}

if ($changeVarTypeFallback) {
$this->phpDocTypeChanger->changeVarType($phpDocInfo, $unionType);
$this->phpDocTypeChanger->changeVarType($property, $phpDocInfo, $unionType);
}

return;
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,15 @@ 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);

if ($phpDocInfo->getReturnType() instanceof NeverType) {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/NodeDecorator/PropertyTypeDecorator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
11 changes: 0 additions & 11 deletions src/Rector/AbstractRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,8 +103,6 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn

private FilePathHelper $filePathHelper;

private DocBlockUpdater $docBlockUpdater;

private NodeConnectingTraverser $nodeConnectingTraverser;

private ?string $toBeRemovedNodeHash = null;
Expand All @@ -130,7 +127,6 @@ public function autowire(
ChangedNodeScopeRefresher $changedNodeScopeRefresher,
RectorOutputStyle $rectorOutputStyle,
FilePathHelper $filePathHelper,
DocBlockUpdater $docBlockUpdater,
NodeConnectingTraverser $nodeConnectingTraverser
): void {
$this->nodeNameResolver = $nodeNameResolver;
Expand All @@ -151,7 +147,6 @@ public function autowire(
$this->changedNodeScopeRefresher = $changedNodeScopeRefresher;
$this->rectorOutputStyle = $rectorOutputStyle;
$this->filePathHelper = $filePathHelper;
$this->docBlockUpdater = $docBlockUpdater;
$this->nodeConnectingTraverser = $nodeConnectingTraverser;
}

Expand Down Expand Up @@ -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);
}
}
Expand Down

0 comments on commit 20ec11e

Please sign in to comment.