[CodeQuality] Use #[Required] autowire() instead of constructor when parent has one in ControllerMethodInjectionToConstructorRector

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

@@ -31,14 +31,17 @@ use Symfony\Component\Routing\Annotation\Route;
 
 final class ChildOfParentWithPrivatePromotedProperty extends ParentControllerWithPrivatePromotedProperty
 {
-    public function __construct(private readonly \Psr\Log\LoggerInterface $logger)
-    {
-    }
+    private \Psr\Log\LoggerInterface $logger;
     #[Route('/example', name: 'example')]
     public function example()
     {
         $this->logger->log('level', 'value');
     }
+    #[\Symfony\Contracts\Service\Attribute\Required]
+    public function autowire(\Psr\Log\LoggerInterface $logger): void
+    {
+        $this->logger = $logger;
+    }
 }
 
 ?>

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

@@ -0,0 +1,51 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Psr\Log\LoggerInterface;
+use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\ParentControllerWithPrivatePromotedProperty;
+use Symfony\Component\Routing\Annotation\Route;
+use Symfony\Contracts\Translation\TranslatorInterface;
+
+final class ParentConstructorMultipleServices extends ParentControllerWithPrivatePromotedProperty
+{
+    #[Route('/example', name: 'example')]
+    public function example(LoggerInterface $logger, TranslatorInterface $translator)
+    {
+        $logger->log('level', $translator->trans('value'));
+    }
+}
+
+?>
+-----
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Psr\Log\LoggerInterface;
+use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\ParentControllerWithPrivatePromotedProperty;
+use Symfony\Component\Routing\Annotation\Route;
+use Symfony\Contracts\Translation\TranslatorInterface;
+
+final class ParentConstructorMultipleServices extends ParentControllerWithPrivatePromotedProperty
+{
+    private \Psr\Log\LoggerInterface $logger;
+    private \Symfony\Contracts\Translation\TranslatorInterface $translator;
+    #[Route('/example', name: 'example')]
+    public function example()
+    {
+        $this->logger->log('level', $this->translator->trans('value'));
+    }
+    #[\Symfony\Contracts\Service\Attribute\Required]
+    public function autowire(\Psr\Log\LoggerInterface $logger, \Symfony\Contracts\Translation\TranslatorInterface $translator): void
+    {
+        $this->logger = $logger;
+        $this->translator = $translator;
+    }
+}
+
+?>

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

@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Psr\Log\LoggerInterface;
+use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\ParentControllerWithAutowireMethod;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class ParentWithAutowireMethod extends ParentControllerWithAutowireMethod
+{
+    #[Route('/example', name: 'example')]
+    public function example(LoggerInterface $logger)
+    {
+        $logger->log('level', 'value');
+    }
+}
+
+?>
+-----
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Psr\Log\LoggerInterface;
+use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\ParentControllerWithAutowireMethod;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class ParentWithAutowireMethod extends ParentControllerWithAutowireMethod
+{
+    private \Psr\Log\LoggerInterface $logger;
+    #[Route('/example', name: 'example')]
+    public function example()
+    {
+        $this->logger->log('level', 'value');
+    }
+    #[\Symfony\Contracts\Service\Attribute\Required]
+    public function autowireServices(\Psr\Log\LoggerInterface $logger): void
+    {
+        $this->logger = $logger;
+    }
+}
+
+?>

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

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source;
+
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\EventDispatcher\EventDispatcherInterface;
+use Symfony\Contracts\Service\Attribute\Required;
+
+abstract class ParentControllerWithAutowireMethod extends AbstractController
+{
+    public function __construct(private readonly EventDispatcherInterface $eventDispatcher)
+    {
+    }
+
+    #[Required]
+    public function autowire(): void
+    {
+    }
+}

rules/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector.php

@@ -5,26 +5,36 @@
 namespace Rector\Symfony\CodeQuality\Rector\Class_;
 
 use Exception;
+use PhpParser\Modifiers;
 use PhpParser\Node;
+use PhpParser\Node\Attribute;
+use PhpParser\Node\AttributeGroup;
+use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
 use PhpParser\Node\Name\FullyQualified;
+use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
 use PhpParser\NodeVisitor;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Type\ObjectType;
 use Rector\NodeManipulator\ClassDependencyManipulator;
+use Rector\NodeManipulator\ClassInsertManipulator;
 use Rector\NodeTypeResolver\Node\AttributeKey;
+use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
 use Rector\PostRector\ValueObject\PropertyMetadata;
 use Rector\Rector\AbstractRector;
 use Rector\Reflection\ReflectionResolver;
 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\SymfonyAttribute;
 use Rector\Symfony\Enum\SymfonyClass;
 use Rector\Symfony\TypeAnalyzer\ControllerAnalyzer;
 use Rector\ValueObject\MethodName;
@@ -43,10 +53,18 @@ final class ControllerMethodInjectionToConstructorRector extends AbstractRector
      */
     private const array COMMON_ENTITY_CONTAINS_SUBNAMESPACES = ["\\Entity\\", "\\Document\\", "\\Model\\"];
 
