Skip to content

Commit dc54cc9

Browse files
fabpotfabpot
committed
feature #37670 [Translation] Translatable objects (natewiebe13)
This PR was squashed before being merged into the 5.2-dev branch. Discussion ---------- [Translation] Translatable objects | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | None | License | MIT | Doc PR | symfony/symfony-docs#... - Will be added if this gets approval **Scenario** When writing business logic, this code typically lives within services. With complex situations, there can be lots of business logic exceptions or messages being created. In order to have the best user experience, we try to display detailed messages to the end user so if it's an error with some data, they know what they need to do to correct the situation. Sometimes this is through a message on a page, an email, etc. In some cases a generic message might be displayed and we log the message instead or send the message in an email to a 3rd party. The service shouldn't need to care how the message is going to be handled, but just needs to create it in a way that it can be translated if needed. **Options** Translating when the message is created. There are two issues with this approach. Autowiring the translator interface to services that will display an error isn't ideal as it will increase the dependencies the service has. The larger issue is that because we would be throwing an exception that contains a string or passing a message that's a string (what the translator returns), if we have generic twig templates that run the translator function on a message, we're doing a double translation and the debug toolbar will indicate that there is a missing message and turn red. This makes finding missing translations much more difficult as we have to determine if there are any false positives and turns the red indicator into white noise. Translating when displaying on a template. This option is possible, but adds complexity to our templates. Each time we display a message, we need to have some kind of logic that checks to see if it should be ran through the translator as-is or we need to get the data from the object and pass into the translator. **Proposed Solution** Adding a Phrase object that we can create and pass around reduces the template complexity and adds a nice way to create objects that can be translated at any point. This something that will be very familiar with Magento developers and other platforms also have similar solutions. **TODO** - [x] Determine if overwriting the parameters should be allowed (currently implemented this way) - [x] Determine the name of the translatable class, currently `Phrase` as `Message` will likely get confused with Messenger - [x] Determine if there should be a global function to allow quickly creating these. Similar to `__()` in Magento - [ ] Add docs - [x] Add tests Happy to get as much feedback as possible. I know a feature like this would be very welcomed by my team. Commits ------- dc6b3bf [Translation] Translatable objects
2 parents 6c094cc + dc6b3bf commit dc54cc9

File tree

15 files changed

+507
-4
lines changed

15 files changed

+507
-4
lines changed

‎src/Symfony/Bridge/Twig/CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ CHANGELOG
55
-----
66

77
* added the `workflow_transition()` function to easily retrieve a specific transition object
8+
* added support for translating `Translatable` objects
9+
* added the `t()` function to easily create `Translatable` objects
10+
* Added support for extracting messages from the `t()` function
811

912
5.0.0
1013
-----

‎src/Symfony/Bridge/Twig/Extension/TranslationExtension.php‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
use Symfony\Bridge\Twig\NodeVisitor\TranslationNodeVisitor;
1616
use Symfony\Bridge\Twig\TokenParser\TransDefaultDomainTokenParser;
1717
use Symfony\Bridge\Twig\TokenParser\TransTokenParser;
18+
use Symfony\Component\Translation\Translatable;
1819
use Symfony\Contracts\Translation\TranslatorInterface;
1920
use Symfony\Contracts\Translation\TranslatorTrait;
2021
use Twig\Extension\AbstractExtension;
2122
use Twig\TwigFilter;
23+
use Twig\TwigFunction;
2224

2325
// Help opcache.preload discover always-needed symbols
2426
class_exists(TranslatorInterface::class);
@@ -54,6 +56,16 @@ public function getTranslator(): TranslatorInterface
5456
return $this->translator;
5557
}
5658

59+
/**
60+
* {@inheritdoc}
61+
*/
62+
public function getFunctions(): array
63+
{
64+
return [
65+
new TwigFunction('t', [$this, 'createTranslatable']),
66+
];
67+
}
68+
5769
/**
5870
* {@inheritdoc}
5971
*/
@@ -91,8 +103,17 @@ public function getTranslationNodeVisitor(): TranslationNodeVisitor
91103
return $this->translationNodeVisitor ?: $this->translationNodeVisitor = new TranslationNodeVisitor();
92104
}
93105

