Skip to content
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
22 changes: 22 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,28 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## 0.3.3 - 2022-01-14

### Added

- Nothing.

### Changed

- Nothing.

### Deprecated

- Nothing.

### Removed

- Nothing.

### Fixed

- Normalize the locale file name before searching for it in `MessageLoader`, to account for differences in case, as well as filesystem case sensitivity (e.g. "en-XB" vs. "en_xb")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Did you mean (e.g. "en-XB" vs. "en-xb") (both hiphens)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I meant en_xb. Even if someone names their locale files EN_XB.json, EN-XB.json, EN-xb.json, etc., the locale en-XB should match.


## 0.3.2 - 2021-12-17

### Added
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,13 @@
*/
class DescriptorCollectorVisitor extends NodeVisitorAbstract
{
public ParserErrorCollection $errors;

private DescriptorCollection $descriptors;
private string $filePath;
private bool $preserveWhitespace;
private IdInterpolator $idInterpolator;
private string $idInterpolatorPattern;
private ParserErrorCollection $errors;
private bool $addGeneratedIdsToSourceCode;

/**
Expand Down
8 changes: 7 additions & 1 deletion src/Extractor/Parser/Descriptor/PragmaCollectorVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,15 @@
*/
class PragmaCollectorVisitor extends NodeVisitorAbstract
{
public ParserErrorCollection $errors;

/**
* @var array<string, string>
*/
private array $parsedPragma = [];

private string $filePath;
private ?string $pragmaName;
private ParserErrorCollection $errors;

public function __construct(string $filePath, string $pragmaName, ParserErrorCollection $errors)
{
Expand Down Expand Up @@ -132,6 +133,11 @@ private function parseMetadata(string $metadata): void

preg_match_all('/(([a-z0-9_\-]+):([a-z0-9_\-]+))+/i', $metadata, $matches);

/**
* @psalm-suppress UnnecessaryVarAnnotation
* @var int $index
* @var string $propertyName
*/
foreach ($matches[2] as $index => $propertyName) {
$compareParsed .= preg_replace('/\s+/', '', strtolower("$propertyName:{$matches[3][$index]}"));
$this->parsedPragma[$propertyName] = $matches[3][$index];
Expand Down
2 changes: 1 addition & 1 deletion src/FormatPHP.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function formatMessage(array $descriptor, array $values = []): string
} catch (UnableToGenerateMessageIdException $exception) {
throw new InvalidArgumentException(
'The message descriptor must have an ID or default message',
is_int($exception->getCode()) ? $exception->getCode() : 0,
is_int($exception->getCode()) ? $exception->getCode() : 0, // @phpstan-ignore-line
$exception,
);
}
Expand Down
28 changes: 26 additions & 2 deletions src/Icu/MessageFormat/Parser/DateTimeSkeletonParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private function setOption(string $skeletonOption, DateTimeFormatOptions $option
'"e..eee" (weekday) patterns are not supported',
);
}
$options->weekday = ['short', 'long', 'narrow', 'short'][$length - 4];
$options->weekday = $this->getWeekdayValue($length - 4);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This fixes a static analysis issue that Psalm raised in CI.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Okay because I don't understand what a narrow weekday is :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These values are the ECMA-402 names that translate to ICU formatting symbols. "Narrow" basically means the shortest possible display. So, while long is "Friday" and short is "Fri," narrow is "F"


break;
case 'c':
Expand All @@ -127,7 +127,7 @@ private function setOption(string $skeletonOption, DateTimeFormatOptions $option
'"c..ccc" (weekday) patterns are not supported',
);
}
$options->weekday = ['short', 'long', 'narrow', 'short'][$length - 4];
$options->weekday = $this->getWeekdayValue($length - 4);

break;
// Period
Expand Down Expand Up @@ -202,4 +202,28 @@ private function setOption(string $skeletonOption, DateTimeFormatOptions $option
);
}
}

/**
* @psalm-return "long" | "narrow" | "short"
*/
private function getWeekdayValue(int $index): string
{
switch ($index) {
case 1:
$value = 'long';

break;
case 2:
$value = 'narrow';

break;
case 0:
default:
$value = 'short';

break;
}

return $value;
}
}
41 changes: 38 additions & 3 deletions src/MessageLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,16 @@
use function array_filter;
use function array_unique;
use function array_values;
use function file_exists;
use function implode;
use function is_callable;
use function scandir;
use function sprintf;
use function str_replace;
use function strtolower;

use const DIRECTORY_SEPARATOR;
use const SCANDIR_SORT_NONE;

