octoleo/rector
1
1
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2024-06-07 11:50:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify PropertyFetchByMethodAnalyzer (#6147)

Browse Source 
Co-authored-by: kaizen-ci <info@kaizen-ci.org>
This commit is contained in:
Tomas Votruba 2021-04-16 00:30:36 +02:00 committed by GitHub
parent 7bf3eeb61d
commit e7e1b091d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 218 additions and 171 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
.github/workflows/rector.yaml vendored
View File

@ -25,10 +25,8 @@ jobs:
fail-fast: false
matrix:
directories:
- src tests
- packages packages-tests
- src tests rules-tests packages packages-tests
- rules
- rules-tests
runs-on: ubuntu-latest
steps:

4
phpstan.neon
View File

@ -527,8 +527,4 @@ parameters:
message: '#\$this as argument is not allowed\. Refactor method to service composition#'
path: rules/TypeDeclaration/Rector/ClassMethod/ParamTypeFromStrictTypedPropertyRector.php
-
message: '#Class cognitive complexity is 32, keep it under 30#'
path: rules/Privatization/Rector/Class_/ChangeLocalPropertyToVariableRector.php
- '#Method Rector\\Core\\PhpParser\\Node\\BetterNodeFinder\:\:findParentTypes\(\) should return T of PhpParser\\Node\|null but returns class\-string<T of PhpParser\\Node\>\|T of PhpParser\\Node#'

1
rules/CodingStyle/Rector/Class_/AddArrayDefaultToArrayPropertyRector.php
View File

@ -262,6 +262,7 @@ CODE_SAMPLE
) && $this->valueResolver->isNull($expr->right)) {
return true;
}
if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($expr->right, $propertyNames)) {
return false;
}

2
rules/DeadCode/Rector/ClassLike/RemoveAnnotationRector.php
View File

@ -11,6 +11,7 @@ use PhpParser\Node\Stmt\ClassLike;
use PhpParser\Node\Stmt\Property;
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagValueNode;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
use Rector\Core\Rector\AbstractRector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
@ -96,6 +97,7 @@ CODE_SAMPLE
}
if ($phpDocInfo->hasChanged()) {
$this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
return null;
}

2
rules/DeadCode/Rector/ClassMethod/RemoveUselessParamTagRector.php
View File

@ -7,6 +7,7 @@ namespace Rector\DeadCode\Rector\ClassMethod;
use PhpParser\Node;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTagRemover;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\PhpDoc\DeadParamTagValueNodeAnalyzer;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@ -93,6 +94,7 @@ CODE_SAMPLE
}
if ($phpDocInfo->hasChanged()) {
$this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
return $node;
}

2
rules/DeadCode/Rector/ClassMethod/RemoveUselessReturnTagRector.php
View File

@ -6,6 +6,7 @@ namespace Rector\DeadCode\Rector\ClassMethod;
use PhpParser\Node;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\PhpDoc\TagRemover\ReturnTagRemover;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@ -77,6 +78,7 @@ CODE_SAMPLE
$this->returnTagRemover->removeReturnTagIfUseless($phpDocInfo, $node);
if ($phpDocInfo->hasChanged()) {
$this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
return $node;
}

2
rules/DeadCode/Rector/Property/RemoveUselessVarTagRector.php
View File

@ -6,6 +6,7 @@ namespace Rector\DeadCode\Rector\Property;
use PhpParser\Node;
use PhpParser\Node\Stmt\Property;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\PhpDoc\TagRemover\VarTagRemover;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@ -69,6 +70,7 @@ CODE_SAMPLE
$this->varTagRemover->removeVarTagIfUseless($phpDocInfo, $node);
if ($phpDocInfo->hasChanged()) {
$this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
return $node;
}

192
rules/Privatization/NodeAnalyzer/PropertyFetchByMethodAnalyzer.php Normal file
View File

