[code-quality] Add ControllerMethodInjectionToConstructorRector

config/sets/symfony/symfony-code-quality.php

@@ -6,6 +6,7 @@
 use Rector\Symfony\CodeQuality\Rector\AttributeGroup\SingleConditionSecurityAttributeToIsGrantedRector;
 use Rector\Symfony\CodeQuality\Rector\BinaryOp\RequestIsMainRector;
 use Rector\Symfony\CodeQuality\Rector\BinaryOp\ResponseStatusCodeRector;
+use Rector\Symfony\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;
 use Rector\Symfony\CodeQuality\Rector\Class_\EventListenerToEventSubscriberRector;
 use Rector\Symfony\CodeQuality\Rector\Class_\InlineClassRoutePrefixRector;
 use Rector\Symfony\CodeQuality\Rector\Class_\LoadValidatorMetadataToAnnotationRector;
@@ -32,7 +33,10 @@
 
         RemoveUnusedRequestParamRector::class,
         ParamTypeFromRouteRequiredRegexRector::class,
+
+        // controller
         ActionSuffixRemoverRector::class,
+        ControllerMethodInjectionToConstructorRector::class,
         LoadValidatorMetadataToAnnotationRector::class,
 
         // request method

config/sets/symfony/symfony7/symfony74/symfony74-json-streamer.php

@@ -9,11 +9,35 @@
 return static function (RectorConfig $rectorConfig): void {
     // @see https://github.com/symfony/symfony/blob/7.4/UPGRADE-7.4.md#jsonstreamer
     $rectorConfig->ruleWithConfiguration(RenameMethodRector::class, [
-        new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'getNativeToStreamValueTransformer', 'getValueTransformers'),
-        new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'getStreamToNativeValueTransformers', 'getValueTransformers'),
-        new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withNativeToStreamValueTransformers', 'withValueTransformers'),
-        new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withStreamToNativeValueTransformers', 'withValueTransformers'),
-        new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withAdditionalNativeToStreamValueTransformer', 'withAdditionalValueTransformer'),
-        new MethodCallRename('Symfony\Component\JsonStreamer\Mapping\PropertyMetadata', 'withAdditionalStreamToNativeValueTransformer', 'withAdditionalValueTransformer'),
+        new MethodCallRename(
+            'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
+            'getNativeToStreamValueTransformer',
+            'getValueTransformers'
+        ),
+        new MethodCallRename(
+            'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
+            'getStreamToNativeValueTransformers',
+            'getValueTransformers'
+        ),
+        new MethodCallRename(
+            'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
+            'withNativeToStreamValueTransformers',
+            'withValueTransformers'
+        ),
+        new MethodCallRename(
+            'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
+            'withStreamToNativeValueTransformers',
+            'withValueTransformers'
+        ),
+        new MethodCallRename(
+            'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
+            'withAdditionalNativeToStreamValueTransformer',
+            'withAdditionalValueTransformer'
+        ),
+        new MethodCallRename(
+            'Symfony\Component\JsonStreamer\Mapping\PropertyMetadata',
+            'withAdditionalStreamToNativeValueTransformer',
+            'withAdditionalValueTransformer'
+        ),
     ]);
 };

rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/ControllerMethodInjectionToConstructorRectorTest.php

