8000 [Translator] Deprecated transChoice and moved it away from contracts by Nyholm · Pull Request #28375 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Translator] Deprecated transChoice and moved it away from contracts #28375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 6, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments. Retry
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[Translator] Deprecated transChoice and moved it away from contracts
  • Loading branch information
Nyholm authored and nicolas-grekas committed Oct 6, 2018
commit d870a850bd71fd1fc0b340cc291f62868f52d317
1 change: 1 addition & 0 deletions UPGRADE-4.2.md
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ Translation
-----------

* The `TranslatorInterface` has been deprecated in favor of `Symfony\Contracts\Translation\TranslatorInterface`
* The `Translator::transChoice()` has been deprecated in favor of using `Translator::trans()` with intl message format
* The `MessageSelector`, `Interval` and `PluralizationRules` classes have been deprecated, use `IdentityTranslator` instead
* The `Translator::getFallbackLocales()` and `TranslationDataCollector::getFallbackLocales()` method have been marked as internal

Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Bridge/Twig/Extension/TranslationExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
use Symfony\Bridge\Twig\TokenParser\TransChoiceTokenParser;
use Symfony\Bridge\Twig\TokenParser\TransDefaultDomainTokenParser;
use Symfony\Bridge\Twig\TokenParser\TransTokenParser;
use Symfony\Component\Translation\LegacyTranslatorTrait;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Contracts\Translation\TranslatorTrait;
use Twig\Extension\AbstractExtension;
Expand All @@ -34,6 +36,9 @@ class TranslationExtension extends AbstractExtension
getLocale as private;
setLocale as private;
trans as private doTrans;
}

use LegacyTranslatorTrait {
transChoice as private doTransChoice;
}

