mirror_php/rector
1
0
Fork 0
mirror of https://github.com/rectorphp/rector.git synced 2025-01-19 14:27:14 +01:00
Code Issues Projects Releases Wiki Activity

[DeadCode] Fix RemoveSetterOnlyPropertyAndMethodCallRector for abstract method contract

Browse Source
This commit is contained in:
TomasVotruba 2020-02-01 10:31:25 +01:00
parent 6cec5bf890
commit 0b94c5c897
4 changed files with 116 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
packages/DeadCode/src/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector.php
View File

@ -5,6 +5,8 @@ declare(strict_types=1);
namespace Rector\DeadCode\Rector\Class_;
use PhpParser\Node;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Interface_;
@ -15,6 +17,7 @@ use Rector\PhpParser\Node\Manipulator\PropertyManipulator;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
use Rector\TypeDeclaration\VendorLock\VendorLockResolver;
/**
* @sponsor Thanks https://spaceflow.io/ for sponsoring this rule - visit them on https://github.com/SpaceFlow-app
@ -28,9 +31,15 @@ final class RemoveSetterOnlyPropertyAndMethodCallRector extends AbstractRector
*/
private $propertyManipulator;
public function __construct(PropertyManipulator $propertyManipulator)
/**
* @var VendorLockResolver
*/
private $vendorLockResolver;
public function __construct(PropertyManipulator $propertyManipulator, VendorLockResolver $vendorLockResolver)
{
$this->propertyManipulator = $propertyManipulator;
$this->vendorLockResolver = $vendorLockResolver;
}
public function getDefinition(): RectorDefinition
@ -93,22 +102,14 @@ PHP
}
$propertyFetches = $this->propertyManipulator->getAllPropertyFetch($node);
$classMethodsToCheck = $this->collectClassMethodsToCheck($propertyFetches);
/** @var ClassMethod[] $methodsToCheck */
$methodsToCheck = [];
foreach ($propertyFetches as $propertyFetch) {
$methodName = $propertyFetch->getAttribute(AttributeKey::METHOD_NAME);
if ($methodName !== '__construct') {
//this rector does not remove empty constructors
$methodsToCheck[$methodName] = $propertyFetch->getAttribute(AttributeKey::METHOD_NODE);
}
}
$vendorLockedClassMethodNames = $this->getVendorLockedClassMethodNames($classMethodsToCheck);
$vendorLockedClassMethodNames = $this->getVendorLockedClassMethodNames($methodsToCheck);
$this->removePropertyAndUsages($node, $vendorLockedClassMethodNames);
/** @var ClassMethod $method */
foreach ($methodsToCheck as $method) {
foreach ($classMethodsToCheck as $method) {
if (! $this->methodHasNoStmtsLeft($method)) {
continue;
}
@ -154,39 +155,34 @@ PHP
return true;
}
if ($this->propertyManipulator->isPropertyUsedInReadContext($propertyProperty)) {
return true;
}
return false;
return $this->propertyManipulator->isPropertyUsedInReadContext($propertyProperty);
}
private function isClassMethodRemovalVendorLocked(ClassMethod $classMethod): bool
/**
* @param PropertyFetch[]|StaticPropertyFetch[] $propertyFetches
* @return ClassMethod[]
*/
private function collectClassMethodsToCheck(array $propertyFetches): array
{
$classMethodName = $this->getName($classMethod);
$classMethodsToCheck = [];
/** @var Class_|null $class */
$class = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
if ($class === null) {
return false;
}
// required by interface?
foreach ($class->implements as $implement) {
$implementedInterfaceName = $this->getName($implement);
if (interface_exists($implementedInterfaceName)) {
$interfaceMethods = get_class_methods($implementedInterfaceName);
if (in_array($classMethodName, $interfaceMethods, true)) {
return true;
}
foreach ($propertyFetches as $propertyFetch) {
$methodName = $propertyFetch->getAttribute(AttributeKey::METHOD_NAME);
// this rector does not remove empty constructors
if ($methodName === '__construct') {
continue;
}
/** @var ClassMethod|null $classMethod */
$classMethod = $propertyFetch->getAttribute(AttributeKey::METHOD_NODE);
if ($classMethod === null) {
continue;
}
$classMethodsToCheck[$methodName] = $classMethod;
}
// required by abstract class?
// @todo
return false;
return $classMethodsToCheck;
}
/**
@ -197,7 +193,7 @@ PHP
{
$vendorLockedClassMethodsNames = [];
foreach ($methodsToCheck as $method) {
if (! $this->isClassMethodRemovalVendorLocked($method)) {
if (! $this->vendorLockResolver->isClassMethodRemovalVendorLocked($method)) {
continue;
}

17
packages/DeadCode/tests/Rector/Class_/RemoveSetterOnlyPropertyAndMethodCallRector/Fixture/skip_abstract_class_required.php.inc Normal file
View File

@ -0,0 +1,17 @@
<?php
namespace Rector\DeadCode\Tests\Rector\Property\RemoveSetterOnlyPropertyAndMethodCallRector\StaticProperty;
class SkipAbstractClassRequired extends AbstractClassRequiring
{
private $name;
public function setName(string $name): void
{
$this->name = $name;
}
}
abstract class AbstractClassRequiring
{
protected abstract function setName(string $name);
}

60
packages/TypeDeclaration/src/VendorLock/VendorLockResolver.php
View File

@ -15,6 +15,7 @@ use Rector\NodeContainer\ParsedNodesByType;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PhpParser\Node\Manipulator\ClassManipulator;
use Rector\PhpParser\Node\Resolver\NameResolver;
use ReflectionClass;
final class VendorLockResolver
{
@ -212,6 +213,65 @@ final class VendorLockResolver
return false;
}
/**
* Checks for:
* - interface required methods
* - abstract classes reqired method
*
* Prevent:
* - removing class methods, that breaks the code
*/
public function isClassMethodRemovalVendorLocked(ClassMethod $classMethod): bool
{
$classMethodName = $this->nameResolver->getName($classMethod);
/** @var Class_|null $class */
$class = $classMethod->getAttribute(AttributeKey::CLASS_NODE);
if ($class === null) {
return false;
}
// required by interface?
foreach ($class->implements as $implement) {
$implementedInterfaceName = $this->nameResolver->getName($implement);
if (interface_exists($implementedInterfaceName)) {
$interfaceMethods = get_class_methods($implementedInterfaceName);
if (in_array($classMethodName, $interfaceMethods, true)) {
return true;
}
}
}
if ($class->extends === null) {
return false;
}
/** @var string $className */
$className = $classMethod->getAttribute(AttributeKey::CLASS_NAME);
$classParents = class_parents($className);
foreach ($classParents as $classParent) {
if (! class_exists($classParent)) {
continue;
}
$parentClassReflection = new ReflectionClass($classParent);
if (! $parentClassReflection->hasMethod($classMethodName)) {
continue;
}
$methodReflection = $parentClassReflection->getMethod($classMethodName);
if (! $methodReflection->isAbstract()) {
continue;
}
return true;
}
return false;
}
// Until we have getProperty (https://github.com/nikic/PHP-Parser/pull/646)
private function getProperty(ClassLike $classLike, string $name)
{

7
src/Rector/AbstractRector/ComplexRemovalTrait.php
View File

@ -241,12 +241,13 @@ trait ComplexRemovalTrait
}
/**
* @param StaticPropertyFetch|PropertyFetch $expr
* @param string[] $classMethodNamesToSkip
*/
private function shouldSkipPropertyForClassMethod(PropertyFetch $propertyFetch, array $classMethodNamesToSkip): bool
private function shouldSkipPropertyForClassMethod(Expr $expr, array $classMethodNamesToSkip): bool
{
/** @var ClassMethod|null $methodNode */
$classMethodNode = $propertyFetch->getAttribute(AttributeKey::METHOD_NODE);
/** @var ClassMethod|null $classMethodNode */
$classMethodNode = $expr->getAttribute(AttributeKey::METHOD_NODE);
if ($classMethodNode === null) {
return false;
}