Skip to content

Commit

Permalink
Remove ununnecesary markAsChanged() in PhpDocTypeChanger, print docbl…
Browse files Browse the repository at this point in the history
…ock in explicit way when changed (#4983)
  • Loading branch information
TomasVotruba committed Sep 11, 2023
1 parent 2e6fcde commit ae3608b
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ final class PhpDocTagRemover
public function removeByName(PhpDocInfo $phpDocInfo, string $name): bool
{
$hasChanged = false;

$phpDocNode = $phpDocInfo->getPhpDocNode();

foreach ($phpDocNode->children as $key => $phpDocChildNode) {
Expand All @@ -26,43 +25,43 @@ public function removeByName(PhpDocInfo $phpDocInfo, string $name): bool
if ($this->areAnnotationNamesEqual($name, $phpDocChildNode->name)) {
unset($phpDocNode->children[$key]);
$hasChanged = true;
$phpDocInfo->markAsChanged();
}

if ($phpDocChildNode->value instanceof DoctrineAnnotationTagValueNode && $phpDocChildNode->value->hasClassName(
$name
)) {
unset($phpDocNode->children[$key]);
$hasChanged = true;
$phpDocInfo->markAsChanged();
}
}

return $hasChanged;
}

public function removeTagValueFromNode(PhpDocInfo $phpDocInfo, Node $desiredNode): void
public function removeTagValueFromNode(PhpDocInfo $phpDocInfo, Node $desiredNode): bool
{
$phpDocNode = $phpDocInfo->getPhpDocNode();
$hasChanged = false;

$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->traverseWithCallable($phpDocNode, '', static function (Node $node) use (
$desiredNode,
$phpDocInfo
&$hasChanged
): ?int {
if ($node instanceof PhpDocTagNode && $node->value === $desiredNode) {
$phpDocInfo->markAsChanged();
$hasChanged = true;
return PhpDocNodeTraverser::NODE_REMOVE;
}

if ($node !== $desiredNode) {
return null;
}

$phpDocInfo->markAsChanged();

$hasChanged = true;
return PhpDocNodeTraverser::NODE_REMOVE;
});

return $hasChanged;
}

private function areAnnotationNamesEqual(string $firstAnnotationName, string $secondAnnotationName): bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ public function changeVarType(Stmt $stmt, PhpDocInfo $phpDocInfo, Type $newType)
if ($currentVarTagValueNode instanceof VarTagValueNode) {
// only change type
$currentVarTagValueNode->type = $newPHPStanPhpDocTypeNode;
$phpDocInfo->markAsChanged();
} else {
// add completely new one
$varTagValueNode = new VarTagValueNode($newPHPStanPhpDocTypeNode, '', '');
Expand Down
6 changes: 4 additions & 2 deletions packages/Comments/NodeDocBlock/DocBlockUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function updateNodeWithPhpDocInfo(Node $node): void
public function updateRefactoredNodeWithPhpDocInfo(Node $node): void
{
// nothing to change? don't save it
$phpDocInfo = $this->resolveChangedPhpDocInfo($node);
$phpDocInfo = $node->getAttribute(AttributeKey::PHP_DOC_INFO);
if (! $phpDocInfo instanceof PhpDocInfo) {
return;
}
Expand All @@ -53,7 +53,9 @@ public function updateRefactoredNodeWithPhpDocInfo(Node $node): void
return;
}

$node->setDocComment(new Doc((string) $phpDocNode));
$printedPhpDoc = $this->printPhpDocInfoToString($phpDocInfo);
$node->setDocComment(new Doc($printedPhpDoc));
// $node->setDocComment(new Doc((string) $phpDocNode));
}

private function setCommentsAttribute(Node $node): void
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

namespace Rector\Tests\CodeQuality\Rector\Class_\InlineConstructorDefaultToPropertyRector\Fixture;

final class MultiProperties
{
private $thing1, $thing2, $thing3;

public function __construct()
{
$this->thing1 = 1;
$this->thing2 = 2;
$this->thing3 = 3;
}
}

?>
6 changes: 5 additions & 1 deletion rules/DeadCode/Rector/ClassLike/RemoveAnnotationRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagValueNode;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
Expand All @@ -29,7 +30,8 @@ final class RemoveAnnotationRector extends AbstractRector implements Configurabl
private array $annotationsToRemove = [];

public function __construct(
private readonly PhpDocTagRemover $phpDocTagRemover
private readonly PhpDocTagRemover $phpDocTagRemover,
private readonly DocBlockUpdater $docBlockUpdater,
) {
}

Expand Down Expand Up @@ -96,6 +98,8 @@ public function refactor(Node $node): ?Node
}

