Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;

use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\SomeEnum;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Routing\Annotation\Route;

final class SkipEnum extends AbstractController
{
#[Route('/some-action', name: 'some_action')]
public function someAction(SomeEnum $someEnum)
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;

use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\SomeEnum;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Security\Core\User\UserInterface;

final class SkipUser extends AbstractController
{
#[Route('/some-action', name: 'some_action')]
public function someAction(UserInterface $user)
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source;

enum SomeEnum
{
}
53 changes: 53 additions & 0 deletions rules/CodeQuality/NodeAnalyzer/ParamConverterClassesResolver.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

namespace Rector\Symfony\CodeQuality\NodeAnalyzer;

use PhpParser\Node\Identifier;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\Doctrine\NodeAnalyzer\AttributeFinder;
use Rector\PhpParser\Node\Value\ValueResolver;
use Rector\Symfony\Enum\SensioAttribute;

final class ParamConverterClassesResolver
{
public function __construct(
private AttributeFinder $attributeFinder,
private ValueResolver $valueResolver,
) {
}

/**
* @return string[]
*/
public function resolveEntityClasses(ClassMethod $classMethod): array
{
$entityClasses = [];

$paramConverterAttributes = $this->attributeFinder->findManyByClass(
$classMethod,
SensioAttribute::PARAM_CONVERTER
);
foreach ($paramConverterAttributes as $paramConverterAttribute) {
foreach ($paramConverterAttribute->args as $arg) {
if (! $arg->name instanceof Identifier) {
continue;
}

if ($arg->name->toString() !== 'class') {
continue;
}

$entityClass = $this->valueResolver->getValue($arg->value);
if (! is_string($entityClass)) {
continue;
}

$entityClasses[] = $entityClass;
}
}

return $entityClasses;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,18 @@
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use Rector\Doctrine\NodeAnalyzer\AttributeFinder;
use PHPStan\Type\ObjectType;
use Rector\NodeManipulator\ClassDependencyManipulator;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\PhpParser\Node\Value\ValueResolver;
use Rector\PostRector\ValueObject\PropertyMetadata;
use Rector\Rector\AbstractRector;
use Rector\StaticTypeMapper\StaticTypeMapper;
use Rector\Symfony\Bridge\NodeAnalyzer\ControllerMethodAnalyzer;
use Rector\Symfony\CodeQuality\NodeAnalyzer\ParamConverterClassesResolver;
use Rector\Symfony\Enum\FosClass;
use Rector\Symfony\Enum\SensioAttribute;
use Rector\Symfony\Enum\SymfonyClass;
use Rector\Symfony\TypeAnalyzer\ControllerAnalyzer;
use Rector\ValueObject\MethodName;
Expand All @@ -44,8 +42,7 @@ public function __construct(
private readonly ControllerMethodAnalyzer $controllerMethodAnalyzer,
private readonly ClassDependencyManipulator $classDependencyManipulator,
private readonly StaticTypeMapper $staticTypeMapper,
private readonly AttributeFinder $attributeFinder,
private readonly ValueResolver $valueResolver,
private readonly ParamConverterClassesResolver $paramConverterClassesResolver,
private readonly ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard
) {
}
Expand Down Expand Up @@ -126,7 +123,7 @@ public function refactor(Node $node): ?Node
continue;
}

$entityClasses = $this->resolveParamConverterEntityClasses($classMethod);
$entityClasses = $this->paramConverterClassesResolver->resolveEntityClasses($classMethod);

foreach ($classMethod->getParams() as $key => $param) {
// skip scalar and empty values, as not services
Expand All @@ -139,12 +136,11 @@ public function refactor(Node $node): ?Node
continue;
}

// request is allowed
if ($this->isNames($param->type, [SymfonyClass::REQUEST, FosClass::PARAM_FETCHER])) {
continue;
}

if ($this->isNames($param->type, $entityClasses)) {
// skip allowed known objectsallowed
if ($this->isNames(
$param->type,
[SymfonyClass::USER_INTERFACE, SymfonyClass::REQUEST, FosClass::PARAM_FETCHER, ...$entityClasses]
)) {
continue;
}

Expand All @@ -154,10 +150,15 @@ public function refactor(Node $node): ?Node
}
}

// @todo allow parameter converter
unset($classMethod->params[$key]);

$paramType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($param->type);

if ($paramType instanceof ObjectType) {
if ($paramType->isEnum()->yes()) {
continue;
}
}

unset($classMethod->params[$key]);
$propertyMetadatas[] = new PropertyMetadata($this->getName($param->var), $paramType);
}
}
Expand All @@ -182,39 +183,9 @@ public function refactor(Node $node): ?Node
continue;
}

// replace param use with property fetch
$this->traverseNodesWithCallable((array) $classMethod->stmts, function (Node $node) use (
$paramNamesToReplace
): Closure|null|PropertyFetch {
if ($node instanceof Closure) {
foreach ($node->uses as $key => $closureUse) {
if ($this->isNames($closureUse->var, $paramNamesToReplace)) {
unset($node->uses[$key]);
}
}

return $node;
}

if (! $node instanceof Variable) {
return null;
}

if (! $this->isNames($node, $paramNamesToReplace)) {
return null;
}

if ($node->getAttribute(AttributeKey::IS_BEING_ASSIGNED) === true) {
return null;
}

$propertyName = $this->getName($node);
return new PropertyFetch(new Variable('this'), $propertyName);
});
$this->replaceParamUseWithPropertyFetch($classMethod, $paramNamesToReplace);
}

// 2. replace in method bodies

return $node;
}

Expand All @@ -232,29 +203,41 @@ private function shouldSkipClassMethod(ClassMethod $classMethod): bool
}

/**
* @return string[]
* @param string[] $paramNamesToReplace
*/
private function resolveParamConverterEntityClasses(ClassMethod $classMethod): array
private function replaceParamUseWithPropertyFetch(ClassMethod $classMethod, array $paramNamesToReplace): void
{
$entityClasses = [];
if ($classMethod->stmts === null) {
return;
}

$paramConverterAttributes = $this->attributeFinder->findManyByClass(
$classMethod,
SensioAttribute::PARAM_CONVERTER
);
foreach ($paramConverterAttributes as $paramConverterAttribute) {
foreach ($paramConverterAttribute->args as $arg) {
if ($arg->name instanceof Identifier && $this->isName($arg->name, 'class')) {
$entityClass = $this->valueResolver->getValue($arg->value);
if (! is_string($entityClass)) {
continue;
$this->traverseNodesWithCallable($classMethod->stmts, function (Node $node) use (
$paramNamesToReplace
): Closure|null|PropertyFetch {
if ($node instanceof Closure) {
foreach ($node->uses as $key => $closureUse) {
if ($this->isNames($closureUse->var, $paramNamesToReplace)) {
unset($node->uses[$key]);
}

$entityClasses[] = $entityClass;
}

return $node;
}

if (! $node instanceof Variable) {
return null;
}

if (! $this->isNames($node, $paramNamesToReplace)) {
return null;
}

if ($node->getAttribute(AttributeKey::IS_BEING_ASSIGNED) === true) {
return null;
}
}

return $entityClasses;
$propertyName = $this->getName($node);
return new PropertyFetch(new Variable('this'), $propertyName);
});
}
}