Bundle does not work properly with new Intl ICU translations

Catalogue/CatalogueCounter.php

@@ -57,7 +57,9 @@ public function getCatalogueStatistics(MessageCatalogueInterface $catalogue): ar
             $result[$domain]['approved'] = 0;
 
             foreach ($catalogue->all($domain) as $key => $text) {
-                $metadata = new Metadata($catalogue->getMetadata($key, $domain));
+                $intlDomain = $domain.'+intl-icu' /* MessageCatalogueInterface::INTL_DOMAIN_SUFFIX */;
+                $rawMetadata = $catalogue->getMetadata($key, $domain) ?: $catalogue->getMetadata($key, $intlDomain);
+                $metadata = new Metadata($rawMetadata);
                 $state = $metadata->getState();
                 if ('new' === $state) {
                     ++$result[$domain]['new'];

Catalogue/CatalogueFetcher.php

@@ -53,8 +53,8 @@ public function getCatalogues(Configuration $config, array $locales = []): array
 
             foreach ($currentCatalogue->getDomains() as $domain) {
                 if (!$this->isValidDomain($config, $domain)) {
-                    $messages = $currentCatalogue->all();
-                    unset($messages[$domain]);
+                    $messages = NSA::getProperty($currentCatalogue, 'messages');
+                    unset($messages[$domain], $messages[$domain.'+intl-icu' /* MessageCatalogueInterface::INTL_DOMAIN_SUFFIX */]);
                     NSA::setProperty($currentCatalogue, 'messages', $messages);
                 }
             }

Catalogue/Operation/ReplaceOperation.php

@@ -11,6 +11,7 @@
 
 namespace Translation\Bundle\Catalogue\Operation;
 
+use Nyholm\NSA;
 use Symfony\Component\Translation\Catalogue\AbstractOperation;
 use Symfony\Component\Translation\MessageCatalogueInterface;
 use Symfony\Component\Translation\MetadataAwareInterface;
@@ -37,26 +38,31 @@ protected function processDomain($domain): void
             'new' => [],
             'obsolete' => [],
         ];
-        if (\defined(sprintf('%s::INTL_DOMAIN_SUFFIX', MessageCatalogueInterface::class))) {
-            $intlDomain = $domain.MessageCatalogueInterface::INTL_DOMAIN_SUFFIX;
-        } else {
-            $intlDomain = $domain;
-        }
+
+        $intlDomain = $domain.'+intl-icu' /* MessageCatalogueInterface::INTL_DOMAIN_SUFFIX */;
+
+        $sourceMessages = NSA::getProperty($this->source, 'messages');
+        $targetMessages = NSA::getProperty($this->target, 'messages');
 
         foreach ($this->source->all($domain) as $id => $message) {
-            $messageDomain = $this->source->defines($id, $intlDomain) ? $intlDomain : $domain;
+            $sourceIdInIntl = \array_key_exists($id, $sourceMessages[$intlDomain] ?? []);
+            $targetIdInIntl = \array_key_exists($id, $targetMessages[$intlDomain] ?? []);
+
+            $sourceMessageDomain = $sourceIdInIntl ? $intlDomain : $domain;
+            $targetMessageDomain = $targetIdInIntl ? $intlDomain : $domain;
+            $resultMessageDomain = $sourceIdInIntl || $targetIdInIntl ? $intlDomain : $domain;
 
             if (!$this->target->has($id, $domain)) {
                 // No merge required
                 $translation = $message;
-                $this->messages[$domain]['new'][$id] = $message;
-                $resultMeta = $this->getMetadata($this->source, $messageDomain, $id);
+                $this->messages[$resultMessageDomain]['new'][$id] = $message;
+                $resultMeta = $this->getMetadata($this->source, $sourceMessageDomain, $id);
             } else {
                 // Merge required
-                $translation = $message ?? $this->target->get($id, $domain);
+                $translation = $message ?? $this->target->get($id, $targetMessageDomain);
                 $resultMeta = null;
-                $sourceMeta = $this->getMetadata($this->source, $messageDomain, $id);
-                $targetMeta = $this->getMetadata($this->target, $this->target->defines($id, $intlDomain) ? $intlDomain : $domain, $id);
+                $sourceMeta = $this->getMetadata($this->source, $sourceMessageDomain, $id);
+                $targetMeta = $this->getMetadata($this->target, $targetMessageDomain, $id);
                 if (\is_array($sourceMeta) && \is_array($targetMeta)) {
                     // We can only merge meta if both is an array
                     $resultMeta = $this->mergeMetadata($sourceMeta, $targetMeta);
@@ -68,11 +74,11 @@ protected function processDomain($domain): void
                 }
             }
 
-            $this->messages[$domain]['all'][$id] = $translation;
-            $this->result->add([$id => $translation], $messageDomain);
+            $this->messages[$resultMessageDomain]['all'][$id] = $translation;
+            $this->result->add([$id => $translation], $resultMessageDomain);
 
             if (!empty($resultMeta)) {
-                $this->result->setMetadata($id, $resultMeta, $messageDomain);
+                $this->result->setMetadata($id, $resultMeta, $resultMessageDomain);
             }
         }
 
@@ -83,14 +89,18 @@ protected function processDomain($domain): void
                 continue;
             }
 
