8000 bug #48830 [Translation] fix PhpAstExtractor also extracts messages i… · symfony/symfony@6f75529 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6f75529

Browse files
fabpotfabpot
committed
bug #48830 [Translation] fix PhpAstExtractor also extracts messages if 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 ------- 96a020b [Translation] fix PhpAstExtractor also extracts messages from t()/trans() that contains both unnamed and named arguments
2 parents 20851ea + 96a020b commit 6f75529

File tree

5 files changed

+31
-6
lines changed

5 files changed

+31
-6
lines changed

‎src/Symfony/Component/Translation/Extractor/Visitor/AbstractVisitor.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,19 @@ protected function hasNodeNamedArguments(Node\Expr\CallLike|Node\Attribute|Node\
6868
return false;
6969
}
7070

71+
protected function nodeFirstNamedArgumentIndex(Node\Expr\CallLike|Node\Attribute|Node\Expr\New_ $node): int
72+
{
73+
$args = $node instanceof Node\Expr\CallLike ? $node->getRawArgs() : $node->args;
74+
75+
foreach ($args as $i => $arg) {
76+
if ($arg instanceof Node\Arg && null !== $arg->name) {
77+
return $i;
78+
}
79+
}
80+
81+
return \PHP_INT_MAX;
82+
}
83+
7184
private function getStringNamedArguments(Node\Expr\CallLike|Node\Attribute $node, string $argumentName = null, bool $isArgumentNamePattern = false): array
7285
{
7386
$args = $node instanceof Node\Expr\CallLike ? $node->getArgs() : $node->args;

‎src/Symfony/Component/Translation/Extractor/Visitor/TransMethodVisitor.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,13 @@ public function enterNode(Node $node): ?Node
3737
$name = (string) $node->name;
3838

3939
if ('trans' === $name || 't' === $name) {
40-
$nodeHasNamedArguments = $this->hasNodeNamedArguments($node);
41-
if (!$messages = $this->getStringArguments($node, $nodeHasNamedArguments ? 'message' : 0)) {
40+
$firstNamedArgumentIndex = $this->nodeFirstNamedArgumentIndex($node);
41+
42+
if (!$messages = $this->getStringArguments($node, 0 < $firstNamedArgumentIndex ? 0 : 'message')) {
4243
return null;
4344
}
4445

45-
$domain = $this->getStringArguments($node, $nodeHasNamedArguments ? 'domain' : 2)[0] ?? null;
46+
$domain = $this->getStringArguments($node, 2 < $firstNamedArgumentIndex ? 2 : 'domain')[0] ?? null;
4647

4748
foreach ($messages as $message) {
4849
$this->addMessageToCatalogue($message, $domain, $node->getStartLine());

‎src/Symfony/Component/Translation/Extractor/Visitor/TranslatableMessageVisitor.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ public function enterNode(Node $node): ?Node
3838
return null;
3939
}
4040

41-
$nodeHasNamedArguments = $this->hasNodeNamedArguments($node);
41+
$firstNamedArgumentIndex = $this->nodeFirstNamedArgumentIndex($node);
4242

43-
if (!$messages = $this->getStringArguments($node, $nodeHasNamedArguments ? 'message' : 0)) {
43+
if (!$messages = $this->getStringArguments($node, 0 < $firstNamedArgumentIndex ? 0 : 'message')) {
4444
return null;
4545
}
4646

47-
$domain = $this->getStringArguments($node, $nodeHasNamedArguments ? 'domain' : 2)[0] ?? null;
47+
$domain = $this->getStringArguments($node, 2 < $firstNamedArgumentIndex ? 2 : 'domain')[0] ?? null;
4848

4949
foreach ($messages as $message) {
50 8000 50
$this->addMessageToCatalogue($message, $domain, $node->getStartLine());

‎src/Symfony/Component/Translation/Tests/Extractor/PhpAstExtractorTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ public function testExtraction(iterable|string $resource)
9191
$expectedNowdoc => 'prefix'.$expectedNowdoc,
9292
'concatenated message with heredoc and nowdoc' => 'prefixconcatenated message with heredoc and nowdoc',
9393
'default domain' => 'prefixdefault domain',
94+
'mix-named-arguments' => 'prefixmix-named-arguments',
95+
'mix-named-arguments-locale' => 'prefixmix-named-arguments-locale',
96+
'mix-named-arguments-without-domain' => 'prefixmix-named-arguments-without-domain',
9497
],
9598
'not_messages' => [
9699
'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)
119122
'variable-assignation-inlined-in-trans-method-call2' => 'prefixvariable-assignation-inlined-in-trans-method-call2',
120123
'variable-assignation-inlined-in-trans-method-call3' => 'prefixvariable-assignation-inlined-in-trans-method-call3',
121124
'variable-assignation-inlined-with-named-arguments-in-trans-method' => 'prefixvariable-assignation-inlined-with-named-arguments-in-trans-method',
125+
'mix-named-arguments-without-parameters' => 'prefixmix-named-arguments-without-parameters',
126+
'mix-named-arguments-disordered' => 'prefixmix-named-arguments-disordered',
122127
],
123128
'validators' => [
124129
'message-in-constraint-attribute' => 'prefixmessage-in-constraint-attribute',

‎src/Symfony/Component/Translation/Tests/fixtures/extractor-ast/translation.html.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,10 @@
5555

5656
<?php echo $view['translator']->trans(domain: $domain = 'not_messages', message: $key = 'variable-assignation-inlined-with-named-arguments-in-trans-method', parameters: $parameters = []); ?>
5757

58+
<?php echo $view['translator']->trans('mix-named-arguments', parameters: ['foo' => 'bar']); ?>
59+
<?php echo $view['translator']->trans('mix-named-arguments-locale', parameters: ['foo' => 'bar'], locale: 'de'); ?>
60+
<?php echo $view['translator']->trans('mix-named-arguments-without-domain', parameters: ['foo' => 'bar']); ?>
61+
<?php echo $view['translator']->trans('mix-named-arguments-without-parameters', domain: 'not_messages'); ?>
62+
<?php echo $view['translator']->trans('mix-named-arguments-disordered', domain: 'not_messages', parameters: []); ?>
63+
5864
<?php echo $view['translator']->trans(...); // should not fail ?>

0 commit comments

Comments
 (0)
0