fix: parse only string literals (#16)

Author: ramsey

src/Extractor/Parser/Descriptor/DescriptorCollectorVisitor.php

@@ -37,6 +37,7 @@
 use function assert;
 use function in_array;
 use function preg_replace;
+use function sprintf;
 use function trim;
 
 /**
@@ -200,9 +201,8 @@ private function parseDescriptorProperties(Node\Expr\Array_ $descriptor): array
 
             assert($item !== null);
             assert($item->key instanceof Node\Scalar\String_);
-            assert($item->value instanceof Node\Scalar\String_);
 
-            $properties[$item->key->value] = $item->value->value;
+            $properties[$item->key->value] = $this->getValue($item->value);
         }
 
         return $properties;
@@ -212,7 +212,7 @@ private function isValidDescriptorItem(?Node\Expr\ArrayItem $item): bool
     {
         return $item !== null
             && $item->key instanceof Node\Scalar\String_
-            && $item->value instanceof Node\Scalar\String_;
+            && $this->isValidValue($item->value);
     }
 
     private function clean(?string $value): ?string
@@ -238,4 +238,35 @@ private function ensureId(DescriptorInterface $descriptor): DescriptorInterface
 
         return $descriptor;
     }
+
+    private function isValidValue(Node\Expr $value): bool
+    {
+        if ($value instanceof Node\Scalar\String_) {
+            return true;
+        }
+
+        if ($value instanceof Node\Expr\BinaryOp\Concat) {
+            $isValid = $this->isValidValue($value->left);
+
+            return $isValid && $this->isValidValue($value->right);
+        }
+
+        throw new UnableToParseDescriptorException(sprintf(
+            'The descriptor must not contain values other than string literals; encountered %s',
+            $value->getType(),
+        ));
+    }
+
+    private function getValue(Node\Expr $value): string
+    {
+        if ($value instanceof Node\Scalar\String_) {
+            return $value->value;
+        }
+
+        assert($value instanceof Node\Expr\BinaryOp\Concat);
+
+        $contents = $this->getValue($value->left);
+
+        return $contents . $this->getValue($value->right);
+    }
 }

tests/Extractor/MessageExtractorTest.php

@@ -424,6 +424,12 @@ public function testProcessWithCustomParser(): void
                 . '/Parser/Descriptor/fixtures/php-parser-04.php on line 40',
             'Descriptor argument must be present in ' . __DIR__
                 . '/Parser/Descriptor/fixtures/php-parser-09.phtml on line 18',
+            'The descriptor must not contain values other than string literals; '
+                . 'encountered Expr_Variable in ' . __DIR__
+                . '/Parser/Descriptor/fixtures/php-parser-10.php on line 6',
+            'The descriptor must not contain values other than string literals; '
+                . 'encountered Scalar_Encapsed in ' . __DIR__
+                . '/Parser/Descriptor/fixtures/php-parser-10.php on line 12',
             'Missing "defaultMessage" in "{{#formatMessage |idWithoutMessage}}{{/formatMessage}}" in '
                 . __DIR__ . '/Parser/Descriptor/fixtures/custom-parser-01.template on line 0',
             'Missing "id" in "{{#formatMessage}}message without ID{{/formatMessage}}" in '
@@ -499,6 +505,12 @@ public function testProcessWithCustomParserAsClosure(): void
             . '/Parser/Descriptor/fixtures/php-parser-04.php on line 40',
             'Descriptor argument must be present in ' . __DIR__
             . '/Parser/Descriptor/fixtures/php-parser-09.phtml on line 18',
+            'The descriptor must not contain values other than string literals; '
+            . 'encountered Expr_Variable in ' . __DIR__
+            . '/Parser/Descriptor/fixtures/php-parser-10.php on line 6',
+            'The descriptor must not contain values other than string literals; '
+            . 'encountered Scalar_Encapsed in ' . __DIR__
+            . '/Parser/Descriptor/fixtures/php-parser-10.php on line 12',
             'Missing "defaultMessage" in "{{#formatMessage |idWithoutMessage}}{{/formatMessage}}" in '
             . __DIR__ . '/Parser/Descriptor/fixtures/custom-parser-01.template on line 0',
             'Missing "id" in "{{#formatMessage}}message without ID{{/formatMessage}}" in '

tests/Extractor/Parser/Descriptor/PhpParserTest.php

@@ -31,7 +31,7 @@ public function testParse01(): void
             [
                 'defaultMessage' => 'This is a default message',
                 'description' => 'A simple description of a fixture for testing purposes.',
-                'end' => 202,
+                'end' => 326,
                 'file' => __DIR__ . '/fixtures/php-parser-01.php',
                 'id' => 'aTestId',
                 'line' => 3,
@@ -323,6 +323,29 @@ public function testParse09(): void
         );
     }
 
+    public function testParse10(): void
+    {
+        $errors = new ParserErrorCollection();
+        $options = new MessageExtractorOptions();
+        $parser = new PhpParser(new FileSystemHelper());
+        $descriptors = $parser(__DIR__ . '/fixtures/php-parser-10.php', $options, $errors);
+        $receivedErrors = $this->compileErrors($errors);
+
+        $this->assertContainsOnlyInstancesOf(DescriptorInterface::class, $descriptors);
+        $this->assertCount(0, $descriptors);
+        $this->assertSame(
+            [
+                'The descriptor must not contain values other than string literals; '
+                    . 'encountered Expr_Variable on line 6 in '
+                    . __DIR__ . '/fixtures/php-parser-10.php',
+                'The descriptor must not contain values other than string literals; '
+                    . 'encountered Scalar_Encapsed on line 12 in '
+                    . __DIR__ . '/fixtures/php-parser-10.php',
+            ],
+            $receivedErrors,
+        );
+    }
+
     /**
      * @return string[]
      */

tests/Extractor/Parser/Descriptor/fixtures/php-parser-01.php

@@ -2,6 +2,9 @@
 
 $internationalization->formatMessage([
     'id' => 'aTestId',
-    'defaultMessage' => 'This is a default message',
-    'description' => 'A simple description of a fixture for testing purposes.',
+    // We're using concatenated string literals on purpose, to test this functionality.
+    'defaultMessage' => 'This is a ' . 'default ' . 'message',
+    'description' => 'A simple description '
+        . 'of a fixture '
+        . 'for testing purposes.',
 ]);

tests/Extractor/Parser/Descriptor/fixtures/php-parser-10.php

@@ -0,0 +1,17 @@
+<?php
+
+$bar = 'default ';
+$description = 'description';
+
+$internationalization->formatMessage([
+    'id' => 'message.with.variable.concat',
+    // Injecting a variable here to break things.
+    'defaultMessage' => 'This is a ' . $bar . 'message',
+]);
+
+$internationalization->formatMessage([
+    'id' => 'message.with.variable.interpolated',
+    // Injecting a variable here to break things.
+    'defaultMessage' => 'This is a default message',
+    'description' => "This is a $description with an interpolated variable.",
+]);