From ec05c89f8ecf9edf6973b7e4d7cc4e571eca1df2 Mon Sep 17 00:00:00 2001 From: Ben Ramsey Date: Wed, 15 Dec 2021 22:40:13 -0600 Subject: [PATCH 1/2] feat: provide message validation during extraction --- CHANGELOG.md | 1 + phpcs.xml.dist | 1 + phpstan.neon.dist | 1 + src/Console/Command/AbstractCommand.php | 9 +- src/Console/Command/ExtractCommand.php | 87 +++++++++++++++ src/Extractor/MessageExtractor.php | 28 +++++ src/Extractor/MessageExtractorOptions.php | 5 + src/Icu/MessageFormat/Parser.php | 1 + src/Icu/MessageFormat/Parser/Error.php | 51 ++++++++- .../Exception/InvalidMessageException.php | 49 +++++++++ .../UnableToParseMessageException.php | 14 +-- src/Icu/MessageFormat/Validator.php | 68 ++++++++++++ tests/Console/Command/ExtractCommandTest.php | 103 ++++++++++++++++++ ...estExecuteWithValidationHasNoErrors__1.txt | 20 ++++ ...mandTest__testExecuteWithValidation__1.txt | 32 ++++++ tests/Extractor/MessageExtractorTest.php | 99 +++++++++++++++++ tests/Icu/MessageFormat/Parser/ErrorTest.php | 70 ++++++++++++ tests/Icu/MessageFormat/ValidatorTest.php | 55 ++++++++++ tests/fixtures/invalid-message.php | 8 ++ 19 files changed, 679 insertions(+), 23 deletions(-) create mode 100644 src/Icu/MessageFormat/Parser/Exception/InvalidMessageException.php create mode 100644 src/Icu/MessageFormat/Validator.php create mode 100644 tests/Console/Command/__snapshots__/ExtractCommandTest__testExecuteWithValidationHasNoErrors__1.txt create mode 100644 tests/Console/Command/__snapshots__/ExtractCommandTest__testExecuteWithValidation__1.txt create mode 100644 tests/Icu/MessageFormat/ValidatorTest.php create mode 100644 tests/fixtures/invalid-message.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aab9e7..4e44e3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Add [Crowdin](https://crowdin.com) as a format for writing and reading extracted messages - Add `pseudo-locale` console command to allow conversion of a locale to one of the supported pseudo-locales (`en-XA`, `en-XB`, `xx-AC`, `xx-HA`, and `xx-LS`). - Provide `--flatten` extraction option to tell the extractor to hoist selectors and flatten sentences as much as possible. For example, `I have {count, plural, one{a dog} other{many dogs}}` becomes `{count, plural, one{I have a dog} other{I have many dogs}}`. The goal is to provide as many full sentences as possible, since fragmented sentences are not translator-friendly. +- Provide `--validate-messages` extraction option to print a list of validation failures and respond with a non-zero exit code on validation failures - Provide `--add-missing-ids` extraction option to update source code with auto-generated identifiers - Add `Util\FormatHelper` that provides `getReader()` and `getWriter()` methods - Introduce `Format\Format` final static class for format constants diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 85ce52c..67865d9 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -8,6 +8,7 @@ ./src ./tests + */tests/fixtures/* */tests/*/fixtures/* diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 2cc10dd..5ffd720 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -5,6 +5,7 @@ parameters: - ./src - ./tests excludePaths: + - */tests/fixtures/* - */tests/*/fixtures/* ignoreErrors: - '#Cannot call method getName\(\) on ReflectionType\|null#' diff --git a/src/Console/Command/AbstractCommand.php b/src/Console/Command/AbstractCommand.php index 0a29260..262fc46 100644 --- a/src/Console/Command/AbstractCommand.php +++ b/src/Console/Command/AbstractCommand.php @@ -33,19 +33,12 @@ */ abstract class AbstractCommand extends SymfonyConsoleCommand { - private const LOG_FORMAT_MAPPING = [ - LogLevel::WARNING => ConsoleLogger::ERROR, - LogLevel::NOTICE => ConsoleLogger::ERROR, - LogLevel::INFO => ConsoleLogger::ERROR, - LogLevel::DEBUG => ConsoleLogger::ERROR, - ]; - private const LOG_VERBOSITY_MAPPING = [ LogLevel::NOTICE => OutputInterface::VERBOSITY_NORMAL, ]; protected function getConsoleLogger(OutputInterface $output): LoggerInterface { - return new ConsoleLogger($output, self::LOG_VERBOSITY_MAPPING, self::LOG_FORMAT_MAPPING); + return new ConsoleLogger($output, self::LOG_VERBOSITY_MAPPING); } } diff --git a/src/Console/Command/ExtractCommand.php b/src/Console/Command/ExtractCommand.php index 59d5896..0365f67 100644 --- a/src/Console/Command/ExtractCommand.php +++ b/src/Console/Command/ExtractCommand.php @@ -29,22 +29,31 @@ use FormatPHP\Extractor\IdInterpolator; use FormatPHP\Extractor\MessageExtractor; use FormatPHP\Extractor\MessageExtractorOptions; +use FormatPHP\Extractor\Parser\ParserErrorCollection; +use FormatPHP\Icu\MessageFormat\Parser\Exception\InvalidMessageException; use FormatPHP\Util\FileSystemHelper; use FormatPHP\Util\FormatHelper; use FormatPHP\Util\Globber; use LogicException; use Ramsey\Collection\Exception\CollectionMismatchException; use Symfony\Component\Console\Exception\InvalidArgumentException as SymfonyInvalidArgumentException; +use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\ConsoleOutputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; use function array_map; use function array_merge; use function array_unique; +use function count; use function explode; use function getcwd; +use function ksort; +use function strlen; +use function substr; use const PHP_EOL; @@ -118,6 +127,15 @@ protected function configure(): void . 'full sentences as possible, since fragmented' . PHP_EOL . 'sentences are not translator-friendly.', ) + ->addOption( + '--validate-messages', + null, + InputOption::VALUE_NONE, + 'Whether to validate messages as proper ICU' . PHP_EOL + . 'message syntax. If any messages fail, this' . PHP_EOL + . 'will respond with a non-zero exit code and' . PHP_EOL + . 'print the error messages to stderr.', + ) ->addOption( '--extract-source-location', null, @@ -210,6 +228,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int $extractor->process($files); + if ($options->validateMessages && $this->printErrors($extractor->getErrors(), $input, $output)) { + return self::FAILURE; + } + return self::SUCCESS; } @@ -249,6 +271,7 @@ private function buildOptions(InputInterface $input): MessageExtractorOptions $options->preserveWhitespace = (bool) $input->getOption('preserve-whitespace'); $options->flatten = (bool) $input->getOption('flatten'); $options->addGeneratedIdsToSourceCode = (bool) $input->getOption('add-missing-ids'); + $options->validateMessages = (bool) $input->getOption('validate-messages'); /** @var string $inputFunctionNames */ $inputFunctionNames = $input->getOption('addl-func') ?? ''; @@ -257,4 +280,68 @@ private function buildOptions(InputInterface $input): MessageExtractorOptions return $options; } + + /** + * @throws LogicException + * @throws SymfonyInvalidArgumentException + */ + private function printErrors(ParserErrorCollection $errors, InputInterface $input, OutputInterface $output): bool + { + $tableErrors = []; + foreach ($errors as $error) { + $message = $error->message; + if ($error->exception instanceof InvalidMessageException) { + $message = 'Syntax Error: ' + . $error->exception->getParserError()->getErrorKindName() + . ' in message "' . $error->exception->getParserError()->message . '"'; + } + + $tableErrors[$error->sourceFile][] = [$error->sourceLine, $message]; + } + + if (count($tableErrors) === 0) { + return false; + } + + if ($output instanceof ConsoleOutputInterface) { + $output = $output->getErrorOutput(); + } + + $style = new SymfonyStyle($input, $output); + $style->warning('The following errors occurred while extracting ICU formatted messages.'); + + ksort($tableErrors); + foreach ($tableErrors as $file => $fileErrors) { + $this->renderTable($file, $fileErrors, $output); + } + + $style->error('Errors encountered during ICU formatted message extraction.'); + + return true; + } + + /** + * @param non-empty-array $errors + * + * @throws LogicException + * @throws SymfonyInvalidArgumentException + */ + private function renderTable(string $file, array $errors, OutputInterface $output): void + { + $fileHeader = strlen($file) > 68 ? '...' . substr($file, -65) : $file; + + $style = Table::getStyleDefinition('borderless'); + $style->setHorizontalBorderChars('-'); + + $table = new Table($output); + $table->setStyle($style); + $table->setColumnMaxWidth(0, 4); + $table->setColumnMaxWidth(1, 68); + $table->setHeaders(['Line', $fileHeader]); + $table->setRows($errors); + + $table->render(); + + $output->write(PHP_EOL); + } } diff --git a/src/Extractor/MessageExtractor.php b/src/Extractor/MessageExtractor.php index 9ee2692..a925732 100644 --- a/src/Extractor/MessageExtractor.php +++ b/src/Extractor/MessageExtractor.php @@ -31,14 +31,17 @@ use FormatPHP\Exception\InvalidArgumentException; use FormatPHP\Exception\UnableToProcessFileException; use FormatPHP\Exception\UnableToWriteFileException; +use FormatPHP\ExtendedDescriptorInterface; use FormatPHP\Extractor\Parser\Descriptor\PhpParser; use FormatPHP\Extractor\Parser\DescriptorParserInterface; +use FormatPHP\Extractor\Parser\ParserError; use FormatPHP\Extractor\Parser\ParserErrorCollection; use FormatPHP\Format\WriterInterface; use FormatPHP\Format\WriterOptions; use FormatPHP\Icu\MessageFormat\Manipulator; use FormatPHP\Icu\MessageFormat\Parser as MessageFormatParser; use FormatPHP\Icu\MessageFormat\Printer; +use FormatPHP\Icu\MessageFormat\Validator; use FormatPHP\Util\FileSystemHelper; use FormatPHP\Util\FormatHelper; use FormatPHP\Util\Globber; @@ -133,6 +136,10 @@ public function process(array $files): void return; } + if ($this->options->validateMessages) { + $this->validateDescriptors($descriptors); + } + $this->write($formatter, $descriptors); } @@ -283,4 +290,25 @@ private function flattenMessage(): Closure return $descriptor; }; } + + private function validateDescriptors(DescriptorCollection $descriptors): void + { + $validator = new Validator(); + + foreach ($descriptors as $descriptor) { + try { + $validator->validate((string) $descriptor->getDefaultMessage()); + } catch (MessageFormatParser\Exception\InvalidMessageException $exception) { + $sourceFile = ''; + $sourceLine = -1; + + if ($descriptor instanceof ExtendedDescriptorInterface) { + $sourceFile = $descriptor->getSourceFile() ?? ''; + $sourceLine = $descriptor->getSourceLine() ?? -1; + } + + $this->errors[] = new ParserError($exception->getMessage(), $sourceFile, $sourceLine, $exception); + } + } + } } diff --git a/src/Extractor/MessageExtractorOptions.php b/src/Extractor/MessageExtractorOptions.php index 8eee0e4..53de4d4 100644 --- a/src/Extractor/MessageExtractorOptions.php +++ b/src/Extractor/MessageExtractorOptions.php @@ -114,4 +114,9 @@ class MessageExtractorOptions * Any IDs already present in the source code will remain unchanged. */ public bool $addGeneratedIdsToSourceCode = false; + + /** + * Whether to validate ICU message syntax during extraction + */ + public bool $validateMessages = false; } diff --git a/src/Icu/MessageFormat/Parser.php b/src/Icu/MessageFormat/Parser.php index 7099519..b781747 100644 --- a/src/Icu/MessageFormat/Parser.php +++ b/src/Icu/MessageFormat/Parser.php @@ -656,6 +656,7 @@ private function parseArgumentOptions( Error::INVALID_NUMBER_SKELETON, $this->message, $styleAndLocation['styleLocation'], + $exception, ), ); } diff --git a/src/Icu/MessageFormat/Parser/Error.php b/src/Icu/MessageFormat/Parser/Error.php index 22becc7..b00efc3 100644 --- a/src/Icu/MessageFormat/Parser/Error.php +++ b/src/Icu/MessageFormat/Parser/Error.php @@ -23,12 +23,24 @@ namespace FormatPHP\Icu\MessageFormat\Parser; use FormatPHP\Icu\MessageFormat\Parser\Type\Location; +use ReflectionObject; +use Throwable; + +use function array_flip; /** * @psalm-type ErrorKind = Error::* */ class Error { + /** + * An error that does not fit with any of the other constants on this class. + * + * If receiving this kind of error, check {@see getThrowable()} to see if + * there is an associated exception. + */ + public const OTHER = 0; + /** * Argument is unclosed (e.g. `{0`) */ @@ -168,6 +180,11 @@ class Error */ public const UNCLOSED_TAG = 27; + /** + * @var string[] + */ + private static array $constants = []; + /** * @var ErrorKind */ @@ -176,13 +193,43 @@ class Error public string $message; public Location $location; + private ?Throwable $throwable; + /** * @param ErrorKind $kind */ - public function __construct(int $kind, string $message, Location $location) - { + public function __construct( + int $kind, + string $message, + Location $location, + ?Throwable $throwable = null + ) { $this->kind = $kind; $this->message = $message; $this->location = $location; + $this->throwable = $throwable; + } + + /** + * May return a Throwable instance if {@see $kind} is {@see OTHER} + */ + public function getThrowable(): ?Throwable + { + return $this->throwable; + } + + /** + * Returns the name for the kind of error this represents + */ + public function getErrorKindName(): string + { + if (self::$constants === []) { + $reflection = new ReflectionObject($this); + + // @phpstan-ignore-next-line + self::$constants = array_flip($reflection->getConstants()); + } + + return self::$constants[$this->kind] ?? ''; } } diff --git a/src/Icu/MessageFormat/Parser/Exception/InvalidMessageException.php b/src/Icu/MessageFormat/Parser/Exception/InvalidMessageException.php new file mode 100644 index 0000000..1f460e2 --- /dev/null +++ b/src/Icu/MessageFormat/Parser/Exception/InvalidMessageException.php @@ -0,0 +1,49 @@ + + * @license https://opensource.org/licenses/MIT MIT License + */ + +declare(strict_types=1); + +namespace FormatPHP\Icu\MessageFormat\Parser\Exception; + +use FormatPHP\Icu\MessageFormat\Parser\Error; +use RuntimeException as PhpRuntimeException; +use Throwable; + +/** + * Thrown when ICU message validation fails + */ +class InvalidMessageException extends PhpRuntimeException implements ParserExceptionInterface +{ + private Error $error; + + public function __construct(Error $error, ?Throwable $previous = null) + { + parent::__construct('Syntax error', 0, $previous); + $this->error = $error; + } + + /** + * Returns the specific syntax error that caused validation to fail + */ + public function getParserError(): Error + { + return $this->error; + } +} diff --git a/src/Icu/MessageFormat/Parser/Exception/UnableToParseMessageException.php b/src/Icu/MessageFormat/Parser/Exception/UnableToParseMessageException.php index 8ffe9eb..7e3bdc2 100644 --- a/src/Icu/MessageFormat/Parser/Exception/UnableToParseMessageException.php +++ b/src/Icu/MessageFormat/Parser/Exception/UnableToParseMessageException.php @@ -23,11 +23,9 @@ namespace FormatPHP\Icu\MessageFormat\Parser\Exception; use FormatPHP\Icu\MessageFormat\Parser\Error; -use ReflectionObject; use RuntimeException as PhpRuntimeException; use Throwable; -use function array_flip; use function sprintf; /** @@ -45,18 +43,8 @@ private function createMessageForError(Error $error): string { return sprintf( 'Syntax error %s found while parsing message "%s"', - $this->getErrorTypeName($error), + $error->getErrorKindName(), $error->message, ); } - - private function getErrorTypeName(Error $error): string - { - $reflection = new ReflectionObject($error); - - // @phpstan-ignore-next-line - $constants = array_flip($reflection->getConstants()); - - return $constants[$error->kind] ?? ''; - } } diff --git a/src/Icu/MessageFormat/Validator.php b/src/Icu/MessageFormat/Validator.php new file mode 100644 index 0000000..28190b3 --- /dev/null +++ b/src/Icu/MessageFormat/Validator.php @@ -0,0 +1,68 @@ + + * @license https://opensource.org/licenses/MIT MIT License + */ + +declare(strict_types=1); + +namespace FormatPHP\Icu\MessageFormat; + +use FormatPHP\Icu\MessageFormat\Parser\Error; +use FormatPHP\Icu\MessageFormat\Parser\Exception\InvalidMessageException; +use FormatPHP\Icu\MessageFormat\Parser\Options; +use FormatPHP\Icu\MessageFormat\Parser\Result; +use FormatPHP\Icu\MessageFormat\Parser\Type\Location; +use FormatPHP\Icu\MessageFormat\Parser\Type\LocationDetails; +use Throwable; + +/** + * A validator to use with the ICU message format syntax + */ +class Validator +{ + /** + * Returns true if the message is valid ICU message syntax + * + * @throws InvalidMessageException + */ + public function validate(string $message): bool + { + $throwable = null; + + $options = new Options(); + $options->shouldParseSkeletons = true; + + try { + $parser = new Parser($message, $options); + $result = $parser->parse(); + } catch (Throwable $throwable) { + // Convert exceptions to errors. + $position = new LocationDetails(-1, $throwable->getLine(), -1); + $result = new Result( + null, + new Error(Error::OTHER, $message, new Location($position, $position), $throwable), + ); + } + + if ($result->err !== null) { + throw new InvalidMessageException($result->err, $throwable); + } + + return true; + } +} diff --git a/tests/Console/Command/ExtractCommandTest.php b/tests/Console/Command/ExtractCommandTest.php index 3be8a16..e323d43 100644 --- a/tests/Console/Command/ExtractCommandTest.php +++ b/tests/Console/Command/ExtractCommandTest.php @@ -9,6 +9,10 @@ use FormatPHP\Test\TestCase; use Symfony\Component\Console\Tester\CommandTester; +use function ob_end_clean; +use function ob_get_contents; +use function ob_start; + class ExtractCommandTest extends TestCase { public function testExecute(): void @@ -23,4 +27,103 @@ public function testExecute(): void $this->assertMatchesTextSnapshot($commandTester->getDisplay()); } + + public function testExecuteWithValidation(): void + { + $application = new Application(); + + /** @var ExtractCommand $command */ + $command = $application->find('extract'); + + $commandTester = new CommandTester($command); + + ob_start(); + $commandTester->execute([ + // All .php and .phtml files in the tests folder. + 'files' => [__DIR__ . '/../../**/*.ph*'], + '--validate-messages' => true, + ], [ + 'capture_stderr_separately' => true, + ]); + $jsonOutput = ob_get_contents(); + ob_end_clean(); + + $this->assertSame(1, $commandTester->getStatusCode()); + + // Use a text snapshot instead of JSON so that characters aren't + // converted to unicode escape sequences. + $this->assertMatchesTextSnapshot($jsonOutput); + + // We can't deterministically snapshot-assert the error output because + // it contains file paths. Boo! + $errorOutput = $commandTester->getErrorOutput(true); + + $this->assertStringContainsString( + '[WARNING] The following errors occurred while extracting ICU formatted', + $errorOutput, + ); + $this->assertStringContainsString( + '[ERROR] Errors encountered during ICU formatted message extraction.', + $errorOutput, + ); + $this->assertStringContainsString( + '32 Descriptor argument must be an array', + $errorOutput, + ); + $this->assertStringContainsString( + '86 Descriptor argument must have at least one of id, defaultMessage, or', + $errorOutput, + ); + $this->assertStringContainsString( + '4 Syntax Error: INVALID_TAG in message "This is a default message", + "description": "A simple description of a fixture for testing purposes." + }, + "photos.count": { + "defaultMessage": "You have {numPhotos, plural, =0 {no photos.} =1 {one photo.} other {# photos.} }", + "description": "A description with multiple lines and extra whitespace." + }, + "welcome": { + "defaultMessage": "Welcome!" + }, + "goodbye": { + "defaultMessage": "Goodbye!" + }, + "Soex4s": { + "defaultMessage": "This is a default message", + "description": "A simple description of a fixture for testing purposes." + }, + "xgMWoP": { + "defaultMessage": "This is a default message" + }, + "Q+U0TW": { + "defaultMessage": "Welcome!" + }, + "myMessage": { + "defaultMessage": "Go home, {name}!" + }, + "foobar": { + "defaultMessage": "foobar" + } +} diff --git a/tests/Extractor/MessageExtractorTest.php b/tests/Extractor/MessageExtractorTest.php index 9bba883..f1b414b 100644 --- a/tests/Extractor/MessageExtractorTest.php +++ b/tests/Extractor/MessageExtractorTest.php @@ -11,6 +11,7 @@ use FormatPHP\Extractor\MessageExtractorOptions; use FormatPHP\Extractor\Parser\DescriptorParserInterface; use FormatPHP\Extractor\Parser\ParserErrorCollection; +use FormatPHP\Icu\MessageFormat\Parser\Exception\InvalidMessageException; use FormatPHP\Test\TestCase; use FormatPHP\Util\FileSystemHelper; use FormatPHP\Util\FormatHelper; @@ -21,6 +22,7 @@ use Psr\Log\NullLogger; use stdClass; +use function count; use function json_decode; use function ob_end_clean; use function ob_get_contents; @@ -833,4 +835,101 @@ public function testProcessFlatten(): void $messages, ); } + + public function testProcessValidate(): void + { + $logger = new NullLogger(); + $options = new MessageExtractorOptions(); + $options->validateMessages = true; + $options->functionNames = ['formatMessage', 'translate']; + + $extractor = new MessageExtractor( + $options, + $logger, + new Globber(new FileSystemHelper()), + new FileSystemHelper(), + new FormatHelper(new FileSystemHelper()), + ); + + ob_start(); + $extractor->process([ + __DIR__ . '/Parser/Descriptor/fixtures/*.ph*', + __DIR__ . '/../fixtures/invalid-message.php', + ]); + $output = ob_get_contents(); + ob_end_clean(); + + $messages = json_decode((string) $output, true); + + $this->assertSame( + [ + 'aTestId' => [ + 'defaultMessage' => 'This is a default message', + 'description' => 'A simple description of a fixture for testing purposes.', + ], + 'OpKKos' => [ + 'defaultMessage' => 'Hello!', + ], + 'photos.count' => [ + 'defaultMessage' => + 'You have {numPhotos, plural, =0 {no photos.} =1 {one photo.} other {# photos.} }', + 'description' => 'A description with multiple lines and extra whitespace.', + ], + 'welcome' => [ + 'defaultMessage' => 'Welcome!', + ], + 'goodbye' => [ + 'defaultMessage' => 'Goodbye!', + ], + 'Soex4s' => [ + 'defaultMessage' => 'This is a default message', + 'description' => 'A simple description of a fixture for testing purposes.', + ], + 'xgMWoP' => [ + 'defaultMessage' => 'This is a default message', + ], + 'Q+U0TW' => [ + 'defaultMessage' => 'Welcome!', + ], + ], + $messages, + ); + + $this->assertGreaterThan(0, count($extractor->getErrors())); + + $errors = []; + foreach ($extractor->getErrors() as $error) { + $message = $error->message; + if ($error->exception instanceof InvalidMessageException) { + $message = 'Syntax Error: ' + . $error->exception->getParserError()->getErrorKindName() + . ' in message "' . $error->exception->getParserError()->message . '"'; + } + + $errors[$error->sourceFile][] = [$error->sourceLine, $message]; + } + + $this->assertSame([ + __DIR__ . '/Parser/Descriptor/fixtures/php-parser-02.php' => [ + [32, 'Descriptor argument must be an array'], + ], + __DIR__ . '/Parser/Descriptor/fixtures/php-parser-03.php' => [ + [8, 'Descriptor argument must be an array'], + ], + __DIR__ . '/Parser/Descriptor/fixtures/php-parser-04.php' => [ + [29, 'Descriptor argument must be an array'], + [40, 'Descriptor argument must be an array'], + ], + __DIR__ . '/Parser/Descriptor/fixtures/php-parser-09.phtml' => [ + [18, 'Descriptor argument must be present'], + ], + __DIR__ . '/Parser/Descriptor/fixtures/php-parser-10.php' => [ + [6, 'The descriptor must not contain values other than string literals; encountered Expr_Variable'], + [12, 'The descriptor must not contain values other than string literals; encountered Scalar_Encapsed'], + ], + __DIR__ . '/../fixtures/invalid-message.php' => [ + [4, 'Syntax Error: INVALID_TAG in message "This is a default message"'], + ], + ], $errors); + } } diff --git a/tests/Icu/MessageFormat/Parser/ErrorTest.php b/tests/Icu/MessageFormat/Parser/ErrorTest.php index 00b986e..5957930 100644 --- a/tests/Icu/MessageFormat/Parser/ErrorTest.php +++ b/tests/Icu/MessageFormat/Parser/ErrorTest.php @@ -8,7 +8,11 @@ use FormatPHP\Icu\MessageFormat\Parser\Type\Location; use FormatPHP\Icu\MessageFormat\Parser\Type\LocationDetails; use FormatPHP\Test\TestCase; +use RuntimeException; +/** + * @psalm-import-type ErrorKind from Error + */ class ErrorTest extends TestCase { public function testConstructor(): void @@ -22,10 +26,12 @@ public function testConstructor(): void $this->assertSame(Error::EMPTY_ARGUMENT, $error->kind); $this->assertSame('a test message', $error->message); $this->assertSame($location, $error->location); + $this->assertNull($error->getThrowable()); } public function testConstantValues(): void { + $this->assertSame(0, Error::OTHER); $this->assertSame(1, Error::EXPECT_ARGUMENT_CLOSING_BRACE); $this->assertSame(2, Error::EMPTY_ARGUMENT); $this->assertSame(3, Error::MALFORMED_ARGUMENT); @@ -53,4 +59,68 @@ public function testConstantValues(): void $this->assertSame(26, Error::UNMATCHED_CLOSING_TAG); $this->assertSame(27, Error::UNCLOSED_TAG); } + + public function testConstructorAcceptsThrowable(): void + { + $start = new LocationDetails(0, 1, 1); + $end = new LocationDetails(2, 4, 6); + $location = new Location($start, $end); + $exception = new RuntimeException(); + + $error = new Error(Error::EMPTY_ARGUMENT, 'a test message', $location, $exception); + + $this->assertSame($exception, $error->getThrowable()); + } + + /** + * @param ErrorKind $kind + * + * @dataProvider provideErrorKind + */ + public function testGetErrorKindName(int $kind, string $expected): void + { + $start = new LocationDetails(0, 1, 1); + $end = new LocationDetails(2, 4, 6); + $location = new Location($start, $end); + + $error = new Error($kind, 'A test message', $location); + + $this->assertSame($expected, $error->getErrorKindName()); + } + + /** + * @return array + */ + public function provideErrorKind(): array + { + return [ + ['kind' => 0, 'expected' => 'OTHER'], + ['kind' => 1, 'expected' => 'EXPECT_ARGUMENT_CLOSING_BRACE'], + ['kind' => 2, 'expected' => 'EMPTY_ARGUMENT'], + ['kind' => 3, 'expected' => 'MALFORMED_ARGUMENT'], + ['kind' => 4, 'expected' => 'EXPECT_ARGUMENT_TYPE'], + ['kind' => 5, 'expected' => 'INVALID_ARGUMENT_TYPE'], + ['kind' => 6, 'expected' => 'EXPECT_ARGUMENT_STYLE'], + ['kind' => 7, 'expected' => 'INVALID_NUMBER_SKELETON'], + ['kind' => 8, 'expected' => 'INVALID_DATE_TIME_SKELETON'], + ['kind' => 9, 'expected' => 'EXPECT_NUMBER_SKELETON'], + ['kind' => 10, 'expected' => 'EXPECT_DATE_TIME_SKELETON'], + ['kind' => 11, 'expected' => 'UNCLOSED_QUOTE_IN_ARGUMENT_STYLE'], + ['kind' => 12, 'expected' => 'EXPECT_SELECT_ARGUMENT_OPTIONS'], + ['kind' => 13, 'expected' => 'EXPECT_PLURAL_ARGUMENT_OFFSET_VALUE'], + ['kind' => 14, 'expected' => 'INVALID_PLURAL_ARGUMENT_OFFSET_VALUE'], + ['kind' => 15, 'expected' => 'EXPECT_SELECT_ARGUMENT_SELECTOR'], + ['kind' => 16, 'expected' => 'EXPECT_PLURAL_ARGUMENT_SELECTOR'], + ['kind' => 17, 'expected' => 'EXPECT_SELECT_ARGUMENT_SELECTOR_FRAGMENT'], + ['kind' => 18, 'expected' => 'EXPECT_PLURAL_ARGUMENT_SELECTOR_FRAGMENT'], + ['kind' => 19, 'expected' => 'INVALID_PLURAL_ARGUMENT_SELECTOR'], + ['kind' => 20, 'expected' => 'DUPLICATE_PLURAL_ARGUMENT_SELECTOR'], + ['kind' => 21, 'expected' => 'DUPLICATE_SELECT_ARGUMENT_SELECTOR'], + ['kind' => 22, 'expected' => 'MISSING_OTHER_CLAUSE'], + ['kind' => 23, 'expected' => 'INVALID_TAG'], + ['kind' => 25, 'expected' => 'INVALID_TAG_NAME'], + ['kind' => 26, 'expected' => 'UNMATCHED_CLOSING_TAG'], + ['kind' => 27, 'expected' => 'UNCLOSED_TAG'], + ]; + } } diff --git a/tests/Icu/MessageFormat/ValidatorTest.php b/tests/Icu/MessageFormat/ValidatorTest.php new file mode 100644 index 0000000..26364f1 --- /dev/null +++ b/tests/Icu/MessageFormat/ValidatorTest.php @@ -0,0 +1,55 @@ +assertTrue($validator->validate('A simple {test} message')); + } + + public function testValidateThrowsExceptionOnInvalidMessage(): void + { + $validator = new Validator(); + $exception = null; + + try { + $validator->validate('This is an message'); + } catch (InvalidMessageException $exception) { + } + + $this->assertInstanceOf(InvalidMessageException::class, $exception); + $this->assertSame(Error::UNCLOSED_TAG, $exception->getParserError()->kind); + $this->assertNull($exception->getParserError()->getThrowable()); + } + + public function testValidateThrowsExceptionOnParserError(): void + { + $validator = new Validator(); + $exception = null; + + try { + $validator->validate('{0, date, ::eee}'); + } catch (InvalidMessageException $exception) { + } + + $this->assertInstanceOf(InvalidMessageException::class, $exception); + $this->assertSame(Error::OTHER, $exception->getParserError()->kind); + $this->assertInstanceOf(InvalidSkeletonOption::class, $exception->getPrevious()); + $this->assertSame( + '"e..eee" (weekday) patterns are not supported', + $exception->getPrevious()->getMessage(), + ); + } +} diff --git a/tests/fixtures/invalid-message.php b/tests/fixtures/invalid-message.php new file mode 100644 index 0000000..4166e1b --- /dev/null +++ b/tests/fixtures/invalid-message.php @@ -0,0 +1,8 @@ +formatMessage([ + 'id' => 'aTestId', + 'defaultMessage' => 'This is a default message', + 'description' => 'A simple description of a fixture for testing purposes.', +]); From 156cb0ccdcb49678bbeb755cf6d0eaad4d642542 Mon Sep 17 00:00:00 2001 From: Ben Ramsey Date: Thu, 16 Dec 2021 09:32:43 -0600 Subject: [PATCH 2/2] fix: do not write output if validation fails --- src/Extractor/MessageExtractor.php | 6 +++ tests/Console/Command/ExtractCommandTest.php | 5 +-- ...mandTest__testExecuteWithValidation__1.txt | 32 ---------------- tests/Extractor/MessageExtractorTest.php | 37 +------------------ 4 files changed, 8 insertions(+), 72 deletions(-) delete mode 100644 tests/Console/Command/__snapshots__/ExtractCommandTest__testExecuteWithValidation__1.txt diff --git a/src/Extractor/MessageExtractor.php b/src/Extractor/MessageExtractor.php index a925732..d25526f 100644 --- a/src/Extractor/MessageExtractor.php +++ b/src/Extractor/MessageExtractor.php @@ -232,6 +232,12 @@ private function write(callable $formatter, DescriptorCollection $descriptors): $descriptors = new DescriptorCollection($flattened); } + if ($this->options->validateMessages === true && count($this->errors) > 0) { + $this->logger->error('Validation errors encountered; extraction failed'); + + return; + } + $file = $this->options->outFile ?? 'php://output'; $writerOptions = new WriterOptions(); diff --git a/tests/Console/Command/ExtractCommandTest.php b/tests/Console/Command/ExtractCommandTest.php index e323d43..dd0096c 100644 --- a/tests/Console/Command/ExtractCommandTest.php +++ b/tests/Console/Command/ExtractCommandTest.php @@ -49,10 +49,7 @@ public function testExecuteWithValidation(): void ob_end_clean(); $this->assertSame(1, $commandTester->getStatusCode()); - - // Use a text snapshot instead of JSON so that characters aren't - // converted to unicode escape sequences. - $this->assertMatchesTextSnapshot($jsonOutput); + $this->assertSame('', $jsonOutput); // We can't deterministically snapshot-assert the error output because // it contains file paths. Boo! diff --git a/tests/Console/Command/__snapshots__/ExtractCommandTest__testExecuteWithValidation__1.txt b/tests/Console/Command/__snapshots__/ExtractCommandTest__testExecuteWithValidation__1.txt deleted file mode 100644 index 3481d79..0000000 --- a/tests/Console/Command/__snapshots__/ExtractCommandTest__testExecuteWithValidation__1.txt +++ /dev/null @@ -1,32 +0,0 @@ -{ - "aTestId": { - "defaultMessage": "This is a default message", - "description": "A simple description of a fixture for testing purposes." - }, - "photos.count": { - "defaultMessage": "You have {numPhotos, plural, =0 {no photos.} =1 {one photo.} other {# photos.} }", - "description": "A description with multiple lines and extra whitespace." - }, - "welcome": { - "defaultMessage": "Welcome!" - }, - "goodbye": { - "defaultMessage": "Goodbye!" - }, - "Soex4s": { - "defaultMessage": "This is a default message", - "description": "A simple description of a fixture for testing purposes." - }, - "xgMWoP": { - "defaultMessage": "This is a default message" - }, - "Q+U0TW": { - "defaultMessage": "Welcome!" - }, - "myMessage": { - "defaultMessage": "Go home, {name}!" - }, - "foobar": { - "defaultMessage": "foobar" - } -} diff --git a/tests/Extractor/MessageExtractorTest.php b/tests/Extractor/MessageExtractorTest.php index f1b414b..403976e 100644 --- a/tests/Extractor/MessageExtractorTest.php +++ b/tests/Extractor/MessageExtractorTest.php @@ -859,42 +859,7 @@ public function testProcessValidate(): void $output = ob_get_contents(); ob_end_clean(); - $messages = json_decode((string) $output, true); - - $this->assertSame( - [ - 'aTestId' => [ - 'defaultMessage' => 'This is a default message', - 'description' => 'A simple description of a fixture for testing purposes.', - ], - 'OpKKos' => [ - 'defaultMessage' => 'Hello!', - ], - 'photos.count' => [ - 'defaultMessage' => - 'You have {numPhotos, plural, =0 {no photos.} =1 {one photo.} other {# photos.} }', - 'description' => 'A description with multiple lines and extra whitespace.', - ], - 'welcome' => [ - 'defaultMessage' => 'Welcome!', - ], - 'goodbye' => [ - 'defaultMessage' => 'Goodbye!', - ], - 'Soex4s' => [ - 'defaultMessage' => 'This is a default message', - 'description' => 'A simple description of a fixture for testing purposes.', - ], - 'xgMWoP' => [ - 'defaultMessage' => 'This is a default message', - ], - 'Q+U0TW' => [ - 'defaultMessage' => 'Welcome!', - ], - ], - $messages, - ); - + $this->assertSame('', $output); $this->assertGreaterThan(0, count($extractor->getErrors())); $errors = [];