Merge pull request #2114 from jeroensmit/ConstructorInjectionToActionInjectionRectorFix2

Fix for issue #2090
This commit is contained in:
Tomáš Votruba 2019-10-11 13:41:11 +01:00 committed by GitHub
commit 3fce9aa551
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 188 additions and 39 deletions

View File

@ -13,6 +13,7 @@ use PhpParser\Node\Stmt\Expression;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\Php\TypeAnalyzer;
use Rector\PhpParser\Node\Manipulator\ClassManipulator;
use Rector\PhpParser\Node\Manipulator\ClassMethodManipulator;
use Rector\Rector\AbstractRector;
use Rector\RectorDefinition\CodeSample;
use Rector\RectorDefinition\RectorDefinition;
@ -42,10 +43,19 @@ final class ConstructorInjectionToActionInjectionRector extends AbstractRector
*/
private $propertyFetchToParamsToRemoveFromConstructor = [];
public function __construct(ClassManipulator $classManipulator, TypeAnalyzer $typeAnalyzer)
{
/**
* @var ClassMethodManipulator
*/
private $classMethodManipulator;
public function __construct(
ClassManipulator $classManipulator,
TypeAnalyzer $typeAnalyzer,
ClassMethodManipulator $classMethodManipulator
) {
$this->classManipulator = $classManipulator;
$this->typeAnalyzer = $typeAnalyzer;
$this->classMethodManipulator = $classMethodManipulator;
}
public function getDefinition(): RectorDefinition
@ -128,7 +138,9 @@ PHP
continue;
}
$this->replacePropertyFetchByInjectedVariables($classMethod);
foreach ($this->propertyFetchToParams as $propertyFetchName => $param) {
$this->changePropertyUsageToParameter($classMethod, $propertyFetchName, $param);
}
}
// collect all property fetches that are relevant to original constructor properties
@ -155,11 +167,13 @@ PHP
return $node;
}
private function replacePropertyFetchByInjectedVariables(ClassMethod $classMethod): void
private function changePropertyUsageToParameter(ClassMethod $classMethod, string $propertyName, Param $param): void
{
$currentlyAddedLocalVariables = [];
$this->traverseNodesWithCallable((array) $classMethod->stmts, function (Node $node) use (
$propertyName,
$param,
&$currentlyAddedLocalVariables
): ?Variable {
if (! $node instanceof PropertyFetch) {
@ -170,14 +184,12 @@ PHP
return null;
}
foreach ($this->propertyFetchToParams as $propertyFetchName => $param) {
if ($this->isName($node, $propertyFetchName)) {
$currentlyAddedLocalVariables[] = $param;
if ($this->isName($node, $propertyName)) {
$currentlyAddedLocalVariables[] = $param;
/** @var string $paramName */
$paramName = $this->getName($param);
return new Variable($paramName);
}
/** @var string $paramName */
$paramName = $this->getName($param);
return new Variable($paramName);
}
return null;
@ -274,41 +286,30 @@ PHP
private function removeUnusedPropertiesAndConstructorParams(Class_ $class, ClassMethod $classMethod): void
{
if ($this->propertyFetchToParamsToRemoveFromConstructor === []) {
return;
$this->removeAssignsFromConstructor($classMethod);
foreach ($this->propertyFetchToParamsToRemoveFromConstructor as $propertyFetchName => $param) {
$this->changePropertyUsageToParameter($classMethod, $propertyFetchName, $param);
}
$this->removeAssignsAndParamsFromConstructor($classMethod);
$this->classMethodManipulator->removeUnusedParameters($classMethod);
$this->removeUnusedProperties($class);
$this->removeEmptyConstruct($class, $classMethod);
$this->removeConstructIfEmpty($class, $classMethod);
}
private function removeAssignsAndParamsFromConstructor(ClassMethod $classMethod): void
private function removeAssignsFromConstructor(ClassMethod $classMethod): void
{
foreach ($this->propertyFetchToParamsToRemoveFromConstructor as $propertyFetchToRemove => $paramToRemove) {
// remove unused params in constructor
foreach ($classMethod->params as $key => $constructorParam) {
if (! $this->areNamesEqual($constructorParam, $paramToRemove)) {
continue;
}
unset($classMethod->params[$key]);
foreach ((array) $classMethod->stmts as $key => $constructorStmt) {
$propertyFetchToVariable = $this->resolveAssignPropertyToVariableOrNull($constructorStmt);
if ($propertyFetchToVariable === null) {
continue;
}
foreach ((array) $classMethod->stmts as $key => $constructorStmt) {
$propertyFetchToVariable = $this->resolveAssignPropertyToVariableOrNull($constructorStmt);
if ($propertyFetchToVariable === null) {
continue;
}
[$propertyFetchName, ] = $propertyFetchToVariable;
if ($propertyFetchName !== $propertyFetchToRemove) {
continue;
}
// remove the assign
unset($classMethod->stmts[$key]);
[$propertyFetchName, ] = $propertyFetchToVariable;
if (! isset($this->propertyFetchToParamsToRemoveFromConstructor[$propertyFetchName])) {
continue;
}
// remove the assign
unset($classMethod->stmts[$key]);
}
}
@ -320,7 +321,7 @@ PHP
}
}
private function removeEmptyConstruct(Class_ $class, ClassMethod $constructClassMethod): void
private function removeConstructIfEmpty(Class_ $class, ClassMethod $constructClassMethod): void
{
if ($constructClassMethod->stmts !== []) {
return;

View File

@ -19,11 +19,13 @@ final class ConstructorInjectionToActionInjectionRectorTest extends AbstractRect
public function provideDataForTest(): Iterator
{
yield [__DIR__ . '/Fixture/fixture.php.inc'];
yield [__DIR__ . '/Fixture/multiple.php.inc'];
yield [__DIR__ . '/Fixture/duplicate.php.inc'];
yield [__DIR__ . '/Fixture/skip_scalars.php.inc'];
yield [__DIR__ . '/Fixture/skip_non_action_methods.php.inc'];
yield [__DIR__ . '/Fixture/only_props_from_this.php.inc'];
yield [__DIR__ . '/Fixture/manage_different_naming.php.inc'];
yield [__DIR__ . '/Fixture/extra_calls_in_constructor.inc.php'];
}
protected function getRectorClass(): string

View File

@ -0,0 +1,62 @@
<?php
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\ExtraCallsInConstructor;
final class SomeController
{
/**
* @var ProductRepository
*/
private $productRepository;
/**
* @var Product
*/
private $default;
/**
* @var int
*/
private $count;
public function __construct(ProductRepository $productRepository)
{
$this->productRepository = $productRepository;
$this->default = $productRepository->getDefault();
$this->count = $this->productRepository->getCount();
}
public function default()
{
$products = $this->productRepository->fetchAll();
}
}
?>
-----
<?php
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\ExtraCallsInConstructor;
final class SomeController
{
/**
* @var Product
*/
private $default;
/**
* @var int
*/
private $count;
public function __construct(ProductRepository $productRepository)
{
$this->default = $productRepository->getDefault();
$this->count = $productRepository->getCount();
}
public function default(ProductRepository $productRepository)
{
$products = $productRepository->fetchAll();
}
}
?>

View File

@ -0,0 +1,64 @@
<?php
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Multiple;
use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
final class SkipNonActionMethodsController
{
/**
* @var ProductRepository
*/
private $productRepository;
/**
* @var ResponseFactory
*/
private $responseFactory;
public function __construct(ProductRepository $productRepository, ResponseFactory $responseFactory)
{
$this->productRepository = $productRepository;
$this->responseFactory = $responseFactory;
}
public function index()
{
return $this->responseFactory->repond($this->isMatch(5));
}
private function isMatch($value): bool
{
return (bool) $this->productRepository->findBy($value);
}
}
?>
-----
<?php
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Multiple;
use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
final class SkipNonActionMethodsController
{
/**
* @var ProductRepository
*/
private $productRepository;
public function __construct(ProductRepository $productRepository)
{
$this->productRepository = $productRepository;
}
public function index(ResponseFactory $responseFactory)
{
return $responseFactory->repond($this->isMatch(5));
}
private function isMatch($value): bool
{
return (bool) $this->productRepository->findBy($value);
}
}
?>

View File

@ -151,6 +151,26 @@ final class ClassMethodManipulator
return $paramName;
}
public function removeParameter(Param $param, ClassMethod $classMethod): void
{
foreach ($classMethod->params as $key => $constructorParam) {
if (! $this->nameResolver->areNamesEqual($constructorParam, $param)) {
continue;
}
unset($classMethod->params[$key]);
}
}
public function removeUnusedParameters(ClassMethod $classMethod): void
{
foreach ($classMethod->getParams() as $param) {
if (! $this->isParameterUsedMethod($param, $classMethod)) {
$this->removeParameter($param, $classMethod);
}
}
}
private function isMethodInParent(string $class, string $method): bool
{
foreach (class_parents($class) as $parentClass) {