Skip to content

Commit

Permalink
[Translator] fix fallback to Locale::getDefault()
Browse files Browse the repository at this point in the history
  • Loading branch information
@nicolas-grekas
nicolas-grekas committed Jul 15, 2021
1 parent 5f70730 commit c1cca3e
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 6 deletions.
4 changes: 2 additions & 2 deletions Tests/TranslatorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function testConstructorValidLocale($locale)
{
$translator = new Translator($locale);

$this->assertSame($locale, $translator->getLocale());
$this->assertSame($locale ?: (class_exists(\Locale::class) ? \Locale::getDefault() : 'en'), $translator->getLocale());
}

/**
Expand Down Expand Up @@ -91,7 +91,7 @@ public function testSetValidLocale($locale)
$translator = new Translator($locale);
$translator->setLocale($locale);

$this->assertEquals($locale, $translator->getLocale());
$this->assertEquals($locale ?: (class_exists(\Locale::class) ? \Locale::getDefault() : 'en'), $translator->getLocale());
}

/**
Expand Down
9 changes: 5 additions & 4 deletions Translator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public function addResource($format, $resource, $locale, $domain = null)
}

$this->assertValidLocale($locale);
$locale ?: $locale = class_exists(\Locale::class) ? \Locale::getDefault() : 'en';

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

Expand All @@ -163,15 +164,15 @@ public function setLocale($locale)
}

$this->assertValidLocale($locale);
$this->locale = $locale ?? (class_exists(\Locale::class) ? \Locale::getDefault() : 'en');
$this->locale = $locale;
}

/**
* {@inheritdoc}
*/
public function getLocale()
{
return $this->locale;
return $this->locale ?: (class_exists(\Locale::class) ? \Locale::getDefault() : 'en');
}

/**
Expand Down Expand Up @@ -281,7 +282,7 @@ public function transChoice($id, $number, array $parameters = [], $domain = null
*/
public function getCatalogue($locale = null)
{
if (null === $locale) {
if (!$locale) {
$locale = $this->getLocale();
} else {
$this->assertValidLocale($locale);
Expand Down Expand Up @@ -505,7 +506,7 @@ protected function computeFallbackLocales($locale)
*/
protected function assertValidLocale($locale)
{
if (null !== $locale && 1 !== preg_match('/^[a-z0-9@_\\.\\-]*$/i', $locale)) {
if (!preg_match('/^[a-z0-9@_\\.\\-]*$/i', (string) $locale)) {
throw new InvalidArgumentException(sprintf('Invalid "%s" locale.', $locale));
}
}
Expand Down

3 comments on commit c1cca3e

@LukeTowers
Copy link

Choose a reason for hiding this comment

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

This broke usage of Carbon on systems that don't have the Locale class present for some reason, still investigating. @briannesbitt @nicolas-grekas

@LukeTowers
Copy link

Choose a reason for hiding this comment

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

Specifically getLocale() returning 'en' instead of null on environments without php-intl installed and enabled (thus the Locale class is not present). An environment that did have it installed and enabled still worked fine, but was reporting the default locale as 'en_US_POSIX'

@LukeTowers
Copy link

Choose a reason for hiding this comment

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

The issue stems from these lines in Carbon here: https://github.com/briannesbitt/Carbon/blob/dad1ee0c17/src/Carbon/Translator.php#L329-L331 which was added in briannesbitt/Carbon@c4806c9#diff-e10ba6a836bd0ec56e856e73ba2145575d9561b3ac40f734b6dc4bca918440d2R254-R256 to prevent unnecessary reloading of the current locale. However, this commit breaks that logic because on the initial load of the Translator instead of $this->getLocale() returning null (and thus failing that check) it now returns 'en' which matches the default locale being initialized by Carbon on some environments causing the first load of the localization to not occur correctly.

Please sign in to comment.