diff --git a/config/set/dead-code/dead-code.yaml b/config/set/dead-code/dead-code.yaml index 5215988676b..fd286e754bc 100644 --- a/config/set/dead-code/dead-code.yaml +++ b/config/set/dead-code/dead-code.yaml @@ -24,3 +24,4 @@ services: Rector\DeadCode\Rector\ClassMethod\RemoveDelegatingParentCallRector: ~ Rector\DeadCode\Rector\Instanceof_\RemoveDuplicatedInstanceOfRector: ~ Rector\DeadCode\Rector\Switch_\RemoveDuplicatedCaseInSwitchRector: ~ + Rector\DeadCode\Rector\Class_\RemoveUnusedDoctrineEntityMethodAndPropertyRector: ~ diff --git a/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php b/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php index a8fb7c817f5..04cb58404b6 100644 --- a/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php +++ b/packages/CodingStyle/src/Rector/Use_/RemoveUnusedAliasRector.php @@ -36,20 +36,20 @@ final class RemoveUnusedAliasRector extends AbstractRector */ private $resolvedDocPossibleAliases = []; - /** - * @var ShortNameResolver - */ - private $shortNameResolver; - /** * @var ClassNaming */ private $classNaming; - public function __construct(ShortNameResolver $shortNameResolver, ClassNaming $classNaming) + /** + * @var ShortNameResolver + */ + private $shortNameResolver; + + public function __construct(ClassNaming $classNaming, ShortNameResolver $shortNameResolver) { - $this->shortNameResolver = $shortNameResolver; $this->classNaming = $classNaming; + $this->shortNameResolver = $shortNameResolver; } public function getDefinition(): RectorDefinition diff --git a/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php b/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php new file mode 100644 index 00000000000..468737144e3 --- /dev/null +++ b/packages/DeadCode/src/Doctrine/DoctrineEntityManipulator.php @@ -0,0 +1,167 @@ +.*?)"#'; + + /** + * @var string + */ + private const TARGET_PROPERTY_PATTERN = '#(inversedBy|mappedBy)="(?.*?)"#'; + + /** + * @var string[] + */ + private const RELATION_ANNOTATIONS = [ + 'Doctrine\ORM\Mapping\OneToMany', + 'Doctrine\ORM\Mapping\ManyToOne', + 'Doctrine\ORM\Mapping\OneToOne', + 'Doctrine\ORM\Mapping\ManyToMany', + ]; + + /** + * @var string + */ + private const MAPPED_OR_INVERSED_BY_PATTERN = '#(,\s+)?(inversedBy|mappedBy)="(?.*?)"#'; + + /** + * @var string + */ + private const JOIN_COLUMN_ANNOTATION = 'Doctrine\ORM\Mapping\JoinColumn'; + + /** + * @var DocBlockManipulator + */ + private $docBlockManipulator; + + /** + * @var NamespaceAnalyzer + */ + private $namespaceAnalyzer; + + public function __construct(DocBlockManipulator $docBlockManipulator, NamespaceAnalyzer $namespaceAnalyzer) + { + $this->docBlockManipulator = $docBlockManipulator; + $this->namespaceAnalyzer = $namespaceAnalyzer; + } + + public function resolveTargetClass(Property $property): ?string + { + foreach (self::RELATION_ANNOTATIONS as $relationAnnotation) { + if (! $this->docBlockManipulator->hasTag($property, $relationAnnotation)) { + continue; + } + + $relationTag = $this->docBlockManipulator->getTagByName($property, $relationAnnotation); + if (! $relationTag->value instanceof GenericTagValueNode) { + throw new ShouldNotHappenException(); + } + + $match = Strings::match($relationTag->value->value, self::TARGET_ENTITY_PATTERN); + if (! isset($match['class'])) { + return null; + } + + $class = $match['class']; + + // fqnize possibly shorten class + if (Strings::contains($class, '\\')) { + return $class; + } + + if (! class_exists($class)) { + return $this->namespaceAnalyzer->resolveTypeToFullyQualified($class, $property); + } + + return $class; + } + + return null; + } + + public function resolveOtherProperty(Property $property): ?string + { + foreach (self::RELATION_ANNOTATIONS as $relationAnnotation) { + if (! $this->docBlockManipulator->hasTag($property, $relationAnnotation)) { + continue; + } + + $relationTag = $this->docBlockManipulator->getTagByName($property, $relationAnnotation); + if (! $relationTag->value instanceof GenericTagValueNode) { + throw new ShouldNotHappenException(); + } + + $match = Strings::match($relationTag->value->value, self::TARGET_PROPERTY_PATTERN); + + return $match['property'] ?? null; + } + + return null; + } + + public function isStandaloneDoctrineEntityClass(Class_ $class): bool + { + if ($class->isAnonymous()) { + return false; + } + + if ($class->isAbstract()) { + return false; + } + + // is parent entity + if ($this->docBlockManipulator->hasTag($class, 'Doctrine\ORM\Mapping\InheritanceType')) { + return false; + } + + return $this->docBlockManipulator->hasTag($class, 'Doctrine\ORM\Mapping\Entity'); + } + + public function removeMappedByOrInversedByFromProperty(Property $property): void + { + $doc = $property->getDocComment(); + if ($doc === null) { + return; + } + + $originalDocText = $doc->getText(); + $clearedDocText = Strings::replace($originalDocText, self::MAPPED_OR_INVERSED_BY_PATTERN); + + // no change + if ($originalDocText === $clearedDocText) { + return; + } + + $property->setDocComment(new Doc($clearedDocText)); + } + + public function isNullableRelation(Property $property): bool + { + if (! $this->docBlockManipulator->hasTag($property, self::JOIN_COLUMN_ANNOTATION)) { + // @see https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/annotations-reference.html#joincolumn + return true; + } + + $joinColumnTag = $this->docBlockManipulator->getTagByName($property, self::JOIN_COLUMN_ANNOTATION); + + if ($joinColumnTag->value instanceof GenericTagValueNode) { + return (bool) Strings::match($joinColumnTag->value->value, '#nullable=true#'); + } + + return false; + } +} diff --git a/packages/DeadCode/src/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector.php b/packages/DeadCode/src/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector.php new file mode 100644 index 00000000000..3b1087c11c4 --- /dev/null +++ b/packages/DeadCode/src/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector.php @@ -0,0 +1,288 @@ +parsedNodesByType = $parsedNodesByType; + $this->classUnusedPrivateClassMethodResolver = $classUnusedPrivateClassMethodResolver; + $this->classManipulator = $classManipulator; + $this->doctrineEntityManipulator = $doctrineEntityManipulator; + } + + public function getDefinition(): RectorDefinition + { + return new RectorDefinition('Removes unused methods and properties from Doctrine entity classes', [ + new CodeSample( + <<<'CODE_SAMPLE' +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class UserEntity +{ + /** + * @ORM\Column + */ + private $name; + + public function getName() + { + return $this->name; + } + + public function setName($name) + { + $this->name = $name; + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class UserEntity +{ +} +CODE_SAMPLE + ), + ]); + } + + /** + * @return string[] + */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Node + { + if (! $this->doctrineEntityManipulator->isStandaloneDoctrineEntityClass($node)) { + return null; + } + + $unusedMethodNames = $this->classUnusedPrivateClassMethodResolver->getClassUnusedMethodNames($node); + if ($unusedMethodNames !== []) { + $node = $this->removeClassMethodsByNames($node, $unusedMethodNames); + } + + $unusedPropertyNames = $this->resolveUnusedPrivatePropertyNames($node); + if ($unusedPropertyNames !== []) { + $node = $this->removeClassPrivatePropertiesByNames($node, $unusedPropertyNames); + } + + return $node; + } + + /** + * Remove unused methods immediately, so we can then remove unused properties. + * @param string[] $unusedMethodNames + */ + private function removeClassMethodsByNames(Class_ $class, array $unusedMethodNames): Class_ + { + foreach ($class->stmts as $key => $classStmt) { + if (! $classStmt instanceof ClassMethod) { + continue; + } + + if ($this->isNames($classStmt, $unusedMethodNames)) { + // remove immediately + unset($class->stmts[$key]); + } + } + + return $class; + } + + /** + * @return string[] + */ + private function resolveUnusedPrivatePropertyNames(Class_ $class): array + { + $privatePropertyNames = $this->classManipulator->getPrivatePropertyNames($class); + + // get list of fetched properties + $usedPropertyNames = $this->resolveClassUsedPropertyFetchNames($class); + + return array_diff($privatePropertyNames, $usedPropertyNames); + } + + /** + * @param string[] $unusedPropertyNames + */ + private function removeClassPrivatePropertiesByNames(Class_ $node, array $unusedPropertyNames): Class_ + { + foreach ($node->stmts as $key => $stmt) { + if (! $stmt instanceof Property) { + continue; + } + + if (! $this->isNames($stmt, $unusedPropertyNames)) { + continue; + } + + unset($node->stmts[$key]); + + // remove "$this->someProperty = new ArrayCollection()" + $propertyName = $this->getName($stmt); + if (isset($this->collectionByPropertyName[$propertyName])) { + $this->removeNode($this->collectionByPropertyName[$propertyName]); + } + + $this->removeInversedByOrMappedByOnRelatedProperty($stmt); + } + + return $node; + } + + private function getOtherRelationProperty(Property $property): ?Property + { + $targetClass = $this->doctrineEntityManipulator->resolveTargetClass($property); + $otherProperty = $this->doctrineEntityManipulator->resolveOtherProperty($property); + + if ($targetClass === null || $otherProperty === null) { + return null; + } + + // get the class property and remove "mappedBy/inversedBy" from annotation + $relatedEntityClass = $this->parsedNodesByType->findClass($targetClass); + if (! $relatedEntityClass instanceof Class_) { + return null; + } + + foreach ($relatedEntityClass->stmts as $relatedEntityClassStmt) { + if (! $relatedEntityClassStmt instanceof Property) { + continue; + } + + if (! $this->isName($relatedEntityClassStmt, $otherProperty)) { + continue; + } + + return $relatedEntityClassStmt; + } + + return null; + } + + private function removeInversedByOrMappedByOnRelatedProperty(Property $property): void + { + $otherRelationProperty = $this->getOtherRelationProperty($property); + if ($otherRelationProperty === null) { + return; + } + + $this->doctrineEntityManipulator->removeMappedByOrInversedByFromProperty($otherRelationProperty); + } + + private function isPropertyFetchAssignOfArrayCollection(PropertyFetch $propertyFetch): bool + { + $parentNode = $propertyFetch->getAttribute(AttributeKey::PARENT_NODE); + if (! $parentNode instanceof Assign) { + return false; + } + + if (! $parentNode->expr instanceof New_) { + return false; + } + + /** @var New_ $new */ + $new = $parentNode->expr; + + return $this->isName($new->class, 'Doctrine\Common\Collections\ArrayCollection'); + } + + /** + * @return string[] + */ + private function resolveClassUsedPropertyFetchNames(Class_ $class): array + { + $usedPropertyNames = []; + + $this->traverseNodesWithCallable($class->stmts, function (Node $node) use (&$usedPropertyNames) { + if (! $node instanceof PropertyFetch) { + return null; + } + + if (! $this->isName($node->var, 'this')) { + return null; + } + + /** @var string $propertyName */ + $propertyName = $this->getName($node->name); + + // skip collection initialization, e.g. "$this->someProperty = new ArrayCollection();" + if ($this->isPropertyFetchAssignOfArrayCollection($node)) { + /** @var Assign $parentNode */ + $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); + $this->collectionByPropertyName[$propertyName] = $parentNode; + return null; + } + + $usedPropertyNames[] = $propertyName; + + return null; + }); + + return $usedPropertyNames; + } +} diff --git a/packages/DeadCode/src/UnusedNodeResolver/ClassUnusedPrivateClassMethodResolver.php b/packages/DeadCode/src/UnusedNodeResolver/ClassUnusedPrivateClassMethodResolver.php new file mode 100644 index 00000000000..217b953fad0 --- /dev/null +++ b/packages/DeadCode/src/UnusedNodeResolver/ClassUnusedPrivateClassMethodResolver.php @@ -0,0 +1,106 @@ +nameResolver = $nameResolver; + $this->parsedNodesByType = $parsedNodesByType; + $this->classManipulator = $classManipulator; + } + + /** + * @return string[] + */ + public function getClassUnusedMethodNames(Class_ $class): array + { + /** @var string $className */ + $className = $this->nameResolver->getName($class); + $classMethodCalls = $this->parsedNodesByType->findMethodCallsOnClass($className); + + $usedMethodNames = array_keys($classMethodCalls); + $classPublicMethodNames = $this->classManipulator->getPublicMethodNames($class); + + return $this->getUnusedMethodNames($class, $classPublicMethodNames, $usedMethodNames); + } + + /** + * @param string[] $classPublicMethodNames + * @param string[] $usedMethodNames + * @return string[] + */ + private function getUnusedMethodNames(Class_ $class, array $classPublicMethodNames, array $usedMethodNames): array + { + $unusedMethods = array_diff($classPublicMethodNames, $usedMethodNames); + + $unusedMethods = $this->filterOutSystemMethods($unusedMethods); + + return $this->filterOutInterfaceRequiredMethods($class, $unusedMethods); + } + + /** + * @param string[] $unusedMethods + * @return string[] + */ + private function filterOutSystemMethods(array $unusedMethods): array + { + foreach ($unusedMethods as $key => $unusedMethod) { + // skip Doctrine-needed methods + if (in_array($unusedMethod, ['getId', 'setId'], true)) { + unset($unusedMethods[$key]); + } + + // skip magic methods + if (Strings::startsWith($unusedMethod, '__')) { + unset($unusedMethods[$key]); + } + } + + return $unusedMethods; + } + + /** + * @param string[] $unusedMethods + * @return string[] + */ + private function filterOutInterfaceRequiredMethods(Class_ $class, array $unusedMethods): array + { + /** @var string $className */ + $className = $this->nameResolver->getName($class); + + $interfaces = class_implements($className); + + $interfaceMethods = []; + foreach ($interfaces as $interface) { + $interfaceMethods = array_merge($interfaceMethods, get_class_methods($interface)); + } + + return array_diff($unusedMethods, $interfaceMethods); + } +} diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/fixture.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/fixture.php.inc new file mode 100644 index 00000000000..79dc6a69c27 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/fixture.php.inc @@ -0,0 +1,43 @@ +name; + } + + public function setName($name) + { + $this->name = $name; + } +} + +?> +----- + diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/remove_inversed_by.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/remove_inversed_by.php.inc new file mode 100644 index 00000000000..399e853c64b --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/remove_inversed_by.php.inc @@ -0,0 +1,81 @@ +voters; + } +} + +$answer = new Answer(); +$answer->getVoters(); + +/** + * @ORM\Entity + */ +class RemoveInversedBy +{ + /** + * @ORM\ManyToMany(targetEntity="Rector\DeadCode\Tests\Rector\Class_\RemoveUnusedDoctrineEntityMethodAndPropertyRector\Fixture\Answer", mappedBy="voters") + */ + private $answers; + + public function __construct() + { + $this->answers = new ArrayCollection; + } +} + +?> +----- +voters; + } +} + +$answer = new Answer(); +$answer->getVoters(); + +/** + * @ORM\Entity + */ +class RemoveInversedBy +{ + public function __construct() + { + } +} + +?> diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/remove_inversed_by_non_fqn.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/remove_inversed_by_non_fqn.php.inc new file mode 100644 index 00000000000..03c52a3e191 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/remove_inversed_by_non_fqn.php.inc @@ -0,0 +1,81 @@ +voters; + } +} + +$answer = new Question(); +$answer->getVoters(); + +/** + * @ORM\Entity + */ +class RemoveInversedByNonFqn +{ + /** + * @ORM\ManyToMany(targetEntity="Question", mappedBy="voters") + */ + private $answers; + + public function __construct() + { + $this->answers = new ArrayCollection; + } +} + +?> +----- +voters; + } +} + +$answer = new Question(); +$answer->getVoters(); + +/** + * @ORM\Entity + */ +class RemoveInversedByNonFqn +{ + public function __construct() + { + } +} + +?> diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_double_entity_call.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_double_entity_call.php.inc new file mode 100644 index 00000000000..ccc379d46ee --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_double_entity_call.php.inc @@ -0,0 +1,48 @@ +name; + } +} + +/** + * @ORM\Entity + */ +class SecondOne +{ + /** + * @ORM\Column + */ + private $name; + + public function getName() + { + return $this->name; + } +} + +class SkipDoubleEntityCall +{ + public function callOnMe($entity) + { + if ($entity instanceof FirstOne || $entity instanceof SecondOne) { + $entity->getName(); + } + } +} diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_id_and_system.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_id_and_system.php.inc new file mode 100644 index 00000000000..8eb1ee72032 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_id_and_system.php.inc @@ -0,0 +1,31 @@ +id = $id; + } + + public function getId() + { + return $this->id; + } + + public function __toString() + { + return 'keep me'; + } +} diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_trait_called_method.php.inc b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_trait_called_method.php.inc new file mode 100644 index 00000000000..16636dc6149 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/Fixture/skip_trait_called_method.php.inc @@ -0,0 +1,29 @@ +name; + } +} + +trait SkipTraitCalledMethod +{ + public function callOnMe(Insider $entity) + { + $entity->getName(); + } +} diff --git a/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/RemoveUnusedDoctrineEntityMethodAndPropertyRectorTest.php b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/RemoveUnusedDoctrineEntityMethodAndPropertyRectorTest.php new file mode 100644 index 00000000000..a66c1625b80 --- /dev/null +++ b/packages/DeadCode/tests/Rector/Class_/RemoveUnusedDoctrineEntityMethodAndPropertyRector/RemoveUnusedDoctrineEntityMethodAndPropertyRectorTest.php @@ -0,0 +1,27 @@ +doTestFiles([ + __DIR__ . '/Fixture/fixture.php.inc', + __DIR__ . '/Fixture/remove_inversed_by.php.inc', + __DIR__ . '/Fixture/remove_inversed_by_non_fqn.php.inc', + // skip + __DIR__ . '/Fixture/skip_double_entity_call.php.inc', + __DIR__ . '/Fixture/skip_id_and_system.php.inc', + __DIR__ . '/Fixture/skip_trait_called_method.php.inc', + ]); + } + + protected function getRectorClass(): string + { + return RemoveUnusedDoctrineEntityMethodAndPropertyRector::class; + } +} diff --git a/packages/NodeTypeResolver/src/PHPStan/Scope/NodeScopeResolver.php b/packages/NodeTypeResolver/src/PHPStan/Scope/NodeScopeResolver.php index 085cfd7075d..e487d4ce3a3 100644 --- a/packages/NodeTypeResolver/src/PHPStan/Scope/NodeScopeResolver.php +++ b/packages/NodeTypeResolver/src/PHPStan/Scope/NodeScopeResolver.php @@ -2,6 +2,7 @@ namespace Rector\NodeTypeResolver\PHPStan\Scope; +use Closure; use PhpParser\Node; use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassLike; @@ -15,6 +16,7 @@ use PHPStan\Broker\Broker; use Rector\Exception\ShouldNotHappenException; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\RemoveDeepChainMethodCallNodeVisitor; +use Rector\NodeTypeResolver\PHPStan\Scope\NodeVisitor\ScopeTraitNodeVisitor; use Rector\NodeTypeResolver\PHPStan\Scope\Stub\ClassReflectionForUnusedTrait; use ReflectionClass; use Symplify\PackageBuilder\Reflection\PrivatesAccessor; @@ -50,18 +52,25 @@ final class NodeScopeResolver */ private $privatesAccessor; + /** + * @var ScopeTraitNodeVisitor + */ + private $scopeTraitNodeVisitor; + public function __construct( ScopeFactory $scopeFactory, PHPStanNodeScopeResolver $phpStanNodeScopeResolver, Broker $broker, RemoveDeepChainMethodCallNodeVisitor $removeDeepChainMethodCallNodeVisitor, - PrivatesAccessor $privatesAccessor + PrivatesAccessor $privatesAccessor, + ScopeTraitNodeVisitor $scopeTraitNodeVisitor ) { $this->scopeFactory = $scopeFactory; $this->phpStanNodeScopeResolver = $phpStanNodeScopeResolver; $this->broker = $broker; $this->removeDeepChainMethodCallNodeVisitor = $removeDeepChainMethodCallNodeVisitor; $this->privatesAccessor = $privatesAccessor; + $this->scopeTraitNodeVisitor = $scopeTraitNodeVisitor; } /** @@ -75,21 +84,22 @@ final class NodeScopeResolver $this->phpStanNodeScopeResolver->setAnalysedFiles([$filePath]); // skip chain method calls, performance issue: https://github.com/phpstan/phpstan/issues/254 - $this->phpStanNodeScopeResolver->processNodes( - $nodes, - $this->scopeFactory->createFromFile($filePath), - function (Node $node, Scope $scope): void { - // the class reflection is resolved AFTER entering to class node - // so we need to get it from the first after this one - if ($node instanceof Class_ || $node instanceof Interface_) { - $scope = $this->resolveClassOrInterfaceScope($node, $scope); - } elseif ($node instanceof Trait_) { - $scope = $this->resolveTraitScope($node, $scope); - } - - $node->setAttribute(AttributeKey::SCOPE, $scope); + $nodeCallback = function (Node $node, Scope $scope): void { + // the class reflection is resolved AFTER entering to class node + // so we need to get it from the first after this one + if ($node instanceof Class_ || $node instanceof Interface_) { + $scope = $this->resolveClassOrInterfaceScope($node, $scope); + } elseif ($node instanceof Trait_) { + $scope = $this->resolveTraitScope($node, $scope); } - ); + + $node->setAttribute(AttributeKey::SCOPE, $scope); + }; + + $scope = $this->scopeFactory->createFromFile($filePath); + $this->phpStanNodeScopeResolver->processNodes($nodes, $scope, $nodeCallback); + + $this->resolveScopeInTrait($nodes, $nodeCallback); return $nodes; } @@ -138,9 +148,6 @@ final class NodeScopeResolver /** @var ScopeContext $scopeContext */ $scopeContext = $this->privatesAccessor->getPrivateProperty($scope, 'context'); - if ($scopeContext->getClassReflection() !== null) { - return $scope->enterTrait($traitReflection); - } // we need to emulate class reflection, because PHPStan is unable to analyze trait without it $classReflection = new ReflectionClass(ClassReflectionForUnusedTrait::class); @@ -158,4 +165,17 @@ final class NodeScopeResolver return $traitScope; } + + /** + * @param Node[] $nodes + */ + private function resolveScopeInTrait(array $nodes, Closure $nodeCallback): void + { + $traitNodeTraverser = new NodeTraverser(); + + $this->scopeTraitNodeVisitor->setNodeCallback($nodeCallback); + $traitNodeTraverser->addVisitor($this->scopeTraitNodeVisitor); + + $traitNodeTraverser->traverse($nodes); + } } diff --git a/packages/NodeTypeResolver/src/PHPStan/Scope/NodeVisitor/ScopeTraitNodeVisitor.php b/packages/NodeTypeResolver/src/PHPStan/Scope/NodeVisitor/ScopeTraitNodeVisitor.php new file mode 100644 index 00000000000..b645571d2cd --- /dev/null +++ b/packages/NodeTypeResolver/src/PHPStan/Scope/NodeVisitor/ScopeTraitNodeVisitor.php @@ -0,0 +1,68 @@ +phpStanNodeScopeResolver = $phpStanNodeScopeResolver; + $this->privatesCaller = $privatesCaller; + } + + public function setNodeCallback(Closure $nodeCallback): void + { + $this->nodeCallback = $nodeCallback; + } + + public function enterNode(Node $node) + { + if ($this->nodeCallback === null) { + throw new ShouldNotHappenException(); + } + + if (! $node instanceof Trait_) { + return null; + } + + $traitScope = $node->getAttribute(AttributeKey::SCOPE); + + $this->privatesCaller->callPrivateMethod( + $this->phpStanNodeScopeResolver, + 'processStmtNodes', + $node, + $node->stmts, + $traitScope, + $this->nodeCallback + ); + + return $node; + } +} diff --git a/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php b/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php index a3b65bea075..18c33b6714a 100644 --- a/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php +++ b/packages/TypeDeclaration/src/PropertyTypeInferer/DoctrineRelationPropertyTypeInferer.php @@ -2,12 +2,9 @@ namespace Rector\TypeDeclaration\PropertyTypeInferer; -use Nette\Utils\Strings; -use PhpParser\Node; use PhpParser\Node\Stmt\Property; -use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; +use Rector\DeadCode\Doctrine\DoctrineEntityManipulator; use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockManipulator; -use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\NamespaceAnalyzer; use Rector\TypeDeclaration\Contract\PropertyTypeInfererInterface; final class DoctrineRelationPropertyTypeInferer implements PropertyTypeInfererInterface @@ -23,11 +20,6 @@ final class DoctrineRelationPropertyTypeInferer implements PropertyTypeInfererIn */ private const TO_ONE_ANNOTATIONS = ['Doctrine\ORM\Mapping\ManyToOne', 'Doctrine\ORM\Mapping\OneToOne']; - /** - * @var string - */ - private const JOIN_COLUMN_ANNOTATION = 'Doctrine\ORM\Mapping\JoinColumn'; - /** * @var string */ @@ -39,14 +31,16 @@ final class DoctrineRelationPropertyTypeInferer implements PropertyTypeInfererIn private $docBlockManipulator; /** - * @var NamespaceAnalyzer + * @var DoctrineEntityManipulator */ - private $namespaceAnalyzer; + private $doctrineEntityManipulator; - public function __construct(DocBlockManipulator $docBlockManipulator, NamespaceAnalyzer $namespaceAnalyzer) - { + public function __construct( + DocBlockManipulator $docBlockManipulator, + DoctrineEntityManipulator $doctrineEntityManipulator + ) { $this->docBlockManipulator = $docBlockManipulator; - $this->namespaceAnalyzer = $namespaceAnalyzer; + $this->doctrineEntityManipulator = $doctrineEntityManipulator; } /** @@ -59,7 +53,7 @@ final class DoctrineRelationPropertyTypeInferer implements PropertyTypeInfererIn continue; } - return $this->processToManyRelation($property, $doctrineRelationAnnotation); + return $this->processToManyRelation($property); } foreach (self::TO_ONE_ANNOTATIONS as $doctrineRelationAnnotation) { @@ -67,7 +61,7 @@ final class DoctrineRelationPropertyTypeInferer implements PropertyTypeInfererIn continue; } - return $this->processToOneRelation($property, $doctrineRelationAnnotation); + return $this->processToOneRelation($property); } return []; @@ -81,12 +75,12 @@ final class DoctrineRelationPropertyTypeInferer implements PropertyTypeInfererIn /** * @return string[] */ - private function processToManyRelation(Property $property, string $doctrineRelationAnnotation): array + private function processToManyRelation(Property $property): array { $types = []; - $relationType = $this->resolveRelationType($property, $doctrineRelationAnnotation); - if ($relationType) { + $relationType = $this->doctrineEntityManipulator->resolveTargetClass($property); + if ($relationType !== null) { $types[] = $relationType . '[]'; } @@ -98,65 +92,19 @@ final class DoctrineRelationPropertyTypeInferer implements PropertyTypeInfererIn /** * @return string[] */ - private function processToOneRelation(Property $property, string $doctrineRelationAnnotation): array + private function processToOneRelation(Property $property): array { $types = []; - $relationType = $this->resolveRelationType($property, $doctrineRelationAnnotation); - if ($relationType) { + $relationType = $this->doctrineEntityManipulator->resolveTargetClass($property); + if ($relationType !== null) { $types[] = $relationType; } - if ($this->isNullableOneRelation($property)) { + if ($this->doctrineEntityManipulator->isNullableRelation($property)) { $types[] = 'null'; } return $types; } - - private function resolveTargetEntity(GenericTagValueNode $genericTagValueNode): ?string - { - $match = Strings::match($genericTagValueNode->value, '#targetEntity=\"(?.*?)\"#'); - - return $match['targetEntity'] ?? null; - } - - private function resolveRelationType(Property $property, string $doctrineRelationAnnotation): ?string - { - $relationTag = $this->docBlockManipulator->getTagByName($property, $doctrineRelationAnnotation); - - if ($relationTag->value instanceof GenericTagValueNode) { - $resolveTargetType = $this->resolveTargetEntity($relationTag->value); - if ($resolveTargetType) { - if (Strings::contains($resolveTargetType, '\\')) { - return $resolveTargetType; - } - - // is FQN? - if (! class_exists($resolveTargetType)) { - return $this->namespaceAnalyzer->resolveTypeToFullyQualified($resolveTargetType, $property); - } - - return $resolveTargetType; - } - } - - return null; - } - - private function isNullableOneRelation(Node $node): bool - { - if (! $this->docBlockManipulator->hasTag($node, self::JOIN_COLUMN_ANNOTATION)) { - // @see https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/annotations-reference.html#joincolumn - return true; - } - - $joinColumnTag = $this->docBlockManipulator->getTagByName($node, self::JOIN_COLUMN_ANNOTATION); - - if ($joinColumnTag->value instanceof GenericTagValueNode) { - return (bool) Strings::match($joinColumnTag->value->value, '#nullable=true#'); - } - - return false; - } } diff --git a/phpstan.neon b/phpstan.neon index 6906e1b6f36..78c1eb7acca 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -179,3 +179,6 @@ parameters: - '#Parameter \#2 \$listener of method Symfony\\Component\\EventDispatcher\\Debug\\TraceableEventDispatcher\:\:getListenerPriority\(\) expects callable\(\)\: mixed, array given#' - '#Parameter \#1 \$kernelClass of method Rector\\Symfony\\Bridge\\DependencyInjection\\SymfonyContainerFactory\:\:createFromKernelClass\(\) expects string, string\|null given#' - '#If condition is always true#' + + # known value + - '#Method Rector\\PhpParser\\Node\\Manipulator\\ClassMethodManipulator\:\:addMethodParameterIfMissing\(\) should return string but returns string\|null#' diff --git a/src/NodeContainer/ParsedNodesByType.php b/src/NodeContainer/ParsedNodesByType.php index 1925b902e07..4404d2f0a91 100644 --- a/src/NodeContainer/ParsedNodesByType.php +++ b/src/NodeContainer/ParsedNodesByType.php @@ -584,15 +584,9 @@ final class ParsedNodesByType */ private function addCall(Node $node): void { - if ($node instanceof MethodCall && $node->var instanceof Variable && $node->var->name === 'this') { - $className = $node->getAttribute(AttributeKey::CLASS_NAME); - } elseif ($node instanceof MethodCall) { - $className = $this->nodeTypeResolver->resolve($node->var)[0] ?? null; - } else { - $className = $this->nodeTypeResolver->resolve($node)[0] ?? null; - } - - if ($className === null) { // anonymous + // one node can be of multiple-class types + $classTypes = $this->resolveNodeClassTypes($node); + if ($classTypes === []) { // anonymous return; } @@ -601,7 +595,9 @@ final class ParsedNodesByType return; } - $this->methodsCallsByTypeAndMethod[$className][$methodName][] = $node; + foreach ($classTypes as $classType) { + $this->methodsCallsByTypeAndMethod[$classType][$methodName][] = $node; + } } /** @@ -669,4 +665,19 @@ final class ParsedNodesByType return false; } + + /** + * @return string[] + */ + private function resolveNodeClassTypes(Node $node): array + { + if ($node instanceof MethodCall && $node->var instanceof Variable && $node->var->name === 'this') { + $className = $node->getAttribute(AttributeKey::CLASS_NAME); + return $className ? [$className] : []; + } elseif ($node instanceof MethodCall) { + return $this->nodeTypeResolver->resolve($node->var); + } + + return $this->nodeTypeResolver->resolve($node); + } } diff --git a/src/PhpParser/Node/Manipulator/ClassManipulator.php b/src/PhpParser/Node/Manipulator/ClassManipulator.php index 958d396da14..ef9587d56d7 100644 --- a/src/PhpParser/Node/Manipulator/ClassManipulator.php +++ b/src/PhpParser/Node/Manipulator/ClassManipulator.php @@ -290,6 +290,53 @@ final class ClassManipulator return null; } + /** + * @return string[] + */ + public function getPrivatePropertyNames(Class_ $class): array + { + $privatePropertyNames = []; + foreach ($class->stmts as $stmt) { + if (! $stmt instanceof Property) { + continue; + } + + if (! $stmt->isPrivate()) { + continue; + } + + /** @var string $propertyName */ + $propertyName = $this->nameResolver->getName($stmt); + $privatePropertyNames[] = $propertyName; + } + + return $privatePropertyNames; + } + + /** + * @return string[] + */ + public function getPublicMethodNames(Class_ $class): array + { + $publicMethodNames = []; + foreach ($class->getMethods() as $method) { + if ($method->isAbstract()) { + continue; + } + + if (! $method->isPublic()) { + continue; + } + + /** @var string $methodName */ + $methodName = $this->nameResolver->getName($method); + + $publicMethodNames[] = $methodName; + } + + return $publicMethodNames; + } + private function tryInsertBeforeFirstMethod(Class_ $classNode, Stmt $stmt): bool { foreach ($classNode->stmts as $key => $classElementNode) { diff --git a/src/PhpParser/Node/Manipulator/FunctionLikeManipulator.php b/src/PhpParser/Node/Manipulator/FunctionLikeManipulator.php index 2848903bda9..c8fdf13faab 100644 --- a/src/PhpParser/Node/Manipulator/FunctionLikeManipulator.php +++ b/src/PhpParser/Node/Manipulator/FunctionLikeManipulator.php @@ -5,6 +5,7 @@ namespace Rector\PhpParser\Node\Manipulator; use Iterator; use PhpParser\Node; use PhpParser\Node\Expr\Closure; +use PhpParser\Node\Expr\Yield_; use PhpParser\Node\FunctionLike; use PhpParser\Node\Identifier; use PhpParser\Node\Name; @@ -174,8 +175,8 @@ final class FunctionLikeManipulator */ private function resolveFromYieldNodes(FunctionLike $functionLike): array { - /** @var Node\Expr\Yield_[] $yieldNodes */ - $yieldNodes = $this->betterNodeFinder->findInstanceOf((array) $functionLike->stmts, Node\Expr\Yield_::class); + /** @var Yield_[] $yieldNodes */ + $yieldNodes = $this->betterNodeFinder->findInstanceOf((array) $functionLike->stmts, Yield_::class); if (count($yieldNodes)) { $this->isVoid = false; diff --git a/utils/PHPStanExtensions/config/phpstan-extensions.neon b/utils/PHPStanExtensions/config/phpstan-extensions.neon index 0b81de20bc7..4b30327deef 100644 --- a/utils/PHPStanExtensions/config/phpstan-extensions.neon +++ b/utils/PHPStanExtensions/config/phpstan-extensions.neon @@ -4,7 +4,7 @@ services: # $node->geAttribute($1) => Type|null by $1 - { class: Rector\PHPStanExtensions\Rector\Type\GetAttributeReturnTypeExtension, tags: [phpstan.broker.dynamicMethodReturnTypeExtension] } - # $nameResolver->resolve() => in some cases always string + # $nameResolver->getName() => in some cases always string - { class: Rector\PHPStanExtensions\Rector\Type\NameResolverReturnTypeExtension, tags: [phpstan.broker.dynamicMethodReturnTypeExtension] } # $nameResolverTrait->getName() => in some cases always string - { class: Rector\PHPStanExtensions\Rector\Type\NameResolverTraitReturnTypeExtension, tags: [phpstan.broker.dynamicMethodReturnTypeExtension] }