ContainerAwareTrans should not add empty strings to the SourceCollection

[nicwortel]

Might be somewhat related to https://github.com/php-translation/extractor/issues/125.

The ContainerAwareTrans visitor unconditionally creates a source location, even if the result of getStringArgument is null.

https://github.com/php-translation/extractor/blob/82fc51a07fb626c0bfb31fb3f041baa88fee6acb/src/Visitor/Php/Symfony/ContainerAwareTrans.php#L38-L44

In my particular case this happens because I concatenate a string with a variable:

$this->translator->trans('message.audit_log.' . $eventType);

Which results in the following translation unit being added to my messages.en.xlf:

    <unit id="47DEQpj" name="">
      <segment>
        <source></source>
        <target></target>
      </segment>
    </unit>

I believe the $this->addLocation call should be wrapped in an if (null !== $label = $this->getStringArgument($node, 1)), similar to what happens in the FlashMessage visitor:

https://github.com/php-translation/extractor/blob/82fc51a07fb626c0bfb31fb3f041baa88fee6acb/src/Visitor/Php/Symfony/FlashMessage.php#L55-L57

[nicwortel]

P.S. I can create a pull request if you agree on this approach.

[nand2]

Hi!

I got the same issue : empty messages were saved in the XLF translation files. A bad consequence for me was that, on each synchronization with localise.biz, a few entries were added for these empty messages.

@nicwortel Your idea is good but there are other places than ContainerAwareTrans.php where empty messages are added.

I did not have time to find them all, so, if it can be useful, here is a patch that will prevent empty messages to be added to the SourceCollection. At least, we are sure no empty messages will be stored.

diff -u a/src/Model/SourceCollection.php b/extractor/src/Model/SourceCollection.php
--- a/src/Model/SourceCollection.php   2019-09-19 17:34:30.264205958 +0200
+++ b/src/Model/SourceCollection.php    2019-09-19 17:34:23.940157572 +0200
@@ -41,6 +41,11 @@
      */
     public function addLocation(SourceLocation $location)
     {
+        // We ignore empty messages, we don't need to translate them
+        if($location->getMessage() == "") {
+            return;
+        }
+
         $this->sourceLocations[] = $location;
     }

[nicwortel]

This issue seems to be fixed thanks to #147.