Skip to content

Commit

Permalink
Make PhpDocINfo::removeByType() return bool to inform about changed n…
Browse files Browse the repository at this point in the history
…ode (#4979)
  • Loading branch information
TomasVotruba committed Sep 11, 2023
1 parent 1593d00 commit 57ec646
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 8 deletions.
11 changes: 9 additions & 2 deletions packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,14 @@ public function findOneByAnnotationClass(string $desiredClass): ?DoctrineAnnotat
* @template T of \PHPStan\PhpDocParser\Ast\Node
* @param class-string<T> $typeToRemove
*/
public function removeByType(string $typeToRemove): void
public function removeByType(string $typeToRemove): bool
{
$hasChanged = false;

$phpDocNodeTraverser = new PhpDocNodeTraverser();
$phpDocNodeTraverser->traverseWithCallable($this->phpDocNode, '', function (Node $node) use (
$typeToRemove
$typeToRemove,
&$hasChanged,
): ?int {
if ($node instanceof PhpDocTagNode && $node->value instanceof $typeToRemove) {
// keep special annotation for tools
Expand All @@ -271,6 +274,7 @@ public function removeByType(string $typeToRemove): void
}

$this->markAsChanged();
$hasChanged = true;
return PhpDocNodeTraverser::NODE_REMOVE;
}

Expand All @@ -279,8 +283,11 @@ public function removeByType(string $typeToRemove): void
}

$this->markAsChanged();
$hasChanged = true;
return PhpDocNodeTraverser::NODE_REMOVE;
});

return $hasChanged;
}

public function addTagValueNode(PhpDocTagValueNode $phpDocTagValueNode): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@

final class PhpDocTagRemover
{
public function removeByName(PhpDocInfo $phpDocInfo, string $name): void
public function removeByName(PhpDocInfo $phpDocInfo, string $name): bool
{
$hasChanged = false;

$phpDocNode = $phpDocInfo->getPhpDocNode();

foreach ($phpDocNode->children as $key => $phpDocChildNode) {
Expand All @@ -23,16 +25,20 @@ public function removeByName(PhpDocInfo $phpDocInfo, string $name): void

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
Expand Down
21 changes: 16 additions & 5 deletions rules/DeadCode/Rector/ClassLike/RemoveAnnotationRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\Property;
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagValueNode;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfo;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
use Rector\Core\Rector\AbstractRector;
Expand Down Expand Up @@ -69,22 +70,32 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): ?Node
{
if ($this->annotationsToRemove === []) {
Assert::notEmpty($this->annotationsToRemove);

$phpDocInfo = $this->phpDocInfoFactory->createFromNode($node);
if (! $phpDocInfo instanceof PhpDocInfo) {
return null;
}

$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
$hasChanged = false;

foreach ($this->annotationsToRemove as $annotationToRemove) {
$this->phpDocTagRemover->removeByName($phpDocInfo, $annotationToRemove);
$namedHasChanged = $this->phpDocTagRemover->removeByName($phpDocInfo, $annotationToRemove);
if ($namedHasChanged) {
$hasChanged = true;
}

if (! is_a($annotationToRemove, PhpDocTagValueNode::class, true)) {

This comment has been minimized.

Copy link
@staabm

staabm Sep 11, 2023

Contributor

should this be instanceof instead to get rid of is_a which works against the static reflection concept?

This comment has been minimized.

Copy link
@TomasVotruba

TomasVotruba Sep 11, 2023

Author Member

The $annotationToRemove is a string value and should trigger autoload as uses mostly PHPStan tag value node types.

Static reflection is for project/native autoload.

continue;
}

$phpDocInfo->removeByType($annotationToRemove);
$typedHasChanged = $phpDocInfo->removeByType($annotationToRemove);
if ($typedHasChanged) {
$hasChanged = true;
}
}

if ($phpDocInfo->hasChanged()) {
if ($hasChanged) {
return $node;
}

Expand Down

0 comments on commit 57ec646

Please sign in to comment.