skip Illuminate\Support\Collection magic for CompleteDynamicPro… (#1721)

skip Illuminate\Support\Collection magic for CompleteDynamicPropertiesRector
This commit is contained in:
Tomáš Votruba 2019-07-10 16:43:15 +02:00 committed by GitHub
commit ba8aa22253
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 135 additions and 40 deletions

View File

@ -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);
}
}

View File

@ -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',
]);
}

View File

@ -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);
});
}
}

View File

@ -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[]

View File

@ -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');
}
}

View File

@ -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');
}
}