NEW Add rule to catch malformed _t() calls (#12)

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

src/PHPStan/TranslationFunctionRule.php

@@ -0,0 +1,152 @@
+<?php
+
+declare(strict_types=1);
+
+namespace SilverStripe\Standards\PHPStan;
+
+use PhpParser\Node;
+use PhpParser\Node\Arg;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\BinaryOp\Concat;
+use PhpParser\Node\Expr\FuncCall;
+use PhpParser\Node\Expr\StaticCall;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Name;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\TypeWithClassName;
+use PHPStan\Type\Generic\GenericClassStringType;
+
+/**
+ * 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 (!$this->callingUnderscoreT($node->name, $scope)) {
+            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 [];
+        }
+        if (!$this->callingUnderscoreT($node->name, $scope)) {
+            return [];
+        }
+        return $this->processArgs($node->getArgs(), $scope);
+    }
+
+    /**
+     * Check if the method/function is called _t
+     */
+    private function callingUnderscoreT(Name|Identifier|Expr $name, Scope $scope)
+    {
+        if (($name instanceof Name) || ($name instanceof Identifier)) {
+            $name = $name->toString();
+        } else {
+            $nameType = $scope->getType($name);
+            // Ignore callables we can't get the names of
+            if (!method_exists($nameType, 'getValue')) {
+                return false;
+            }
+            $name = $nameType->getValue();
+        }
+        return $name === '_t';
+    }
+
+    /**
+     * 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 = $args[0]->value;
+        $argValue = $this->getStringValue($entityArg, $scope);
+        // If phpstan can't get us a nice clear value, text collector almost certainly can't either.
+        if ($argValue === null) {
+            return [
+                RuleErrorBuilder::message(
+                    'Can\'t determine value of first argument to _t(). Use a simpler value.'
+                )->build()
+            ];
+        }
+
+        if (substr_count($argValue, '.') !== 1) {
+            return [RuleErrorBuilder::message('First argument passed to _t() must have exactly one period.')->build()];
+        }
+
+        return [];
+    }
+
+    /**
+     * Get a string value from a node, if one can be derived.
+     */
+    private function getStringValue(Node $node, Scope $scope): ?string
+    {
+        // e.g. MyClass
+        if (($node instanceof Name) || ($node instanceof Identifier)) {
+            return $node->toString();
+        }
+        $type = $scope->getType($node);
+        // Variables and other types PHPStan can directly reason about
+        if (method_exists($type, 'getValue')) {
+            return $type->getValue();
+        }
+        // e.g. static::class
+        if (($type instanceof GenericClassStringType) && ($type->getGenericType() instanceof TypeWithClassName)) {
+            return $type->getGenericType()->getClassName();
+        }
+        // e.g. static::class . '.myKey'
+        if ($node instanceof Concat) {
+            $left = $this->getStringValue($node->left, $scope);
+            $right = $this->getStringValue($node->right, $scope);
+            if ($left === null || $right === null) {
+                return null;
+            }
+            return $left . $right;
+        }
+        return null;
+    }
+}

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,17],
+            ],
+            '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,22,23],
+            ],
+            '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);
+    }
+}

tests/PHPStan/TranslationFunctionRuleTest/InClass.php

@@ -0,0 +1,39 @@
+<?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(__CLASS__ . InClass::MY_ENTITY);
+        $variable = 'somethingwrong';
+        _t($variable);
+        i18n::_t('nodot');
+        $callableString = '_t';
+        $callableString('nodot');
+        i18n::_t(static::class . 'key');
+
+        // These should be ignored
+        SomeClass::_t('abc');
+        $x = new SomeClass();
+        $x->_t('abc');
+        $callable = [i18n::class, '_t'];
+        $callable('abc');
+        $callable2 = fn ($x) => $x;
+        $callable2('abc');
+        $callable3 = function ($x) {
+            return $x;
+        };
+        $callable3('abc');
+        i18n::set_locale('en-us');
+    }
+}

tests/PHPStan/TranslationFunctionRuleTest/InClassCorrect.php

@@ -0,0 +1,24 @@
+<?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(__CLASS__ . '.' . InClass::MY_ENTITY);
+        $variable = 'nothing.wrong';
+        _t($variable);
+        i18n::_t('with.dot');
+        $callableString = '_t';
+        $callableString('with.dot');
+        i18n::_t(static::class . '.key');
+    }
+}

tests/PHPStan/TranslationFunctionRuleTest/raw-php-correct.php

@@ -0,0 +1,16 @@
+<?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');
+$callableString = '_t';
+$callableString('with.dot');

tests/PHPStan/TranslationFunctionRuleTest/raw-php.php

@@ -0,0 +1,31 @@
+<?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');
+$callableString = '_t';
+$callableString('nodot');
+
+// These should be ignored
+SomeClass::_t('abc');
+$x = new SomeClass();
+$x->_t('abc');
+$callable = [i18n::class, '_t'];
+$callable('abc');
+$callable2 = fn ($x) => $x;
+$callable2('abc');
+$callable3 = function ($x) {
+    return $x;
+};
+$callable3('abc');
+i18n::set_locale('en-us');