Skip to content

Commit

Permalink
NEW Add rule to catch malformed _t() calls
Browse files Browse the repository at this point in the history
  • Loading branch information
GuySartorelli committed Aug 6, 2024
1 parent ee6a851 commit d5819ee
Show file tree
Hide file tree
Showing 7 changed files with 321 additions and 0 deletions.
1 change: 1 addition & 0 deletions rules.neon
Original file line number Diff line number Diff line change
@@ -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
Expand Down
152 changes: 152 additions & 0 deletions src/PHPStan/TranslationFunctionRule.php
Original file line number Diff line number Diff line change
@@ -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;
}
}
58 changes: 58 additions & 0 deletions tests/PHPStan/TranslationFunctionRuleTest.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
39 changes: 39 additions & 0 deletions tests/PHPStan/TranslationFunctionRuleTest/InClass.php
Original file line number Diff line number Diff line change
@@ -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');
}
}
24 changes: 24 additions & 0 deletions tests/PHPStan/TranslationFunctionRuleTest/InClassCorrect.php
Original file line number Diff line number Diff line change
@@ -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');
}
}
16 changes: 16 additions & 0 deletions tests/PHPStan/TranslationFunctionRuleTest/raw-php-correct.php
Original file line number Diff line number Diff line change
@@ -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');
31 changes: 31 additions & 0 deletions tests/PHPStan/TranslationFunctionRuleTest/raw-php.php
Original file line number Diff line number Diff line change
@@ -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');

0 comments on commit d5819ee

Please sign in to comment.