Skip to content

Commit

Permalink
bug #29461 [Contracts] extract LocaleAwareInterface out of Translator…
Browse files Browse the repository at this point in the history
…Interface (nicolas-grekas)

This PR was merged into the 4.2 branch.

Discussion
----------

[Contracts] extract LocaleAwareInterface out of TranslatorInterface

| Q             | A
| ------------- | ---
| Branch?       | 4.2
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

While reviewing #28929, I realized we have a design issue in the Translation contract: we missed segregating `setLocale()`/`getLocale()` out of `TranslatorInterface`. E.g. when people type-hint for `TranslatorInterface`, they *should not* be auto-suggested with the `setLocale()` mutator.

Technically, that's a BC break. I think we should do it. The release is close enough in time and that will save us from maintenance issues this will create in the future otherwise.

Commits
-------

73e4a1a [Contracts] extract LocaleAwareInterface out of TranslatorInterface
  • Loading branch information
nicolas-grekas committed Dec 6, 2018
2 parents b932300 + 73e4a1a commit e2ae5d9
Show file tree
Hide file tree
Showing 20 changed files with 63 additions and 74 deletions.
2 changes: 2 additions & 0 deletions UPGRADE-4.2.md
Expand Up @@ -68,6 +68,7 @@ Finder
Form
----

* The `symfony/translation` dependency has been removed - run `composer require symfony/translation` if you need the component
* The `getExtendedType()` method of the `FormTypeExtensionInterface` is deprecated and will be removed in 5.0. Type
extensions must implement the static `getExtendedTypes()` method instead and return an iterable of extended types.

Expand Down Expand Up @@ -381,6 +382,7 @@ TwigBundle
Validator
---------

* The `symfony/translation` dependency has been removed - run `composer require symfony/translation` if you need the component
* The `checkMX` and `checkHost` options of the `Email` constraint are deprecated
* The component is now decoupled from `symfony/translation` and uses `Symfony\Contracts\Translation\TranslatorInterface` instead
* The `ValidatorBuilderInterface` has been deprecated and `ValidatorBuilder` made final
Expand Down
Expand Up @@ -19,17 +19,4 @@ public function trans($id, array $parameters = array(), $domain = null, $locale
{
return '[trans]'.$id.'[/trans]';
}

public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null)
{
return '[trans]'.$id.'[/trans]';
}

public function setLocale($locale)
{
}

public function getLocale()
{
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/composer.json
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.1.3",
"symfony/contracts": "^1.0",
"symfony/contracts": "^1.0.2",
"twig/twig": "^1.35|^2.4.4"
},
"require-dev": {
Expand Down
Expand Up @@ -111,16 +111,4 @@ class TranslatorWithTranslatorBag implements TranslatorInterface
public function trans($id, array $parameters = array(), $domain = null, $locale = null)
{
}

public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null)
{
}

public function setLocale($locale)
{
}

public function getLocale()
{
}
}
Expand Up @@ -19,17 +19,4 @@ public function trans($id, array $parameters = array(), $domain = null, $locale
{
return '[trans]'.$id.'[/trans]';
}

public function transChoice($id, $number, array $parameters = array(), $domain = null, $locale = null)
{
return '[trans]'.$id.'[/trans]';
}

public function setLocale($locale)
{
}

public function getLocale()
{
}
}
3 changes: 2 additions & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Expand Up @@ -19,8 +19,9 @@
"php": "^7.1.3",
"ext-xml": "*",
"symfony/cache": "~4.2",
"symfony/dependency-injection": "^4.2",
"symfony/config": "~4.2",
"symfony/contracts": "^1.0.2",
"symfony/dependency-injection": "^4.2",
"symfony/event-dispatcher": "^4.1",
"symfony/http-foundation": "^4.1.2",
"symfony/http-kernel": "^4.2",
Expand Down
Expand Up @@ -17,8 +17,8 @@
use Symfony\Component\HttpKernel\Event\FinishRequestEvent;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Component\Translation\TranslatorInterface;
use Symfony\Contracts\Translation\LocaleAwareInterface;