-            $messageDomain = $this->target->defines($id, $intlDomain) ? $intlDomain : $domain;
-            $this->messages[$domain]['all'][$id] = $message;
-            $this->messages[$domain]['obsolete'][$id] = $message;
-            $this->result->add([$id => $message], $messageDomain);
+            $sourceIdInIntl = \array_key_exists($id, $sourceMessages[$intlDomain] ?? []);
+            $targetIdInIntl = \array_key_exists($id, $targetMessages[$intlDomain] ?? []);
+
+            $resultMessageDomain = $sourceIdInIntl || $targetIdInIntl ? $intlDomain : $domain;
+
+            $this->messages[$resultMessageDomain]['all'][$id] = $message;
+            $this->messages[$resultMessageDomain]['obsolete'][$id] = $message;
+            $this->result->add([$id => $message], $resultMessageDomain);
 
-            $resultMeta = $this->getMetadata($this->target, $messageDomain, $id);
+            $resultMeta = $this->getMetadata($this->target, $resultMessageDomain, $id);
             if (!empty($resultMeta)) {
-                $this->result->setMetadata($id, $resultMeta, $messageDomain);
+                $this->result->setMetadata($id, $resultMeta, $resultMessageDomain);
             }
         }
     }

Command/CheckMissingCommand.php

@@ -80,6 +80,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
                 'blacklist_domains' => $config->getBlacklistDomains(),
                 'whitelist_domains' => $config->getWhitelistDomains(),
                 'project_root' => $config->getProjectRoot(),
+                'new_message_format' => $config->getNewMessageFormat(),
             ]
         );
 

Command/ExtractCommand.php

@@ -106,6 +106,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             'blacklist_domains' => $config->getBlacklistDomains(),
             'whitelist_domains' => $config->getWhitelistDomains(),
             'project_root' => $config->getProjectRoot(),
+            'new_message_format' => $config->getNewMessageFormat(),
         ]);
         $errors = $result->getErrors();
 

DependencyInjection/Configuration.php

@@ -164,6 +164,28 @@ private function configsNode(ArrayNodeDefinition $root): void
                         ->scalarNode('output_dir')->cannotBeEmpty()->defaultValue('%kernel.project_dir%/Resources/translations')->end()
                         ->scalarNode('project_root')->info("The root dir of your project. By default this will be kernel_root's parent.")->end()
                         ->scalarNode('xliff_version')->info('The version of XLIFF XML you want to use (if dumping to this format).')->defaultValue('2.0')->end()
+                        ->scalarNode('new_message_format')
+                            ->info('Use "icu" to place new translations in "{domain}+intl-icu.{locale}.{ext}" container')
+                            ->defaultValue('icu')
+                            ->beforeNormalization()
+                                ->ifTrue(
+                                    function ($format) {
+                                        return \is_string($format);
+                                    }
+                                )
+                                ->then(
+                                    function (string $format) {
+                                        return strtolower($format);
+                                    }
+                                )
+                            ->end()
+                            ->validate()
+                                ->ifTrue(function ($value) {
+                                    return !\is_string($value) || !\in_array($value, ['', 'icu']);
+                                })
+                                ->thenInvalid('The "new_message_format" must be either: "" or "icu"; got "%s"')
+                            ->end()
+                        ->end()
                         ->variableNode('local_file_storage_options')
                             ->info('Options passed to the local file storage\'s dumper.')
                             ->defaultValue([])

Model/Configuration.php

@@ -92,6 +92,11 @@ final class Configuration
      */
     private $xliffVersion;
 
+    /**
+     * @var string
+     */
+    private $newMessageFormat;
+
     public function __construct(array $data)
     {
         $this->name = $data['name'];
@@ -106,6 +111,7 @@ public function __construct(array $data)
         $this->blacklistDomains = $data['blacklist_domains'];
         $this->whitelistDomains = $data['whitelist_domains'];
         $this->xliffVersion = $data['xliff_version'];
+        $this->newMessageFormat = $data['new_message_format'];
     }
 
     public function getName(): string
@@ -177,6 +183,15 @@ public function getXliffVersion(): string
         return $this->xliffVersion;
     }
 
