Skip to content

[Translation] deprecate passing a null locale #32415

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/Symfony/Component/Translation/MessageCatalogue.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterf
*/
public function __construct(?string $locale, array $messages = [])
{
if (null === $locale) {
@trigger_error(sprintf('Passing "null" to the first argument of the "%s" method has been deprecated since Symfony 4.4 and will throw an error in 5.0.', __METHOD__), E_USER_DEPRECATED);
}

$this->locale = $locale;
$this->messages = $messages;
}
Expand Down
11 changes: 11 additions & 0 deletions src/Symfony/Component/Translation/Tests/MessageCatalogueTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ public function testGetLocale()
$this->assertEquals('en', $catalogue->getLocale());
}

/**
* @group legacy
* @expectedDeprecation Passing "null" to the first argument of the "Symfony\Component\Translation\MessageCatalogue::__construct" method has been deprecated since Symfony 4.4 and will throw an error in 5.0.
*/
public function testGetNullLocale()
{
$catalogue = new MessageCatalogue(null);

$this->assertNull($catalogue->getLocale());
}

public function testGetDomains()
{
$catalogue = new MessageCatalogue('en', ['domain1' => [], 'domain2' => [], 'domain2+intl-icu' => [], 'domain3+intl-icu' => []]);
Expand Down
29 changes: 28 additions & 1 deletion src/Symfony/Component/Translation/Tests/TranslatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,25 @@ public function testAddResourceInvalidLocales($locale)
*/
public function testAddResourceValidLocales($locale)
{
if (null === $locale) {
$this->markTestSkipped('null is not a valid locale');
Copy link
Member

Choose a reason for hiding this comment

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

why not remove null from getValidLocalesTests instead? this would look legit to me.

Copy link
Contributor Author

@Simperfit Simperfit Jul 8, 2019

Choose a reason for hiding this comment

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

because of #32415 (comment) I don't know it seems that it used in places that can be null

EDIT: Just checked it's used

}
$translator = new Translator('fr');
$translator->addResource('array', ['foo' => 'foofoo'], $locale);
// no assertion. this method just asserts that no exception is thrown
$this->addToAssertionCount(1);
}

/**
* @group legacy
* @expectedDeprecation Passing "null" to the third argument of the "Symfony\Component\Translation\Translator::addResource" method has been deprecated since Symfony 4.4 and will throw an error in 5.0.
*/
public function testAddResourceNull()
{
$translator = new Translator('fr');
$translator->addResource('array', ['foo' => 'foofoo'], null);
}

public function testAddResourceAfterTrans()
{
$translator = new Translator('fr');
Expand Down Expand Up @@ -367,10 +380,13 @@ public function testTransInvalidLocale($locale)
}

/**
* @dataProvider getValidLocalesTests
* @dataProvider getValidLocalesTests
*/
public function testTransValidLocale($locale)
{
if (null === $locale) {
$this->markTestSkipped('null is not a valid locale');
}
$translator = new Translator($locale);
$translator->addLoader('array', new ArrayLoader());
$translator->addResource('array', ['test' => 'OK'], $locale);
Expand All @@ -379,6 +395,17 @@ public function testTransValidLocale($locale)
$this->assertEquals('OK', $translator->trans('test', [], null, $locale));
}

/**
* @group legacy
* @expectedDeprecation Passing "null" to the third argument of the "Symfony\Component\Translation\Translator::addResource" method has been deprecated since Symfony 4.4 and will throw an error in 5.0.
*/
public function testTransNullLocale()
{
$translator = new Translator(null);
$translator->addLoader('array', new ArrayLoader());
$translator->addResource('array', ['test' => 'OK'], null);
}

/**
* @dataProvider getFlattenedTransTests
*/
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/Translation/Translator.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ public function addResource($format, $resource, $locale, $domain = null)
$domain = 'messages';
}

if (null === $locale) {
@trigger_error(sprintf('Passing "null" to the third argument of the "%s" method has been deprecated since Symfony 4.4 and will throw an error in 5.0.', __METHOD__), E_USER_DEPRECATED);
}

$this->assertValidLocale($locale);

$this->resources[$locale][] = [$format, $resource, $domain];
Expand Down