@ -0,0 +1,192 @@
<?php
declare(strict_types=1);
namespace Rector\Privatization\NodeAnalyzer;
use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Do_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\While_;
use PhpParser\NodeTraverser;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\Core\ValueObject\MethodName;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Symplify\Astral\NodeTraverser\SimpleCallableNodeTraverser;
final class PropertyFetchByMethodAnalyzer
{
/**
* @var array<class-string<Stmt>>
*/
private const SCOPE_CHANGING_NODE_TYPES = [Do_::class, While_::class, If_::class, Else_::class];
/**
* @var NodeNameResolver
*/
private $nodeNameResolver;
/**
* @var PropertyFetchAnalyzer
*/
private $propertyFetchAnalyzer;
/**
* @var SimpleCallableNodeTraverser
*/
private $simpleCallableNodeTraverser;
public function __construct(
NodeNameResolver $nodeNameResolver,
PropertyFetchAnalyzer $propertyFetchAnalyzer,
SimpleCallableNodeTraverser $simpleCallableNodeTraverser
) {
$this->nodeNameResolver = $nodeNameResolver;
$this->propertyFetchAnalyzer = $propertyFetchAnalyzer;
$this->simpleCallableNodeTraverser = $simpleCallableNodeTraverser;
}
/**
* @param string[] $propertyNames
* @return array<string, string[]>
*/
public function collectPropertyFetchByMethods(Class_ $class, array $propertyNames): array
{
$propertyUsageByMethods = [];
foreach ($propertyNames as $propertyName) {
foreach ($class->getMethods() as $classMethod) {
// assigned in constructor injection → skip
if ($this->nodeNameResolver->isName($classMethod, MethodName::CONSTRUCT)) {
return [];
}
if (! $this->propertyFetchAnalyzer->containsLocalPropertyFetchName($classMethod, $propertyName)) {
continue;
}
if ($this->isPropertyChangingInMultipleMethodCalls($classMethod, $propertyName)) {
continue;
}
$classMethodName = $this->nodeNameResolver->getName($classMethod);
$propertyUsageByMethods[$propertyName][] = $classMethodName;
}
}
return $propertyUsageByMethods;
}
/**
* Covers https://github.com/rectorphp/rector/pull/2558#discussion_r363036110
*/
private function isPropertyChangingInMultipleMethodCalls(ClassMethod $classMethod, string $propertyName): bool
{
$isPropertyChanging = false;
$isPropertyReadInIf = false;
$isIfFollowedByAssign = false;
$this->simpleCallableNodeTraverser->traverseNodesWithCallable(
(array) $classMethod->getStmts(),
function (Node $node) use (
&$isPropertyChanging,
$propertyName,
&$isPropertyReadInIf,
&$isIfFollowedByAssign
): ?int {
if ($isPropertyReadInIf) {
if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($node, [$propertyName])) {
return null;
}
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
if ($parentNode instanceof Assign && $parentNode->var === $node) {
$isIfFollowedByAssign = true;
}
}
if (! $this->isScopeChangingNode($node)) {
return null;
}
if ($node instanceof If_) {
$isPropertyReadInIf = $this->refactorIf($node, $propertyName);
}
$isPropertyChanging = $this->isPropertyChanging($node, $propertyName);
if (! $isPropertyChanging) {
return null;
}
return NodeTraverser::STOP_TRAVERSAL;
}
);
return $isPropertyChanging || $isIfFollowedByAssign || $isPropertyReadInIf;
}
private function isScopeChangingNode(Node $node): bool
{
foreach (self::SCOPE_CHANGING_NODE_TYPES as $type) {
if (is_a($node, $type, true)) {
return true;
}
}
return false;
}
private function refactorIf(If_ $if, string $privatePropertyName): ?bool
{
$this->simpleCallableNodeTraverser->traverseNodesWithCallable($if->cond, function (Node $node) use (
$privatePropertyName,
&$isPropertyReadInIf
): ?int {
if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($node, [$privatePropertyName])) {
return null;
}
$isPropertyReadInIf = true;
return NodeTraverser::STOP_TRAVERSAL;
});
return $isPropertyReadInIf;
}
private function isPropertyChanging(Node $node, string $privatePropertyName): bool
{
$isPropertyChanging = false;
// here cannot be any property assign
$this->simpleCallableNodeTraverser->traverseNodesWithCallable($node, function (Node $node) use (
&$isPropertyChanging,
$privatePropertyName
): ?int {
if (! $node instanceof Assign) {
return null;
}
if (! $node->var instanceof PropertyFetch) {
return null;
}
if (! $this->nodeNameResolver->isName($node->var->name, $privatePropertyName)) {
return null;
}
$isPropertyChanging = true;
return NodeTraverser::STOP_TRAVERSAL;
});
return $isPropertyChanging;
}
}

