diff --git a/packages/CodeQuality/src/Rector/Class_/CompleteDynamicPropertiesRector.php b/packages/CodeQuality/src/Rector/Class_/CompleteDynamicPropertiesRector.php index c95572f73ec..0df05436b47 100644 --- a/packages/CodeQuality/src/Rector/Class_/CompleteDynamicPropertiesRector.php +++ b/packages/CodeQuality/src/Rector/Class_/CompleteDynamicPropertiesRector.php @@ -6,7 +6,9 @@ use Nette\Utils\Arrays; use PhpParser\Node; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Stmt\Class_; +use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Property; use Rector\NodeTypeResolver\Node\AttributeKey; use Rector\NodeTypeResolver\PhpDoc\NodeAnalyzer\DocBlockManipulator; @@ -22,6 +24,11 @@ use Rector\RectorDefinition\RectorDefinition; */ final class CompleteDynamicPropertiesRector extends AbstractRector { + /** + * @var string + */ + private const LARAVEL_COLLECTION_CLASS = 'Illuminate\Support\Collection'; + /** * @var TypeToStringResolver */ @@ -84,26 +91,21 @@ CODE_SAMPLE */ public function refactor(Node $node): ?Node { - if ($node->isAnonymous()) { + if (! $this->isNonAnonymousClass($node)) { return null; } /** @var string $class */ $class = $this->getName($node); + // properties are accessed via magic, nothing we can do if (method_exists($class, '__set') || method_exists($class, '__get')) { return null; } + // special case for Laravel Collection macro magic $fetchedLocalPropertyNameToTypes = $this->resolveFetchedLocalPropertyNameToTypes($node); - $propertyNames = []; - $this->traverseNodesWithCallable($node->stmts, function (Node $node) use (&$propertyNames) { - if (! $node instanceof Property) { - return null; - } - - $propertyNames[] = $this->getName($node); - }); + $propertyNames = $this->getClassPropertyNames($node); $fetchedLocalPropertyNames = array_keys($fetchedLocalPropertyNameToTypes); $propertiesToComplete = array_diff($fetchedLocalPropertyNames, $propertyNames); @@ -150,7 +152,9 @@ CODE_SAMPLE ->getNode(); } else { $newProperty = $propertyBuilder->getNode(); - $this->docBlockManipulator->changeVarTag($newProperty, $propertyTypesAsString); + if ($propertyTypesAsString) { + $this->docBlockManipulator->changeVarTag($newProperty, $propertyTypesAsString); + } } $newProperties[] = $newProperty; @@ -176,25 +180,69 @@ CODE_SAMPLE return null; } - $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); - - // fallback type - $propertyFetchType = ['mixed']; - - // possible get type - if ($parentNode instanceof Assign) { - $assignedValueStaticType = $this->getStaticType($parentNode->expr); - if ($assignedValueStaticType) { - $propertyFetchType = $this->typeToStringResolver->resolve($assignedValueStaticType); - } + // special Laravel collection scope + if ($this->shouldSkipForLaravelCollection($node)) { + return null; } /** @var string $propertyName */ $propertyName = $this->getName($node->name); + $propertyFetchType = $this->resolvePropertyFetchType($node); $fetchedLocalPropertyNameToTypes[$propertyName][] = $propertyFetchType; }); return $fetchedLocalPropertyNameToTypes; } + + /** + * @return string[] + */ + private function getClassPropertyNames(Class_ $class): array + { + $propertyNames = []; + + $this->traverseNodesWithCallable($class->stmts, function (Node $node) use (&$propertyNames) { + if (! $node instanceof Property) { + return null; + } + + $propertyNames[] = $this->getName($node); + }); + + return $propertyNames; + } + + /** + * @return string[] + */ + private function resolvePropertyFetchType(Node $node): array + { + $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE); + + // possible get type + if ($parentNode instanceof Assign) { + $assignedValueStaticType = $this->getStaticType($parentNode->expr); + if ($assignedValueStaticType) { + return $this->typeToStringResolver->resolve($assignedValueStaticType); + } + } + + // fallback type + return ['mixed']; + } + + private function shouldSkipForLaravelCollection(Node $node): bool + { + $staticCallOrClassMethod = $this->betterNodeFinder->findFirstAncestorInstancesOf( + $node, + [ClassMethod::class, StaticCall::class] + ); + + if (! $staticCallOrClassMethod instanceof StaticCall) { + return false; + } + + return $this->isName($staticCallOrClassMethod->class, self::LARAVEL_COLLECTION_CLASS); + } } diff --git a/packages/CodeQuality/tests/Rector/Class_/CompleteDynamicPropertiesRector/CompleteDynamicPropertiesRectorTest.php b/packages/CodeQuality/tests/Rector/Class_/CompleteDynamicPropertiesRector/CompleteDynamicPropertiesRectorTest.php index 7192e0f188e..1d9773ec857 100644 --- a/packages/CodeQuality/tests/Rector/Class_/CompleteDynamicPropertiesRector/CompleteDynamicPropertiesRectorTest.php +++ b/packages/CodeQuality/tests/Rector/Class_/CompleteDynamicPropertiesRector/CompleteDynamicPropertiesRectorTest.php @@ -17,6 +17,7 @@ final class CompleteDynamicPropertiesRectorTest extends AbstractRectorTestCase __DIR__ . '/Fixture/skip_trait_used.php.inc', __DIR__ . '/Fixture/skip_magic_parent.php.inc', __DIR__ . '/Fixture/skip_magic.php.inc', + __DIR__ . '/Fixture/skip_laravel_closure_binding.php.inc', ]); } diff --git a/packages/CodeQuality/tests/Rector/Class_/CompleteDynamicPropertiesRector/Fixture/skip_laravel_closure_binding.php.inc b/packages/CodeQuality/tests/Rector/Class_/CompleteDynamicPropertiesRector/Fixture/skip_laravel_closure_binding.php.inc new file mode 100644 index 00000000000..0693d5b4d78 --- /dev/null +++ b/packages/CodeQuality/tests/Rector/Class_/CompleteDynamicPropertiesRector/Fixture/skip_laravel_closure_binding.php.inc @@ -0,0 +1,23 @@ +<?php + +namespace Rector\CodeQuality\Tests\Rector\Class_\CompleteDynamicPropertiesRector\Fixture; + +use Illuminate\Support\Collection; + +class SkipLaravelClosureBinding +{ + /** + * @return void + */ + public function registerCollectionMacros(): void + { + Collection::macro('ksort', function (): Collection { + // macros callbacks are bound to collection so we can safely access + // protected Collection::items + $list = $this->items; + ksort($list); + + return new static($list); + }); + } +} diff --git a/src/PhpParser/Node/BetterNodeFinder.php b/src/PhpParser/Node/BetterNodeFinder.php index 9fbdccfca26..7d3ec163748 100644 --- a/src/PhpParser/Node/BetterNodeFinder.php +++ b/src/PhpParser/Node/BetterNodeFinder.php @@ -75,6 +75,26 @@ final class BetterNodeFinder return null; } + /** + * @param Node $node + * @param string[] $types + */ + public function findFirstAncestorInstancesOf(Node $node, array $types): ?Node + { + $currentNode = $node->getAttribute(AttributeKey::PARENT_NODE); + while ($currentNode !== null) { + foreach ($types as $type) { + if (is_a($currentNode, $type, true)) { + return $currentNode; + } + } + + $currentNode = $currentNode->getAttribute(AttributeKey::PARENT_NODE); + } + + return null; + } + /** * @param Node|Node[]|Stmt[] $nodes * @return Node[] diff --git a/src/Rector/AbstractRector/AbstractRectorTrait.php b/src/Rector/AbstractRector/AbstractRectorTrait.php index 16276403ba6..8202fcc22b5 100644 --- a/src/Rector/AbstractRector/AbstractRectorTrait.php +++ b/src/Rector/AbstractRector/AbstractRectorTrait.php @@ -2,6 +2,10 @@ namespace Rector\Rector\AbstractRector; +use Nette\Utils\Strings; +use PhpParser\Node; +use PhpParser\Node\Stmt\Class_; + trait AbstractRectorTrait { use AppliedRectorCollectorTrait; @@ -14,4 +18,22 @@ trait AbstractRectorTrait use VisibilityTrait; use ValueResolverTrait; use CallableNodeTraverserTrait; + + protected function isNonAnonymousClass(?Node $node): bool + { + if ($node === null) { + return false; + } + + if (! $node instanceof Class_) { + return false; + } + + $name = $this->getName($node); + if ($name === null) { + return false; + } + + return ! Strings::contains($name, 'AnonymousClass'); + } } diff --git a/src/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php b/src/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php index cd4b3b35319..acd5f38bd1a 100644 --- a/src/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php +++ b/src/Rector/MethodCall/ServiceGetterToConstructorInjectionRector.php @@ -2,7 +2,6 @@ namespace Rector\Rector\MethodCall; -use Nette\Utils\Strings; use PhpParser\Node; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; @@ -162,22 +161,4 @@ CODE_SAMPLE return $node; } - - private function isNonAnonymousClass(?Node $node): bool - { - if ($node === null) { - return false; - } - - if (! $node instanceof Class_) { - return false; - } - - $name = $this->getName($node); - if ($name === null) { - return false; - } - - return ! Strings::contains($name, 'AnonymousClass'); - } }