94-
public function trans(?string $message, array $arguments = [], string $domain = null, string $locale = null, int $count = null): string
106+
/**
107+
* @param ?string|Translatable $message The message id (may also be an object that can be cast to string)
108+
*/
109+
public function trans($message, array $arguments = [], string $domain = null, string $locale = null, int $count = null): string
95110
{
111+
if ($message instanceof Translatable) {
112+
$arguments += $message->getParameters();
113+
$domain = $message->getDomain();
114+
$message = $message->getMessage();
115+
}
116+
96117
if (null === $message || '' === $message) {
97118
return '';
98119
}
@@ -103,4 +124,9 @@ public function trans(?string $message, array $arguments = [], string $domain =
103124

104125
return $this->getTranslator()->trans($message, $arguments, $domain, $locale);
105126
}
127+
128+
public function createTranslatable(string $message, array $parameters = [], string $domain = 'messages'): Translatable
129+
{
130+
return new Translatable($message, $parameters, $domain);
131+
}
106132
}

‎src/Symfony/Bridge/Twig/NodeVisitor/TranslationNodeVisitor.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Twig\Environment;
1616
use Twig\Node\Expression\ConstantExpression;
1717
use Twig\Node\Expression\FilterExpression;
18+
use Twig\Node\Expression\FunctionExpression;
1819
use Twig\Node\Node;
1920
use Twig\NodeVisitor\AbstractNodeVisitor;
2021

@@ -66,6 +67,20 @@ protected function doEnterNode(Node $node, Environment $env): Node
6667
$node->getNode('node')->getAttribute('value'),
6768
$this->getReadDomainFromArguments($node->getNode('arguments'), 1),
6869
];
70+
} elseif (
71+
$node instanceof FilterExpression &&
72+
'trans' === $node->getNode('filter')->getAttribute('value') &&
73+
$node->getNode('node') instanceof FunctionExpression &&
74+
't' === $node->getNode('node')->getAttribute('name')
75+
) {
76+
$nodeArguments = $node->getNode('node')->getNode('arguments');
77+
78+
if ($nodeArguments->getIterator()->current() instanceof ConstantExpression) {
79+
$this->messages[] = [
80+
$this->getReadMessageFromArguments($nodeArguments, 0),
81+
$this->getReadDomainFromArguments($nodeArguments, 2),
82+
];
83+
}
6984
} elseif (
7085
$node instanceof FilterExpression &&
7186
'transchoice' === $node->getNode('filter')->getAttribute('value') &&
@@ -103,6 +118,28 @@ public function getPriority(): int
103118
return 0;
104119
}
105120

121+
private function getReadMessageFromArguments(Node $arguments, int $index): ?string
122+
{
123+
if ($arguments->hasNode('message')) {
124+
$argument = $arguments->getNode('message');
125+
} elseif ($arguments->hasNode($index)) {
126+
$argument = $arguments->getNode($index);
127+
} else {
128+
return null;
129+
}
130+
131+
return $this->getReadMessageFromNode($argument);
132+
}
133+
134+
private function getReadMessageFromNode(Node $node): ?string
135+
{
136+
if ($node instanceof ConstantExpression) {
137+
return $node->getAttribute('value');
138+
}
139+
140+
return null;
141+
}
142+
106143
private function getReadDomainFromArguments(Node $arguments, int $index): ?string
107144
{
108145
if ($arguments->hasNode('domain')) {

‎src/Symfony/Bridge/Twig/Tests/Extension/TranslationExtensionTest.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,23 @@ public function getTransTests()
122122
// trans filter with null message
123123
['{{ null|trans }}', ''],
124124
['{{ foo|trans }}', '', ['foo' => null]],
125+
126+
// trans object
127+
['{{ t("Hello")|trans }}', 'Hello'],
128+
['{{ t(name)|trans }}', 'Symfony', ['name' => 'Symfony']],
129+
['{{ t(hello)|trans({ \'%name%\': \'Symfony\' }) }}', 'Hello Symfony', ['hello' => 'Hello %name%']],
130+
['{{ t(hello, { \'%name%\': \'Symfony\' })|trans }}', 'Hello Symfony', ['hello' => 'Hello %name%']],
131+
['{{ t(hello, { \'%name%\': \'Another Name\' })|trans({ \'%name%\': \'Symfony\' }) }}', 'Hello Symfony', ['hello' => 'Hello %name%']],
132+
['{% set vars = { \'%name%\': \'Symfony\' } %}{{ t(hello)|trans(vars) }}', 'Hello Symfony', ['hello' => 'Hello %name%']],
133+
['{% set vars = { \'%name%\': \'Symfony\' } %}{{ t(hello, vars)|trans }}', 'Hello Symfony', ['hello' => 'Hello %name%']],
134+
['{{ t("Hello")|trans(locale="fr") }}', 'Hello'],
135+
['{{ t("Hello", {}, "messages")|trans(locale="fr") }}', 'Hello'],
136+
137+
// trans object with count
138+
['{{ t("{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples")|trans(count=count) }}', 'There is 5 apples', ['count' => 5]],
139+
['{{ t(text)|trans(count=5, arguments={\'%name%\': \'Symfony\'}) }}', 'There is 5 apples (Symfony)', ['text' => '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples (%name%)']],
140+
['{{ t(text, {\'%name%\': \'Symfony\'})|trans(count=5) }}', 'There is 5 apples (Symfony)', ['text' => '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples (%name%)']],
141+
['{{ t("{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples", {}, "messages")|trans(locale="fr", count=count) }}', 'There is 5 apples', ['count' => 5]],
125142
];
126143
}
127144

