[symfony] Add NoControllerMethodInjectionRule (#241)

Author: TomasVotruba

config/symfony-rules.neon

@@ -7,6 +7,9 @@ rules:
     - Symplify\PHPStanRules\Rules\Symfony\RequireInvokableControllerRule
     - Symplify\PHPStanRules\Rules\Symfony\FormTypeClassNameRule
 
+    # controllers
+    - Symplify\PHPStanRules\Rules\Symfony\NoControllerMethodInjectionRule
+
     # routing
     - Symplify\PHPStanRules\Rules\Symfony\NoRoutingPrefixRule
     - Symplify\PHPStanRules\Rules\Symfony\NoClassLevelRouteRule

src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php

@@ -61,4 +61,6 @@ final class SymfonyRuleIdentifier
     public const RULE_IDENTIFIER = 'symfony.noServiceAutowireDuplicate';
 
     public const NO_SET_CLASS_SERVICE_DUPLICATE = 'symfony.noSetClassServiceDuplicate';
+
+    public const NO_CONTROLLER_METHOD_INJECTION = 'symfony.noControllerMethodInjection';
 }

src/Enum/SymfonyClass.php

@@ -43,4 +43,6 @@ final class SymfonyClass
     public const IS_GRANTED = 'Symfony\Component\Security\Http\Attribute\IsGranted';
 
     public const ATTRIBUTE = 'Symfony\Component\DependencyInjection\Attribute\Autowire';
+
+    public const REQUEST = 'Symfony\Component\HttpFoundation\Request';
 }

src/Rules/Symfony/NoControllerMethodInjectionRule.php

@@ -0,0 +1,85 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Symfony;
+
+use PhpParser\Node;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PhpParser\Node\Stmt\Class_;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Symplify\PHPStanRules\Enum\RuleIdentifier\SymfonyRuleIdentifier;
+use Symplify\PHPStanRules\Enum\SymfonyClass;
+
+/**
+ * @see \Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\NoControllerMethodInjectionRuleTest
+ *
+ * @implements Rule<Class_>
+ */
+final class NoControllerMethodInjectionRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'Instead of service "%s" action injection, use __construct() and invokable controller with __invoke() to clearly separate services and parameters';
+
+    public function getNodeType(): string
+    {
+        return Class_::class;
+    }
+
+    /**
+     * @param Class_ $node
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $ruleErrors = [];
+
+        if (! $node->name instanceof Identifier) {
+            return [];
+        }
+
+        $className = $node->name->toString();
+
+        if (! str_ends_with($className, 'Controller')) {
+            return [];
+        }
+
+        foreach ($node->getMethods() as $classMethod) {
+            if (! $classMethod->isPublic()) {
+                continue;
+            }
+
+            if ($classMethod->isMagic()) {
+                continue;
+            }
+
+            if ($classMethod->getParams() === []) {
+                continue;
+            }
+
+            foreach ($classMethod->getParams() as $param) {
+                if (! $param->type instanceof Name) {
+                    continue;
+                }
+
+                // Request is allwoed
+                $typeName = $param->type->toString();
+                if ($typeName === SymfonyClass::REQUEST) {
+                    continue;
+                }
+
+                $identifierRuleError = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $typeName))
+                    ->identifier(SymfonyRuleIdentifier::NO_CONTROLLER_METHOD_INJECTION)
+                    ->build();
+
+                $ruleErrors[] = $identifierRuleError;
+            }
+        }
+
+        return $ruleErrors;
+    }
+}

stubs/Symfony/Component/HttpFoundation/Request.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\HttpFoundation;
+
+class Request
+{
+}

tests/Rules/Symfony/NoControllerMethodInjectionRule/Fixture/SkipRequestParameterController.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\Fixture;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Routing\Annotation\Route;
+
+final class SkipRequestParameterController
+{
+    /**
+     * @Route("/some-action", name="some_action")
+     */
+    public function someRequired(Request $request)
+    {
+    }
+}

tests/Rules/Symfony/NoControllerMethodInjectionRule/Fixture/SkipScalarParameterController.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\Fixture;
+
+use Symfony\Component\Routing\Annotation\Route;
+use Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\Source\SomeService;
+
+final class SkipScalarParameterController
+{
+    /**
+     * @Route("/some-action", name="some_action")
+     */
+    public function someRequired(int $id)
+    {
+    }
+}

tests/Rules/Symfony/NoControllerMethodInjectionRule/Fixture/SomeActionInjectionController.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\Fixture;
+
+use Symfony\Component\Routing\Annotation\Route;
+use Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\Source\SomeService;
+
+final class SomeActionInjectionController
+{
+    /**
+     * @Route("/some-action", name="some_action")
+     */
+    public function someRequired(SomeService $someService)
+    {
+    }
+}

tests/Rules/Symfony/NoControllerMethodInjectionRule/NoControllerMethodInjectionRuleTest.php

@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule;
+
+use Iterator;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symplify\PHPStanRules\Rules\Symfony\NoControllerMethodInjectionRule;
+use Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\Source\SomeService;
+
+final class NoControllerMethodInjectionRuleTest extends RuleTestCase
+{
+    /**
+     * @param mixed[] $expectedErrorMessagesWithLines
+     */
+    #[DataProvider('provideData')]
+    public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
+    {
+        $this->analyse([$filePath], $expectedErrorMessagesWithLines);
+    }
+
+    public static function provideData(): Iterator
+    {
+        yield [__DIR__ . '/Fixture/SomeActionInjectionController.php', [[
+            sprintf(NoControllerMethodInjectionRule::ERROR_MESSAGE, SomeService::class),
+            10,
+        ]]];
+
+        yield [__DIR__ . '/Fixture/SkipRequestParameterController.php', []];
+        yield [__DIR__ . '/Fixture/SkipScalarParameterController.php', []];
+    }
+
+    protected function getRule(): Rule
+    {
+        return new NoControllerMethodInjectionRule();
+    }
+}

tests/Rules/Symfony/NoControllerMethodInjectionRule/Source/SomeService.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\NoControllerMethodInjectionRule\Source;
+
+final class SomeService
+{
+
+}