[symfony] Add TaggedIteratorOverRepeatedServiceCallRule (#209)

* [symfony] Add TaggedIteratorOverRepeatedServiceCallRule

* README

Author: TomasVotruba

README.md

@@ -1438,6 +1438,49 @@ rules:
 <br>
 
 
+### TaggedIteratorOverRepeatedServiceCallRuleTest
+
+Instead of repeated "->call(%s, ...)" calls, pass services as tagged iterator argument to the constructor
+
+```yaml
+rules:
+    - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\TaggedIteratorOverRepeatedServiceCallRule
+```
+
+```php
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ref;
+
+return static function (ContainerConfigurator $containerConfigurator): void {
+    $services = $containerConfigurator->services();
+
+    $services->set(SomeService::class)
+        ->call('setService', [ref('service1')])
+        ->call('setService', [ref('service2')])
+        ->call('setService', [ref('service3')]);
+};
+```
+
+:x:
+
+<br>
+
+```php
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+use Symfony\Component\DependencyInjection\Loader\Configurator\tagged_iterator;
+
+return static function (ContainerConfigurator $containerConfigurator): void {
+    $services = $containerConfigurator->services();
+
+    $services->set(SomeService::class)
+        ->arg('$services', tagged_iterator('SomeServiceTag'));
+};
+```
+
+:+1:
+
+<br>
+
 ### NoGetInCommandRule
 
 Prevents using `$this->get(...)` in commands, to promote dependency injection.

config/symfony-config-rules.neon

@@ -2,3 +2,4 @@ rules:
     - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule
     - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoBundleResourceConfigRule
     - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\AlreadyRegisteredAutodiscoveryServiceRule
+    - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\TaggedIteratorOverRepeatedServiceCallRule

src/Enum/SymfonyFunctionName.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Enum;
+
+final class SymfonyFunctionName
+{
+    public const REFERENCE = 'Symfony\Component\DependencyInjection\Loader\Configurator\ref';
+
+    public const SERVICE = 'Symfony\Component\DependencyInjection\Loader\Configurator\service';
+}

src/Rules/Symfony/ConfigClosure/TaggedIteratorOverRepeatedServiceCallRule.php

@@ -0,0 +1,81 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Rules\Symfony\ConfigClosure;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Closure;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Stmt\Expression;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleError;
+use PHPStan\Rules\RuleErrorBuilder;
+use Symplify\PHPStanRules\Symfony\NodeAnalyzer\SymfonyClosureDetector;
+use Symplify\PHPStanRules\Symfony\NodeFinder\RepeatedServiceAdderCallNameFinder;
+
+/**
+ * @implements Rule<Closure>
+ *
+ * @see \Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\TaggedIteratorOverRepeatedServiceCallRule\TaggedIteratorOverRepeatedServiceCallRuleTest
+ */
+final class TaggedIteratorOverRepeatedServiceCallRule implements Rule
+{
+    /**
+     * @var string
+     */
+    public const ERROR_MESSAGE = 'Instead of repeated "->call(%s, ...)" calls, pass services as tagged iterator argument to the constructor';
+
+    /**
+     * @var string
+     */
+    private const RULE_IDENTIFIER = 'symfony.taggedIteratorOverRepeatedServiceCall';
+
+    public function getNodeType(): string
+    {
+        return Closure::class;
+    }
+
+    /**
+     * @param Closure $node
+     * @return RuleError[]
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (! SymfonyClosureDetector::detect($node)) {
+            return [];
+        }
+
+        $ruleErrors = [];
+
+        foreach ($node->stmts as $stmt) {
+            if (! $stmt instanceof Expression) {
+                continue;
+            }
+
+            $nestedExpr = $stmt->expr;
+            if (! $nestedExpr instanceof MethodCall) {
+                continue;
+            }
+
+            if ($nestedExpr->isFirstClassCallable()) {
+                continue;
+            }
+
+            $adderCallName = RepeatedServiceAdderCallNameFinder::find($nestedExpr);
+            if (! is_string($adderCallName)) {
+                continue;
+            }
+
+            $ruleError = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $adderCallName))
+                ->identifier(self::RULE_IDENTIFIER)
+                ->line($stmt->getStartLine())
+                ->build();
+
+            $ruleErrors[] = $ruleError;
+        }
+
+        return $ruleErrors;
+    }
+}

src/Symfony/ConfigClosure/SymfonyServiceReferenceFunctionAnalyzer.php

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Symfony\ConfigClosure;
+
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Name;
+use Symplify\PHPStanRules\Enum\SymfonyFunctionName;
+
+final class SymfonyServiceReferenceFunctionAnalyzer
+{
+    public static function isReferenceCall(Expr $expr): bool
+    {
+        if (! $expr instanceof FuncCall) {
+            return false;
+        }
+
+        if (! $expr->name instanceof Name) {
+            return false;
+        }
+
+        $functionName = $expr->name->toString();
+
+        return in_array($functionName, [
+            SymfonyFunctionName::REFERENCE,
+            SymfonyFunctionName::SERVICE,
+        ]);
+    }
+}

src/Symfony/NodeFinder/RepeatedServiceAdderCallNameFinder.php

@@ -0,0 +1,88 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Symfony\NodeFinder;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\NodeFinder;
+use Symplify\PHPStanRules\Symfony\ConfigClosure\SymfonyServiceReferenceFunctionAnalyzer;
+
+final class RepeatedServiceAdderCallNameFinder
+{
+    private const CALL_NAME = 'call';
+
+    private const MIN_ALERT_COUNT = 3;
+
+    public static function find(MethodCall $methodCall): ?string
+    {
+        $callMethodCalls = self::findCallMethodCalls($methodCall);
+
+        $callMethodNames = [];
+
+        foreach ($callMethodCalls as $callMethodCall) {
+            /** @var String_ $calledMethodNameExpr */
+            $calledMethodNameExpr = $callMethodCall->getArgs()[0]->value;
+            $callMethodName = $calledMethodNameExpr->value;
+
+            // is passing a service references?
+            $passedExpr = $callMethodCall->getArgs()[1]->value;
+            if (! $passedExpr instanceof Array_) {
+                continue;
+            }
+
+            if (count($passedExpr->items) !== 1) {
+                continue;
+            }
+
+            $firstArrayItem = $passedExpr->items[0];
+            if (! SymfonyServiceReferenceFunctionAnalyzer::isReferenceCall($firstArrayItem->value)) {
+                continue;
+            }
+
+            $callMethodNames[] = $callMethodName;
+        }
+
+        $methodNamesToCount = array_count_values($callMethodNames);
+        foreach ($methodNamesToCount as $methodName => $count) {
+            if ($count < self::MIN_ALERT_COUNT) {
+                continue;
+            }
+
+            return $methodName;
+        }
+
+        return null;
+    }
+
+    /**
+     * @return MethodCall[]
+     */
+    private static function findCallMethodCalls(MethodCall $methodCall): array
+    {
+        $nodeFinder = new NodeFinder();
+
+        /** @var MethodCall[] $callMethodCalls */
+        $callMethodCalls = $nodeFinder->find($methodCall, function (Node $node): bool {
+            if (! $node instanceof MethodCall) {
+                return false;
+            }
+
+            if (! fast_node_named($node->name, self::CALL_NAME)) {
+                return false;
+            }
+
+            if (count($node->getArgs()) !== 2) {
+                return false;
+            }
+
+            $callNameExpr = $node->getArgs()[0]->value;
+            return $callNameExpr instanceof String_;
+        });
+
+        return $callMethodCalls;
+    }
+}