‎src/Symfony/Bridge/Twig/Tests/Translation/TwigExtractorTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ public function getExtractData()
6060
['{% trans from "domain" %}new key{% endtrans %}', ['new key' => 'domain']],
6161
['{% set foo = "new key" | trans %}', ['new key' => 'messages']],
6262
['{{ 1 ? "new key" | trans : "another key" | trans }}', ['new key' => 'messages', 'another key' => 'messages']],
63+
['{{ t("new key") | trans() }}', ['new key' => 'messages']],
64+
['{{ t("new key", {}, "domain") | trans() }}', ['new key' => 'domain']],
65+
['{{ 1 ? t("new key") | trans : t("another key") | trans }}', ['new key' => 'messages', 'another key' => 'messages']],
6366

6467
// make sure 'trans_default_domain' tag is supported
6568
['{% trans_default_domain "domain" %}{{ "new key"|trans }}', ['new key' => 'domain']],

‎src/Symfony/Component/Translation/CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ CHANGELOG
55
-----
66

77
* added `PseudoLocalizationTranslator`
8+
* added `Translatable` objects that represent a message that can be translated
9+
* added the `t()` function to easily create `Translatable` objects
10+
* Added support for extracting messages from `Translatable` objects
811

912
5.1.0
1013
-----

‎src/Symfony/Component/Translation/Extractor/PhpExtractor.php‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,66 @@ class PhpExtractor extends AbstractFileExtractor implements ExtractorInterface
5454
'(',
5555
self::MESSAGE_TOKEN,
5656
],
57+
[
58+
'new',
59+
'Translatable',
60+
'(',
61+
self::MESSAGE_TOKEN,
62+
',',
63+
self::METHOD_ARGUMENTS_TOKEN,
64+
',',
65+
self::DOMAIN_TOKEN,
66+
],
67+
[
68+
'new',
69+
'Translatable',
70+
'(',
71+
self::MESSAGE_TOKEN,
72+
],
73+
[
74+
'new',
75+
'\\',
76+
'Symfony',
77+
'\\',
78+
'Component',
79+
'\\',
80+
'Translation',
81+
'\\',
82+
'Translatable',
83+
'(',
84+
self::MESSAGE_TOKEN,
85+
',',
86+
self::METHOD_ARGUMENTS_TOKEN,
87+
',',
88+
self::DOMAIN_TOKEN,
89+
],
90+
[
91+
'new',
92+
'\\',
93+
'Symfony',
94+
'\\',
95+
'Component',
96+
'\\',
97+
'Translation',
98+
'\\',
99+
'Translatable',
100+
'(',
101+
self::MESSAGE_TOKEN,
102+
],
103+
[
104+
't',
105+
'(',
106+
self::MESSAGE_TOKEN,
107+
',',
108+
self::METHOD_ARGUMENTS_TOKEN,
109+
',',
110+
self::DOMAIN_TOKEN,
111+
],
112+
[
113+
't',
114+
'(',
115+
self::MESSAGE_TOKEN,
116+
],
57117
];
58118

