Fixed file visitor for extracting exception messages (#232)

* Fixed file visitor for extracting exception messages * Fixed translation configuration (#237)
ibexa · Aug 24, 2023 · e40d311 · e40d311
1 parent 73cd091
commit e40d311
Show file tree

Hide file tree

Showing 15 changed files with 470 additions and 359 deletions.
diff --git a/src/bundle/Core/DependencyInjection/IbexaCoreExtension.php b/src/bundle/Core/DependencyInjection/IbexaCoreExtension.php
@@ -1024,10 +1024,10 @@ private function prependJMSTranslation(ContainerBuilder $container): void
             'configs' => [
                 'ibexa_core' => [
                     'dirs' => [
-                        __DIR__ . '/../',
+                        __DIR__ . '/../../../',
                     ],
                     'output_dir' => __DIR__ . '/../Resources/translations/',
-                    'output_format' => 'xliff',
+                    'output_format' => 'xlf',
                     'excluded_dirs' => ['Behat', 'Tests', 'node_modules', 'Features'],
                 ],
             ],

diff --git a/src/bundle/Core/Resources/translations/content_fields.en.xlf b/src/bundle/Core/Resources/translations/content_fields.en.xlf
@@ -8,12 +8,12 @@
     <body>
       <trans-unit id="f337a8fd9042dbc4de8b5d305b6d8df3c0c374c4" resname="content-field.latitude.not_set">
         <source>Not set</source>
-        <target state="new">Not set</target>
+        <target>Not set</target>
         <note>key: content-field.latitude.not_set</note>
       </trans-unit>
       <trans-unit id="687973ca20983f27a99c50243e364e346c9c4814" resname="content-field.longitude.not_set">
         <source>Not set</source>
-        <target state="new">Not set</target>
+        <target>Not set</target>
         <note>key: content-field.longitude.not_set</note>
       </trans-unit>
     </body>

diff --git a/src/bundle/Core/Resources/translations/fielddefinition.en.xlf b/src/bundle/Core/Resources/translations/fielddefinition.en.xlf
diff --git a/...ore/Resources/translations/forms.en.xliff → .../Core/Resources/translations/forms.en.xlf b/...ore/Resources/translations/forms.en.xliff → .../Core/Resources/translations/forms.en.xlf
diff --git a/src/bundle/Core/Resources/translations/messages.en.xlf b/src/bundle/Core/Resources/translations/messages.en.xlf
@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <xliff xmlns="urn:oasis:names:tc:xliff:document:1.2" xmlns:jms="urn:jms:translation" version="1.2">
-  <file date="2017-01-26T14:55:58Z" source-language="en" target-language="en" datatype="plaintext" original="not.available">
+  <file source-language="en" target-language="en" datatype="plaintext" original="not.available">
     <header>
       <tool tool-id="JMSTranslationBundle" tool-name="JMSTranslationBundle" tool-version="1.1.0-DEV"/>
       <note>The source node in most cases contains the sample message as written by the developer. If it looks like a dot-delimitted string such as "form.label.firstname", then the developer has not provided a default message.</note>
@@ -10,32 +10,26 @@
         <source>Enter login or email</source>
         <target>Enter login or email</target>
         <note>key: Enter login or email</note>
-        <jms:reference-file line="18">src/bundle/Core/Resources/views/Security/login.html.twig</jms:reference-file>
       </trans-unit>
       <trans-unit id="570591c060a999e9ab1592763c74d881b7654ee1" resname="Enter password">
         <source>Enter password</source>
         <target>Enter password</target>
         <note>key: Enter password</note>
-        <jms:reference-file line="22">src/bundle/Core/Resources/views/Security/login.html.twig</jms:reference-file>
       </trans-unit>
       <trans-unit id="4e5a2893bdcc7d239c1db72e4c4ffbe4bea73174" resname="Login">
         <source>Login</source>
         <target>Login</target>
         <note>key: Login</note>
-        <jms:reference-file line="32">src/bundle/Core/Resources/views/Security/login.html.twig</jms:reference-file>
-        <jms:reference-file line="6">src/bundle/Core/Resources/views/Security/login.html.twig</jms:reference-file>
       </trans-unit>
       <trans-unit id="be81ab98cb9ccf753975f50fa70cd32581a821fc" resname="Password:">
         <source>Password:</source>
         <target>Password:</target>
         <note>key: Password:</note>
-        <jms:reference-file line="21">src/bundle/Core/Resources/views/Security/login.html.twig</jms:reference-file>
       </trans-unit>
       <trans-unit id="17dfb031e086e3415d71f3928e6ce08d6b1c1d4d" resname="Username:">
         <source>Username:</source>
         <target>Username:</target>
         <note>key: Username:</note>
-        <jms:reference-file line="17">src/bundle/Core/Resources/views/Security/login.html.twig</jms:reference-file>
       </trans-unit>
     </body>
   </file>

diff --git a/src/bundle/Core/Resources/translations/repository_exceptions.en.xlf b/src/bundle/Core/Resources/translations/repository_exceptions.en.xlf
diff --git a/src/lib/Base/Exceptions/ContentTypeValidationException.php b/src/lib/Base/Exceptions/ContentTypeValidationException.php
@@ -9,6 +9,7 @@
 use Ibexa\Contracts\Core\Repository\Exceptions\ContentTypeValidationException as APIContentTypeValidationException;
 use Ibexa\Core\Base\Translatable;
 use Ibexa\Core\Base\TranslatableBase;
+use JMS\TranslationBundle\Annotation\Ignore;
 
 /**
  * This Exception is thrown on create or update content type when content type is not valid.
@@ -25,8 +26,7 @@ class ContentTypeValidationException extends APIContentTypeValidationException i
      */
     public function __construct($messageTemplate, array $parameters = [])
     {
-        /** @Ignore */
-        $this->setMessageTemplate($messageTemplate);
+        $this->setMessageTemplate(/** @Ignore */$messageTemplate);
         $this->setParameters($parameters);
 
         parent::__construct($this->getBaseTranslation());

diff --git a/src/lib/Base/Exceptions/ContentValidationException.php b/src/lib/Base/Exceptions/ContentValidationException.php
@@ -9,6 +9,7 @@
 use Ibexa\Contracts\Core\Repository\Exceptions\ContentValidationException as APIContentValidationException;
 use Ibexa\Core\Base\Translatable;
 use Ibexa\Core\Base\TranslatableBase;
+use JMS\TranslationBundle\Annotation\Ignore;
 
 /**
  * This Exception is thrown on create or update content one or more given fields are not valid.
@@ -25,8 +26,7 @@ class ContentValidationException extends APIContentValidationException implement
      */
     public function __construct($messageTemplate, array $parameters = [])
     {
-        /** @Ignore */
-        $this->setMessageTemplate($messageTemplate);
+        $this->setMessageTemplate(/** @Ignore */$messageTemplate);
         $this->setParameters($parameters);
 
         parent::__construct($this->getBaseTranslation());

diff --git a/src/lib/Helper/FieldsGroups/ArrayTranslatorFieldsGroupsList.php b/src/lib/Helper/FieldsGroups/ArrayTranslatorFieldsGroupsList.php
@@ -7,6 +7,7 @@
 namespace Ibexa\Core\Helper\FieldsGroups;
 
 use Ibexa\Contracts\Core\Repository\Values\ContentType\FieldDefinition;
+use JMS\TranslationBundle\Annotation\Ignore;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
 /**
@@ -39,6 +40,7 @@ public function getGroups()
 
         foreach ($this->groups as $groupIdentifier) {
             $translatedGroups[$groupIdentifier] = $this->translator->trans(
+                /** @Ignore */
                 $groupIdentifier,
                 [],
                 'ezplatform_fields_groups'

diff --git a/src/lib/MVC/Symfony/Templating/Twig/Extension/FileSizeExtension.php b/src/lib/MVC/Symfony/Templating/Twig/Extension/FileSizeExtension.php
@@ -8,6 +8,7 @@
 
 use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface;
 use Ibexa\Core\MVC\Symfony\Locale\LocaleConverterInterface;
+use JMS\TranslationBundle\Annotation\Ignore;
 use Locale;
 use NumberFormatter;
 use Symfony\Contracts\Translation\TranslatorInterface;
@@ -110,7 +111,9 @@ public function sizeFilter($number, $precision)
             $i = ($index - 1);
         }
         $formatter = new NumberFormatter($this->getLocale(), NumberFormatter::PATTERN_DECIMAL);
-        $formatter->setPattern($formatter->getPattern() . ' ' . $this->translator->trans($this->suffixes[$i]));
+        $formatter->setPattern(
+            $formatter->getPattern() . ' ' . $this->translator->trans(/** @Ignore */$this->suffixes[$i])
+        );
         $formatter->setAttribute(NumberFormatter::MAX_FRACTION_DIGITS, $precision);
 
         return $formatter->format($number);

diff --git a/src/lib/MVC/Symfony/Translation/ExceptionMessageTemplateFileVisitor.php b/src/lib/MVC/Symfony/Translation/ExceptionMessageTemplateFileVisitor.php
@@ -6,13 +6,126 @@
  */
 namespace Ibexa\Core\MVC\Symfony\Translation;
 
+use Doctrine\Common\Annotations\DocParser;
+use JMS\TranslationBundle\Annotation\Ignore;
+use JMS\TranslationBundle\Model\Message;
+use JMS\TranslationBundle\Model\MessageCatalogue;
 use JMS\TranslationBundle\Translation\Extractor\File\DefaultPhpFileExtractor;
+use JMS\TranslationBundle\Translation\FileSourceFactory;
+use PhpParser\Node;
+use PhpParser\Node\Scalar\String_;
+use PhpParser\NodeTraverser;
+use Psr\Log\LoggerAwareTrait;
+use SplFileInfo;
 
 class ExceptionMessageTemplateFileVisitor extends DefaultPhpFileExtractor
 {
-    protected $methodsToExtractFrom = ['setmessagetemplate' => -1];
+    use LoggerAwareTrait;
 
-    protected $defaultDomain = 'repository_exceptions';
+    /** @var array<string, int> */
+    protected $methodsToExtractFrom = ['setMessageTemplate' => -1];
+
+    protected string $defaultDomain = 'repository_exceptions';
+
+    private FileSourceFactory $fileSourceFactory;
+
+    private NodeTraverser $traverser;
+
+    private SplFileInfo $file;
+
+    private MessageCatalogue $catalogue;
+
+    private Node $previousNode;
+
+    private DocParser $docParser;
+
+    public function __construct(DocParser $docParser, FileSourceFactory $fileSourceFactory)
+    {
+        parent::__construct($docParser, $fileSourceFactory);
+        $this->fileSourceFactory = $fileSourceFactory;
+        $this->docParser = $docParser;
+        $this->traverser = new NodeTraverser();
+        $this->traverser->addVisitor($this);
+    }
+
+    public function enterNode(Node $node): void
+    {
+        $methodCallNodeName = null;
+        if ($node instanceof Node\Expr\MethodCall) {
+            $methodCallNodeName = $node->name instanceof Node\Identifier ? $node->name->name : $node->name;
+        }
+        if (
+            !is_string($methodCallNodeName)
+            || !array_key_exists($methodCallNodeName, $this->methodsToExtractFrom)
+        ) {
+            $this->previousNode = $node;
+
+            return;
+        }
+
+        $ignore = $this->isIgnore($node);
+
+        if (!$node->args[0]->value instanceof String_) {
+            if ($ignore) {
+                return;
+            }
+
+            $message = sprintf('Can only extract the translation id from a scalar string, but got "%s". Please refactor your code to make it extractable, or add the doc comment /** @Ignore */ to this code element (in %s on line %d).', get_class($node->args[0]->value), $this->file, $node->args[0]->value->getLine());
+
+            $this->logger->error($message);
+
+            return;
+        }
+
+        $id = $node->args[0]->value->value;
+
+        $message = new Message($id, $this->defaultDomain);
+        $message->addSource($this->fileSourceFactory->create($this->file, $node->getLine()));
+        $this->catalogue->add($message);
+    }
+
+    public function visitPhpFile(SplFileInfo $file, MessageCatalogue $catalogue, array $ast): void
+    {
+        $this->file = $file;
+        $this->catalogue = $catalogue;
+        $this->traverser->traverse($ast);
+    }
+
+    private function getDocCommentForNode(Node $node): ?string
+    {
+        if (null !== $comment = $node->args[0]->getDocComment()) {
+            return $comment->getText();
+        }
+
+        if (null !== $comment = $node->getDocComment()) {
+            return $comment->getText();
+        }
+
+        if (null !== $this->previousNode && $this->previousNode->getDocComment() !== null) {
+            $comment = $this->previousNode->getDocComment();
+
+            return is_object($comment) ? $comment->getText() : $comment;
+        }
+
+        return null;
+    }
+
+    private function isIgnore($node): bool
+    {
+        if (null !== $docComment = $this->getDocCommentForNode($node)) {
+            $annotations = $this->docParser->parse(
+                $docComment,
+                'file ' . $this->file . ' near line ' . $node->getLine()
+            );
+            foreach ($annotations as $annot) {
+                if ($annot instanceof Ignore) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
 }
 
 class_alias(ExceptionMessageTemplateFileVisitor::class, 'eZ\Publish\Core\MVC\Symfony\Translation\ExceptionMessageTemplateFileVisitor');
diff --git a/tests/lib/MVC/Symfony/Translation/ExceptionMessageTemplateFileVisitorTest.php b/tests/lib/MVC/Symfony/Translation/ExceptionMessageTemplateFileVisitorTest.php
@@ -0,0 +1,102 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Tests\Core\MVC\Symfony\Translation;
+
+use Doctrine\Common\Annotations\DocParser;
+use Ibexa\Core\MVC\Symfony\Translation\ExceptionMessageTemplateFileVisitor;
+use JMS\TranslationBundle\Model\Message;
+use JMS\TranslationBundle\Model\MessageCatalogue;
+use JMS\TranslationBundle\Translation\FileSourceFactory;
+use PhpParser\Lexer;
+use PhpParser\Parser;
+use PhpParser\ParserFactory;
+use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
+use SplFileInfo;
+
+final class ExceptionMessageTemplateFileVisitorTest extends TestCase
+{
+    private const FIXTURES_DIR = __DIR__ . '/fixtures/';
+
+    private Parser $phpParser;
+
+    private ExceptionMessageTemplateFileVisitor $exceptionMessageTemplateFileVisitor;
+
+    protected function setUp(): void
+    {
+        $docParser = new DocParser();
+        $fileSourceFactory = new FileSourceFactory(
+            self::FIXTURES_DIR,
+        );
+        $lexer = new Lexer();
+        $factory = new ParserFactory();
+        $this->phpParser = $factory->create(ParserFactory::PREFER_PHP7, $lexer);
+        $this->exceptionMessageTemplateFileVisitor = new ExceptionMessageTemplateFileVisitor(
+            $docParser,
+            $fileSourceFactory
+        );
+    }
+
+    public function testExtractTranslation(): void
+    {
+        $messageCatalogue = new MessageCatalogue();
+        $file = self::FIXTURES_DIR . 'SetMessageTemplate.php';
+        $fileInfo = new SplFileInfo($file);
+
+        $ast = $this->phpParser->parse(file_get_contents($file));
+        $this->exceptionMessageTemplateFileVisitor->visitPhpFile(
+            $fileInfo,
+            $messageCatalogue,
+            $ast
+        );
+
+        $expectedMessage = new Message('Foo exception', 'repository_exceptions');
+
+        self::assertTrue(
+            $messageCatalogue->has($expectedMessage)
+        );
+    }
+
+    public function testNoTranslationToExtract(): void
+    {
+        $messageCatalogue = new MessageCatalogue();
+        $file = self::FIXTURES_DIR . 'NoTranslationToExtract.php';
+        $fileInfo = new SplFileInfo($file);
+
+        $ast = $this->phpParser->parse(file_get_contents($file));
+        $this->exceptionMessageTemplateFileVisitor->visitPhpFile(
+            $fileInfo,
+            $messageCatalogue,
+            $ast
+        );
+
+        self::assertEmpty($messageCatalogue->getDomains());
+    }
+
+    public function testWrongTranslationId(): void
+    {
+        $messageCatalogue = new MessageCatalogue();
+        $file = self::FIXTURES_DIR . 'WrongTranslationId.php';
+        $fileInfo = new SplFileInfo($file);
+
+        $ast = $this->phpParser->parse(file_get_contents($file));
+
+        $logger = $this->createMock(LoggerInterface::class);
+        $logger
+            ->expects($this->once())
+            ->method('error');
+
+        $this->exceptionMessageTemplateFileVisitor->setLogger($logger);
+        $this->exceptionMessageTemplateFileVisitor->visitPhpFile(
+            $fileInfo,
+            $messageCatalogue,
+            $ast
+        );
+    }
+}
diff --git a/tests/lib/MVC/Symfony/Translation/fixtures/NoTranslationToExtract.php b/tests/lib/MVC/Symfony/Translation/fixtures/NoTranslationToExtract.php
@@ -0,0 +1,24 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Tests\Core\MVC\Symfony\Translation\fixtures;
+
+use Exception;
+use Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException as APIInvalidArgumentException;
+use Ibexa\Core\Base\Translatable;
+use Ibexa\Core\Base\TranslatableBase;
+
+final class NoTranslationToExtract extends APIInvalidArgumentException implements Translatable
+{
+    use TranslatableBase;
+
+    public function __construct(?Exception $previous = null)
+    {
+        parent::__construct($this->getBaseTranslation(), 0, $previous);
+    }
+}