[code-quality] Skip enum in ControllerMethodInjectionToConstructorRector (#893)

* extract ParamConverterClassesResolver

* skip enum in ControllerMethodInjectionToConstructorRector

* skip user interface

Author: TomasVotruba

rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/Fixture/skip_enum.php.inc

@@ -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)
+    {
+    }
+}

rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/Fixture/skip_user.php.inc

@@ -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)
+    {
+    }
+}

rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/Source/SomeEnum.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source;
+
+enum SomeEnum
+{
+}

rules/CodeQuality/NodeAnalyzer/ParamConverterClassesResolver.php

@@ -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;
+    }
+}

rules/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector.php

@@ -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;
@@ -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
     ) {
     }
@@ -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
@@ -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;
                 }
 
@@ -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);
             }
         }
@@ -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;
     }
 
@@ -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);
+        });
     }
 }