/**
* Loads messages for a given locale from the file system or cache
Expand All @@ -48,6 +53,8 @@
*/
class MessageLoader
{
private const MESSAGE_FILE_EXTENSION = '.json';

private ConfigInterface $config;
private FileSystemHelper $fileSystemHelper;
private FormatHelper $formatHelper;
Expand Down Expand Up @@ -108,8 +115,9 @@ private function getLocaleMessages(): array

foreach ($this->getFallbackLocales() as $locale) {
try {
$messagesFile = $this->messagesDirectory . DIRECTORY_SEPARATOR . $locale . '.json';
$messagesContents = $this->fileSystemHelper->getJsonContents($messagesFile);
$messagesContents = $this->fileSystemHelper->getJsonContents(
$this->getFilePathForLocale($locale),
);

break;
} catch (UnableToProcessFileException $exception) {
Expand All @@ -119,8 +127,9 @@ private function getLocaleMessages(): array

if ($messagesContents === false) {
throw new LocaleNotFoundException(sprintf(
'Unable to find a suitable locale for "%s"; please set a default locale',
'Unable to find a suitable locale for "%s" in %s; please set a default locale',
$this->config->getLocale()->toString(),
$this->messagesDirectory,
));
}

Expand Down Expand Up @@ -171,4 +180,30 @@ private function loadFormatReader($formatReader): callable

return $this->formatHelper->getReader($formatReader);
}

private function getFilePathForLocale(string $locale): string
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This method fixes the case-sensitivity problem.

{
$filePath = $this->messagesDirectory . DIRECTORY_SEPARATOR . $locale . self::MESSAGE_FILE_EXTENSION;
if (file_exists($filePath)) {
return $filePath;
}

// If the file doesn't exist, check for alternate casings and notations.
// e.g., en-XB, en_XB, en-xb, en_xb, EN-XB, EN_XB, eN-xB, etc.
$normalize = fn (string $filename): string => str_replace('_', '-', strtolower($filename));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

$searchFile = $normalize($locale . self::MESSAGE_FILE_EXTENSION);
$localeFiles = scandir($this->messagesDirectory, SCANDIR_SORT_NONE) ?: [];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would it be worth putting in a short-circuit before the scandir in case the file can be found directly?

Just thinking of cases where messagesDirectory might have a bunch of files in it and everything is named (and cased) properly, so we can save the disk IO.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good call!


foreach ($localeFiles as $localeFile) {
if ($normalize($localeFile) === $searchFile) {
return $this->messagesDirectory . DIRECTORY_SEPARATOR . $localeFile;
}
}

throw new UnableToProcessFileException(sprintf(
'Could not find file for locale "%s" in %s',
$locale,
$this->messagesDirectory,
));
}
}
31 changes: 29 additions & 2 deletions tests/MessageLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,22 @@ public function testExceptionWhenUnableToFindSuitableLocale(): void
// Esperanto, Latin script, US region.
$locale = new Locale('eo-Latn-US');

$messagesDirectory = __DIR__ . '/fixtures/locales';

/** @var ReaderInterface $reader */
$reader = $this->mockery(ReaderInterface::class);

$loader = new MessageLoader(
__DIR__ . '/fixtures/locales',
$messagesDirectory,
new Config($locale),
$reader,
);

$this->expectException(LocaleNotFoundException::class);
$this->expectExceptionMessage('Unable to find a suitable locale for "eo-Latn-US"; please set a default locale');
$this->expectExceptionMessage(
'Unable to find a suitable locale for "eo-Latn-US" in ' . $messagesDirectory
. '; please set a default locale',
);

$loader->loadMessages();
}
Expand Down Expand Up @@ -144,4 +149,26 @@ public function provideCustomReader(): array
['customReader' => null],
];
}

public function testLoadMessagesNormalizesFilenames(): void
{
$locale = new Locale('en-XB');
$defaultLocale = new Locale('en');

$loader = new MessageLoader(
__DIR__ . '/fixtures/locales',
new Config($locale, $defaultLocale),
new FormatPHPReader(),
);

$collection = $loader->loadMessages();

$this->assertGreaterThanOrEqual(1, $collection->count());
$this->assertNotNull($collection['about.inspire']);
$this->assertInstanceOf(MessageInterface::class, $collection['about.inspire']);
$this->assertSame(
'[!! Ḁṭ Ṡǩíííĺĺśśśḫâŕŕŕè, ẘè èṁṗṗṗŏẘèèèŕ ṁṁṁèṁḃḃḃèŕśśś ṭŏŏŏ ĝèèèṭ íííńśṗṗṗíŕèèèḋ. !!]',
$collection['about.inspire']->getMessage(),
);
}
}
14 changes: 14 additions & 0 deletions tests/fixtures/locales/en_xb.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"about.inspire": {
"defaultMessage": "[!! Ḁṭ Ṡǩíííĺĺśśśḫâŕŕŕè, ẘè èṁṗṗṗŏẘèèèŕ ṁṁṁèṁḃḃḃèŕśśś ṭŏŏŏ ĝèèèṭ íííńśṗṗṗíŕèèèḋ. !!]"
},
"how.many.pets": {
"defaultMessage": "[!! Ļâśśśṭ ṭṭṭíṁèèè Ḭ ćḫèèèćǩèèèḋ, !!]{gender, select, male{<italicized>[!! ḫè !!]</italicized>[!! ḫâââḋ !!]} female{<italicized>[!! śḫèèè !!]</italicized>[!! ḫâââḋ !!]} other{<italicized>[!! ṭḫèèèẏ !!]</italicized>[!! ḫâââḋ !!]}}[!! !!]{petCount, plural, =0{<bold>[!! ńŏ !!]</bold>[!! ṗèèèṭś !!]} =1{<bold>[!! â !!]</bold>[!! ṗèèèṭ !!]} other{<bold>#</bold>[!! ṗèèèṭś !!]}}[!! . !!]"
},
"start.with.tag": {
"defaultMessage": "<foo>{argument}</foo>"
},
"start.with.argument": {
"defaultMessage": "{argument}"
}
}