+    /**
+     * If set to "icu" it'll place all new translations in "{domain}+intl-icu.{locale}.{ext}" file.
+     * Otherwise normal "{domain}.{locale}.{ext}" file will be used.
+     */
+    public function getNewMessageFormat(): string
+    {
+        return $this->newMessageFormat;
+    }
+
     /**
      * Reconfigures the directories so we can use one configuration for extracting
      * the messages only from one bundle.

Service/Importer.php

@@ -11,6 +11,7 @@
 
 namespace Translation\Bundle\Service;
 
+use Nyholm\NSA;
 use Symfony\Component\Finder\Finder;
 use Symfony\Component\Translation\MessageCatalogue;
 use Translation\Bundle\Catalogue\Operation\ReplaceOperation;
@@ -74,11 +75,11 @@ public function extractToCatalogues(Finder $finder, array $catalogues, array $co
         $results = [];
         foreach ($catalogues as $catalogue) {
             $target = new MessageCatalogue($catalogue->getLocale());
-            $this->convertSourceLocationsToMessages($target, $sourceCollection);
+            $this->convertSourceLocationsToMessages($target, $sourceCollection, $catalogue);
 
             // Remove all SourceLocation and State form catalogue.
-            foreach ($catalogue->getDomains() as $domain) {
-                foreach ($catalogue->all($domain) as $key => $translation) {
+            foreach (NSA::getProperty($catalogue, 'messages') as $domain => $translations) {
+                foreach ($translations as $key => $translation) {
                     $meta = $this->getMetadata($catalogue, $key, $domain);
                     $meta->removeAllInCategory('file-source');
                     $meta->removeAllInCategory('state');
@@ -90,28 +91,37 @@ public function extractToCatalogues(Finder $finder, array $catalogues, array $co
             $result = $merge->getResult();
             $domains = $merge->getDomains();
 
+            $resultMessages = NSA::getProperty($result, 'messages');
+
             // Mark new messages as new/obsolete
             foreach ($domains as $domain) {
+                $intlDomain = $domain.'+intl-icu' /* MessageCatalogueInterface::INTL_DOMAIN_SUFFIX */;
+
                 foreach ($merge->getNewMessages($domain) as $key => $translation) {
-                    $meta = $this->getMetadata($result, $key, $domain);
+                    $messageDomain = \array_key_exists($key, $resultMessages[$intlDomain] ?? []) ? $intlDomain : $domain;
+
+                    $meta = $this->getMetadata($result, $key, $messageDomain);
                     $meta->setState('new');
-                    $this->setMetadata($result, $key, $domain, $meta);
+                    $this->setMetadata($result, $key, $messageDomain, $meta);
 
                     // Add custom translations that we found in the source
                     if (null === $translation) {
                         if (null !== $newTranslation = $meta->getTranslation()) {
-                            $result->set($key, $newTranslation, $domain);
+                            $result->set($key, $newTranslation, $messageDomain);
                             // We do not want "translation" key stored anywhere.
                             $meta->removeAllInCategory('translation');
                         } elseif (null !== ($newTranslation = $meta->getDesc()) && $catalogue->getLocale() === $this->defaultLocale) {
-                            $result->set($key, $newTranslation, $domain);
+                            $result->set($key, $newTranslation, $messageDomain);
                         }
                     }
                 }
+
                 foreach ($merge->getObsoleteMessages($domain) as $key => $translation) {
-                    $meta = $this->getMetadata($result, $key, $domain);
+                    $messageDomain = \array_key_exists($key, $resultMessages[$intlDomain] ?? []) ? $intlDomain : $domain;
+
+                    $meta = $this->getMetadata($result, $key, $messageDomain);
                     $meta->setState('obsolete');
-                    $this->setMetadata($result, $key, $domain, $meta);
+                    $this->setMetadata($result, $key, $messageDomain, $meta);
                 }
             }
             $results[] = $result;
@@ -120,30 +130,48 @@ public function extractToCatalogues(Finder $finder, array $catalogues, array $co
         return new ImportResult($results, $sourceCollection->getErrors());
     }
 
