Merge pull request #11603 from creative-commoners/pulls/5/fix-i18n

FIX Prevent invalid i18n sources from being collected
silverstripe · Feb 12, 2025 · 41c6d6c · 41c6d6c
2 parents 61a384d + fad032a
commit 41c6d6c
Show file tree

Hide file tree

Showing 4 changed files with 81 additions and 3 deletions.
diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php
@@ -4429,15 +4429,19 @@ public function provideI18nEntities()
         $pluralName = $this->plural_name();
         $singularName = $this->singular_name();
         $conjunction = preg_match('/^[aeiou]/i', $singularName ?? '') ? 'An ' : 'A ';
-        return [
-            static::class . '.CLASS_DESCRIPTION' => $this->classDescription(),
+        $entities = [
             static::class . '.SINGULARNAME' => $singularName,
             static::class . '.PLURALNAME' => $pluralName,
             static::class . '.PLURALS' => [
                 'one' => $conjunction . $singularName,
                 'other' => '{count} ' . $pluralName
             ]
         ];
+        $classDescription = $this->classDescription();
+        if ($classDescription) {
+            $entities[static::class . '.CLASS_DESCRIPTION'] = $classDescription;
+        }
+        return $entities;
     }
 
     /**

diff --git a/src/i18n/TextCollection/i18nTextCollector.php b/src/i18n/TextCollection/i18nTextCollector.php
@@ -883,7 +883,7 @@ public function collectFromCode($content, $fileName, Module $module)
                     $inTransFn = false;
                     $inConcat = false;
                     // Ensure key is valid before saving
-                    if (!empty($currentEntity[0])) {
+                    if (!empty($currentEntity[0]) && !str_ends_with($currentEntity[0], '.')) {
                         $key = $currentEntity[0];
                         $default = $currentEntity[1] ?? '';
                         $comment = $currentEntity[2] ?? '';

diff --git a/tests/php/ORM/DataObjectTest.php b/tests/php/ORM/DataObjectTest.php
@@ -2840,4 +2840,56 @@ public function testExceptionForUniqueIndexViolation(array $fieldsRecordOne, arr
         $this->expectExceptionMessage($expectedMessage);
         $record2->write();
     }
+
+    public static function provideProvideI18nEntities(): array
+    {
+        return [
+            'has-class-description' => [
+                'classDescription' => 'A fluffy cloud',
+                'expected' => true,
+            ],
+            'no-class-description' => [
+                'classDescription' => null,
+                'expected' => false,
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideProvideI18nEntities
+     */
+    public function testProvideI18nEntities(?string $classDescription, bool $expected): void
+    {
+        $obj = new class extends DataObject {
+            public $classDescription;
+            public function singular_name()
+            {
+                return 'Cloud';
+            }
+            public function plural_name()
+            {
+                return 'Clouds';
+            }
+            public function classDescription()
+            {
+                return $this->classDescription;
+            }
+        };
+        $obj->classDescription = $classDescription;
+        $entities = $obj->provideI18nEntities();
+        // Fix up anonymous class keys
+        foreach ($entities as $key => $entity) {
+            unset($entities[$key]);
+            $newKey = preg_replace('#^.+?\.([A-Z_]+)$#', '$1', $key);
+            $entities[$newKey] = $entity;
+        }
+        $this->assertSame('Cloud', $entities['SINGULARNAME']);
+        $this->assertSame('Clouds', $entities['PLURALNAME']);
+        $this->assertSame(['one' => 'A Cloud', 'other' => '{count} Clouds'], $entities['PLURALS']);
+        if ($expected) {
+            $this->assertSame('A fluffy cloud', $entities['CLASS_DESCRIPTION']);
+        } else {
+            $this->assertFalse(array_key_exists('CLASS_DESCRIPTION', $entities));
+        }
+    }
 }
diff --git a/tests/php/i18n/i18nTextCollectorTest.php b/tests/php/i18n/i18nTextCollectorTest.php
@@ -1028,4 +1028,26 @@ public function testItCanUseVariableAsContext()
             'TestEntity.REGULARCONTEXT' => "test {type}",
         ], $collectedTranslatables);
     }
+
+    public function testDoesNotCollectInvalidKeys()
+    {
+        // From the code below this will previously collect `'.' => 'generic'`
+        // Code was added to i18nTextCollector::collectFromCode() to ignore keys
+        // that end with "."
+        $c = i18nTextCollector::create();
+        $mymodule = ModuleLoader::inst()->getManifest()->getModule('i18ntestmodule');
+        $php = <<<'PHP'
+        $data = [
+            $foo,
+            static::get_something($foo),
+            'generic'
+        ];
+        _t(
+            __CLASS__ . '.' . ucfirst($foo) . 'Type',
+            $hello[$foo]
+        );
+        PHP;
+        $collectedTranslatables = $c->collectFromCode($php, null, $mymodule);
+        $this->assertEmpty($collectedTranslatables);
+    }
 }