tests/Rules/Symfony/ConfigClosure/TaggedIteratorOverRepeatedServiceCallRule/Fixture/ConfigWithRepeatedCalls.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
+
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+use function Symfony\Component\DependencyInjection\Loader\Configurator\service;
+
+return function (ContainerConfigurator $container) {
+    $services = $container->services();
+
+    $services->set('some')
+        ->call('repeatedMethod', [service('some')])
+        ->call('repeatedMethod', [service('some')])
+        ->call('repeatedMethod', [service('some')])
+        ->call('repeatedMethod', [service('some')])
+        ->call('repeatedMethod', [service('some')])
+        ->call('repeatedMethod', [service('some')])
+        ->call('repeatedMethod', [service('some')]);
+};

tests/Rules/Symfony/ConfigClosure/TaggedIteratorOverRepeatedServiceCallRule/Fixture/SkipNonServiceCalls.php

@@ -0,0 +1,18 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
+
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+
+return function (ContainerConfigurator $container) {
+    $services = $container->services();
+
+    $services->set('some')
+        ->call('repeatedMethod', [100])
+        ->call('repeatedMethod', [100])
+        ->call('repeatedMethod', [100])
+        ->call('repeatedMethod', [100])
+        ->call('repeatedMethod', [100])
+        ->call('repeatedMethod', [100])
+        ->call('repeatedMethod', [100]);
+};

tests/Rules/Symfony/ConfigClosure/TaggedIteratorOverRepeatedServiceCallRule/Fixture/SkipOnlyFewRepeats.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\ServicesExcludedDirectoryMustExistRule\Fixture;
+
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+
+return function (ContainerConfigurator $container) {
+    $services = $container->services();
+
+    $services->set('some')
+        ->call('repeatedMethod', [])
+        ->call('repeatedMethod', []);
+};

tests/Rules/Symfony/ConfigClosure/TaggedIteratorOverRepeatedServiceCallRule/TaggedIteratorOverRepeatedServiceCallRuleTest.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\TaggedIteratorOverRepeatedServiceCallRule;
+
+use Iterator;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use PHPUnit\Framework\Attributes\DataProvider;
+use Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\TaggedIteratorOverRepeatedServiceCallRule;
+
+final class TaggedIteratorOverRepeatedServiceCallRuleTest 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/ConfigWithRepeatedCalls.php', [
+            [
+                sprintf(TaggedIteratorOverRepeatedServiceCallRule::ERROR_MESSAGE, 'repeatedMethod'),
+                11,
+            ],
+        ]];
+
+        yield [__DIR__ . '/Fixture/SkipOnlyFewRepeats.php', []];
+        yield [__DIR__ . '/Fixture/SkipNonServiceCalls.php', []];
+    }
+
+    protected function getRule(): Rule
+    {
+        return new TaggedIteratorOverRepeatedServiceCallRule();
+    }
+}