-    private function convertSourceLocationsToMessages(MessageCatalogue $catalogue, SourceCollection $collection): void
-    {
+    private function convertSourceLocationsToMessages(
+        MessageCatalogue $catalogue,
+        SourceCollection $collection,
+        MessageCatalogue $currentCatalogue
+    ): void {
+        $currentMessages = NSA::getProperty($currentCatalogue, 'messages');
+
         /** @var SourceLocation $sourceLocation */
         foreach ($collection as $sourceLocation) {
             $context = $sourceLocation->getContext();
             $domain = $context['domain'] ?? 'messages';
+
             // Check with white/black list
             if (!$this->isValidDomain($domain)) {
                 continue;
             }
 
+            $intlDomain = $domain.'+intl-icu' /* MessageCatalogueInterface::INTL_DOMAIN_SUFFIX */;
+
             $key = $sourceLocation->getMessage();
-            $catalogue->add([$key => null], $domain);
+
+            if (\array_key_exists($key, $currentMessages[$intlDomain] ?? [])) {
+                $messageDomain = $intlDomain;
+            } elseif (\array_key_exists($key, $currentMessages[$domain] ?? [])) {
+                $messageDomain = $domain;
+            } else {
+                // New translation
+                $messageDomain = 'icu' === $this->config['new_message_format'] ? $intlDomain : $domain;
+            }
+
+            $catalogue->add([$key => null], $messageDomain);
             $trimLength = 1 + \strlen($this->config['project_root']);
 
-            $meta = $this->getMetadata($catalogue, $key, $domain);
+            $meta = $this->getMetadata($catalogue, $key, $messageDomain);
             $meta->addCategory('file-source', sprintf('%s:%s', substr($sourceLocation->getPath(), $trimLength), $sourceLocation->getLine()));
             if (isset($sourceLocation->getContext()['desc'])) {
                 $meta->addCategory('desc', $sourceLocation->getContext()['desc']);
             }
             if (isset($sourceLocation->getContext()['translation'])) {
                 $meta->addCategory('translation', $sourceLocation->getContext()['translation']);
             }
-            $this->setMetadata($catalogue, $key, $domain, $meta);
+            $this->setMetadata($catalogue, $key, $messageDomain, $meta);
         }
     }
 
@@ -178,6 +206,7 @@ private function processConfig(array $config): void
             'project_root' => '',
             'blacklist_domains' => [],
             'whitelist_domains' => [],
+            'new_message_format' => 'icu',
         ];
 
         $config = array_merge($default, $config);

Tests/Functional/Catalogue/CatalogueFetcherTest.php

@@ -81,6 +81,7 @@ public static function getDefaultData(): array
             'blacklist_domains' => ['getBlacklistDomains'],
             'whitelist_domains' => ['getWhitelistDomains'],
             'xliff_version' => ['getXliffVersion'],
+            'new_message_format' => ['getNewMessageFormat'],
         ];
     }
 

Tests/Functional/Command/ExtractCommandTest.php

@@ -111,9 +111,9 @@ public function testExecute(): void
         }
 
         $meta = new Metadata($catalogue->getMetadata('not.in.source'));
-        $this->assertTrue('obsolete' === $meta->getState());
+        self::assertSame('obsolete', $meta->getState(), 'Expect meta state to be correct');
 
         $meta = new Metadata($catalogue->getMetadata('translated.title'));
-        $this->assertTrue('new' === $meta->getState());
+        self::assertSame('new', $meta->getState(), 'Expect meta state to be correct');
     }
 }

Tests/Functional/Command/StatusCommandTest.php

@@ -55,9 +55,9 @@ public function testExecute(): void
         $this->assertArrayHasKey('new', $total);
         $this->assertArrayHasKey('obsolete', $total);
         $this->assertArrayHasKey('approved', $total);
-        $this->assertEquals(2, $total['defined']);
-        $this->assertEquals(1, $total['new']);
+        $this->assertEquals(4, $total['defined']);
+        $this->assertEquals(2, $total['new']);
         $this->assertEquals(0, $total['obsolete']);
-        $this->assertEquals(1, $total['approved']);
+        $this->assertEquals(2, $total['approved']);
     }
 }

Tests/Functional/app/Resources/translations/.gitignore

@@ -1,2 +1,3 @@
+messages+intl-icu.sv.xlf
 messages.sv.xlf
 *~

Tests/Functional/app/Resources/translations/messages+intl-icu.en.xlf

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<xliff xmlns="urn:oasis:names:tc:xliff:document:2.0" version="2.0" srcLang="fr-FR" trgLang="en-US">
+    <file id="messages.en_US">
+        <unit id="IcuLCa0a2j">
+            <notes>
+                <note category="state">new</note>
+                <note category="approved">true</note>
+                <note category="section" priority="1">user login</note>
+            </notes>
+            <segment>
+                <source>key2</source>
+                <target>trans2</target>
+            </segment>
+        </unit>
+        <unit id="IcuLCa0a2b">
+            <notes>
+                <note category="file-source" priority="1">Resources/views/translated.html.twig:12</note>
+                <note>file-source</note>
+                <note>status:new</note>
+            </notes>
+            <segment>
+                <source>key3</source>
+                <target>trans3</target>
+            </segment>
+        </unit>
+    </file>
+</xliff>

Tests/Functional/app/config/normal_config.yaml

@@ -9,7 +9,9 @@ translation:
       dirs: ["%test.project_dir%/Resources/views"]
       output_dir: "%test.project_dir%/Resources/translations"
       project_root: "%test.project_dir%"
+      new_message_format: ''
     app_with_transchoice:
       dirs: ["%test.project_dir%/Resources/views_with_transchoice"]
       output_dir: "%test.project_dir%/Resources/translations"
       project_root: "%test.project_dir%"
+      new_message_format: ''