if ($hasChanged) {
$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node);

return $node;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PhpParser\Node\Stmt\ClassMethod;
use PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\NodeCollector\UnusedParameterResolver;
use Rector\DeadCode\NodeManipulator\VariadicFunctionLikeDetector;
Expand All @@ -26,7 +27,8 @@ final class RemoveUnusedPrivateMethodParameterRector extends AbstractRector
public function __construct(
private readonly VariadicFunctionLikeDetector $variadicFunctionLikeDetector,
private readonly UnusedParameterResolver $unusedParameterResolver,
private readonly PhpDocTagRemover $phpDocTagRemover
private readonly PhpDocTagRemover $phpDocTagRemover,
private readonly DocBlockUpdater $docBlockUpdater,
) {
}

Expand Down Expand Up @@ -201,6 +203,7 @@ private function shouldSkipClassMethod(ClassMethod $classMethod): bool
private function clearPhpDocInfo(ClassMethod $classMethod, array $unusedParameters): void
{
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);
$hasChanged = false;

foreach ($unusedParameters as $unusedParameter) {
$parameterName = $this->getName($unusedParameter->var);
Expand All @@ -217,7 +220,11 @@ private function clearPhpDocInfo(ClassMethod $classMethod, array $unusedParamete
continue;
}

$this->phpDocTagRemover->removeTagValueFromNode($phpDocInfo, $paramTagValueNode);
$hasChanged = $this->phpDocTagRemover->removeTagValueFromNode($phpDocInfo, $paramTagValueNode);
}

if ($hasChanged) {
$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($classMethod);
}
}
}
7 changes: 6 additions & 1 deletion rules/Php80/Rector/Class_/AnnotationToAttributeRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Rector\BetterPhpDocParser\PhpDoc\DoctrineAnnotationTagValueNode;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\PhpVersionFeature;
Expand Down Expand Up @@ -58,7 +59,8 @@ public function __construct(
private readonly PhpDocTagRemover $phpDocTagRemover,
private readonly AttributeGroupNamedArgumentManipulator $attributeGroupNamedArgumentManipulator,
private readonly UseImportsResolver $useImportsResolver,
private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer
private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer,
private readonly DocBlockUpdater $docBlockUpdater,
) {
}

Expand Down Expand Up @@ -141,6 +143,9 @@ public function refactor(Node $node): ?Node
return null;
}

// 3. Reprint docblock
$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node);

$this->attributeGroupNamedArgumentManipulator->decorate($attributeGroups);
$node->attrGroups = array_merge($node->attrGroups, $attributeGroups);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey;
use Rector\Comments\NodeDocBlock\DocBlockUpdater;
use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\PhpVersion;
Expand Down Expand Up @@ -46,7 +47,8 @@ public function __construct(
private readonly UseImportsResolver $useImportsResolver,
private readonly PhpDocTagRemover $phpDocTagRemover,
private readonly NestedAttrGroupsFactory $nestedAttrGroupsFactory,
private readonly UseNodesToAddCollector $useNodesToAddCollector
private readonly UseNodesToAddCollector $useNodesToAddCollector,
private readonly DocBlockUpdater $docBlockUpdater,
) {
}

Expand Down Expand Up @@ -121,6 +123,9 @@ public function refactor(Node $node): ?Node
return null;
}

// 3. Reprint docblock
$this->docBlockUpdater->updateRefactoredNodeWithPhpDocInfo($node);

$node->attrGroups = array_merge($node->attrGroups, $attributeGroups);
$this->completeExtraUseImports($attributeGroups);

Expand Down
4 changes: 0 additions & 4 deletions src/ValueObject/PhpVersionFeature.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ final class PhpVersionFeature
*/
public const CLASSNAME_CONSTANT = PhpVersion::PHP_55;

/*
* @var int
*/

/**
* @var int
*/
Expand Down

0 comments on commit ae3608b

Please sign in to comment.