diff --git a/AGENTS.md b/AGENTS.md index 77a92bf0..56fa49ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -26,7 +26,7 @@ Repository: `Svaroh/JsFormValidatorBundle` (default branch: `main`) Last verified in the Nix shell on PHP 8.5.6 / Node 24.16.0: -- `composer test`: 81 tests, 250 assertions. +- `composer test`: 95 tests, 268 assertions. - `composer phpstan`: no errors. - `composer coverage`: PHP line coverage ~97%, threshold `80%`. - `npm run test:unit`: Jest 608 tests. diff --git a/Tests/Unit/PluralMessageTest.php b/Tests/Unit/PluralMessageTest.php index 6f9a4efa..07094ae6 100644 --- a/Tests/Unit/PluralMessageTest.php +++ b/Tests/Unit/PluralMessageTest.php @@ -4,6 +4,8 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; +use Psr\Log\LogLevel; use Svaroh\JsFormValidatorBundle\Factory\JsFormValidatorFactory; use Svaroh\JsFormValidatorBundle\Form\Extension\FormExtension; use Symfony\Component\Form\Extension\Core\Type\FormType; @@ -16,6 +18,7 @@ use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Validation; +use Symfony\Component\Validator\Validator\ValidatorInterface; /** * A pluralized message carries every form of the translation in one string. @@ -58,26 +61,122 @@ public function testChoosesTheUkrainianFormOfTheLimit(int $min, string $expected $this->assertSame($expected, $options['minMessage']); } + /** + * The catalogue is given here rather than taken from Symfony's own English + * source, so that a rewording of a constraint message upstream cannot fail + * this for a reason that has nothing to do with choosing a form. + */ public function testChoosesTheEnglishSingularAndPlural() { - $singular = $this->parseConstraint(new Assert\Length(max: 1), 'en'); - $plural = $this->parseConstraint(new Assert\Length(max: 10), 'en'); + $catalogue = array( + (new Assert\Length(max: 1))->maxMessage => + 'It should have {{ limit }} character or less.' + . '|It should have {{ limit }} characters or less.', + ); - $this->assertStringContainsString('character or less', $singular['maxMessage']); - $this->assertStringContainsString('characters or less', $plural['maxMessage']); - $this->assertStringNotContainsString('|', $singular['maxMessage']); + $singular = $this->parseConstraint(new Assert\Length(max: 1), 'en', $catalogue); + $plural = $this->parseConstraint(new Assert\Length(max: 10), 'en', $catalogue); + + $this->assertSame('It should have {{ limit }} character or less.', $singular['maxMessage']); + $this->assertSame('It should have {{ limit }} characters or less.', $plural['maxMessage']); } /** * Length falls back to exactMessage when min and max are equal, and picks - * its form by that same limit. + * its form by that same limit. The naming convention cannot reach this one: + * there is no "exact" option to read the limit from, so it is the table in + * the factory that answers for it. */ public function testPluralizesTheExactMessageByTheLimit() { - $options = $this->parseConstraint(new Assert\Length(min: 1, max: 1), 'en'); + $catalogue = array( + (new Assert\Length(min: 1, max: 1))->exactMessage => + 'It should have exactly {{ limit }} character.' + . '|It should have exactly {{ limit }} characters.', + ); + + $one = $this->parseConstraint(new Assert\Length(min: 1, max: 1), 'en', $catalogue); + $many = $this->parseConstraint(new Assert\Length(min: 7, max: 7), 'en', $catalogue); + + $this->assertSame('It should have exactly {{ limit }} character.', $one['exactMessage']); + $this->assertSame('It should have exactly {{ limit }} characters.', $many['exactMessage']); + } + + /** Count has an exactMessage of its own, chosen by the same limit. */ + public function testPluralizesTheExactMessageOfACollection() + { + $catalogue = array( + (new Assert\Count(min: 1, max: 1))->exactMessage => + 'Ця колекція повинна містити рівно {{ limit }} елемент.' + . '|Ця колекція повинна містити рівно {{ limit }} елемента.' + . '|Ця колекція повинна містити рівно {{ limit }} елементів.', + ); + + $options = $this->parseConstraint(new Assert\Count(min: 5, max: 5), 'uk', $catalogue); + + $this->assertSame('Ця колекція повинна містити рівно {{ limit }} елементів.', $options['exactMessage']); + } + + /** Both limits of a Choice are pluralized, each by its own option. */ + public function testPluralizesBothLimitsOfAChoice() + { + $reference = new Assert\Choice(choices: array('a'), multiple: true); + $catalogue = array( + $reference->minMessage => 'щонайменше {{ limit }} варіант' + . '|щонайменше {{ limit }} варіанти' + . '|щонайменше {{ limit }} варіантів', + $reference->maxMessage => 'щонайбільше {{ limit }} варіант' + . '|щонайбільше {{ limit }} варіанти' + . '|щонайбільше {{ limit }} варіантів', + ); + + $options = $this->parseConstraint( + new Assert\Choice(choices: array('a', 'b', 'c'), multiple: true, min: 3, max: 5), + 'uk', + $catalogue + ); + + $this->assertSame('щонайменше {{ limit }} варіанти', $options['minMessage']); + $this->assertSame('щонайбільше {{ limit }} варіантів', $options['maxMessage']); + } + + /** + * The one entry of the table that the naming convention could never + * reproduce: "filenameTooLongMessage" is chosen by "filenameMaxLength". + */ + public function testPluralizesTheFilenameLengthOfAFile() + { + $catalogue = array( + (new Assert\File())->filenameTooLongMessage => + 'Назва файлу задовга. Вона має містити {{ filename_max_length }} символ.' + . '|Назва файлу задовга. Вона має містити {{ filename_max_length }} символи.' + . '|Назва файлу задовга. Вона має містити {{ filename_max_length }} символів.', + ); + + $options = $this->parseConstraint(new Assert\File(filenameMaxLength: 30), 'uk', $catalogue); + + $this->assertSame( + 'Назва файлу задовга. Вона має містити {{ filename_max_length }} символів.', + $options['filenameTooLongMessage'] + ); + } + + /** WordCount pluralizes both of its limits as well. */ + public function testPluralizesTheLimitsOfAWordCount() + { + if (!extension_loaded('intl')) { + $this->markTestSkipped('The WordCount constraint requires the intl extension.'); + } + + $catalogue = array( + (new Assert\WordCount(min: 1))->minMessage => 'щонайменше {{ min }} слово' + . '|щонайменше {{ min }} слова' + . '|щонайменше {{ min }} слів', + ); + + $options = $this->parseConstraint(new Assert\WordCount(min: 5), 'uk', $catalogue); - $this->assertStringContainsString('exactly {{ limit }} character.', $options['exactMessage']); - $this->assertStringNotContainsString('|', $options['exactMessage']); + $this->assertSame('щонайменше {{ min }} слів', $options['minMessage']); } public function testLeavesAMessageWithoutFormsAlone() @@ -91,6 +190,68 @@ public function testLeavesAMessageWithoutFormsAlone() $this->assertSame('Значення не повинно бути порожнім.', $options['message']); } + public static function constraintsSymfonyDoesNotPluralize(): array + { + $piped = 'Значення має бути {{ limit }} або більше (див. А|Б).'; + + return array( + // A numeric option sits right next to each of these messages, and + // the limit is 1, so a two-form rule would quietly return the head + // of the string. Symfony pluralizes neither of them. + 'Range::min' => array(new Assert\Range(min: 1, max: 10), 'minMessage', $piped), + 'Count::divisibleBy' => array(new Assert\Count(divisibleBy: 1), 'divisibleByMessage', $piped), + 'Image::minRatio' => array(new Assert\Image(minRatio: 1.0), 'minRatioMessage', $piped), + + // A constraint of an application's own that extends one of + // Symfony's keeps Symfony's options and Symfony's validator, so it + // pluralizes exactly what Symfony pluralizes and nothing more. + 'ApplicationRange::min' => array(new ApplicationRange(min: 1, max: 10), 'minMessage', $piped), + 'ApplicationCount::divisibleBy' => array( + new ApplicationCount(divisibleBy: 1), + 'divisibleByMessage', + $piped, + ), + ); + } + + /** + * The table covers every pluralized message of Symfony's own constraints, + * so the naming convention is not applied to them, nor to a constraint that + * extends one of them. It would match options that count nothing, and a + * translation that happens to carry a literal "|" would be cut at a + * separator that was never a plural one. + */ + #[DataProvider('constraintsSymfonyDoesNotPluralize')] + public function testLeavesAMessageSymfonyDoesNotPluralizeWhole( + Constraint $constraint, + string $messageOption, + string $message + ) { + $options = $this->parseConstraint( + $constraint, + 'uk', + array($constraint->{$messageOption} => $message) + ); + + $this->assertSame($message, $options[$messageOption]); + } + + /** + * Leaving the rest of an extended constraint alone must not cost it the + * messages the table does name: those are matched with "instanceof", so + * Count's row answers for a constraint that extends Count. + */ + public function testPluralizesTheTableRowsOfAnExtendedConstraint() + { + $options = $this->parseConstraint( + new ApplicationCount(min: 5), + 'uk', + array((new Assert\Count(min: 1))->minMessage => self::UK_COUNT_MIN) + ); + + $this->assertSame('Ця колекція повинна містити {{ limit }} елементів чи більше.', $options['minMessage']); + } + /** * A translation can offer fewer forms than the locale needs, and then no * form can be chosen. Handing the whole message to the browser leaves the @@ -110,6 +271,36 @@ public function testKeepsTheWholeMessageWhenTheTranslationHasTooFewForms() $this->assertSame($twoForms, $options['minMessage']); } + /** + * A message that still carries its separators is all the browser shows, and + * nothing else says why, so the factory reports it where a logger is given. + * What it reports is the message the catalogue is keyed by, because that is + * the entry whoever reads the log has to go and finish. + */ + public function testReportsAMessageWhoseFormCouldNotBeChosen() + { + $twoForms = 'один елемент|багато елементів'; + $messageId = (new Assert\Count(min: 1))->minMessage; + + $logger = new CollectingLogger(); + + $factory = $this->createFactory('uk', array($messageId => $twoForms)); + $factory->setLogger($logger); + + $this->parseConstraint(new Assert\Count(min: 5), 'uk', array(), $factory); + + $reported = array_values(array_filter( + $logger->records, + static fn (array $record): bool => $messageId === ($record['context']['message'] ?? null) + )); + + $this->assertCount(1, $reported); + $this->assertSame(LogLevel::WARNING, $reported[0]['level']); + $this->assertStringContainsString('Could not choose a plural form', $reported[0]['message']); + $this->assertStringContainsString($twoForms, $reported[0]['context']['reason']); + $this->assertInstanceOf(\InvalidArgumentException::class, $reported[0]['context']['exception']); + } + /** * Symfony's constraints name the option a message is pluralized by after * the message itself, so a custom constraint that keeps the convention is @@ -130,30 +321,117 @@ public function testPluralizesACustomConstraintByItsNamingConvention() } /** - * Runs one constraint through the factory and returns its options as the - * browser receives them, messages translated. - * - * @param array $catalogue - * - * @return array + * The class of the factory is a documented extension point, + * "svaroh_js_form_validator.factory.class", and translateMessage() has been + * part of it since long before a plural form was chosen here. An override + * that knows only its original two arguments keeps working: declaring + * LegacyFactory below would be a fatal error otherwise, and every message + * still goes through it, pluralized or not. + */ + public function testHonoursAnOverriddenTranslateMessage() + { + $factory = $this->createFactory( + 'uk', + array((new Assert\NotBlank())->message => 'Значення не повинно бути порожнім.'), + LegacyFactory::class + ); + + $options = $this->parseConstraint(new Assert\NotBlank(), 'uk', array(), $factory); + + $this->assertSame('[Значення не повинно бути порожнім.]', $options['message']); + } + + /** + * A pluralized message is handed to translateMessage() as well, with the + * chosen count among its parameters, so an override sees it like any other. + */ + public function testHonoursAnOverriddenTranslateMessageOnAPluralizedMessage() + { + $factory = $this->createFactory( + 'uk', + array((new Assert\Count(min: 1))->minMessage => self::UK_COUNT_MIN), + LegacyFactory::class + ); + + $options = $this->parseConstraint(new Assert\Count(min: 5), 'uk', array(), $factory); + + $this->assertSame( + '[Ця колекція повинна містити {{ limit }} елементів чи більше.]', + $options['minMessage'] + ); + } + + /** + * The fallback of a pluralized message runs through translateMessage() too, + * so an override still sees the messages no form could be chosen for, and + * sees them once rather than twice. */ - private function parseConstraint(Constraint $constraint, string $locale, array $catalogue = array()): array + public function testHonoursAnOverriddenTranslateMessageOnTheFallback() { + $twoForms = 'один елемент|багато елементів'; + + $factory = $this->createFactory( + 'uk', + array((new Assert\Count(min: 1))->minMessage => $twoForms), + LegacyFactory::class + ); + + $options = $this->parseConstraint(new Assert\Count(min: 5), 'uk', array(), $factory); + + $this->assertSame('[' . $twoForms . ']', $options['minMessage']); + } + + /** + * The validator the factory was built with, which the form is given as + * well: in the container both are the one "validator" service. + */ + private ?ValidatorInterface $validator = null; + + /** + * Builds a factory that translates against the given catalogue. + * + * @param array $catalogue + * @param class-string $class + */ + private function createFactory( + string $locale, + array $catalogue = array(), + string $class = JsFormValidatorFactory::class + ): JsFormValidatorFactory { $translator = new Translator($locale); $translator->addLoader('array', new ArrayLoader()); $translator->addResource('array', $catalogue, $locale, 'validators'); - $validator = Validation::createValidator(); $router = $this->createStub(UrlGeneratorInterface::class); $router->method('generate')->willReturn('/generated-route'); - $factory = new JsFormValidatorFactory( - $validator, + $this->validator = Validation::createValidator(); + + return new $class( + $this->validator, $translator, $router, array('js_validation' => true), 'validators' ); + } + + /** + * Runs one constraint through the factory and returns its options as the + * browser receives them, messages translated. + * + * @param array $catalogue + * + * @return array + */ + private function parseConstraint( + Constraint $constraint, + string $locale, + array $catalogue = array(), + ?JsFormValidatorFactory $factory = null + ): array { + $factory = $factory ?? $this->createFactory($locale, $catalogue); + $validator = $this->validator ?? Validation::createValidator(); $form = Forms::createFormFactoryBuilder() ->addExtension(new ValidatorExtension($validator)) @@ -167,7 +445,11 @@ private function parseConstraint(Constraint $constraint, string $locale, array $ $model = $factory->createJsModel($form); $parsed = $model->children['field']->data['form']['constraints'][get_class($constraint)][0]; - return get_object_vars($parsed); + // A File constraint is exported as a plain option list, because its + // "maxSize" option is a protected property behind a magic getter that + // the generic object export cannot see. Every other constraint is + // exported as itself. + return is_array($parsed) ? $parsed : get_object_vars($parsed); } } @@ -189,3 +471,48 @@ public function getTargets(): string|array return self::PROPERTY_CONSTRAINT; } } + +/** + * A constraint of an application's own that extends one of Symfony's. It keeps + * Symfony's options and Symfony's validator, so it pluralizes what Symfony + * pluralizes: nothing, in the case of Range. + */ +class ApplicationRange extends Assert\Range +{ +} + +/** The same, over a constraint that does have rows in the factory's table. */ +class ApplicationCount extends Assert\Count +{ +} + +/** + * Keeps what it was told, so a test can look at every record rather than at the + * order they arrived in. + */ +class CollectingLogger extends AbstractLogger +{ + /** @var array */ + public array $records = array(); + + public function log($level, string|\Stringable $message, array $context = array()): void + { + $this->records[] = array( + 'level' => $level, + 'message' => (string) $message, + 'context' => $context, + ); + } +} + +/** + * A factory of an application's own that overrides translateMessage() with the + * two arguments it has always taken. + */ +class LegacyFactory extends JsFormValidatorFactory +{ + protected function translateMessage($message, ?array $parameters = null) + { + return '[' . parent::translateMessage($message, $parameters) . ']'; + } +} diff --git a/composer.json b/composer.json index 8bf691f4..52a51be6 100644 --- a/composer.json +++ b/composer.json @@ -20,6 +20,7 @@ "require": { "php": "^8.4", "doctrine/persistence": "^2.5 || ^3.0 || ^4.0", + "psr/log": "^3.0", "symfony/config": "^8.0", "symfony/dependency-injection": "^8.0", "symfony/doctrine-bridge": "^8.0", diff --git a/src/Factory/JsFormValidatorFactory.php b/src/Factory/JsFormValidatorFactory.php index 2437a7cd..be4ea502 100644 --- a/src/Factory/JsFormValidatorFactory.php +++ b/src/Factory/JsFormValidatorFactory.php @@ -1,6 +1,7 @@ Message" naming convention is translated without a count, which - * is what the library did before. + * This table is the whole of what the library knows about Symfony's own + * constraints: a message of theirs that is missing from it is translated + * without a count, keeping every form, which is what the library did before + * the table existed. Constraints from elsewhere are matched by the naming + * convention in getPluralCount() instead. */ protected const PLURAL_COUNT_OPTIONS = array( Choice::class => array('minMessage' => 'min', 'maxMessage' => 'max'), @@ -55,6 +58,21 @@ class JsFormValidatorFactory WordCount::class => array('minMessage' => 'min', 'maxMessage' => 'max'), ); + /** + * The namespace Symfony's own constraints live in. + * + * Their pluralized messages are enumerated in PLURAL_COUNT_OPTIONS, so the + * naming convention is never applied to them. It would match options that + * are not counts of anything -- Range::$min, Image::$minRatio, + * Count::$divisibleBy -- and a translation of one of those that happens to + * contain a "|" would be cut at a separator that was never a plural one. + * + * A constraint that extends one of Symfony's inherits those same options + * and the same validator, which does not call setPlural() for them either, + * so the whole ancestry is looked at rather than the class itself. + */ + protected const SYMFONY_CONSTRAINT_NAMESPACE = 'Symfony\\Component\\Validator\\Constraints\\'; + /** * @var ValidatorInterface */ @@ -90,6 +108,11 @@ class JsFormValidatorFactory */ protected $transDomain; + /** + * @var LoggerInterface|null + */ + protected $logger = null; + /** * @param ValidatorInterface $validator * @param TranslatorInterface $translator @@ -111,6 +134,21 @@ public function __construct( $this->transDomain = $domain; } + /** + * The logger the factory reports a message it could not translate to + * + * Optional: without one those messages are still handed to the browser in + * the shape they came in, they are just not reported anywhere. + * + * @param LoggerInterface|null $logger + * + * @return void + */ + public function setLogger(?LoggerInterface $logger) + { + $this->logger = $logger; + } + /** * Gets metadata from system using the entity class name * @@ -127,47 +165,78 @@ protected function getMetadataFor($className) /** * Translate a single message * - * A pluralized message carries every form of the translation in one string, - * separated by "|", and which of them applies depends on the locale: two - * forms in English, three in Ukrainian, six in Arabic. Given the number the - * message speaks about, the translator picks the form for the current - * locale here, so the browser receives the one form it has to show. + * @param string $message + * + * @return string + */ + protected function translateMessage($message, ?array $parameters = null) + { + return $this->translator->trans($message, $parameters ?? array(), $this->transDomain); + } + + /** + * Translate a message that carries every plural form of the translation + * + * Such a message holds those forms in one string, separated by "|", and + * which of them applies depends on the locale: two forms in English, three + * in Ukrainian, six in Arabic. Given the number the message speaks about, + * the translator picks the form for the current locale here, so the browser + * receives the one form it has to show. + * + * The number is handed to translateMessage() as the "%count%" parameter it + * has always been, so every message still goes through that one method and + * an override of it sees the pluralized ones too. * * @param string $message * @param int|null $plural The number the message is pluralized by, if any * * @return string */ - protected function translateMessage($message, ?array $parameters = null, $plural = null) + protected function translatePluralMessage($message, $plural, ?array $parameters = null) { - $parameters = $parameters ?? array(); - if (null !== $plural) { try { - return $this->translator->trans( + return $this->translateMessage( $message, - array('%count%' => $plural) + $parameters, - $this->transDomain + array('%count%' => $plural) + ($parameters ?? array()) ); } catch (\InvalidArgumentException $e) { - // The translation offers fewer forms than the locale needs, so - // no form can be chosen. Fall through and hand the whole - // message to the browser, as this library did before, rather - // than break the rendering of the form over a translation. + // Most often the translation offers fewer forms than the locale + // needs, so no form can be chosen. Hand the whole message to the + // browser then, as this library did before, rather than break + // the rendering of a form over a translation. The catch is wider + // than that one case on purpose, so say what was swallowed: a + // message that reaches the browser still carrying its "|" + // separators gives nobody anything else to go on. It is a + // warning rather than a debug note because that message is what + // the form actually shows, which is a defect in the rendering + // and not a diagnostic detail. + if (null !== $this->logger) { + $this->logger->warning( + 'Could not choose a plural form for the message "{message}", ' + . 'handing every form to the browser: {reason}', + array('message' => $message, 'reason' => $e->getMessage(), 'exception' => $e) + ); + } } } - return $this->translator->trans($message, $parameters, $this->transDomain); + return $this->translateMessage($message, $parameters); } /** * The number a pluralized message option picks its form by, or null when the * constraint does not pluralize that message. * - * Constraints of Symfony's own are listed in PLURAL_COUNT_OPTIONS. Anything - * else is matched by the naming convention those constraints follow, where - * "minMessage" is pluralized by the "min" option, so a custom constraint - * that keeps the convention is covered without being listed. + * Constraints of Symfony's own, and constraints extending one of them, are + * answered from PLURAL_COUNT_OPTIONS and from nothing else, because that + * table covers every pluralized message they have. A constraint of an + * application's own is matched by the naming convention Symfony's + * constraints follow, where "minMessage" is pluralized by the "min" option, + * so it is covered without being listed. + * + * A limit that is not a whole number is truncated towards zero, the same + * cast Symfony's own setPlural(int $number) applies to it. * * @param object $constraint * @param string $messageOption @@ -186,6 +255,10 @@ protected function getPluralCount($constraint, $messageOption) } if (null === $countOption) { + if ($this->isSymfonyConstraint($constraint)) { + return null; + } + if (!preg_match('/^(?