mirror of
https://github.com/rectorphp/rector.git
synced 2025-01-17 21:38:22 +01:00
[Architecture] Add ConstructorInjectionToActionInjectionRector
This commit is contained in:
parent
fe85a71698
commit
fbf0aa275f
@ -76,7 +76,8 @@
|
||||
"Rector\\SOLID\\": "packages/SOLID/src",
|
||||
"Rector\\Legacy\\": "packages/Legacy/src",
|
||||
"Rector\\ElasticSearchDSL\\": "packages/ElasticSearchDSL/src",
|
||||
"Rector\\SymfonyPHPUnit\\": "packages/SymfonyPHPUnit/src"
|
||||
"Rector\\SymfonyPHPUnit\\": "packages/SymfonyPHPUnit/src",
|
||||
"Rector\\Architecture\\": "packages/Architecture/src"
|
||||
}
|
||||
},
|
||||
"autoload-dev": {
|
||||
@ -114,7 +115,8 @@
|
||||
"Rector\\SOLID\\Tests\\": "packages/SOLID/tests",
|
||||
"Rector\\Legacy\\Tests\\": "packages/Legacy/tests",
|
||||
"Rector\\ElasticSearchDSL\\Tests\\": "packages/ElasticSearchDSL/tests",
|
||||
"Rector\\SymfonyPHPUnit\\Tests\\": "packages/SymfonyPHPUnit/tests"
|
||||
"Rector\\SymfonyPHPUnit\\Tests\\": "packages/SymfonyPHPUnit/tests",
|
||||
"Rector\\Architecture\\Tests\\": "packages/Architecture/tests"
|
||||
},
|
||||
"classmap": [
|
||||
"packages/Symfony/tests/Rector/FrameworkBundle/AbstractToConstructorInjectionRectorSource",
|
||||
@ -166,4 +168,4 @@
|
||||
"dev-master": "0.5-dev"
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
@ -0,0 +1,2 @@
|
||||
services:
|
||||
Rector\Architecture\Rector\Class_\ConstructorInjectionToActionInjectionRector: ~
|
1
ecs.yaml
1
ecs.yaml
@ -102,6 +102,7 @@ parameters:
|
||||
|
||||
Symplify\CodingStandard\Sniffs\CleanCode\CognitiveComplexitySniff:
|
||||
# tough logic
|
||||
- 'packages/Architecture/src/Rector/Class_/ConstructorInjectionToActionInjectionRector.php'
|
||||
- 'src/PhpParser/Node/Commander/NodeRemovingCommander.php'
|
||||
- 'packages/BetterPhpDocParser/src/*'
|
||||
- 'packages/Symfony/src/Rector/Class_/MakeCommandLazyRector.php'
|
||||
|
8
packages/Architecture/config/config.yaml
Normal file
8
packages/Architecture/config/config.yaml
Normal file
@ -0,0 +1,8 @@
|
||||
services:
|
||||
_defaults:
|
||||
autowire: true
|
||||
public: true
|
||||
|
||||
Rector\Architecture\:
|
||||
resource: '../src'
|
||||
exclude: '../src/{Rector/**/*Rector.php}'
|
7
packages/Architecture/src/Cleaner/ClassMethodCleaner.php
Normal file
7
packages/Architecture/src/Cleaner/ClassMethodCleaner.php
Normal file
@ -0,0 +1,7 @@
|
||||
<?php declare(strict_types=1);
|
||||
|
||||
namespace Rector\Architecture\Cleaner;
|
||||
|
||||
final class ClassMethodCleaner
|
||||
{
|
||||
}
|
@ -0,0 +1,345 @@
|
||||
<?php declare(strict_types=1);
|
||||
|
||||
namespace Rector\Architecture\Rector\Class_;
|
||||
|
||||
use PhpParser\Node;
|
||||
use PhpParser\Node\Expr\Assign;
|
||||
use PhpParser\Node\Expr\PropertyFetch;
|
||||
use PhpParser\Node\Expr\Variable;
|
||||
use PhpParser\Node\Param;
|
||||
use PhpParser\Node\Stmt\Class_;
|
||||
use PhpParser\Node\Stmt\ClassMethod;
|
||||
use PhpParser\Node\Stmt\Expression;
|
||||
use Rector\NodeTypeResolver\Node\AttributeKey;
|
||||
use Rector\Php\TypeAnalyzer;
|
||||
use Rector\PhpParser\Node\Manipulator\ClassManipulator;
|
||||
use Rector\Rector\AbstractRector;
|
||||
use Rector\RectorDefinition\CodeSample;
|
||||
use Rector\RectorDefinition\RectorDefinition;
|
||||
|
||||
final class ConstructorInjectionToActionInjectionRector extends AbstractRector
|
||||
{
|
||||
/**
|
||||
* @var ClassManipulator
|
||||
*/
|
||||
private $classManipulator;
|
||||
|
||||
/**
|
||||
* @var TypeAnalyzer
|
||||
*/
|
||||
private $typeAnalyzer;
|
||||
|
||||
/**
|
||||
* @var Param[]
|
||||
*/
|
||||
private $propertyFetchToParams = [];
|
||||
|
||||
/**
|
||||
* @var Param[]
|
||||
*/
|
||||
private $propertyFetchToParamsToRemoveFromConstructor = [];
|
||||
|
||||
public function __construct(ClassManipulator $classManipulator, TypeAnalyzer $typeAnalyzer)
|
||||
{
|
||||
$this->classManipulator = $classManipulator;
|
||||
$this->typeAnalyzer = $typeAnalyzer;
|
||||
}
|
||||
|
||||
public function getDefinition(): RectorDefinition
|
||||
{
|
||||
return new RectorDefinition('', [
|
||||
new CodeSample(
|
||||
<<<'CODE_SAMPLE'
|
||||
final class SomeController
|
||||
{
|
||||
/**
|
||||
* @var ProductRepository
|
||||
*/
|
||||
private $productRepository;
|
||||
|
||||
public function __construct(ProductRepository $productRepository)
|
||||
{
|
||||
$this->productRepository = $productRepository;
|
||||
}
|
||||
|
||||
public function default()
|
||||
{
|
||||
$products = $this->productRepository->fetchAll();
|
||||
}
|
||||
}
|
||||
CODE_SAMPLE
|
||||
,
|
||||
<<<'CODE_SAMPLE'
|
||||
final class SomeController
|
||||
{
|
||||
public function default(ProductRepository $productRepository)
|
||||
{
|
||||
$products = $productRepository->fetchAll();
|
||||
}
|
||||
}
|
||||
CODE_SAMPLE
|
||||
),
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return string[]
|
||||
*/
|
||||
public function getNodeTypes(): array
|
||||
{
|
||||
return [Class_::class];
|
||||
}
|
||||
|
||||
/**
|
||||
* @param Class_ $node
|
||||
*/
|
||||
public function refactor(Node $node): ?Node
|
||||
{
|
||||
$this->reset();
|
||||
|
||||
// only in controllers
|
||||
if (! $this->isName($node, '*Controller')) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if ($node->isAbstract()) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$constructMethod = $node->getMethod('__construct');
|
||||
// no constructor, nothing to do
|
||||
if ($constructMethod === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// traverse constructor dependencies and names of their properties
|
||||
$this->collectPropertyFetchToParams($constructMethod);
|
||||
|
||||
// replace them in property fetches with particular class methods and use variable instead
|
||||
foreach ($node->stmts as $classStmt) {
|
||||
if (! $classStmt instanceof ClassMethod) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($this->isName($classStmt, '__construct')) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (! $classStmt->isPublic()) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$this->replacePropertyFetchByInjectedVariables($classStmt);
|
||||
}
|
||||
|
||||
// collect all property fetches that are relevant to original constructor properties
|
||||
$this->traverseNodesWithCallable($node->stmts, function (Node $node) {
|
||||
if (! $node instanceof PropertyFetch) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// only scan non-action methods
|
||||
/** @var ClassMethod $methdoNode */
|
||||
$methdoNode = $node->getAttribute(AttributeKey::METHOD_NODE);
|
||||
if ($methdoNode->isPublic()) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$usedPropertyFetchName = $this->getName($node);
|
||||
if (isset($this->propertyFetchToParams[$usedPropertyFetchName])) {
|
||||
unset($this->propertyFetchToParamsToRemoveFromConstructor[$usedPropertyFetchName]);
|
||||
}
|
||||
});
|
||||
|
||||
$this->removeUnusedPropertiesAndConstructorParams($node, $constructMethod);
|
||||
|
||||
return $node;
|
||||
}
|
||||
|
||||
private function getPositionStmtByTypeAndName(Class_ $node, string $name, string $type): ?int
|
||||
{
|
||||
foreach ($node->stmts as $key => $stmt) {
|
||||
if (! is_a($stmt, $type, true)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (! $this->isName($stmt, $name)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
return $key;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
private function removeEmptyConstruct(Class_ $class, ClassMethod $constructClassMethod): void
|
||||
{
|
||||
if ($constructClassMethod->stmts !== []) {
|
||||
return;
|
||||
}
|
||||
|
||||
/** @var int $constructMethodPosition */
|
||||
$constructMethodPosition = $this->getPositionStmtByTypeAndName($class, '__construct', ClassMethod::class);
|
||||
if ($constructMethodPosition) {
|
||||
unset($class->stmts[$constructMethodPosition]);
|
||||
}
|
||||
}
|
||||
|
||||
private function removeUnusedProperties(Class_ $class): void
|
||||
{
|
||||
foreach (array_keys($this->propertyFetchToParamsToRemoveFromConstructor) as $propertyFetchName) {
|
||||
/** @var string $propertyFetchName */
|
||||
$this->classManipulator->removeProperty($class, $propertyFetchName);
|
||||
}
|
||||
}
|
||||
|
||||
private function replacePropertyFetchByInjectedVariables(ClassMethod $classMethod): void
|
||||
{
|
||||
$currentlyAddedLocalVariables = [];
|
||||
|
||||
$this->traverseNodesWithCallable((array) $classMethod->stmts, function (Node $node) use (
|
||||
&$currentlyAddedLocalVariables
|
||||
): ?Variable {
|
||||
if (! $node instanceof PropertyFetch) {
|
||||
return null;
|
||||
}
|
||||
|
||||
foreach ($this->propertyFetchToParams as $propertyFetchName => $param) {
|
||||
if ($this->isName($node, $propertyFetchName)) {
|
||||
$currentlyAddedLocalVariables[] = $param;
|
||||
|
||||
/** @var string $paramName */
|
||||
$paramName = $this->getName($param);
|
||||
return new Variable($paramName);
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
});
|
||||
|
||||
foreach ($currentlyAddedLocalVariables as $param) {
|
||||
// is param already present?
|
||||
foreach ($classMethod->params as $existingParam) {
|
||||
if ($this->areNamesEqual($existingParam, $param)) {
|
||||
continue 2;
|
||||
}
|
||||
}
|
||||
|
||||
$classMethod->params[] = $param;
|
||||
}
|
||||
}
|
||||
|
||||
private function collectPropertyFetchToParams(ClassMethod $classMethod): void
|
||||
{
|
||||
foreach ((array) $classMethod->stmts as $constructorStmt) {
|
||||
$propertyToVariable = $this->resolveAssignPropertyToVariableOrNull($constructorStmt);
|
||||
if ($propertyToVariable === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
[$propertyFetchName, $variableName] = $propertyToVariable;
|
||||
|
||||
$param = $this->classManipulator->findMethodParamByName($classMethod, $variableName);
|
||||
if ($param === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// random type, we cannot autowire in action
|
||||
if ($param->type === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$paramType = $this->getName($param->type);
|
||||
if ($paramType === null) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($this->typeAnalyzer->isPhpReservedType($paramType)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// it's a match
|
||||
$this->propertyFetchToParams[$propertyFetchName] = $param;
|
||||
}
|
||||
|
||||
$this->propertyFetchToParamsToRemoveFromConstructor = $this->propertyFetchToParams;
|
||||
}
|
||||
|
||||
private function reset(): void
|
||||
{
|
||||
$this->propertyFetchToParams = [];
|
||||
$this->propertyFetchToParamsToRemoveFromConstructor = [];
|
||||
}
|
||||
|
||||
private function removeAssignsAndParamsFromConstructor(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;
|
||||
}
|
||||
|
||||
[$propertyFetchName, ] = $propertyFetchToVariable;
|
||||
if ($propertyFetchName !== $propertyFetchToRemove) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// remove the assign
|
||||
unset($classMethod->stmts[$key]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private function resolveAssignPropertyToVariableOrNull(Node $node): ?array
|
||||
{
|
||||
if ($node instanceof Expression) {
|
||||
$node = $node->expr;
|
||||
}
|
||||
|
||||
if (! $node instanceof Assign) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (! $node->var instanceof PropertyFetch) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (! $node->expr instanceof Variable) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$propertyFetchName = $this->getName($node->var);
|
||||
$variableName = $this->getName($node->expr);
|
||||
if ($propertyFetchName === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if ($variableName === null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return [$propertyFetchName, $variableName];
|
||||
}
|
||||
|
||||
private function removeUnusedPropertiesAndConstructorParams(Class_ $class, ClassMethod $classMethod): void
|
||||
{
|
||||
if ($this->propertyFetchToParamsToRemoveFromConstructor === []) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->removeAssignsAndParamsFromConstructor($classMethod);
|
||||
$this->removeUnusedProperties($class);
|
||||
$this->removeEmptyConstruct($class, $classMethod);
|
||||
}
|
||||
}
|
@ -0,0 +1,25 @@
|
||||
<?php declare(strict_types=1);
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector;
|
||||
|
||||
use Rector\Architecture\Rector\Class_\ConstructorInjectionToActionInjectionRector;
|
||||
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
|
||||
|
||||
final class ConstructorInjectionToActionInjectionRectorTest extends AbstractRectorTestCase
|
||||
{
|
||||
public function test(): void
|
||||
{
|
||||
$this->doTestFiles([
|
||||
__DIR__ . '/Fixture/fixture.php.inc',
|
||||
__DIR__ . '/Fixture/duplicate.php.inc',
|
||||
__DIR__ . '/Fixture/skip_scalars.php.inc',
|
||||
__DIR__ . '/Fixture/skip_non_action_methods.php.inc',
|
||||
__DIR__ . '/Fixture/manage_different_naming.php.inc',
|
||||
]);
|
||||
}
|
||||
|
||||
protected function getRectorClass(): string
|
||||
{
|
||||
return ConstructorInjectionToActionInjectionRector::class;
|
||||
}
|
||||
}
|
@ -0,0 +1,43 @@
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
|
||||
|
||||
final class DuplicateController
|
||||
{
|
||||
/**
|
||||
* @var ProductRepository
|
||||
*/
|
||||
private $productRepository;
|
||||
|
||||
public function __construct(ProductRepository $productRepository)
|
||||
{
|
||||
$this->productRepository = $productRepository;
|
||||
}
|
||||
|
||||
public function actionInjection(ProductRepository $productRepository)
|
||||
{
|
||||
$products = $productRepository->fetchAll();
|
||||
$products = $this->productRepository->fetchAll();
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
||||
-----
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
|
||||
|
||||
final class DuplicateController
|
||||
{
|
||||
public function actionInjection(ProductRepository $productRepository)
|
||||
{
|
||||
$products = $productRepository->fetchAll();
|
||||
$products = $productRepository->fetchAll();
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
@ -0,0 +1,37 @@
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
final class SomeController
|
||||
{
|
||||
/**
|
||||
* @var ProductRepository
|
||||
*/
|
||||
private $productRepository;
|
||||
|
||||
public function __construct(ProductRepository $productRepository)
|
||||
{
|
||||
$this->productRepository = $productRepository;
|
||||
}
|
||||
|
||||
public function default()
|
||||
{
|
||||
$products = $this->productRepository->fetchAll();
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
||||
-----
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
final class SomeController
|
||||
{
|
||||
public function default(ProductRepository $productRepository)
|
||||
{
|
||||
$products = $productRepository->fetchAll();
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
@ -0,0 +1,41 @@
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
|
||||
|
||||
final class ManageDifferentNamingController
|
||||
{
|
||||
/**
|
||||
* @var ProductRepository
|
||||
*/
|
||||
private $repository;
|
||||
|
||||
public function __construct(ProductRepository $productRepository)
|
||||
{
|
||||
$this->repository = $productRepository;
|
||||
}
|
||||
|
||||
public function index()
|
||||
{
|
||||
return $this->repository->get(5);
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
||||
-----
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
use Rector\Tests\Rector\Architecture\DependencyInjection\ActionInjectionToConstructorInjectionRector\Source\ProductRepository;
|
||||
|
||||
final class ManageDifferentNamingController
|
||||
{
|
||||
public function index(ProductRepository $productRepository)
|
||||
{
|
||||
return $productRepository->get(5);
|
||||
}
|
||||
}
|
||||
|
||||
?>
|
@ -0,0 +1,28 @@
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
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()
|
||||
{
|
||||
return $this->isMatch(5);
|
||||
}
|
||||
|
||||
private function isMatch($value): bool
|
||||
{
|
||||
return (bool) $this->productRepository->findBy($value);
|
||||
}
|
||||
}
|
@ -0,0 +1,21 @@
|
||||
<?php
|
||||
|
||||
namespace Rector\Architecture\Tests\Rector\Class_\ConstructorInjectionToActionInjectionRector\Fixture;
|
||||
|
||||
final class SkipScalarsController
|
||||
{
|
||||
/**
|
||||
* @var mixed[]
|
||||
*/
|
||||
private $items = [];
|
||||
|
||||
public function __construct(array $items)
|
||||
{
|
||||
$this->items = $items;
|
||||
}
|
||||
|
||||
public function actionInjection()
|
||||
{
|
||||
$item = $this->items[5];
|
||||
}
|
||||
}
|
@ -6,6 +6,7 @@ use PhpParser\Node;
|
||||
use PhpParser\Node\Expr\Assign;
|
||||
use PhpParser\Node\Expr\PropertyFetch;
|
||||
use PhpParser\Node\Name;
|
||||
use PhpParser\Node\Param;
|
||||
use PhpParser\Node\Stmt;
|
||||
use PhpParser\Node\Stmt\Class_;
|
||||
use PhpParser\Node\Stmt\ClassLike;
|
||||
@ -260,6 +261,35 @@ final class ClassManipulator
|
||||
return false;
|
||||
}
|
||||
|
||||
public function removeProperty(Class_ $class, string $propertyName): void
|
||||
{
|
||||
foreach ($class->stmts as $key => $classStmt) {
|
||||
if (! $classStmt instanceof Node\Stmt\Property) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (! $this->nameResolver->isName($classStmt, $propertyName)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
unset($class->stmts[$key]);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
public function findMethodParamByName(ClassMethod $classMethod, string $name): ?Param
|
||||
{
|
||||
foreach ($classMethod->params as $param) {
|
||||
if (! $this->nameResolver->isName($param, $name)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
return $param;
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
private function tryInsertBeforeFirstMethod(Class_ $classNode, Stmt $stmt): bool
|
||||
{
|
||||
foreach ($classNode->stmts as $key => $classElementNode) {
|
||||
|
Loading…
x
Reference in New Issue
Block a user