mirror of
https://github.com/rectorphp/rector.git
synced 2024-09-06 15:41:59 +00:00
fix RemoveSetterOnlyPropertyAndMethodCallRector race condition
This commit is contained in:
Tomas Votruba
parent
779e082213
commit
4b584b9507
165
packages/DeadCode/src/Analyzer/SetterOnlyMethodAnalyzer.php
Normal file
165
packages/DeadCode/src/Analyzer/SetterOnlyMethodAnalyzer.php
Normal file
@ -0,0 +1,165 @@
|
||||
<?php declare(strict_types=1);
|
||||
|
||||
namespace Rector\DeadCode\Analyzer;
|
||||
|
||||
use PhpParser\Node;
|
||||
use PhpParser\Node\Expr\Assign;
|
||||
use PhpParser\Node\Expr\PropertyFetch;
|
||||
use PhpParser\Node\Stmt\Class_;
|
||||
use PhpParser\Node\Stmt\ClassMethod;
|
||||
use PhpParser\Node\Stmt\Expression;
|
||||
use Rector\NodeContainer\ParsedNodesByType;
|
||||
use Rector\PhpParser\Node\Manipulator\ClassManipulator;
|
||||
use Rector\PhpParser\Node\Resolver\NameResolver;
|
||||
use Rector\PhpParser\NodeTraverser\CallableNodeTraverser;
|
||||
|
||||
final class SetterOnlyMethodAnalyzer
|
||||
{
|
||||
/**
|
||||
* @var ParsedNodesByType
|
||||
*/
|
||||
private $parsedNodesByType;
|
||||
|
||||
/**
|
||||
* @var ClassManipulator
|
||||
*/
|
||||
private $classManipulator;
|
||||
|
||||
/**
|
||||
* @var NameResolver
|
||||
*/
|
||||
private $nameResolver;
|
||||
|
||||
/**
|
||||
* @var CallableNodeTraverser
|
||||
*/
|
||||
private $callableNodeTraverser;
|
||||
|
||||
/**
|
||||
* @var string[][][]
|
||||
*/
|
||||
private $propertiesAndMethodsToRemoveByType = [];
|
||||
|
||||
public function __construct(
|
||||
ParsedNodesByType $parsedNodesByType,
|
||||
ClassManipulator $classManipulator,
|
||||
NameResolver $nameResolver,
|
||||
CallableNodeTraverser $callableNodeTraverser
|
||||
) {
|
||||
$this->parsedNodesByType = $parsedNodesByType;
|
||||
$this->classManipulator = $classManipulator;
|
||||
$this->nameResolver = $nameResolver;
|
||||
$this->callableNodeTraverser = $callableNodeTraverser;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns 1. setter only properties + 2. setter only method names by class type
|
||||
* @return string[][][]
|
||||
*/
|
||||
public function provideSetterOnlyPropertiesAndMethodsByType(): array
|
||||
{
|
||||
if ($this->propertiesAndMethodsToRemoveByType !== []) {
|
||||
return $this->propertiesAndMethodsToRemoveByType;
|
||||
}
|
||||
|
||||
foreach ($this->parsedNodesByType->getClasses() as $class) {
|
||||
$type = $this->nameResolver->getName($class);
|
||||
|
||||
// 1. setter only properties by class
|
||||
$assignOnlyPrivatePropertyNames = $this->classManipulator->getAssignOnlyPrivatePropertyNames($class);
|
||||
if ($assignOnlyPrivatePropertyNames) {
|
||||
$this->propertiesAndMethodsToRemoveByType[$type]['properties'] = $assignOnlyPrivatePropertyNames;
|
||||
}
|
||||
|
||||
// 2. setter only methods by class
|
||||
$setterOnlyMethodNames = $this->resolveSetterOnlyMethodNames($class, $assignOnlyPrivatePropertyNames);
|
||||
if ($setterOnlyMethodNames) {
|
||||
$this->propertiesAndMethodsToRemoveByType[$type]['methods'] = $setterOnlyMethodNames;
|
||||
}
|
||||
}
|
||||
|
||||
return $this->propertiesAndMethodsToRemoveByType;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string[] $assignOnlyPrivatePropertyNames
|
||||
* @return string[]
|
||||
*/
|
||||
private function resolveSetterOnlyMethodNames(Class_ $class, array $assignOnlyPrivatePropertyNames): array
|
||||
{
|
||||
$methodNamesToBeRemoved = [];
|
||||
|
||||
$this->callableNodeTraverser->traverseNodesWithCallable($class, function (Node $node) use (
|
||||
$assignOnlyPrivatePropertyNames,
|
||||
&$methodNamesToBeRemoved
|
||||
): void {
|
||||
if (! $this->isClassMethodWithSinglePropertyAssignOfNames($node, $assignOnlyPrivatePropertyNames)) {
|
||||
return;
|
||||
}
|
||||
/** @var string $classMethodName */
|
||||
$classMethodName = $this->nameResolver->getName($node);
|
||||
$methodNamesToBeRemoved[] = $classMethodName;
|
||||
});
|
||||
|
||||
return $methodNamesToBeRemoved;
|
||||
}
|
||||
|
||||
/**
|
||||
* Looks for:
|
||||
*
|
||||
* public function <someMethod>($value)
|
||||
* {
|
||||
* $this->value = $value
|
||||
* }
|
||||
*
|
||||
* @param string[] $propertyNames
|
||||
*/
|
||||
private function isClassMethodWithSinglePropertyAssignOfNames(Node $node, array $propertyNames): bool
|
||||
{
|
||||
if (! $node instanceof ClassMethod) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->nameResolver->isName($node, '__construct')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (count((array) $node->stmts) !== 1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (! $node->stmts[0] instanceof Expression) {
|
||||
return false;
|
||||
}
|
||||
|
||||
/** @var Expression $onlyExpression */
|
||||
$onlyExpression = $node->stmts[0];
|
||||
|
||||
$onlyStmt = $onlyExpression->expr;
|
||||
|
||||
return $this->isPropertyAssignWithPropertyNames($onlyStmt, $propertyNames);
|
||||
}
|
||||
|
||||
/**
|
||||
* Is: "$this->value = <$value>"
|
||||
*
|
||||
* @param string[] $propertyNames
|
||||
*/
|
||||
private function isPropertyAssignWithPropertyNames(Node $node, array $propertyNames): bool
|
||||
{
|
||||
if (! $node instanceof Assign) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (! $node->var instanceof PropertyFetch) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$propertyFetch = $node->var;
|
||||
if (! $this->nameResolver->isName($propertyFetch->var, 'this')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $this->nameResolver->isNames($propertyFetch->name, $propertyNames);
|
||||
}
|
||||
}
|
||||
@ -3,13 +3,11 @@
|
||||
namespace Rector\DeadCode\Rector\Class_;
|
||||
|
||||
use PhpParser\Node;
|
||||
use PhpParser\Node\Expr\Assign;
|
||||
use PhpParser\Node\Expr\PropertyFetch;
|
||||
use PhpParser\Node\Stmt\Class_;
|
||||
use PhpParser\Node\Expr\MethodCall;
|
||||
use PhpParser\Node\Stmt\ClassMethod;
|
||||
use PhpParser\Node\Stmt\Expression;
|
||||
use Rector\NodeContainer\ParsedNodesByType;
|
||||
use Rector\PhpParser\Node\Manipulator\ClassManipulator;
|
||||
use PhpParser\Node\Stmt\Property;
|
||||
use Rector\DeadCode\Analyzer\SetterOnlyMethodAnalyzer;
|
||||
use Rector\NodeTypeResolver\Node\AttributeKey;
|
||||
use Rector\Rector\AbstractRector;
|
||||
use Rector\RectorDefinition\CodeSample;
|
||||
use Rector\RectorDefinition\RectorDefinition;
|
||||
@ -21,24 +19,13 @@ use Rector\RectorDefinition\RectorDefinition;
|
||||
final class RemoveSetterOnlyPropertyAndMethodCallRector extends AbstractRector
|
||||
{
|
||||
/**
|
||||
* @var ClassManipulator
|
||||
* @var SetterOnlyMethodAnalyzer
|
||||
*/
|
||||
private $classManipulator;
|
||||
private $setterOnlyMethodAnalyzer;
|
||||
|
||||
/**
|
||||
* @var ParsedNodesByType
|
||||
*/
|
||||
private $parsedNodesByType;
|
||||
|
||||
/**
|
||||
* @var string[]
|
||||
*/
|
||||
private $methodCallNamesToBeRemoved = [];
|
||||
|
||||
public function __construct(ClassManipulator $classManipulator, ParsedNodesByType $parsedNodesByType)
|
||||
public function __construct(SetterOnlyMethodAnalyzer $setterOnlyMethodAnalyzer)
|
||||
{
|
||||
$this->classManipulator = $classManipulator;
|
||||
$this->parsedNodesByType = $parsedNodesByType;
|
||||
$this->setterOnlyMethodAnalyzer = $setterOnlyMethodAnalyzer;
|
||||
}
|
||||
|
||||
public function getDefinition(): RectorDefinition
|
||||
@ -88,125 +75,60 @@ CODE_SAMPLE
|
||||
*/
|
||||
public function getNodeTypes(): array
|
||||
{
|
||||
return [Class_::class];
|
||||
return [Property::class, MethodCall::class, ClassMethod::class];
|
||||
}
|
||||
|
||||
/**
|
||||
* @param Class_ $node
|
||||
* @param Property|MethodCall|ClassMethod $node
|
||||
*/
|
||||
public function refactor(Node $node): ?Node
|
||||
{
|
||||
$this->methodCallNamesToBeRemoved = [];
|
||||
$setterOnlyPropertiesAndMethods = $this->resolveSetterOnlyPropertiesAndMethodsForClass($node);
|
||||
if ($setterOnlyPropertiesAndMethods === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// 1. get assign only private properties
|
||||
$assignOnlyPrivatePropertyNames = $this->classManipulator->getAssignOnlyPrivatePropertyNames($node);
|
||||
$this->classManipulator->removeProperties($node, $assignOnlyPrivatePropertyNames);
|
||||
|
||||
// 2. remove assigns + class methods with only setter assign
|
||||
$this->removePropertyAssigns($node, $assignOnlyPrivatePropertyNames);
|
||||
|
||||
// 3. remove setter method calls
|
||||
$this->removeSetterMethodCalls($node);
|
||||
|
||||
return $node;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param string[] $assignOnlyPrivatePropertyNames
|
||||
*/
|
||||
private function removePropertyAssigns(Class_ $class, array $assignOnlyPrivatePropertyNames): void
|
||||
{
|
||||
$this->traverseNodesWithCallable($class, function (Node $node) use ($assignOnlyPrivatePropertyNames): void {
|
||||
if ($this->isClassMethodWithSinglePropertyAssignOfNames($node, $assignOnlyPrivatePropertyNames)) {
|
||||
/** @var string $classMethodName */
|
||||
$classMethodName = $this->getName($node);
|
||||
$this->methodCallNamesToBeRemoved[] = $classMethodName;
|
||||
|
||||
$this->removeNode($node);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
if ($this->isPropertyAssignWithPropertyNames($node, $assignOnlyPrivatePropertyNames)) {
|
||||
// 1. remove class properties
|
||||
if ($node instanceof Property) {
|
||||
if ($this->isNames($node, $setterOnlyPropertiesAndMethods['properties'] ?? [])) {
|
||||
$this->removeNode($node);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
private function removeSetterMethodCalls(Node $node): void
|
||||
{
|
||||
/** @var string $className */
|
||||
$className = $this->getName($node);
|
||||
$methodCallsByMethodName = $this->parsedNodesByType->findMethodCallsOnClass($className);
|
||||
|
||||
/** @var string $methodName */
|
||||
foreach ($methodCallsByMethodName as $methodName => $classMethodCalls) {
|
||||
if (! in_array($methodName, $this->methodCallNamesToBeRemoved, true)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
foreach ($classMethodCalls as $classMethodCall) {
|
||||
$this->removeNode($classMethodCall);
|
||||
// 2. remove class methods
|
||||
if ($node instanceof ClassMethod) {
|
||||
if ($this->isNames($node, $setterOnlyPropertiesAndMethods['methods'] ?? [])) {
|
||||
$this->removeNode($node);
|
||||
}
|
||||
}
|
||||
|
||||
// 3. remove method calls
|
||||
if ($node instanceof MethodCall) {
|
||||
if ($this->isNames($node->name, $setterOnlyPropertiesAndMethods['methods'] ?? [])) {
|
||||
$this->removeNode($node);
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Looks for:
|
||||
*
|
||||
* public function <someMethod>($value)
|
||||
* {
|
||||
* $this->value = $value
|
||||
* }
|
||||
*
|
||||
* @param string[] $propertyNames
|
||||
* @param Property|ClassMethod|MethodCall $node
|
||||
* @return string[][]][]|null
|
||||
*/
|
||||
private function isClassMethodWithSinglePropertyAssignOfNames(Node $node, array $propertyNames): bool
|
||||
private function resolveSetterOnlyPropertiesAndMethodsForClass(Node $node): ?array
|
||||
{
|
||||
if (! $node instanceof ClassMethod) {
|
||||
return false;
|
||||
if ($node instanceof Property || $node instanceof ClassMethod) {
|
||||
$className = $node->getAttribute(AttributeKey::CLASS_NAME);
|
||||
} elseif ($node instanceof MethodCall) {
|
||||
$className = $this->getTypes($node->var)[0] ?? null;
|
||||
if ($className === null) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
if ($this->isName($node, '__construct')) {
|
||||
return false;
|
||||
}
|
||||
$setterOnlyPropertiesAndMethodsByType = $this->setterOnlyMethodAnalyzer->provideSetterOnlyPropertiesAndMethodsByType();
|
||||
|
||||
if (count((array) $node->stmts) !== 1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (! $node->stmts[0] instanceof Expression) {
|
||||
return false;
|
||||
}
|
||||
|
||||
/** @var Expression $onlyExpression */
|
||||
$onlyExpression = $node->stmts[0];
|
||||
|
||||
$onlyStmt = $onlyExpression->expr;
|
||||
|
||||
return $this->isPropertyAssignWithPropertyNames($onlyStmt, $propertyNames);
|
||||
}
|
||||
|
||||
/**
|
||||
* Is: "$this->value = <$value>"
|
||||
*
|
||||
* @param string[] $propertyNames
|
||||
*/
|
||||
private function isPropertyAssignWithPropertyNames(Node $node, array $propertyNames): bool
|
||||
{
|
||||
if (! $node instanceof Assign) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (! $node->var instanceof PropertyFetch) {
|
||||
return false;
|
||||
}
|
||||
|
||||
$propertyFetch = $node->var;
|
||||
if (! $this->isName($propertyFetch->var, 'this')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return $this->isNames($propertyFetch->name, $propertyNames);
|
||||
return $setterOnlyPropertiesAndMethodsByType[$className] ?? null;
|
||||
}
|
||||
}
|
||||
|
||||
@ -118,7 +118,7 @@ final class RectorApplication
|
||||
foreach ($fileInfos as $fileInfo) {
|
||||
$this->tryCatchWrapper($fileInfo, function (SmartFileInfo $smartFileInfo): void {
|
||||
$this->fileProcessor->parseFileInfoToLocalCache($smartFileInfo);
|
||||
});
|
||||
}, 'parsing');
|
||||
}
|
||||
|
||||
// active only one rule
|
||||
@ -131,14 +131,14 @@ final class RectorApplication
|
||||
foreach ($fileInfos as $fileInfo) {
|
||||
$this->tryCatchWrapper($fileInfo, function (SmartFileInfo $smartFileInfo): void {
|
||||
$this->fileProcessor->refactor($smartFileInfo);
|
||||
});
|
||||
}, 'refactoring');
|
||||
}
|
||||
|
||||
// 3. print to file or string
|
||||
foreach ($fileInfos as $fileInfo) {
|
||||
$this->tryCatchWrapper($fileInfo, function (SmartFileInfo $smartFileInfo): void {
|
||||
$this->processFileInfo($smartFileInfo);
|
||||
});
|
||||
}, 'printing');
|
||||
}
|
||||
|
||||
if ($this->configuration->showProgressBar()) {
|
||||
@ -149,9 +149,9 @@ final class RectorApplication
|
||||
$this->removedAndAddedFilesProcessor->run();
|
||||
}
|
||||
|
||||
private function tryCatchWrapper(SmartFileInfo $smartFileInfo, callable $callback): void
|
||||
private function tryCatchWrapper(SmartFileInfo $smartFileInfo, callable $callback, string $phase): void
|
||||
{
|
||||
$this->advance($smartFileInfo);
|
||||
$this->advance($smartFileInfo, $phase);
|
||||
|
||||
try {
|
||||
if (in_array($smartFileInfo, $this->notParsedFiles, true)) {
|
||||
@ -195,10 +195,10 @@ final class RectorApplication
|
||||
$this->fileSystemFileProcessor->processFileInfo($fileInfo);
|
||||
}
|
||||
|
||||
private function advance(SmartFileInfo $smartFileInfo): void
|
||||
private function advance(SmartFileInfo $smartFileInfo, string $phase): void
|
||||
{
|
||||
if ($this->symfonyStyle->isVerbose()) {
|
||||
$this->symfonyStyle->writeln('[parsing] ' . $smartFileInfo->getRealPath());
|
||||
$this->symfonyStyle->writeln(sprintf('[%s] %s', $phase, $smartFileInfo->getRealPath()));
|
||||
} elseif ($this->configuration->showProgressBar()) {
|
||||
$this->symfonyStyle->progressAdvance();
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user