From ad4c718391f74fa2b6f5097e06476da01ab1b359 Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Wed, 24 Apr 2024 16:52:08 +0200 Subject: [PATCH 1/4] Fixed string substitution for Plural Value Object when casting to string `\Ibexa\Contracts\Core\Repository\Values\Translation\Plural` had an incorrect casting to string implementation which resulted in the effect opposite to the desired one --- src/contracts/Repository/Values/Translation/Plural.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/contracts/Repository/Values/Translation/Plural.php b/src/contracts/Repository/Values/Translation/Plural.php index 73b30d1950..08ff2a744c 100644 --- a/src/contracts/Repository/Values/Translation/Plural.php +++ b/src/contracts/Repository/Values/Translation/Plural.php @@ -74,7 +74,9 @@ public function __construct($singular, $plural, array $values) */ public function __toString() { - return strtr(current($this->values) == 1 ? $this->plural : $this->singular, $this->values); + $firstValue = !empty($this->values) ? current(array_values($this->values)) : null; + + return strtr((int)$firstValue === 1 ? $this->singular : $this->plural, $this->values); } } From 1699474d577718bb2c92e7b81a989e1d08f7f579 Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Wed, 24 Apr 2024 16:58:21 +0200 Subject: [PATCH 2/4] [Tests] Added test coverage for Message and Plural Value Objects --- .../Values/Translation/MessageTest.php | 61 ++++++++++++++++++ .../Values/Translation/PluralTest.php | 63 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tests/lib/Repository/Values/Translation/MessageTest.php create mode 100644 tests/lib/Repository/Values/Translation/PluralTest.php diff --git a/tests/lib/Repository/Values/Translation/MessageTest.php b/tests/lib/Repository/Values/Translation/MessageTest.php new file mode 100644 index 0000000000..278847cb4b --- /dev/null +++ b/tests/lib/Repository/Values/Translation/MessageTest.php @@ -0,0 +1,61 @@ + + */ + public static function getDataForTestStringable(): iterable + { + yield 'message with substitution values' => [ + new Message( + 'Anna has some oranges in %object%', + [ + '%object%' => 'a basket', + ] + ), + 'Anna has some oranges in a basket', + ]; + + yield 'message with multiple substitution values' => [ + new Message( + '%first_name% has some data in %storage_type%', + [ + '%first_name%' => 'Anna', + '%storage_type%' => 'her database', + ] + ), + 'Anna has some data in her database', + ]; + + yield 'message with no substitution values' => [ + new Message( + 'This value is not correct', + [] + ), + 'This value is not correct', + ]; + } +} diff --git a/tests/lib/Repository/Values/Translation/PluralTest.php b/tests/lib/Repository/Values/Translation/PluralTest.php new file mode 100644 index 0000000000..d0d6e57f96 --- /dev/null +++ b/tests/lib/Repository/Values/Translation/PluralTest.php @@ -0,0 +1,63 @@ + + */ + public static function getDataForTestStringable(): iterable + { + yield 'singular form' => [ + new Plural( + 'John has %apple_count% apple', + 'John has %apple_count% apples', + [ + '%apple_count%' => 1, + ] + ), + 'John has 1 apple', + ]; + + yield 'plural form' => [ + new Plural( + 'John has %apple_count% apple', + 'John has %apple_count% apples', + [ + '%apple_count%' => 2, + ] + ), + 'John has 2 apples', + ]; + + yield 'no substitution values' => [ + new Plural( + 'John has some apples', + 'John has a lot of apples', + [] + ), + 'John has a lot of apples', + ]; + } +} From c2566e56be4ad0530cafa0261ff067ba4bb6d745 Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Wed, 24 Apr 2024 16:59:59 +0200 Subject: [PATCH 3/4] Added strict types to Message and Plural Value Objects --- .../Repository/Values/Translation/Message.php | 18 +++---- .../Repository/Values/Translation/Plural.php | 50 ++++++++----------- 2 files changed, 29 insertions(+), 39 deletions(-) diff --git a/src/contracts/Repository/Values/Translation/Message.php b/src/contracts/Repository/Values/Translation/Message.php index 6d1ec6e973..16fcd84552 100644 --- a/src/contracts/Repository/Values/Translation/Message.php +++ b/src/contracts/Repository/Values/Translation/Message.php @@ -19,34 +19,30 @@ class Message extends Translation /** * Message string. Might use replacements like %foo%, which are replaced by * the values specified in the values array. - * - * @var string */ - protected $message; + protected string $message; /** * Translation value objects. May not contain any numbers, which might * result in requiring plural forms. Use Plural for that. * - * @var array + * @var array */ - protected $values; + protected array $values; /** * Construct singular only message from string and optional value array. * - * @param string $message - * @param array $values + * @param array $values */ - public function __construct($message, array $values = []) + public function __construct(string $message, array $values = []) { $this->message = $message; $this->values = $values; + + parent::__construct(); } - /** - * {@inheritdoc} - */ public function __toString() { return strtr($this->message, $this->values); diff --git a/src/contracts/Repository/Values/Translation/Plural.php b/src/contracts/Repository/Values/Translation/Plural.php index 08ff2a744c..720c8f5347 100644 --- a/src/contracts/Repository/Values/Translation/Plural.php +++ b/src/contracts/Repository/Values/Translation/Plural.php @@ -11,67 +11,61 @@ /** * Class for translatable messages, which may contain plural forms. * - * The message might include replacements, in the form %[A-Za-z]%. Those are + * The message might include replacements, in the form %[A-Za-z]%. Those are * replaced by the values provided. A raw % can be escaped like %%. * * You need to provide a singular and plural variant for the string. The - * strings provided should be english and will be translated depending on the + * strings provided should be English and will be translated depending on the * environment language. * - * This interface follows the interfaces of XLiff, gettext, Symfony2 - * Translations and Zend_Translate. For singular forms you just provide a plain - * string (with optional placeholders without effects on the plural forms). For - * potential plural forms you always provide a singular variant and an english - * simple plural variant. No implementation supports multiple different plural - * forms in one single message. + * This interface follows the interfaces of XLIFF, gettext, Symfony Translations and Zend_Translate. + * For singular forms you just provide a plain string (with optional placeholders without effects on the plural forms). + * For potential plural forms you always provide a singular variant and an English simple plural variant. + * An instance of this class can be cast to a string. In such case whether to use singular or plural form is determined + * based on the value of first element of $values array (it needs to be 1 for singular, anything else for plural). + * If plurality cannot be inferred from $values, a plural form is assumed as default. To force singular form, + * use {@see \Ibexa\Contracts\Core\Repository\Values\Translation\Message} instead. * - * The singular / plural string could, for Symfony2, for example be converted - * to "$singular|$plural", and you would call gettext like: ngettext( - * $singular, $plural, $count ). + * No implementation supports multiple different plural forms in one single message. + * + * The singular / plural string could, for Symfony, for example be converted + * to "$singular|$plural", and you would call gettext like: ngettext($singular, $plural, $count ). */ class Plural extends Translation { /** * Singular string. Might use replacements like %foo%, which are replaced by * the values specified in the values array. - * - * @var string */ - protected $singular; + protected string $singular; /** * Message string. Might use replacements like %foo%, which are replaced by * the values specified in the values array. - * - * @var string */ - protected $plural; + protected string $plural; /** - * Translation value objects. May not contain any numbers, which might - * result in requiring plural forms. Use MessagePlural for that. + * Translation value objects. * - * @var array + * @var array */ - protected $values; + protected array $values; /** * Construct plural message from singular, plural and value array. * - * @param string $singular - * @param string $plural - * @param array $values + * @param array $values */ - public function __construct($singular, $plural, array $values) + public function __construct(string $singular, string $plural, array $values) { $this->singular = $singular; $this->plural = $plural; $this->values = $values; + + parent::__construct(); } - /** - * {@inheritdoc} - */ public function __toString() { $firstValue = !empty($this->values) ? current(array_values($this->values)) : null; From 1d8d800732e95bb9e55b6b0a73e2f6a46759cbeb Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Thu, 27 Jun 2024 15:58:54 +0200 Subject: [PATCH 4/4] [PHPStan] Aligned baseline with the changes --- phpstan-baseline.neon | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f5a64a5191..df24cda6eb 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -7410,26 +7410,6 @@ parameters: count: 1 path: src/contracts/Repository/Values/ObjectState/ObjectStateCreateStruct.php - - - message: "#^Method Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Translation\\\\Message\\:\\:__construct\\(\\) has parameter \\$values with no value type specified in iterable type array\\.$#" - count: 1 - path: src/contracts/Repository/Values/Translation/Message.php - - - - message: "#^Property Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Translation\\\\Message\\:\\:\\$values type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/contracts/Repository/Values/Translation/Message.php - - - - message: "#^Method Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Translation\\\\Plural\\:\\:__construct\\(\\) has parameter \\$values with no value type specified in iterable type array\\.$#" - count: 1 - path: src/contracts/Repository/Values/Translation/Plural.php - - - - message: "#^Property Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Translation\\\\Plural\\:\\:\\$values type has no value type specified in iterable type array\\.$#" - count: 1 - path: src/contracts/Repository/Values/Translation/Plural.php - - message: "#^Class Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\URL\\\\SearchResult implements generic interface IteratorAggregate but does not specify its types\\: TKey, TValue$#" count: 1