177
rules/Privatization/Rector/Class_/ChangeLocalPropertyToVariableRector.php
View File

@ -5,21 +5,10 @@ declare(strict_types=1);
namespace Rector\Privatization\Rector\Class_;
use PhpParser\Node;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Do_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\While_;
use PhpParser\NodeTraverser;
use Rector\Core\NodeAnalyzer\PropertyFetchAnalyzer;
use Rector\Core\NodeManipulator\ClassManipulator;
use Rector\Core\Rector\AbstractRector;
use Rector\Core\ValueObject\MethodName;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Privatization\NodeAnalyzer\PropertyFetchByMethodAnalyzer;
use Rector\Privatization\NodeReplacer\PropertyFetchWithVariableReplacer;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@ -29,34 +18,29 @@ use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
*/
final class ChangeLocalPropertyToVariableRector extends AbstractRector
{
/**
* @var array<class-string<Stmt>>
*/
private const SCOPE_CHANGING_NODE_TYPES = [Do_::class, While_::class, If_::class, Else_::class];
/**
* @var ClassManipulator
*/
private $classManipulator;
/**
* @var PropertyFetchAnalyzer
*/
private $propertyFetchAnalyzer;
/**
* @var PropertyFetchWithVariableReplacer
*/
private $propertyFetchWithVariableReplacer;
/**
* @var PropertyFetchByMethodAnalyzer
*/
private $propertyFetchByMethodAnalyzer;
public function __construct(
ClassManipulator $classManipulator,
PropertyFetchAnalyzer $propertyFetchAnalyzer,
PropertyFetchWithVariableReplacer $propertyFetchWithVariableReplacer
PropertyFetchWithVariableReplacer $propertyFetchWithVariableReplacer,
PropertyFetchByMethodAnalyzer $propertyFetchByMethodAnalyzer
) {
$this->classManipulator = $classManipulator;
$this->propertyFetchAnalyzer = $propertyFetchAnalyzer;
$this->propertyFetchWithVariableReplacer = $propertyFetchWithVariableReplacer;
$this->propertyFetchByMethodAnalyzer = $propertyFetchByMethodAnalyzer;
}
public function getRuleDefinition(): RuleDefinition
@ -110,7 +94,10 @@ CODE_SAMPLE
$privatePropertyNames = $this->classManipulator->getPrivatePropertyNames($node);
$propertyUsageByMethods = $this->collectPropertyFetchByMethods($node, $privatePropertyNames);
$propertyUsageByMethods = $this->propertyFetchByMethodAnalyzer->collectPropertyFetchByMethods(
$node,
$privatePropertyNames
);
if ($propertyUsageByMethods === []) {
return null;
}
@ -137,142 +124,4 @@ CODE_SAMPLE
return $node;
}
/**
* @param string[] $privatePropertyNames
* @return array<string, string[]>
*/
private function collectPropertyFetchByMethods(Class_ $class, array $privatePropertyNames): array
{
$propertyUsageByMethods = [];
foreach ($privatePropertyNames as $privatePropertyName) {
foreach ($class->getMethods() as $classMethod) {
// assigned in constructor injection → skip
if ($this->isName($classMethod, MethodName::CONSTRUCT)) {
return [];
}
if (! $this->propertyFetchAnalyzer->containsLocalPropertyFetchName(
$classMethod,
$privatePropertyName
)) {
continue;
}
if ($this->isPropertyChangingInMultipleMethodCalls($classMethod, $privatePropertyName)) {
continue;
}
/** @var string $classMethodName */
$classMethodName = $this->getName($classMethod);
$propertyUsageByMethods[$privatePropertyName][] = $classMethodName;
}
}
return $propertyUsageByMethods;
}
/**
* Covers https://github.com/rectorphp/rector/pull/2558#discussion_r363036110
*/
private function isPropertyChangingInMultipleMethodCalls(ClassMethod $classMethod, string $propertyName): bool
{
$isPropertyChanging = false;
$isPropertyReadInIf = false;
$isIfFollowedByAssign = false;
$this->traverseNodesWithCallable((array) $classMethod->getStmts(), function (Node $node) use (
&$isPropertyChanging,
$propertyName,
&$isPropertyReadInIf,
&$isIfFollowedByAssign
): ?int {
if ($isPropertyReadInIf) {
if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($node, [$propertyName])) {
return null;
}
$parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
if ($parentNode instanceof Assign && $parentNode->var === $node) {
$isIfFollowedByAssign = true;
}
}
if (! $this->isScopeChangingNode($node)) {
return null;
}
if ($node instanceof If_) {
$isPropertyReadInIf = $this->refactorIf($node, $propertyName);
}
$isPropertyChanging = $this->isPropertyChanging($node, $propertyName);
if (! $isPropertyChanging) {
return null;
}
return NodeTraverser::STOP_TRAVERSAL;
});
return $isPropertyChanging || $isIfFollowedByAssign || $isPropertyReadInIf;
}
private function isScopeChangingNode(Node $node): bool
{
foreach (self::SCOPE_CHANGING_NODE_TYPES as $type) {
if (is_a($node, $type, true)) {
return true;
}
}
return false;
}
private function refactorIf(If_ $if, string $privatePropertyName): ?bool
{
$this->traverseNodesWithCallable($if->cond, function (Node $node) use (
$privatePropertyName,
&$isPropertyReadInIf
): ?int {
if (! $this->propertyFetchAnalyzer->isLocalPropertyOfNames($node, [$privatePropertyName])) {
return null;
}
$isPropertyReadInIf = true;
return NodeTraverser::STOP_TRAVERSAL;
});
return $isPropertyReadInIf;
}
private function isPropertyChanging(Node $node, string $privatePropertyName): bool
{
$isPropertyChanging = false;
// here cannot be any property assign
$this->traverseNodesWithCallable($node, function (Node $node) use (
&$isPropertyChanging,
$privatePropertyName
): ?int {
if (! $node instanceof Assign) {
return null;
}
if (! $node->var instanceof PropertyFetch) {
return null;
}
if (! $this->isName($node->var->name, $privatePropertyName)) {
return null;
}
$isPropertyChanging = true;
return NodeTraverser::STOP_TRAVERSAL;
});
return $isPropertyChanging;
}
}

3
rules/TypeDeclaration/Rector/ClassMethod/AddArrayParamDocTypeRector.php
View File

@ -12,6 +12,7 @@ use PHPStan\Type\MixedType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
use Rector\ChangesReporting\ValueObject\RectorWithLineChange;
use Rector\Core\Rector\AbstractRector;
use Rector\DeadCode\PhpDoc\TagRemover\ParamTagRemover;
use Rector\TypeDeclaration\TypeInferer\ParamTypeInferer;
@ -120,10 +121,12 @@ CODE_SAMPLE
}
$paramName = $this->getName($param);
$this->phpDocTypeChanger->changeParamType($phpDocInfo, $paramType, $param, $paramName);
}
if ($phpDocInfo->hasChanged()) {
$this->file->addRectorClassWithLine(new RectorWithLineChange($this, $node->getLine()));
$this->paramTagRemover->removeParamTagsIfUseless($phpDocInfo, $node);
return $node;
}