From ae50f5ae1572c79f81b03980badef63c9aa6dd3a Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <guy.sartorelli@silverstripe.com>
Date: Tue, 6 Aug 2024 11:18:37 +1200
Subject: [PATCH] NEW Add rule to catch malformed _t() calls

---
 rules.neon                                    |  1 +
 src/PHPStan/TranslationFunctionRule.php       | 96 +++++++++++++++++++
 tests/PHPStan/TranslationFunctionRuleTest.php | 58 +++++++++++
 .../TranslationFunctionRuleTest/InClass.php   | 27 ++++++
 .../InClassCorrect.php                        | 21 ++++
 .../raw-php-correct.php                       | 14 +++
 .../TranslationFunctionRuleTest/raw-php.php   | 20 ++++
 7 files changed, 237 insertions(+)
 create mode 100644 src/PHPStan/TranslationFunctionRule.php
 create mode 100644 tests/PHPStan/TranslationFunctionRuleTest.php
 create mode 100644 tests/PHPStan/TranslationFunctionRuleTest/InClass.php
 create mode 100644 tests/PHPStan/TranslationFunctionRuleTest/InClassCorrect.php
 create mode 100644 tests/PHPStan/TranslationFunctionRuleTest/raw-php-correct.php
 create mode 100644 tests/PHPStan/TranslationFunctionRuleTest/raw-php.php

diff --git a/rules.neon b/rules.neon
index 56a6367..827b7ee 100644
--- a/rules.neon
+++ b/rules.neon
@@ -1,6 +1,7 @@
 rules:
   - SilverStripe\Standards\PHPStan\MethodAnnotationsRule
   - SilverStripe\Standards\PHPStan\KeywordSelfRule
+  - SilverStripe\Standards\PHPStan\TranslationFunctionRule
 
 parameters:
   # Setting customRulestUsed to true allows us to avoid using the built-in rules