Expand Down Expand Up @@ -107,7 +112,7 @@ public function trans($message, array $arguments = array(), $domain = null, $loc

public function transchoice($message, $count, array $arguments = array(), $domain = null, $locale = null)
{
if (null === $this->translator) {
if (null === $this->translator || !$this->translator instanceof LegacyTranslatorInterface) {
return $this->doTransChoice($message, $count, array_merge(array('%count%' => $count), $arguments), $domain, $locale);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
namespace Symfony\Bundle\FrameworkBundle\Templating\Helper;

use Symfony\Component\Templating\Helper\Helper;
use Symfony\Component\Translation\LegacyTranslatorTrait;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Contracts\Translation\TranslatorTrait;

Expand All @@ -24,6 +26,9 @@ class TranslatorHelper extends Helper
getLocale as private;
setLocale as private;
trans as private doTrans;
}

use LegacyTranslatorTrait {
transChoice as private doTransChoice;
}

Expand Down Expand Up @@ -51,7 +56,7 @@ public function trans($id, array $parameters = array(), $domain = 'messages', $l
*/
public function transChoice($id, $number, array $parameters = array(), $domain = 'messages', $locale = null)
{
if (null === $this->translator) {
if (null === $this->translator || !$this->translator instanceof LegacyTranslatorInterface) {
return $this->doTransChoice($id, $number, $parameters, $domain, $locale);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,25 @@ public function testTransWithoutCaching()
$this->assertEquals('foo (FR)', $translator->trans('foo'));
$this->assertEquals('bar (EN)', $translator->trans('bar'));
$this->assertEquals('foobar (ES)', $translator->trans('foobar'));
$this->assertEquals('choice 0 (EN)', $translator->transChoice('choice', 0));
$this->assertEquals('no translation', $translator->trans('no translation'));
$this->assertEquals('foobarfoo (PT-PT)', $translator->trans('foobarfoo'));
$this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
$this->assertEquals('foobarbaz (fr.UTF-8)', $translator->trans('foobarbaz'));
$this->assertEquals('foobarbax (sr@latin)', $translator->trans('foobarbax'));
}

/**
* @group legacy
*/
public function testTransChoiceWithoutCaching()
{
$translator = $this->getTranslator($this->getLoader());
$translator->setLocale('fr');
$translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR', 'fr.UTF-8', 'sr@latin'));

$this->assertEquals('choice 0 (EN)', $translator->transChoice('choice', 0));
$this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
}

public function testTransWithCaching()
{
// prime the cache
Expand All @@ -70,10 +81,8 @@ public function testTransWithCaching()
$this->assertEquals('foo (FR)', $translator->trans('foo'));
$this->assertEquals('bar (EN)', $translator->trans('bar'));
$this->assertEquals('foobar (ES)', $translator->trans('foobar'));
$this->assertEquals('choice 0 (EN)', $translator->transChoice('choice', 0));
$this->assertEquals('no translation', $translator->trans('no translation'));
$this->assertEquals('foobarfoo (PT-PT)', $translator->trans('foobarfoo'));
$this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
$this->assertEquals('foobarbaz (fr.UTF-8)', $translator->trans('foobarbaz'));
$this->assertEquals('foobarbax (sr@latin)', $translator->trans('foobarbax'));

Expand All @@ -88,14 +97,37 @@ public function testTransWithCaching()
$this->assertEquals('foo (FR)', $translator->trans('foo'));
$this->assertEquals('bar (EN)', $translator->trans('bar'));
$this->assertEquals('foobar (ES)', $translator->trans('foobar'));
$this->assertEquals('choice 0 (EN)', $translator->transChoice('choice', 0));
$this->assertEquals('no translation', $translator->trans('no translation'));
$this->assertEquals('foobarfoo (PT-PT)', $translator->trans('foobarfoo'));
$this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
$this->assertEquals('foobarbaz (fr.UTF-8)', $translator->trans('foobarbaz'));
$this->assertEquals('foobarbax (sr@latin)', $translator->trans('foobarbax'));
}

/**
* @group legacy
*/
public function testTransChoiceWithCaching()
{
// prime the cache
$translator = $this->getTranslator($this->getLoader(), array('cache_dir' => $this->tmpDir));
$translator->setLocale('fr');
$translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR', 'fr.UTF-8', 'sr@latin'));

$this->assertEquals('choice 0 (EN)', $translator->transChoice('choice', 0));
$this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));

// do it another time as the cache is primed now
$loader = $this->getMockBuilder('Symfony\Component\Translation\Loader\LoaderInterface')->getMock();
$loader->expects($this->never())->method('load');

$translator = $this->getTranslator($loader, array('cache_dir' => $this->tmpDir));
$translator->setLocale('fr');
$translator->setFallbackLocales(array('en', 'es', 'pt-PT', 'pt_BR', 'fr.UTF-8', 'sr@latin'));

$this->assertEquals('choice 0 (EN)', $translator->transChoice('choice', 0));
$this->assertEquals('other choice 1 (PT-BR)', $translator->transChoice('other choice', 1));
}

/**
* @expectedException \InvalidArgumentException
* @expectedExceptionMessage Invalid "invalid locale" locale.
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/Translation/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CHANGELOG
-----

* Started using ICU parent locales as fallback locales.
* deprecated `Translator::transChoice()` in favor of using `Translator::trans()` with intl message format
* deprecated `TranslatorInterface` in favor of `Symfony\Contracts\Translation\TranslatorInterface`
* deprecated `MessageSelector`, `Interval` and `PluralizationRules`; use `IdentityTranslator` instead
* Added `IntlMessageFormatter` and `FallbackMessageFormatter`
Expand Down
11 changes: 10 additions & 1 deletion src/Symfony/Component/Translation/DataCollectorTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
*/
class DataCollectorTranslator implements LegacyTranslatorInterface, TranslatorBagInterface
{
use LegacyTranslatorTrait {
transChoice as private doTransChoice;
}

const MESSAGE_DEFINED = 0;
const MESSAGE_MISSING = 1;
const MESSAGE_EQUALS_FALLBACK = 2;
Expand Down Expand Up @@ -59,7 +63,12 @@ public function trans($id, array $parameters = array(), $domain = null, $locale
*/
public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null)
{
$trans = $this->translator->transChoice($id, $number, $parameters, $domain, $locale);
if ($this->translator instanceof LegacyTranslatorInterface) {
$trans = $this->translator->transChoice($id, $number, $parameters, $domain, $locale);
} else {
$trans = $this->doTransChoice($id, $number, $parameters, $domain, $locale);
}

$this->collectMessage($locale, $domain, $id, $trans, $parameters, $number);

return $trans;
Expand Down
11 changes: 4 additions & 7 deletions src/Symfony/Component/Translation/IdentityTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
*/
class IdentityTranslator implements TranslatorInterface
{
use TranslatorTrait {
use TranslatorTrait;
use LegacyTranslatorTrait {
transChoice as private doTransChoice;
}

Expand All @@ -43,15 +44,11 @@ public function __construct(MessageSelector $selector = null)
*/
public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null)
{
@trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2, use the "trans()" method with intl formatted messages instead.', __METHOD__), E_USER_DEPRECATED);
if ($this->selector) {
return strtr($this->selector->choose((string) $id, (int) $number, $locale ?: $this->getLocale()), $parameters);
return strtr($this->selector->choose((string) $id, $number, $locale ?: $this->getLocale()), $parameters);
}

return $this->doTransChoice($id, $number, $parameters, $domain, $locale);
}

private function getPluralizationRule(int $number, string $locale): int
{
return PluralizationRules::get($number, $locale, false);
}
}
110 changes: 110 additions & 0 deletions src/Symfony/Component/Translation/LegacyTranslatorTrait.php
F987
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Translation;

use Symfony\Component\Translation\Exception\InvalidArgumentException;

/**
* @author Tobias Nyholm <tobias.nyholm@gmail.com>
* @author Fabien Potencier <fabien@symfony.com>
*
* @deprecated since Symfony 4.2, use IdentityTranslator instead
*/
trait LegacyTranslatorTrait
{
/**
* Implementation of Symfony\Component\Translation\TranslationInterface::transChoice
* {@inheritdoc}
*/
public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null)
{
@trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2, use the "trans()" method with intl formatted messages instead.', __METHOD__), E_USER_DEPRECATED);

$id = (string) $id;
$number = (float) $number;
$locale = (string) $locale ?: $this->getLocale();

$parts = array();
if (preg_match('/^\|++$/', $id)) {
$parts = explode('|', $id);
} elseif (preg_match_all('/(?:\|\||[^\|])++/', $id, $matches)) {
$parts = $matches[0];
}

$intervalRegexp = <<<'EOF'
/^(?P<interval>
({\s*
(\-?\d+(\.\d+)?[\s*,\s*\-?\d+(\.\d+)?]*)
\s*})

|

(?P<left_delimiter>[\[\]])
\s*
(?P<left>-Inf|\-?\d+(\.\d+)?)
\s*,\s*
(?P<right>\+?Inf|\-?\d+(\.\d+)?)
\s*
(?P<right_delimiter>[\[\]])
)\s*(?P<message>.*?)$/xs
EOF;

$standardRules = array();
foreach ($parts as $part) {
$part = trim(str_replace('||', '|', $part));

// try to match an explicit rule, then fallback to the standard ones
if (preg_match($intervalRegexp, $part, $matches)) {
if ($matches[2]) {
foreach (explode(',', $matches[3]) as $n) {
if ($number == $n) {
return strtr($matches['message'], $parameters);
}
}
} else {
$leftNumber = '-Inf' === $matches['left'] ? -INF : (float) $matches['left'];
$rightNumber = \is_numeric($matches['right']) ? (float) $matches['right'] : INF;

if (('[' === $matches['left_delimiter'] ? $number >= $leftNumber : $number > $leftNumber)
&& (']' === $matches['right_delimiter'] ? $number <= $rightNumber : $number < $rightNumber)
) {
return strtr($matches['message'], $parameters);
}
}
} elseif (preg_match('/^\w+\:\s*(.*?)$/', $part, $matches)) {
$standardRules[] = $matches[1];
} else {
$standardRules[] = $part;
}
}

$position = PluralizationRules::get($number, $locale, false);

if (!isset($standardRules[$position])) {
// when there's exactly one rule given, and that rule is a standard
// rule, use this rule
if (1 === \count($parts) && isset($standardRules[0])) {
return strtr($standardRules[0], $parameters);
}

$message = sprintf('Unable to choose a translation for "%s" with locale "%s" for value "%d". Double check that this translation has the correct plural options (e.g. "There is one apple|There are %%count%% apples").', $id, $locale, $number);

if (\class_exists(InvalidArgumentException::class)) {
throw new InvalidArgumentException($message);
}

throw new \InvalidArgumentException($message);
}

return strtr($standardRules[$position], $parameters);
}
}
11 changes: 10 additions & 1 deletion src/Symfony/Component/Translation/LoggingTranslator.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
*/
class LoggingTranslator implements LegacyTranslatorInterface, TranslatorBagInterface
{
use LegacyTranslatorTrait {
transChoice as private doTransChoice;
}

/**
* @var TranslatorInterface|TranslatorBagInterface
*/
Expand Down Expand Up @@ -58,7 +62,12 @@ public function trans($id, array $parameters = array(), $domain = null, $locale
*/
public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null)
{
$trans = $this->translator->transChoice($id, $number, $parameters, $domain, $locale);
if ($this->translator instanceof LegacyTranslatorInterface) {
$trans = $this->translator->transChoice($id, $number, $parameters, $domain, $locale);
} else {
$trans = $this->doTransChoice($id, $number, $parameters, $domain, $locale);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct to use the trait here and in the DataCollector?

}

$this->log($id, $domain, $locale);

return $trans;
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Component/Translation/MessageSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ class MessageSelector
* The two methods can also be mixed:
* {0} There are no apples|one: There is one apple|more: There are %count% apples
*
* @param string $message The message being translated
* @param int $number The number of items represented for the message
* @param string $locale The locale to use for choosing
* @param string $message The message being translated
* @param int|float $number The number of items represented for the message
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually support floats.

* @param string $locale The locale to use for choosing
*
* @return string
*
Expand Down
Loading
0