59119
/**

‎src/Symfony/Component/Translation/Resources/functions/translatable.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Component\Translation\Translatable;
13+
14+
if (!function_exists('t')) {
15+
/**
16+
* @author Nate Wiebe <nate@northern.co>
17+
*/
18+
function t(string $message, array $parameters = [], string $domain = 'messages'): Translatable
19+
{
20+
return new Translatable($message, $parameters, $domain);
21+
}
22+
}

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

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,39 @@ public function testExtraction($resource)
4141
// Assert
4242
$expectedCatalogue = [
4343
'messages' => [
44+
'translatable single-quoted key' => 'prefixtranslatable single-quoted key',
45+
'translatable double-quoted key' => 'prefixtranslatable double-quoted key',
46+
'translatable heredoc key' => 'prefixtranslatable heredoc key',
47+
'translatable nowdoc key' => 'prefixtranslatable nowdoc key',
48+
"translatable double-quoted key with whitespace and escaped \$\n\" sequences" => "prefixtranslatable double-quoted key with whitespace and escaped \$\n\" sequences",
49+
'translatable single-quoted key with whitespace and nonescaped \$\n\' sequences' => 'prefixtranslatable single-quoted key with whitespace and nonescaped \$\n\' sequences',
50+
'translatable single-quoted key with "quote mark at the end"' => 'prefixtranslatable single-quoted key with "quote mark at the end"',
51+
'translatable '.$expectedHeredoc => 'prefixtranslatable '.$expectedHeredoc,
52+
'translatable '.$expectedNowdoc => 'prefixtranslatable '.$expectedNowdoc,
53+
'translatable concatenated message with heredoc and nowdoc' => 'prefixtranslatable concatenated message with heredoc and nowdoc',
54+
'translatable default domain' => 'prefixtranslatable default domain',
55+
'translatable-fqn single-quoted key' => 'prefixtranslatable-fqn single-quoted key',
56+
'translatable-fqn double-quoted key' => 'prefixtranslatable-fqn double-quoted key',
57+
'translatable-fqn heredoc key' => 'prefixtranslatable-fqn heredoc key',
58+
'translatable-fqn nowdoc key' => 'prefixtranslatable-fqn nowdoc key',
59+
"translatable-fqn double-quoted key with whitespace and escaped \$\n\" sequences" => "prefixtranslatable-fqn double-quoted key with whitespace and escaped \$\n\" sequences",
60+
'translatable-fqn single-quoted key with whitespace and nonescaped \$\n\' sequences' => 'prefixtranslatable-fqn single-quoted key with whitespace and nonescaped \$\n\' sequences',
61+
'translatable-fqn single-quoted key with "quote mark at the end"' => 'prefixtranslatable-fqn single-quoted key with "quote mark at the end"',
62+
'translatable-fqn '.$expectedHeredoc => 'prefixtranslatable-fqn '.$expectedHeredoc,
63+
'translatable-fqn '.$expectedNowdoc => 'prefixtranslatable-fqn '.$expectedNowdoc,
64+
'translatable-fqn concatenated message with heredoc and nowdoc' => 'prefixtranslatable-fqn concatenated message with heredoc and nowdoc',
65+
'translatable-fqn default domain' => 'prefixtranslatable-fqn default domain',
66+
'translatable-short single-quoted key' => 'prefixtranslatable-short single-quoted key',
67+
'translatable-short double-quoted key' => 'prefixtranslatable-short double-quoted key',
68+
'translatable-short heredoc key' => 'prefixtranslatable-short heredoc key',
69+
'translatable-short nowdoc key' => 'prefixtranslatable-short nowdoc key',
70+
"translatable-short double-quoted key with whitespace and escaped \$\n\" sequences" => "prefixtranslatable-short double-quoted key with whitespace and escaped \$\n\" sequences",
71+
'translatable-short single-quoted key with whitespace and nonescaped \$\n\' sequences' => 'prefixtranslatable-short single-quoted key with whitespace and nonescaped \$\n\' sequences',
72+
'translatable-short single-quoted key with "quote mark at the end"' => 'prefixtranslatable-short single-quoted key with "quote mark at the end"',
73+
'translatable-short '.$expectedHeredoc => 'prefixtranslatable-short '.$expectedHeredoc,
74+
'translatable-short '.$expectedNowdoc => 'prefixtranslatable-short '.$expectedNowdoc,
75+
'translatable-short concatenated message with heredoc and nowdoc' => 'prefixtranslatable-short concatenated message with heredoc and nowdoc',
76+
'translatable-short default domain' => 'prefixtranslatable-short default domain',
4477
'single-quoted key' => 'prefixsingle-quoted key',
4578
'double-quoted key' => 'prefixdouble-quoted key',
4679
'heredoc key' => 'prefixheredoc key',
@@ -54,6 +87,21 @@ public function testExtraction($resource)
5487
'default domain' => 'prefixdefault domain',
5588
],
5689
'not_messages' => [
90+
'translatable other-domain-test-no-params-short-array' => 'prefixtranslatable other-domain-test-no-params-short-array',
91+
'translatable other-domain-test-no-params-long-array' => 'prefixtranslatable other-domain-test-no-params-long-array',
92+
'translatable other-domain-test-params-short-array' => 'prefixtranslatable other-domain-test-params-short-array',
93+
'translatable other-domain-test-params-long-array' => 'prefixtranslatable other-domain-test-params-long-array',
94+
'translatable typecast' => 'prefixtranslatable typecast',
95+
'translatable-fqn other-domain-test-no-params-short-array' => 'prefixtranslatable-fqn other-domain-test-no-params-short-array',
96+
'translatable-fqn other-domain-test-no-params-long-array' => 'prefixtranslatable-fqn other-domain-test-no-params-long-array',
97+
'translatable-fqn other-domain-test-params-short-array' => 'prefixtranslatable-fqn other-domain-test-params-short-array',
98+
'translatable-fqn other-domain-test-params-long-array' => 'prefixtranslatable-fqn other-domain-test-params-long-array',
99+
'translatable-fqn typecast' => 'prefixtranslatable-fqn typecast',
100+
'translatable-short other-domain-test-no-params-short-array' => 'prefixtranslatable-short other-domain-test-no-params-short-array',
101+
'translatable-short other-domain-test-no-params-long-array' => 'prefixtranslatable-short other-domain-test-no-params-long-array',
102+
'translatable-short other-domain-test-params-short-array' => 'prefixtranslatable-short other-domain-test-params-short-array',
103+
'translatable-short other-domain-test-params-long-array' => 'prefixtranslatable-short other-domain-test-params-long-array',
104+
'translatable-short typecast' => 'prefixtranslatable-short typecast',
57105
'other-domain-test-no-params-short-array' => 'prefixother-domain-test-no-params-short-array',
58106
'other-domain-test-no-params-long-array' => 'prefixother-domain-test-no-params-long-array',
59107
'other-domain-test-params-short-array' => 'prefixother-domain-test-params-short-array',
@@ -65,6 +113,18 @@ public function testExtraction($resource)
65113

66114
$this->assertEquals($expectedCatalogue, $actualCatalogue);
67115

116+
$filename = str_replace(\DIRECTORY_SEPARATOR, '/', __DIR__).'/../fixtures/extractor/translatable.html.php';
117+
$this->assertEquals(['sources' => [$filename.':2']], $catalogue->getMetadata('translatable single-quoted key'));
118+
$this->assertEquals(['sources' => [$filename.':37']], $catalogue->getMetadata('translatable other-domain-test-no-params-short-array', 'not_messages'));
119+
120+
$filename = str_replace(\DIRECTORY_SEPARATOR, '/', __DIR__).'/../fixtures/extractor/translatable-fqn.html.php';
121+
$this->assertEquals(['sources' => [$filename.':2']], $catalogue->getMetadata('translatable-fqn single-quoted key'));
122+
$this->assertEquals(['sources' => [$filename.':37']], $catalogue->getMetadata('translatable-fqn other-domain-test-no-params-short-array', 'not_messages'));
123+
124+
$filename = str_replace(\DIRECTORY_SEPARATOR, '/', __DIR__).'/../fixtures/extractor/translatable-short.html.php';
125+
$this->assertEquals(['sources' => [$filename.':2']], $catalogue->getMetadata('translatable-short single-quoted key'));
126+
$this->assertEquals(['sources' => [$filename.':37']], $catalogue->getMetadata('translatable-short other-domain-test-no-params-short-array', 'not_messages'));
127+
68128
$filename = str_replace(\DIRECTORY_SEPARATOR, '/', __DIR__).'/../fixtures/extractor/translation.html.php';
69129
$this->assertEquals(['sources' => [$filename.':2']], $catalogue->getMetadata('single-quoted key'));
70130
$this->assertEquals(['sources' => [$filename.':37']], $catalogue->getMetadata('other-domain-test-no-params-short-array', 'not_messages'));
@@ -73,20 +133,21 @@ public function testExtraction($resource)
73133
public function resourcesProvider()
74134
{
75135
$directory = __DIR__.'/../fixtures/extractor/';
136+
$phpFiles = [];
76137
$splFiles = [];
77138
foreach (new \DirectoryIterator($directory) as $fileInfo) {
78139
if ($fileInfo->isDot()) {
79140
continue;
80141
}
81-
if ('translation.html.php' === $fileInfo->getBasename()) {
82-
$phpFile = $fileInfo->getPathname();
142+
if (\in_array($fileInfo->getBasename(), ['translatable.html.php', 'translatable-fqn.html.php', 'translatable-short.html.php', 'translation.html.php'], true)) {
143+
$phpFiles[] = $fileInfo->getPathname();
83144
}
84145
$splFiles[] = $fileInfo->getFileInfo();
85146
}
86147

87148
return [
88149
[$directory],
89-
[$phpFile],
150+
[$phpFiles],
90151
[glob($directory.'*')],
91152
[$splFiles],
92153
[new \ArrayObject(glob($directory.'*'))],

0 commit comments

Comments
 (0)