bug #48830 [Translation] fix PhpAstExtractor also extracts messages i…

…f t() contains both unnamed and named arguments (gassan)

This PR was merged into the 6.2 branch.

Discussion
----------

[Translation] fix PhpAstExtractor also extracts messages if t() contains both unnamed and named arguments

The case for $translator->trans('some message', domain: 'foo') was not covered by PhpAstExtractor.

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| License       | MIT
<!--
Replace this notice by a short README for your feature/bugfix.
This will help reviewers and should be a good start for the documentation.

Additionally (see https://symfony.com/releases):
 - Always add tests and ensure they pass.
 - Bug fixes must be submitted against the lowest maintained branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the latest branch.
 - For new features, provide some code snippets to help understand usage.
 - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
 - Never break backward compatibility (see https://symfony.com/bc).
-->

Commits
-------

96a020bb57 [Translation] fix PhpAstExtractor also extracts messages from t()/trans() that contains both unnamed and named arguments

Extractor/Visitor/AbstractVisitor.php

@@ -68,6 +68,19 @@ protected function hasNodeNamedArguments(Node\Expr\CallLike|Node\Attribute|Node\
         return false;
     }
 
+    protected function nodeFirstNamedArgumentIndex(Node\Expr\CallLike|Node\Attribute|Node\Expr\New_ $node): int
+    {
+        $args = $node instanceof Node\Expr\CallLike ? $node->getRawArgs() : $node->args;
+
+        foreach ($args as $i => $arg) {
+            if ($arg instanceof Node\Arg && null !== $arg->name) {
+                return $i;
+            }
+        }
+
+        return \PHP_INT_MAX;
+    }
+
     private function getStringNamedArguments(Node\Expr\CallLike|Node\Attribute $node, string $argumentName = null, bool $isArgumentNamePattern = false): array
     {
         $args = $node instanceof Node\Expr\CallLike ? $node->getArgs() : $node->args;

Extractor/Visitor/TransMethodVisitor.php

@@ -37,12 +37,13 @@ public function enterNode(Node $node): ?Node
         $name = (string) $node->name;
 
         if ('trans' === $name || 't' === $name) {
-            $nodeHasNamedArguments = $this->hasNodeNamedArguments($node);
-            if (!$messages = $this->getStringArguments($node, $nodeHasNamedArguments ? 'message' : 0)) {
+            $firstNamedArgumentIndex = $this->nodeFirstNamedArgumentIndex($node);
+
+            if (!$messages = $this->getStringArguments($node, 0 < $firstNamedArgumentIndex ? 0 : 'message')) {
                 return null;
             }
 
-            $domain = $this->getStringArguments($node, $nodeHasNamedArguments ? 'domain' : 2)[0] ?? null;
+            $domain = $this->getStringArguments($node, 2 < $firstNamedArgumentIndex ? 2 : 'domain')[0] ?? null;
 
             foreach ($messages as $message) {
                 $this->addMessageToCatalogue($message, $domain, $node->getStartLine());

Extractor/Visitor/TranslatableMessageVisitor.php

@@ -38,13 +38,13 @@ public function enterNode(Node $node): ?Node
             return null;
         }
 
-        $nodeHasNamedArguments = $this->hasNodeNamedArguments($node);
+        $firstNamedArgumentIndex = $this->nodeFirstNamedArgumentIndex($node);
 
-        if (!$messages = $this->getStringArguments($node, $nodeHasNamedArguments ? 'message' : 0)) {
+        if (!$messages = $this->getStringArguments($node, 0 < $firstNamedArgumentIndex ? 0 : 'message')) {
             return null;
         }
 
-        $domain = $this->getStringArguments($node, $nodeHasNamedArguments ? 'domain' : 2)[0] ?? null;
+        $domain = $this->getStringArguments($node, 2 < $firstNamedArgumentIndex ? 2 : 'domain')[0] ?? null;
 
         foreach ($messages as $message) {
             $this->addMessageToCatalogue($message, $domain, $node->getStartLine());

Tests/Extractor/PhpAstExtractorTest.php

@@ -91,6 +91,9 @@ public function testExtraction(iterable|string $resource)
                 $expectedNowdoc => 'prefix'.$expectedNowdoc,
                 'concatenated message with heredoc and nowdoc' => 'prefixconcatenated message with heredoc and nowdoc',
                 'default domain' => 'prefixdefault domain',
+                'mix-named-arguments' => 'prefixmix-named-arguments',
+                'mix-named-arguments-locale' => 'prefixmix-named-arguments-locale',
+                'mix-named-arguments-without-domain' => 'prefixmix-named-arguments-without-domain',
             ],
             'not_messages' => [
                 'translatable other-domain-test-no-params-short-array' => 'prefixtranslatable other-domain-test-no-params-short-array',
@@ -119,6 +122,8 @@ public function testExtraction(iterable|string $resource)
                 'variable-assignation-inlined-in-trans-method-call2' => 'prefixvariable-assignation-inlined-in-trans-method-call2',
                 'variable-assignation-inlined-in-trans-method-call3' => 'prefixvariable-assignation-inlined-in-trans-method-call3',
                 'variable-assignation-inlined-with-named-arguments-in-trans-method' => 'prefixvariable-assignation-inlined-with-named-arguments-in-trans-method',
+                'mix-named-arguments-without-parameters' => 'prefixmix-named-arguments-without-parameters',
+                'mix-named-arguments-disordered' => 'prefixmix-named-arguments-disordered',
             ],
             'validators' => [
                 'message-in-constraint-attribute' => 'prefixmessage-in-constraint-attribute',

Tests/fixtures/extractor-ast/translation.html.php

@@ -55,4 +55,10 @@
 
 <?php echo $view['translator']->trans(domain: $domain = 'not_messages', message: $key = 'variable-assignation-inlined-with-named-arguments-in-trans-method', parameters: $parameters = []); ?>
 
+<?php echo $view['translator']->trans('mix-named-arguments', parameters: ['foo' => 'bar']); ?>
+<?php echo $view['translator']->trans('mix-named-arguments-locale', parameters: ['foo' => 'bar'], locale: 'de'); ?>
+<?php echo $view['translator']->trans('mix-named-arguments-without-domain', parameters: ['foo' => 'bar']); ?>
+<?php echo $view['translator']->trans('mix-named-arguments-without-parameters', domain: 'not_messages'); ?>
+<?php echo $view['translator']->trans('mix-named-arguments-disordered', domain: 'not_messages', parameters: []); ?>
+
 <?php echo $view['translator']->trans(...); // should not fail ?>