diff --git a/src/Rules/Symfony/NoFindTaggedServiceIdsCallRule.php b/src/Rules/Symfony/NoFindTaggedServiceIdsCallRule.php index 16851495..5ace320b 100644 --- a/src/Rules/Symfony/NoFindTaggedServiceIdsCallRule.php +++ b/src/Rules/Symfony/NoFindTaggedServiceIdsCallRule.php @@ -5,8 +5,14 @@ namespace Symplify\PHPStanRules\Rules\Symfony; use PhpParser\Node; +use PhpParser\Node\Expr\ArrayDimFetch; +use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Stmt\Foreach_; +use PhpParser\NodeFinder; use PHPStan\Analyser\Scope; +use PHPStan\Node\InClassNode; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; @@ -15,38 +21,133 @@ use Symplify\PHPStanRules\PHPUnit\TestClassDetector; /** - * @implements Rule + * @implements Rule */ -final class NoFindTaggedServiceIdsCallRule implements Rule +final readonly class NoFindTaggedServiceIdsCallRule implements Rule { public const string ERROR_MESSAGE = 'Instead of "$this->findTaggedServiceIds()" use more reliable registerForAutoconfiguration() and tagged iterator attribute. Those work outside any configuration and avoid missed tag errors'; + private NodeFinder $nodeFinder; + + public function __construct() + { + $this->nodeFinder = new NodeFinder(); + } + public function getNodeType(): string { - return MethodCall::class; + return InClassNode::class; } /** - * @param MethodCall $node + * @param InClassNode $node * @return IdentifierRuleError[] */ public function processNode(Node $node, Scope $scope): array { - if (! NamingHelper::isName($node->name, 'findTaggedServiceIds')) { - return []; - } - // tagged service ids are commonly used in tests to assert service registration if (TestClassDetector::isTestClass($scope)) { return []; } - $identifierRuleError = RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier(SymfonyRuleIdentifier::NO_FIND_TAGGED_SERVICE_IDS_CALL) - ->build(); + $classLike = $node->getOriginalNode(); + + $ruleErrors = []; + + foreach ($this->findTaggedServiceIdsCalls($classLike) as $methodCall) { + // reading tag attributes (e.g. $tags[0]['alias']) cannot be expressed by a tagged iterator, keep it + if ($this->usesTagAttributes($classLike, $methodCall)) { + continue; + } + + $ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE) + ->identifier(SymfonyRuleIdentifier::NO_FIND_TAGGED_SERVICE_IDS_CALL) + ->line($methodCall->getStartLine()) + ->build(); + } + + return $ruleErrors; + } + + /** + * @return MethodCall[] + */ + private function findTaggedServiceIdsCalls(Node $classLike): array + { + $methodCalls = $this->nodeFinder->findInstanceOf($classLike, MethodCall::class); + + return array_filter( + $methodCalls, + static fn (MethodCall $methodCall): bool => NamingHelper::isName($methodCall->name, 'findTaggedServiceIds') + ); + } + + private function usesTagAttributes(Node $classLike, MethodCall $methodCall): bool + { + $assignedVariableName = $this->resolveAssignedVariableName($classLike, $methodCall); + if ($assignedVariableName === null) { + return false; + } + + foreach ($this->findForeachesOverVariable($classLike, $assignedVariableName) as $foreach) { + if (! $foreach->valueVar instanceof Variable || ! is_string($foreach->valueVar->name)) { + continue; + } + + if ($this->hasArrayDimFetchOnVariable($foreach->stmts, $foreach->valueVar->name)) { + return true; + } + } + + return false; + } + + private function resolveAssignedVariableName(Node $classLike, MethodCall $methodCall): ?string + { + foreach ($this->nodeFinder->findInstanceOf($classLike, Assign::class) as $assign) { + if ($assign->expr !== $methodCall) { + continue; + } + + if ($assign->var instanceof Variable && is_string($assign->var->name)) { + return $assign->var->name; + } + } + + return null; + } + + /** + * @return Foreach_[] + */ + private function findForeachesOverVariable(Node $classLike, string $variableName): array + { + $foreaches = $this->nodeFinder->findInstanceOf($classLike, Foreach_::class); + + return array_filter( + $foreaches, + static fn (Foreach_ $foreach): bool => $foreach->expr instanceof Variable && $foreach->expr->name === $variableName + ); + } + + /** + * @param Node[] $stmts + */ + private function hasArrayDimFetchOnVariable(array $stmts, string $variableName): bool + { + $arrayDimFetch = $this->nodeFinder->findFirst($stmts, static function (Node $subNode) use ($variableName): bool { + if (! $subNode instanceof ArrayDimFetch) { + return false; + } + + $rootVariable = $subNode->var; + while ($rootVariable instanceof ArrayDimFetch) { + $rootVariable = $rootVariable->var; + } + + return $rootVariable instanceof Variable && $rootVariable->name === $variableName; + }); - return [ - $identifierRuleError, - ]; + return $arrayDimFetch instanceof ArrayDimFetch; } } diff --git a/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/Fixture/FindTaggedServiceIdsSimpleForeach.php b/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/Fixture/FindTaggedServiceIdsSimpleForeach.php new file mode 100644 index 00000000..fe2a4f65 --- /dev/null +++ b/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/Fixture/FindTaggedServiceIdsSimpleForeach.php @@ -0,0 +1,21 @@ +getDefinition('some_service'); + + $taggedServices = $containerBuilder->findTaggedServiceIds('some_tag'); + foreach ($taggedServices as $id => $tag) { + $definition->addMethodCall('addTransport', [$tag]); + } + } +} diff --git a/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/Fixture/SkipTagAttributesInForeach.php b/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/Fixture/SkipTagAttributesInForeach.php new file mode 100644 index 00000000..c6342a97 --- /dev/null +++ b/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/Fixture/SkipTagAttributesInForeach.php @@ -0,0 +1,27 @@ +getDefinition('some_service'); + + $taggedServices = $containerBuilder->findTaggedServiceIds('some_tag'); + foreach ($taggedServices as $id => $tags) { + $definition->addMethodCall('addTransport', [ + $id, + new Reference($id), + ! empty($tags[0]['alias']) ? $tags[0]['alias'] : $id, + ! empty($tags[0]['integrationAlias']) ? $tags[0]['integrationAlias'] : $id, + ]); + } + } +} diff --git a/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/NoFindTaggedServiceIdsCallRuleTest.php b/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/NoFindTaggedServiceIdsCallRuleTest.php index f1a0b964..15131045 100644 --- a/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/NoFindTaggedServiceIdsCallRuleTest.php +++ b/tests/Rules/Symfony/NoFindTaggedServiceIdsCallRule/NoFindTaggedServiceIdsCallRuleTest.php @@ -32,6 +32,10 @@ public static function provideData(): Iterator { yield [__DIR__ . '/Fixture/FindTaggedServiceIdsInPass.php', [[NoFindTaggedServiceIdsCallRule::ERROR_MESSAGE, 14]]]; + yield [__DIR__ . '/Fixture/FindTaggedServiceIdsSimpleForeach.php', [[NoFindTaggedServiceIdsCallRule::ERROR_MESSAGE, 16]]]; + + yield [__DIR__ . '/Fixture/SkipTagAttributesInForeach.php', []]; + yield [__DIR__ . '/Fixture/SkipFindTaggedServiceIdsInContext.php', []]; }