+    private const string AUTOWIRE_METHOD_NAME = 'autowire';
+
+    /**
+     * Used when a parent class already defines autowire(), to avoid overriding it
+     */
+    private const string FALLBACK_AUTOWIRE_METHOD_NAME = 'autowireServices';
+
     public function __construct(
         private readonly ControllerAnalyzer $controllerAnalyzer,
         private readonly ControllerMethodAnalyzer $controllerMethodAnalyzer,
         private readonly ClassDependencyManipulator $classDependencyManipulator,
+        private readonly ClassInsertManipulator $classInsertManipulator,
         private readonly StaticTypeMapper $staticTypeMapper,
         private readonly ParamConverterClassesResolver $paramConverterClassesResolver,
         private readonly ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard,
@@ -57,7 +75,7 @@ public function __construct(
     public function getRuleDefinition(): RuleDefinition
     {
         return new RuleDefinition(
-            'Change Symfony controller method injection to direct constructor dependency, to separate params and services clearly',
+            'Change Symfony controller method injection to direct constructor dependency, to separate params and services clearly. If a parent class has a constructor, use #[Required] autowire() method instead, to avoid repeating all parent params',
             [
                 new CodeSample(
                     <<<'CODE_SAMPLE'
@@ -96,6 +114,42 @@ public function someAction(
         $data = $this->someService->getData();
     }
 }
+CODE_SAMPLE
+                ),
+                new CodeSample(
+                    <<<'CODE_SAMPLE'
+use Symfony\Component\Routing\Annotation\Route;
+
+final class SomeController extends SomeParentControllerWithConstructor
+{
+    #[Route('/some-path', name: 'some_name')]
+    public function someAction(SomeService $someService)
+    {
+        $data = $someService->getData();
+    }
+}
+CODE_SAMPLE
+                    ,
+                    <<<'CODE_SAMPLE'
+use Symfony\Component\Routing\Annotation\Route;
+use Symfony\Contracts\Service\Attribute\Required;
+
+final class SomeController extends SomeParentControllerWithConstructor
+{
+    private SomeService $someService;
+
+    #[Route('/some-path', name: 'some_name')]
+    public function someAction()
+    {
+        $data = $this->someService->getData();
+    }
+
+    #[Required]
+    public function autowire(SomeService $someService): void
+    {
+        $this->someService = $someService;
+    }
+}
 CODE_SAMPLE
                 ),
             ]
@@ -254,9 +308,16 @@ public function refactor(Node $node): ?Node
             unset($methodToModify->params[$paramKey]);
         }
 
-        // 1. update constructor
-        foreach ($propertyMetadatas as $propertyMetadata) {
-            $this->classDependencyManipulator->addConstructorDependency($node, $propertyMetadata);
+        // 1. add dependencies
+        if ($propertyMetadatas !== []) {
+            if ($this->hasParentConstructor($node)) {
+                // constructor would have to repeat all parent params, use setter injection instead
+                $this->addRequiredAutowireClassMethod($node, $propertyMetadatas);
+            } else {
+                foreach ($propertyMetadatas as $propertyMetadata) {
+                    $this->classDependencyManipulator->addConstructorDependency($node, $propertyMetadata);
+                }
+            }
         }
 
         foreach ($node->getMethods() as $classMethod) {
@@ -390,6 +451,76 @@ private function hasAccessibleParentProperty(Class_ $class, string $propertyName
         return false;
     }
 
+    /**
+     * @param PropertyMetadata[] $propertyMetadatas
+     */
+    private function addRequiredAutowireClassMethod(Class_ $class, array $propertyMetadatas): void
+    {
+        $autowireMethodName = $this->resolveAutowireMethodName($class);
+
+        $autowireClassMethod = $class->getMethod($autowireMethodName);
+        $isNewClassMethod = ! $autowireClassMethod instanceof ClassMethod;
+
+        if (! $autowireClassMethod instanceof ClassMethod) {
+            $autowireClassMethod = new ClassMethod(new Identifier($autowireMethodName), [
+                'flags' => Modifiers::PUBLIC,
+                'returnType' => new Identifier('void'),
+                'attrGroups' => [
+                    new AttributeGroup([new Attribute(new FullyQualified(SymfonyAttribute::REQUIRED))]),
+                ],
+                'stmts' => [],
+            ]);
+        }
+
+        foreach ($propertyMetadatas as $propertyMetadata) {
+            $propertyName = $propertyMetadata->getName();
+            $propertyType = $propertyMetadata->getType();
+
+            $property = $this->nodeFactory->createPrivatePropertyFromNameAndType($propertyName, $propertyType);
+            $this->classInsertManipulator->addAsFirstMethod($class, $property);
+
+            $autowireClassMethod->params[] = new Param(
+                new Variable($propertyName),
+                null,
+                $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode($propertyType, TypeKind::PARAM)
+            );
+
+            $autowireClassMethod->stmts[] = new Expression(
+                new Assign(new PropertyFetch(new Variable('this'), $propertyName), new Variable($propertyName))
+            );
+        }
+
+        if ($isNewClassMethod) {
+            $class->stmts[] = $autowireClassMethod;
+        }
+    }
+
+    private function resolveAutowireMethodName(Class_ $class): string
+    {
+        $classReflection = $this->reflectionResolver->resolveClassReflection($class);
+        if (! $classReflection instanceof ClassReflection) {
+            return self::AUTOWIRE_METHOD_NAME;
+        }
+
+        foreach ($classReflection->getParents() as $parentClassReflection) {
+            if ($parentClassReflection->hasNativeMethod(self::AUTOWIRE_METHOD_NAME)) {
+                return self::FALLBACK_AUTOWIRE_METHOD_NAME;
+            }
+        }
+
+        return self::AUTOWIRE_METHOD_NAME;
+    }
+
+    private function hasParentConstructor(Class_ $class): bool
+    {
+        $classReflection = $this->reflectionResolver->resolveClassReflection($class);
+        if (! $classReflection instanceof ClassReflection) {
+            return false;
+        }
+
+        return array_any($classReflection->getParents(), fn (ClassReflection $parentClassReflection): bool => $parentClassReflection->hasNativeMethod(MethodName::CONSTRUCT));
+    }
+
     private function isUsedInStaticClosureUse(ClassMethod $classMethod, string $paramName): bool
     {
         if ($classMethod->stmts === null) {