From c1f36a43fcb0ed6ff37726a06122420b6850d9a7 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sat, 4 Jun 2022 19:29:23 +0200 Subject: [PATCH] Apply RemoveJustPropertyFetchRector and improve skipping for nodes --- .../PhpDocManipulator/PhpDocTagRemover.php | 4 +--- .../PhpDocParser/ClassAnnotationMatcher.php | 6 ++---- .../DoctrineAnnotationDecorator.php | 3 +-- .../Reflection/FamilyRelationsAnalyzer.php | 6 ++---- .../NodeNameResolver/FuncCallNameResolver.php | 6 ++---- .../NodeFactory/PhpAttributeGroupFactory.php | 6 +++--- rector.php | 3 +++ .../Fixture/skip_dim_fetch_assign.php.inc | 20 +++++++++++++++++++ .../skip_on_nodes_as_mostly_used.php.inc | 15 ++++++++++++++ .../RemoveJustPropertyFetchRector.php | 19 ++++++++++++++++++ rules/Naming/Naming/AliasNameResolver.php | 4 ++-- src/NodeManipulator/ArrayManipulator.php | 3 +-- src/NodeManipulator/IfManipulator.php | 6 ++---- 13 files changed, 73 insertions(+), 28 deletions(-) create mode 100644 rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_dim_fetch_assign.php.inc create mode 100644 rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_on_nodes_as_mostly_used.php.inc diff --git a/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTagRemover.php b/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTagRemover.php index 3772a5b76ec..fb264c4907b 100644 --- a/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTagRemover.php +++ b/packages/BetterPhpDocParser/PhpDocManipulator/PhpDocTagRemover.php @@ -27,9 +27,7 @@ public function removeByName(PhpDocInfo $phpDocInfo, string $name): void } if ($phpDocChildNode->value instanceof DoctrineAnnotationTagValueNode) { - $doctrineAnnotationTagValueNode = $phpDocChildNode->value; - - if ($doctrineAnnotationTagValueNode->hasClassName($name)) { + if ($phpDocChildNode->value->hasClassName($name)) { unset($phpDocNode->children[$key]); $phpDocInfo->markAsChanged(); } diff --git a/packages/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher.php b/packages/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher.php index dc5cb67602f..c9a089c84cb 100644 --- a/packages/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher.php +++ b/packages/BetterPhpDocParser/PhpDocParser/ClassAnnotationMatcher.php @@ -81,15 +81,13 @@ private function resolveAsAliased(array $uses, string $tag): string $prefix = $use instanceof GroupUse ? $use->prefix . '\\' : ''; - $useUses = $use->uses; - foreach ($useUses as $useUse) { + foreach ($use->uses as $useUse) { if (! $useUse->alias instanceof Identifier) { continue; } - $alias = $useUse->alias; - if ($alias->toString() === $tag) { + if ($useUse->alias->toString() === $tag) { return $prefix . $useUse->name->toString(); } } diff --git a/packages/BetterPhpDocParser/PhpDocParser/DoctrineAnnotationDecorator.php b/packages/BetterPhpDocParser/PhpDocParser/DoctrineAnnotationDecorator.php index 0d1aa7ed393..e37d913a7dc 100644 --- a/packages/BetterPhpDocParser/PhpDocParser/DoctrineAnnotationDecorator.php +++ b/packages/BetterPhpDocParser/PhpDocParser/DoctrineAnnotationDecorator.php @@ -242,11 +242,10 @@ private function createSpacelessPhpDocTagNode( GenericTagValueNode $genericTagValueNode, string $fullyQualifiedAnnotationClass ): SpacelessPhpDocTagNode { - $annotationContent = $genericTagValueNode->value; $formerStartEnd = $genericTagValueNode->getAttribute(PhpDocAttributeKey::START_AND_END); return $this->createDoctrineSpacelessPhpDocTagNode( - $annotationContent, + $genericTagValueNode->value, $tagName, $fullyQualifiedAnnotationClass, $formerStartEnd diff --git a/packages/FamilyTree/Reflection/FamilyRelationsAnalyzer.php b/packages/FamilyTree/Reflection/FamilyRelationsAnalyzer.php index 860aa878e75..0441007b54f 100644 --- a/packages/FamilyTree/Reflection/FamilyRelationsAnalyzer.php +++ b/packages/FamilyTree/Reflection/FamilyRelationsAnalyzer.php @@ -138,10 +138,8 @@ public function getClassLikeAncestorNames(Class_ | Interface_ | Name $classOrNam if ($classLike instanceof Class_) { if ($classLike->extends instanceof Name) { - $extendName = $classLike->extends; - - $ancestorNames[] = $this->nodeNameResolver->getName($extendName); - $ancestorNames = array_merge($ancestorNames, $this->getClassLikeAncestorNames($extendName)); + $ancestorNames[] = $this->nodeNameResolver->getName($classLike->extends); + $ancestorNames = array_merge($ancestorNames, $this->getClassLikeAncestorNames($classLike->extends)); } foreach ($classLike->implements as $implement) { diff --git a/packages/NodeNameResolver/NodeNameResolver/FuncCallNameResolver.php b/packages/NodeNameResolver/NodeNameResolver/FuncCallNameResolver.php index d66edd7c6ae..0362f34b76c 100644 --- a/packages/NodeNameResolver/NodeNameResolver/FuncCallNameResolver.php +++ b/packages/NodeNameResolver/NodeNameResolver/FuncCallNameResolver.php @@ -39,9 +39,7 @@ public function resolve(Node $node): ?string return null; } - $functionName = $node->name; - - $namespaceName = $functionName->getAttribute(AttributeKey::NAMESPACED_NAME); + $namespaceName = $node->name->getAttribute(AttributeKey::NAMESPACED_NAME); if ($namespaceName instanceof FullyQualified) { $functionFqnName = $namespaceName->toString(); @@ -50,6 +48,6 @@ public function resolve(Node $node): ?string } } - return (string) $functionName; + return (string) $node->name; } } diff --git a/packages/PhpAttribute/NodeFactory/PhpAttributeGroupFactory.php b/packages/PhpAttribute/NodeFactory/PhpAttributeGroupFactory.php index e096d72c9ca..78bebdb6cd8 100644 --- a/packages/PhpAttribute/NodeFactory/PhpAttributeGroupFactory.php +++ b/packages/PhpAttribute/NodeFactory/PhpAttributeGroupFactory.php @@ -125,12 +125,12 @@ private function removeUnwrappedItems(string $attributeClass, array $items): arr continue; } - $arrayItemKey = $item->key; - if (! $arrayItemKey instanceof String_) { + if (! $item->key instanceof String_) { continue; } - if (! in_array($arrayItemKey->value, $unwrappeColumns, true)) { + $stringItemKey = $item->key; + if (! in_array($stringItemKey->value, $unwrappeColumns, true)) { continue; } diff --git a/rector.php b/rector.php index 04aa04eb910..1c1cd5de70d 100644 --- a/rector.php +++ b/rector.php @@ -7,6 +7,7 @@ use Rector\CodingStyle\Rector\MethodCall\PreferThisOrSelfMethodCallRector; use Rector\CodingStyle\ValueObject\ReturnArrayClassMethodToYield; use Rector\Config\RectorConfig; +use Rector\DeadCode\Rector\StmtsAwareInterface\RemoveJustPropertyFetchRector; use Rector\Nette\Set\NetteSetList; use Rector\Php55\Rector\String_\StringClassNameToClassConstantRector; use Rector\Php81\Rector\Class_\MyCLabsClassToEnumRector; @@ -43,6 +44,8 @@ new ReturnArrayClassMethodToYield('PHPUnit\Framework\TestCase', '*provide*'), ]); + // $rectorConfig->rule(RemoveJustPropertyFetchRector::class); + $rectorConfig->paths([ __DIR__ . '/bin', __DIR__ . '/src', diff --git a/rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_dim_fetch_assign.php.inc b/rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_dim_fetch_assign.php.inc new file mode 100644 index 00000000000..76aa8f26ea0 --- /dev/null +++ b/rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_dim_fetch_assign.php.inc @@ -0,0 +1,20 @@ +items; + + // $this, self, static, FQN + $firstItemValue = $items[0]->value; + } +} diff --git a/rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_on_nodes_as_mostly_used.php.inc b/rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_on_nodes_as_mostly_used.php.inc new file mode 100644 index 00000000000..8d47ba7b353 --- /dev/null +++ b/rules-tests/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector/Fixture/skip_on_nodes_as_mostly_used.php.inc @@ -0,0 +1,15 @@ +value; + + return $name; + } +} diff --git a/rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector.php b/rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector.php index c9f4b50da27..cc30d4d783e 100644 --- a/rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector.php +++ b/rules/DeadCode/Rector/StmtsAwareInterface/RemoveJustPropertyFetchRector.php @@ -14,6 +14,7 @@ use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\While_; use PHPStan\PhpDocParser\Ast\PhpDoc\VarTagValueNode; +use PHPStan\Type\ObjectType; use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface; use Rector\Core\Rector\AbstractRector; use Rector\DeadCode\ValueObject\PropertyFetchToVariableAssign; @@ -206,6 +207,10 @@ private function matchVariableToPropertyAssign(Stmt $stmt): ?PropertyFetchToVari return null; } + if ($this->isPropertyFetchCallerNode($assign->expr)) { + return null; + } + // keep property fetch nesting if ($assign->expr->var instanceof PropertyFetch) { return null; @@ -217,4 +222,18 @@ private function matchVariableToPropertyAssign(Stmt $stmt): ?PropertyFetchToVari return new PropertyFetchToVariableAssign($assign->var, $assign->expr); } + + private function isPropertyFetchCallerNode(PropertyFetch $propertyFetch): bool + { + // skip nodes as mostly used with public property fetches + $propertyFetchCallerType = $this->getType($propertyFetch->var); + + if (! $propertyFetchCallerType instanceof ObjectType) { + return false; + } + + $nodeObjectType = new ObjectType('PhpParser\Node'); + return $nodeObjectType->isSuperTypeOf($propertyFetchCallerType) + ->yes(); + } } diff --git a/rules/Naming/Naming/AliasNameResolver.php b/rules/Naming/Naming/AliasNameResolver.php index e4be6d88efa..fd370424631 100644 --- a/rules/Naming/Naming/AliasNameResolver.php +++ b/rules/Naming/Naming/AliasNameResolver.php @@ -24,8 +24,8 @@ public function resolveByName(Name $name): ?string $prefix = $use instanceof GroupUse ? $use->prefix . '\\' : ''; - $useUses = $use->uses; - foreach ($useUses as $useUse) { + + foreach ($use->uses as $useUse) { if (! $useUse->alias instanceof Identifier) { continue; } diff --git a/src/NodeManipulator/ArrayManipulator.php b/src/NodeManipulator/ArrayManipulator.php index e4cceb253fe..1bc6e8735da 100644 --- a/src/NodeManipulator/ArrayManipulator.php +++ b/src/NodeManipulator/ArrayManipulator.php @@ -39,8 +39,7 @@ public function isDynamicArray(Array_ $array): bool return true; } - $value = $item->value; - if (! $this->isAllowedArrayValue($value)) { + if (! $this->isAllowedArrayValue($item->value)) { return true; } } diff --git a/src/NodeManipulator/IfManipulator.php b/src/NodeManipulator/IfManipulator.php index 642d1191f08..55cd830d0be 100644 --- a/src/NodeManipulator/IfManipulator.php +++ b/src/NodeManipulator/IfManipulator.php @@ -72,13 +72,11 @@ public function matchIfNotNullReturnValue(If_ $if): ?Expr */ public function matchIfValueReturnValue(If_ $if): ?Expr { - $stmts = $if->stmts; - - if (count($stmts) !== 1) { + if (count($if->stmts) !== 1) { return null; } - $insideIfStmt = $stmts[0]; + $insideIfStmt = $if->stmts[0]; if (! $insideIfStmt instanceof Return_) { return null; }