/**
* Synchronizes the locale between the request and the translator.
Expand All @@ -31,12 +31,12 @@ class TranslatorListener implements EventSubscriberInterface
private $requestStack;

/**
* @param TranslatorInterface $translator
* @param LocaleAwareInterface $translator
*/
public function __construct($translator, RequestStack $requestStack)
{
if (!$translator instanceof LegacyTranslatorInterface && !$translator instanceof TranslatorInterface) {
throw new \TypeError(sprintf('Argument 1 passed to %s() must be an instance of %s, %s given.', __METHOD__, TranslatorInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));
if (!$translator instanceof TranslatorInterface && !$translator instanceof LocaleAwareInterface) {
throw new \TypeError(sprintf('Argument 1 passed to %s() must be an instance of %s, %s given.', __METHOD__, LocaleAwareInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));
}
$this->translator = $translator;
$this->requestStack = $requestStack;
Expand Down
Expand Up @@ -17,7 +17,7 @@
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\EventListener\TranslatorListener;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Contracts\Translation\LocaleAwareInterface;

class TranslatorListenerTest extends TestCase
{
Expand All @@ -27,7 +27,7 @@ class TranslatorListenerTest extends TestCase

protected function setUp()
{
$this->translator = $this->getMockBuilder(TranslatorInterface::class)->getMock();
$this->translator = $this->getMockBuilder(LocaleAwareInterface::class)->getMock();
$this->requestStack = $this->getMockBuilder('Symfony\Component\HttpFoundation\RequestStack')->getMock();
$this->listener = new TranslatorListener($this->translator, $this->requestStack);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpKernel/composer.json
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.1.3",
"symfony/contracts": "^1.0",
"symfony/contracts": "^1.0.2",
"symfony/event-dispatcher": "~4.1",
"symfony/http-foundation": "^4.1.1",
"symfony/debug": "~3.4|~4.0",
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/Translation/DataCollectorTranslator.php
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Translation\Exception\InvalidArgumentException;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\LocaleAwareInterface;
use Symfony\Contracts\Translation\TranslatorInterface;

/**
Expand All @@ -39,8 +40,8 @@ public function __construct($translator)
if (!$translator instanceof LegacyTranslatorInterface && !$translator instanceof TranslatorInterface) {
throw new \TypeError(sprintf('Argument 1 passed to %s() must be an instance of %s, %s given.', __METHOD__, TranslatorInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));
}
if (!$translator instanceof TranslatorBagInterface) {
throw new InvalidArgumentException(sprintf('The Translator "%s" must implement TranslatorInterface and TranslatorBagInterface.', \get_class($translator)));
if (!$translator instanceof TranslatorBagInterface || !$translator instanceof LocaleAwareInterface) {
throw new InvalidArgumentException(sprintf('The Translator "%s" must implement TranslatorInterface, TranslatorBagInterface and LocaleAwareInterface.', \get_class($translator)));
}

$this->translator = $translator;
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Component/Translation/LoggingTranslator.php
Expand Up @@ -14,6 +14,7 @@
use Psr\Log\LoggerInterface;
use Symfony\Component\Translation\Exception\InvalidArgumentException;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\LocaleAwareInterface;
use Symfony\Contracts\Translation\TranslatorInterface;

/**
Expand All @@ -37,8 +38,8 @@ public function __construct($translator, LoggerInterface $logger)
if (!$translator instanceof LegacyTranslatorInterface && !$translator instanceof TranslatorInterface) {
throw new \TypeError(sprintf('Argument 1 passed to %s() must be an instance of %s, %s given.', __METHOD__, TranslatorInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));
}
if (!$translator instanceof TranslatorBagInterface) {
throw new InvalidArgumentException(sprintf('The Translator "%s" must implement TranslatorInterface and TranslatorBagInterface.', \get_class($translator)));
if (!$translator instanceof TranslatorBagInterface || !$translator instanceof LocaleAwareInterface) {
throw new InvalidArgumentException(sprintf('The Translator "%s" must implement TranslatorInterface, TranslatorBagInterface and LocaleAwareInterface.', \get_class($translator)));
}

$this->translator = $translator;
Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Translation/TranslatorInterface.php
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Translation;

use Symfony\Component\Translation\Exception\InvalidArgumentException;
use Symfony\Contracts\Translation\LocaleAwareInterface;

/**
* TranslatorInterface.
Expand All @@ -20,7 +21,7 @@
*
* @deprecated since Symfony 4.2, use Symfony\Contracts\Translation\TranslatorInterface instead
*/
interface TranslatorInterface
interface TranslatorInterface extends LocaleAwareInterface
{
/**
* Translates the given message.
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Translation/composer.json
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.1.3",
"symfony/contracts": "^1.0",
"symfony/contracts": "^1.0.2",
"symfony/polyfill-mbstring": "~1.0"
},
"require-dev": {
Expand Down
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Validator\DependencyInjection\AddValidatorInitializersPass;
use Symfony\Component\Validator\Util\LegacyTranslatorProxy;
use Symfony\Contracts\Translation\LocaleAwareInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Contracts\Translation\TranslatorTrait;

Expand Down Expand Up @@ -72,7 +73,7 @@ public function testLegacyTranslatorProxy()
}
}

class TestTranslator implements TranslatorInterface
class TestTranslator implements TranslatorInterface, LocaleAwareInterface
{
use TranslatorTrait;
}
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\Validator\Util;

use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\LocaleAwareInterface;
use Symfony\Contracts\Translation\TranslatorInterface;

/**
Expand All @@ -23,6 +24,9 @@ class LegacyTranslatorProxy implements LegacyTranslatorInterface, TranslatorInte

public function __construct(TranslatorInterface $translator)
{
if (!$translator instanceof LocaleAwareInterface) {
throw new \InvalidArgumentException(sprintf('The translator passed to "%s()" must implement "%s".', __METHOD__, LocaleAwareInterface::class));
}
$this->translator = $translator;
}

Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/Validator/ValidatorBuilder.php
Expand Up @@ -30,6 +30,7 @@
use Symfony\Component\Validator\Mapping\Loader\YamlFileLoader;
use Symfony\Component\Validator\Util\LegacyTranslatorProxy;
use Symfony\Component\Validator\Validator\RecursiveValidator;
use Symfony\Contracts\Translation\LocaleAwareInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Contracts\Translation\TranslatorTrait;

Expand Down Expand Up @@ -332,7 +333,7 @@ public function getValidator()
$translator = $this->translator;

if (null === $translator) {
$translator = new class() implements TranslatorInterface {
$translator = new class() implements TranslatorInterface, LocaleAwareInterface {
use TranslatorTrait;
};
// Force the locale to be 'en' when no translator is provided rather than relying on the Intl default locale
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/composer.json
Expand Up @@ -17,7 +17,7 @@
],
"require": {
"php": "^7.1.3",
"symfony/contracts": "^1.0",
"symfony/contracts": "^1.0.2",
"symfony/polyfill-ctype": "~1.8",
"symfony/polyfill-mbstring": "~1.0"
},
Expand Down
31 changes: 31 additions & 0 deletions src/Symfony/Contracts/Translation/LocaleAwareInterface.php
@@ -0,0 +1,31 @@
<?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\Contracts\Translation;

interface LocaleAwareInterface
{
/**
* Sets the current locale.
*
* @param string $locale The locale
*
* @throws \InvalidArgumentException If the locale contains invalid characters
*/
public function setLocale($locale);

/**
* Returns the current locale.
*
* @return string The locale
*/
public function getLocale();
}
16 changes: 0 additions & 16 deletions src/Symfony/Contracts/Translation/TranslatorInterface.php
Expand Up @@ -62,20 +62,4 @@ interface TranslatorInterface
* @throws \InvalidArgumentException If the locale contains invalid characters
*/
public function trans($id, array $parameters = array(), $domain = null, $locale = null);

/**
* Sets the current locale.
*
* @param string $locale The locale
*
* @throws \InvalidArgumentException If the locale contains invalid characters
*/
public function setLocale($locale);

/**
* Returns the current locale.
*
* @return string The locale
*/
public function getLocale();
}
2 changes: 1 addition & 1 deletion src/Symfony/Contracts/Translation/TranslatorTrait.php
Expand Up @@ -14,7 +14,7 @@
use Symfony\Component\Translation\Exception\InvalidArgumentException;

/**
* A trait to help implement TranslatorInterface.
* A trait to help implement TranslatorInterface and LocaleAwareInterface.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
Expand Down

0 comments on commit e2ae5d9

Please sign in to comment.