From 157147cb8eaa1800a8473db547c6964a5205e83d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Mar 2021 20:09:42 +0200 Subject: [PATCH 1/8] "Symfony\Component\Translation\PluralizationRules" is deprecated Signed-off-by: Joas Schilling --- lib/private/L10N/L10N.php | 30 ++++++++++-------------------- lib/private/L10N/L10NString.php | 33 +++++++++++++-------------------- 2 files changed, 23 insertions(+), 40 deletions(-) diff --git a/lib/private/L10N/L10N.php b/lib/private/L10N/L10N.php index 3b32ec2827..72f124f775 100644 --- a/lib/private/L10N/L10N.php +++ b/lib/private/L10N/L10N.php @@ -32,7 +32,7 @@ namespace OC\L10N; use OCP\IL10N; use OCP\L10N\IFactory; use Punic\Calendar; -use Symfony\Component\Translation\PluralizationRules; +use Symfony\Component\Translation\IdentityTranslator; class L10N implements IL10N { @@ -48,11 +48,8 @@ class L10N implements IL10N { /** @var string Locale of this object */ protected $locale; - /** @var string Plural forms (string) */ - private $pluralFormString = 'nplurals=2; plural=(n != 1);'; - - /** @var string Plural forms (function) */ - private $pluralFormFunction = null; + /** @var IdentityTranslator */ + private $identityTranslator; /** @var string[] */ private $translations = []; @@ -214,20 +211,16 @@ class L10N implements IL10N { } /** - * Returnsed function accepts the argument $n - * - * Called by \OC_L10N_String - * @return \Closure the plural form function + * @internal + * @return IdentityTranslator */ - public function getPluralFormFunction(): \Closure { - if (\is_null($this->pluralFormFunction)) { - $lang = $this->getLanguageCode(); - $this->pluralFormFunction = function ($n) use ($lang) { - return PluralizationRules::get($n, $lang); - }; + public function getIdentityTranslator(): IdentityTranslator { + if (\is_null($this->identityTranslator)) { + $this->identityTranslator = new IdentityTranslator(); + $this->identityTranslator->setLocale($this->getLocaleCode()); } - return $this->pluralFormFunction; + return $this->identityTranslator; } /** @@ -242,9 +235,6 @@ class L10N implements IL10N { return false; } - if (!empty($json['pluralForm'])) { - $this->pluralFormString = $json['pluralForm']; - } $this->translations = array_merge($this->translations, $json['translations']); return true; } diff --git a/lib/private/L10N/L10NString.php b/lib/private/L10N/L10NString.php index a82228189b..0eadadf9be 100644 --- a/lib/private/L10N/L10NString.php +++ b/lib/private/L10N/L10NString.php @@ -32,7 +32,7 @@ namespace OC\L10N; class L10NString implements \JsonSerializable { - /** @var \OC\L10N\L10N */ + /** @var L10N */ protected $l10n; /** @var string */ @@ -45,38 +45,31 @@ class L10NString implements \JsonSerializable { protected $count; /** - * @param \OC\L10N\L10N $l10n + * @param L10N $l10n * @param string|string[] $text * @param array $parameters * @param int $count */ - public function __construct(\OC\L10N\L10N $l10n, $text, $parameters, $count = 1) { + public function __construct(L10N $l10n, $text, array $parameters, int $count = 1) { $this->l10n = $l10n; $this->text = $text; $this->parameters = $parameters; $this->count = $count; } - /** - * @return string - */ - public function __toString() { + public function __toString(): string { $translations = $this->l10n->getTranslations(); + $identityTranslator = $this->l10n->getIdentityTranslator(); - $text = $this->text; - if (array_key_exists($this->text, $translations)) { - if (is_array($translations[$this->text])) { - $fn = $this->l10n->getPluralFormFunction(); - $id = $fn($this->count); - $text = $translations[$this->text][$id]; - } else { - $text = $translations[$this->text]; - } - } + $parameters = $this->parameters; + // Add $count as %count% as per \Symfony\Contracts\Translation\TranslatorInterface + $parameters['%count%'] = $this->count; - // Replace %n first (won't interfere with vsprintf) - $text = str_replace('%n', (string)$this->count, $text); - return vsprintf($text, $this->parameters); + // Use the indexed version as per \Symfony\Contracts\Translation\TranslatorInterface + $identity = implode('|', $translations[$this->text]); + $identity = str_replace('%n', '%count%', $identity); + + return $identityTranslator->trans($identity, $parameters); } From a4c6749b020d5cb4e63a54b0aa9ca5ebe961ee4d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Mar 2021 20:19:41 +0200 Subject: [PATCH 2/8] Add a check for the pipe character Signed-off-by: Joas Schilling --- build/translation-checker.php | 5 +++++ lib/private/L10N/L10NString.php | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/build/translation-checker.php b/build/translation-checker.php index 1f7ec343af..2c3a7856d8 100644 --- a/build/translation-checker.php +++ b/build/translation-checker.php @@ -48,6 +48,11 @@ foreach ($directories as $dir) { $content = file_get_contents($file->getPathname()); $json = json_decode($content, true); + $translations = json_encode($json['translations']); + if (strpos($content, '|') !== false) { + $errors[] = $file->getPathname() . "\n" . ' ' . 'Contains a | in the translations' . "\n"; + } + if (json_last_error() !== JSON_ERROR_NONE) { $errors[] = $file->getPathname() . "\n" . ' ' . json_last_error_msg() . "\n"; } else { diff --git a/lib/private/L10N/L10NString.php b/lib/private/L10N/L10NString.php index 0eadadf9be..ae90f52a02 100644 --- a/lib/private/L10N/L10NString.php +++ b/lib/private/L10N/L10NString.php @@ -59,6 +59,12 @@ class L10NString implements \JsonSerializable { public function __toString(): string { $translations = $this->l10n->getTranslations(); + + $pipeCheck = implode('', $translations[$this->text]); + if (strpos($pipeCheck, '|') !== false) { + return 'Can not use pipe character in translations'; + } + $identityTranslator = $this->l10n->getIdentityTranslator(); $parameters = $this->parameters; From 439457d2b4cdc511fbce826764b2b27e30abee50 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 30 Mar 2021 21:04:41 +0200 Subject: [PATCH 3/8] Fix languages that miss a string in the translation Signed-off-by: Joas Schilling --- lib/private/L10N/L10NString.php | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/private/L10N/L10NString.php b/lib/private/L10N/L10NString.php index ae90f52a02..ef33babac5 100644 --- a/lib/private/L10N/L10NString.php +++ b/lib/private/L10N/L10NString.php @@ -59,12 +59,6 @@ class L10NString implements \JsonSerializable { public function __toString(): string { $translations = $this->l10n->getTranslations(); - - $pipeCheck = implode('', $translations[$this->text]); - if (strpos($pipeCheck, '|') !== false) { - return 'Can not use pipe character in translations'; - } - $identityTranslator = $this->l10n->getIdentityTranslator(); $parameters = $this->parameters; @@ -72,7 +66,22 @@ class L10NString implements \JsonSerializable { $parameters['%count%'] = $this->count; // Use the indexed version as per \Symfony\Contracts\Translation\TranslatorInterface - $identity = implode('|', $translations[$this->text]); + $identity = $this->text; + if (array_key_exists($this->text, $translations)) { + $identity = $translations[$this->text]; + } + + if (is_array($identity)) { + $pipeCheck = implode('', $identity); + if (strpos($pipeCheck, '|') !== false) { + return 'Can not use pipe character in translations'; + } + + $identity = implode('|', $identity); + } else if (strpos($identity, '|') !== false) { + return 'Can not use pipe character in translations'; + } + $identity = str_replace('%n', '%count%', $identity); return $identityTranslator->trans($identity, $parameters); From 5a514a9a419f40800e5dc70c33b9de9f33cf7a76 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 31 Mar 2021 09:54:23 +0200 Subject: [PATCH 4/8] Correctly replace all PHP placeholders with the parameters Signed-off-by: Joas Schilling --- lib/private/L10N/L10NString.php | 9 ++++----- tests/data/l10n/de.json | 5 ++++- tests/lib/L10N/L10nTest.php | 21 +++++++++++++++++++++ 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/private/L10N/L10NString.php b/lib/private/L10N/L10NString.php index ef33babac5..0722057de0 100644 --- a/lib/private/L10N/L10NString.php +++ b/lib/private/L10N/L10NString.php @@ -61,10 +61,6 @@ class L10NString implements \JsonSerializable { $translations = $this->l10n->getTranslations(); $identityTranslator = $this->l10n->getIdentityTranslator(); - $parameters = $this->parameters; - // Add $count as %count% as per \Symfony\Contracts\Translation\TranslatorInterface - $parameters['%count%'] = $this->count; - // Use the indexed version as per \Symfony\Contracts\Translation\TranslatorInterface $identity = $this->text; if (array_key_exists($this->text, $translations)) { @@ -84,7 +80,10 @@ class L10NString implements \JsonSerializable { $identity = str_replace('%n', '%count%', $identity); - return $identityTranslator->trans($identity, $parameters); + // $count as %count% as per \Symfony\Contracts\Translation\TranslatorInterface + $text = $identityTranslator->trans($identity, ['%count%' => $this->count]); + + return vsprintf($text, $this->parameters); } diff --git a/tests/data/l10n/de.json b/tests/data/l10n/de.json index c2b6f34c08..d9e75d303c 100644 --- a/tests/data/l10n/de.json +++ b/tests/data/l10n/de.json @@ -1,6 +1,9 @@ { "translations" : { - "_%n file_::_%n files_": ["%n Datei", "%n Dateien"] + "_%n file_::_%n files_": ["%n Datei", "%n Dateien"], + "Ordered placeholders one %s two %s": "Placeholder one %s two %s", + "Reordered placeholders one %s two %s": "Placeholder two %2$s one %1$s", + "Reordered placeholders one %1$s two %2$s": "Placeholder two %2$s one %1$s" }, "pluralForm" : "nplurals=2; plural=(n != 1);" } diff --git a/tests/lib/L10N/L10nTest.php b/tests/lib/L10N/L10nTest.php index 0de09386fb..3fb22b3f66 100644 --- a/tests/lib/L10N/L10nTest.php +++ b/tests/lib/L10N/L10nTest.php @@ -76,6 +76,27 @@ class L10nTest extends TestCase { $this->assertEquals('5 oken', (string)$l->n('%n window', '%n windows', 5)); } + public function dataPlaceholders(): array { + return [ + ['Ordered placeholders one %s two %s', 'Placeholder one 1 two 2'], + ['Reordered placeholders one %s two %s', 'Placeholder two 2 one 1'], + ['Reordered placeholders one %1$s two %2$s', 'Placeholder two 2 one 1'], + ]; + } + + /** + * @dataProvider dataPlaceholders + * + * @param $string + * @param $expected + */ + public function testPlaceholders($string, $expected): void { + $transFile = \OC::$SERVERROOT.'/tests/data/l10n/de.json'; + $l = new L10N($this->getFactory(), 'test', 'de', 'de_AT', [$transFile]); + + $this->assertEquals($expected, $l->t($string, ['1', '2'])); + } + public function localizationData() { return [ // timestamp as string From 2b8e47dcace5d329a874c0dfa9a30cfbc551ca80 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 31 Mar 2021 09:56:36 +0200 Subject: [PATCH 5/8] Correclty use plural for share exception Signed-off-by: Joas Schilling --- lib/private/Share20/Manager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 7211a4eb1e..00020c3a8f 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -441,7 +441,7 @@ class Manager implements IManager { $date->setTime(0, 0, 0); $date->add(new \DateInterval('P' . $defaultExpireDays . 'D')); if ($date < $expirationDate) { - $message = $this->l->t('Can’t set expiration date more than %s days in the future', [$defaultExpireDays]); + $message = $this->l->n('Can’t set expiration date more than %n day in the future', 'Can’t set expiration date more than %n days in the future', $defaultExpireDays); throw new GenericShareException($message, $message, 404); } } @@ -517,7 +517,7 @@ class Manager implements IManager { $date->setTime(0, 0, 0); $date->add(new \DateInterval('P' . $this->shareApiLinkDefaultExpireDays() . 'D')); if ($date < $expirationDate) { - $message = $this->l->t('Can’t set expiration date more than %s days in the future', [$this->shareApiLinkDefaultExpireDays()]); + $message = $this->l->n('Can’t set expiration date more than %n day in the future', 'Can’t set expiration date more than %n days in the future', $this->shareApiLinkDefaultExpireDays()); throw new GenericShareException($message, $message, 404); } } From 3cf447ac448afb4d12131a0b0393e130e2613ea0 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 31 Mar 2021 10:08:43 +0200 Subject: [PATCH 6/8] Fix PHP CS Signed-off-by: Joas Schilling --- lib/private/L10N/L10NString.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/L10N/L10NString.php b/lib/private/L10N/L10NString.php index 0722057de0..9d219ea177 100644 --- a/lib/private/L10N/L10NString.php +++ b/lib/private/L10N/L10NString.php @@ -74,7 +74,7 @@ class L10NString implements \JsonSerializable { } $identity = implode('|', $identity); - } else if (strpos($identity, '|') !== false) { + } elseif (strpos($identity, '|') !== false) { return 'Can not use pipe character in translations'; } From 2228b285c5bff8627669e966ff672a857a66f097 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 1 Apr 2021 11:12:00 +0200 Subject: [PATCH 7/8] Only check translations insteda of the full file Signed-off-by: Joas Schilling --- build/translation-checker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/translation-checker.php b/build/translation-checker.php index 2c3a7856d8..53c139a1d6 100644 --- a/build/translation-checker.php +++ b/build/translation-checker.php @@ -49,7 +49,7 @@ foreach ($directories as $dir) { $json = json_decode($content, true); $translations = json_encode($json['translations']); - if (strpos($content, '|') !== false) { + if (strpos($translations, '|') !== false) { $errors[] = $file->getPathname() . "\n" . ' ' . 'Contains a | in the translations' . "\n"; } From 79ebc7f24c9c908efb5b41422150bca9cf0b7966 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 14 Apr 2021 14:46:07 +0200 Subject: [PATCH 8/8] Fix test by defining plurals Signed-off-by: Joas Schilling --- tests/lib/Share20/ManagerTest.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index e17f179b60..a4dd621522 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -126,6 +126,10 @@ class ManagerTest extends \Test\TestCase { ->willReturnCallback(function ($text, $parameters = []) { return vsprintf($text, $parameters); }); + $this->l->method('n') + ->willReturnCallback(function ($singular, $plural, $count, $parameters = []) { + return vsprintf(str_replace('%n', $count, ($count === 1) ? $singular : $plural), $parameters); + }); $this->factory = new DummyFactory(\OC::$server); @@ -1957,7 +1961,7 @@ class ManagerTest extends \Test\TestCase { $data = []; // No exclude groups - $data[] = ['no', null, null, null, false]; + $data[] = ['no', null, null, [], false]; // empty exclude list, user no groups $data[] = ['yes', '', json_encode(['']), [], false];