@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;
+
+use Iterator;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Rector\Testing\PHPUnit\AbstractRectorTestCase;
+
+final class ControllerMethodInjectionToConstructorRectorTest extends AbstractRectorTestCase
+{
+    #[DataProvider('provideData')]
+    public function test(string $filePath): void
+    {
+        $this->doTestFile($filePath);
+    }
+
+    public static function provideData(): Iterator
+    {
+        return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
+    }
+
+    public function provideConfigFilePath(): string
+    {
+        return __DIR__ . '/config/configured_rule.php';
+    }
+}

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

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Psr\Log\LoggerInterface;
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class ReUseExistingService extends AbstractController
+{
+    public function __construct(private LoggerInterface $logger)
+    {
+    }
+
+    #[Route('/some-action', name: 'some_action')]
+    public function someAction(
+        \Symfony\Component\HttpFoundation\Request $request,
+        \Psr\Log\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 Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class ReUseExistingService extends AbstractController
+{
+    public function __construct(private readonly \Psr\Log\LoggerInterface $logger)
+    {
+    }
+
+    #[Route('/some-action', name: 'some_action')]
+    public function someAction(
+        \Symfony\Component\HttpFoundation\Request $request
+    ) {
+        $this->logger->log('level', 'value');
+    }
+}
+
+?>

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

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class SkipScalarAndRequestParams extends AbstractController
+{
+    #[Route('/some-action', name: 'some_action')]
+    public function someAction(
+        \Symfony\Component\HttpFoundation\Request $request,
+        int $number
+    ) {
+    }
+}

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

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class SomeControllerWithMethodInjection extends AbstractController
+{
+    #[Route('/some-action', name: 'some_action')]
+    public function someAction(
+        \Symfony\Component\HttpFoundation\Request $request,
+        \Psr\Log\LoggerInterface $logger
+    ) {
+        $logger->log('level', 'value');
+    }
+}
+
+?>
+-----
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Fixture;
+
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class SomeControllerWithMethodInjection extends AbstractController
+{
+    public function __construct(private readonly \Psr\Log\LoggerInterface $logger)
+    {
+    }
+    #[Route('/some-action', name: 'some_action')]
+    public function someAction(
+        \Symfony\Component\HttpFoundation\Request $request
+    ) {
+        $this->logger->log('level', 'value');
+    }
+}
+
+?>

rules-tests/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector/config/configured_rule.php

@@ -0,0 +1,10 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\Symfony\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(ControllerMethodInjectionToConstructorRector::class);
+};

rules/CodeQuality/Rector/Class_/ControllerMethodInjectionToConstructorRector.php

@@ -0,0 +1,169 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Symfony\CodeQuality\Rector\Class_;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\Class_;
+use Rector\NodeManipulator\ClassDependencyManipulator;
+use Rector\PostRector\ValueObject\PropertyMetadata;
+use Rector\Rector\AbstractRector;
+use Rector\StaticTypeMapper\StaticTypeMapper;
+use Rector\Symfony\Bridge\NodeAnalyzer\ControllerMethodAnalyzer;
+use Rector\Symfony\Enum\SymfonyClass;
+use Rector\Symfony\TypeAnalyzer\ControllerAnalyzer;
+use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
+use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
+
+/**
+ * @see \Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\ControllerMethodInjectionToConstructorRectorTest
+ */
+final class ControllerMethodInjectionToConstructorRector extends AbstractRector
+{
+    public function __construct(
+        private readonly ControllerAnalyzer $controllerAnalyzer,
+        private readonly ControllerMethodAnalyzer $controllerMethodAnalyzer,
+        private readonly ClassDependencyManipulator $classDependencyManipulator,
+        private readonly StaticTypeMapper $staticTypeMapper,
+    ) {
+    }
+
+    public function getRuleDefinition(): RuleDefinition
+    {
+        return new RuleDefinition(
+            'Change Symfony controller method injection to direct constructor dependency, to separate params and services clearly',
+            [
+                new CodeSample(
+                    <<<'CODE_SAMPLE'
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class SomeController extends AbstractController
+{
+    #[Route('/some-path', name: 'some_name')]
+    public function someAction(
+        Request $request,
+        SomeService $someService
+    ) {
+        $data = $someService->getData();
+    }
+}
+CODE_SAMPLE
+                    ,
+                    <<<'CODE_SAMPLE'
+use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class SomeController extends AbstractController
+{
+    public function __construct(
+        private readonly SomeService $someService
+    ) {
+    }
+
+    #[Route('/some-path', name: 'some_name')]
+    public function someAction(
+        Request $request
+    ) {
+        $data = $this->someService->getData();
+    }
+}
+CODE_SAMPLE
+                ),
+            ]
+        );
+    }
+
+    /**
+     * @return array<class-string<Node>>
+     */
+    public function getNodeTypes(): array
+    {
+        return [Class_::class];
+    }
+
+    /**
+     * @param Class_ $node
+     */
+    public function refactor(Node $node): ?Node
+    {
+        if (! $this->controllerAnalyzer->isController($node)) {
+            return null;
+        }
+
+        $propertyMetadatas = [];
+
+        foreach ($node->getMethods() as $classMethod) {
+            if (! $this->controllerMethodAnalyzer->isAction($classMethod)) {
+                continue;
+            }
+
+            foreach ($classMethod->getParams() as $key => $param) {
+                // skip scalar and empty values, as not services
+                if ($param->type === null || $param->type instanceof Identifier) {
+                    continue;
+                }
+
+                // request is allowed
+                if ($param->type instanceof Name && $this->isName($param->type, SymfonyClass::REQUEST)) {
+                    continue;
+                }
+
+                // @todo allow parameter converter
+                unset($classMethod->params[$key]);
+
+                $paramType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($param->type);
+                $propertyMetadatas[] = new PropertyMetadata($this->getName($param->var), $paramType);
+            }
+        }
+
+        // nothing to move
+        if ($propertyMetadatas === []) {
+            return null;
+        }
+
+        $paramNamesToReplace = [];
+        foreach ($propertyMetadatas as $propertyMetadata) {
+            $paramNamesToReplace[] = $propertyMetadata->getName();
+        }
+
+        // 1. update constructor
+        foreach ($propertyMetadatas as $propertyMetadata) {
+            $this->classDependencyManipulator->addConstructorDependency($node, $propertyMetadata);
+        }
+
+        foreach ($node->getMethods() as $classMethod) {
+            if (! $this->controllerMethodAnalyzer->isAction($classMethod)) {
+                continue;
+            }
+
+            // replace param use with property fetch
+            $this->traverseNodesWithCallable((array) $classMethod->stmts, function (Node $node) use (
+                $paramNamesToReplace
+            ): ?PropertyFetch {
+                if (! $node instanceof Variable) {
+                    return null;
+                }
+
+                if (! $this->isNames($node, $paramNamesToReplace)) {
+                    return null;
+
+                }
+
+                $propertyName = $this->getName($node);
+                return new PropertyFetch(new Variable('this'), $propertyName);
+            });
+        }
+
+        // 2. replace in method bodies
+
+        return $node;
+    }
+}