diff --git a/src/PHPStan/TranslationFunctionRule.php b/src/PHPStan/TranslationFunctionRule.php
new file mode 100644
index 0000000..2b1d127
--- /dev/null
+++ b/src/PHPStan/TranslationFunctionRule.php
@@ -0,0 +1,96 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SilverStripe\Standards\PHPStan;
+
+use PhpParser\Node;
+use PhpParser\Node\Arg;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Name;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\Constant\ConstantStringType;
+
+/**
+ * Validates that the first argument to the _t() function isn't malformed.
+ *
+ * See https://phpstan.org/developing-extensions/rules
+ *
+ * @implements Rule<FuncCall|StaticCall>
+ */
+class TranslationFunctionRule implements Rule
+{
+    public function getNodeType(): string
+    {
+        return Node::class;
+    }
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if ($node instanceof FuncCall) {
+            return $this->processFuncCall($node, $scope);
+        }
+        if ($node instanceof StaticCall) {
+            return $this->processStaticCall($node, $scope);
+        }
+        return [];
+    }
+
+    /**
+     * Process calls to the global function _t()
+     */
+    private function processFuncCall(FuncCall $node, Scope $scope): array
+    {
+        if ($node->name->__toString() !== '_t') {
+            return [];
+        }
+        return $this->processArgs($node->getArgs(), $scope);
+    }
+
+    /**
+     * Process calls to the static method i18n::_t()
+     */
+    private function processStaticCall(StaticCall $node, Scope $scope): array
+    {
+        $class = $node->class;
+        if (!($class instanceof Name)) {
+            return [];
+        }
+        if ($class->toString() !== 'SilverStripe\i18n\i18n') {
+            return [];
+        }
+        return $this->processArgs($node->getArgs(), $scope);
+    }
+
+    /**
+     * Check that the first arg value can be evaluated and has exactly one period.
+     *
+     * @param Arg[] $args
+     */
+    private function processArgs(array $args, Scope $scope): array
+    {
+        // If we have no args PHP itself will complain and it'll be caught by other linting, so just skip.
+        if (count($args) < 1) {
+            return [];
+        }
+
+        $entityArg = $scope->getType($args[0]->value);
+        // If phpstan can't get us a nice clear value, text collector almost certainly can't either.
+        if (!($entityArg instanceof ConstantStringType)) {
+            return [
+                RuleErrorBuilder::message(
+                    'Can\'t determine value of first argument to _t(). Use a simpler value.'
+                )->build()
+            ];
+        }
+
+        if (substr_count($entityArg->getValue(), '.') !== 1) {
+            return [RuleErrorBuilder::message('First argument passed to _t() must have exactly one period.')->build()];
+        }
+
+        return [];
+    }
+}
diff --git a/tests/PHPStan/TranslationFunctionRuleTest.php b/tests/PHPStan/TranslationFunctionRuleTest.php
new file mode 100644
index 0000000..0dada4d
--- /dev/null
+++ b/tests/PHPStan/TranslationFunctionRuleTest.php
@@ -0,0 +1,58 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SilverStripe\Standards\Tests\PHPStan;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use SilverStripe\Standards\PHPStan\TranslationFunctionRule;
+
+/**
+ * @extends RuleTestCase<TranslationFunctionRule>
+ */
+class TranslationFunctionRuleTest extends RuleTestCase
+{
+    protected function getRule(): Rule
+    {
+        return new TranslationFunctionRule();
+    }
+
+    public function provideRule()
+    {
+        return [
+            'no class scope' => [
+                'filePaths' => [__DIR__ . '/TranslationFunctionRuleTest/raw-php.php'],
+                'errorMessage' => 'First argument passed to _t() must have exactly one period.',
+                'errorLines' => [8,9,10,11,12,14,15],
+            ],
+            'no class scope, no errors' => [
+                'filePaths' => [__DIR__ . '/TranslationFunctionRuleTest/raw-php-correct.php'],
+                'errorMessage' => '',
+                'errorLines' => [],
+            ],
+            'in class scope' => [
+                'filePaths' => [__DIR__ . '/TranslationFunctionRuleTest/InClass.php'],
+                'errorMessage' => 'First argument passed to _t() must have exactly one period.',
+                'errorLines' => [13,14,15,16,17,19,20],
+            ],
+            'in class scope, no errors' => [
+                'filePaths' => [__DIR__ . '/TranslationFunctionRuleTest/InClassCorrect.php'],
+                'errorMessage' => '',
+                'errorLines' => [],
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideRule
+     */
+    public function testRule(array $filePaths, string $errorMessage, array $errorLines): void
+    {
+        $errors = [];
+        foreach ($errorLines as $line) {
+            $errors[] = [$errorMessage, $line];
+        }
+        $this->analyse($filePaths, $errors);
+    }
+}
diff --git a/tests/PHPStan/TranslationFunctionRuleTest/InClass.php b/tests/PHPStan/TranslationFunctionRuleTest/InClass.php
new file mode 100644
index 0000000..885bb23
--- /dev/null
+++ b/tests/PHPStan/TranslationFunctionRuleTest/InClass.php
@@ -0,0 +1,27 @@
+<?php
+
+namespace SilverStripe\Standards\Tests\PHPStan\TranslationFunctionRuleTest;
+
+use SilverStripe\i18n\i18n;
+
+class InClass
+{
+    public const MY_ENTITY = 'entity';
+
+    public function myMethod()
+    {
+        _t('abc');
+        _t(InClass::class);
+        _t(InClass::class . 'somekey');
+        _t(InClass::MY_ENTITY);
+        _t(InClass::class . InClass::MY_ENTITY);
+        $variable = 'somethingwrong';
+        _t($variable);
+        i18n::_t('nodot');
+
+        // These should be ignored
+        SomeClass::_t('abc');
+        $x = new SomeClass();
+        $x->_t('abc');
+    }
+}
diff --git a/tests/PHPStan/TranslationFunctionRuleTest/InClassCorrect.php b/tests/PHPStan/TranslationFunctionRuleTest/InClassCorrect.php
new file mode 100644
index 0000000..f9621dc
--- /dev/null
+++ b/tests/PHPStan/TranslationFunctionRuleTest/InClassCorrect.php
@@ -0,0 +1,21 @@
+<?php
+
+namespace SilverStripe\Standards\Tests\PHPStan\TranslationFunctionRuleTest;
+
+use SilverStripe\i18n\i18n;
+
+class InClassCorrect
+{
+    public const MY_ENTITY = 'entity';
+
+    public function myMethod()
+    {
+        _t('abc.123');
+        _t(InClass::class . '.somekey');
+        _t('something.' . InClass::MY_ENTITY);
+        _t(InClass::class . '.' . InClass::MY_ENTITY);
+        $variable = 'nothing.wrong';
+        _t($variable);
+        i18n::_t('with.dot');
+    }
+}
diff --git a/tests/PHPStan/TranslationFunctionRuleTest/raw-php-correct.php b/tests/PHPStan/TranslationFunctionRuleTest/raw-php-correct.php
new file mode 100644
index 0000000..7623a17
--- /dev/null
+++ b/tests/PHPStan/TranslationFunctionRuleTest/raw-php-correct.php
@@ -0,0 +1,14 @@
+<?php
+
+use App\SomeClass;
+use SilverStripe\i18n\i18n;
+
+const MY_ENTITY = 'entity';
+
+_t('abc.123');
+_t(SomeClass::class . '.somekey');
+_t('something.' . MY_ENTITY);
+_t(SomeClass::class . '.' . MY_ENTITY);
+$variable = 'nothing.wrong';
+_t($variable);
+i18n::_t('with.dot');
diff --git a/tests/PHPStan/TranslationFunctionRuleTest/raw-php.php b/tests/PHPStan/TranslationFunctionRuleTest/raw-php.php
new file mode 100644
index 0000000..a2182f7
--- /dev/null
+++ b/tests/PHPStan/TranslationFunctionRuleTest/raw-php.php
@@ -0,0 +1,20 @@
+<?php
+
+use App\SomeClass;
+use SilverStripe\i18n\i18n;
+
+const MY_ENTITY = 'entity';
+
+_t('abc');
+_t(SomeClass::class);
+_t(SomeClass::class . 'somekey');
+_t(MY_ENTITY);
+_t(SomeClass::class . MY_ENTITY);
+$variable = 'somethingwrong';
+_t($variable);
+i18n::_t('nodot');
+
+// These should be ignored
+SomeClass::_t('abc');
+$x = new SomeClass();
+$x->_t('abc');