Skip to content

Commit

Permalink
NEW Add rule to avoid use of "new" keyword
Browse files Browse the repository at this point in the history
  • Loading branch information
GuySartorelli committed Jul 23, 2024
1 parent 62b0416 commit 8795b5f
Show file tree
Hide file tree
Showing 12 changed files with 322 additions and 1 deletion.
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"license": "BSD-3-Clause",
"require": {
"php": "^8.1",
"phpstan/phpstan": "^1.10"
"phpstan/phpstan": "^1.11"
},
"require-dev": {
"phpunit/phpunit": "^9.6",
Expand Down
1 change: 1 addition & 0 deletions rules.neon
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
rules:
- SilverStripe\Standards\PHPStan\MethodAnnotationsRule
- SilverStripe\Standards\PHPStan\KeywordSelfRule

parameters:
# Setting customRulestUsed to true allows us to avoid using the built-in rules
Expand Down
75 changes: 75 additions & 0 deletions src/PHPStan/KeywordSelfRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

declare(strict_types=1);

namespace SilverStripe\Standards\PHPStan;

use PhpParser\Node;
use PhpParser\Node\Name;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

/**
* Validates that the `self` keyword is only used in situations where it's not avoidable.
*
* See https://phpstan.org/developing-extensions/rules
*
* @implements Rule<Node>
*/
class KeywordSelfRule implements Rule
{
public function getNodeType(): string
{
return Node::class;
}

public function processNode(Node $node, Scope $scope): array
{
switch (get_class($node)) {
// fetching a constant, e.g. `self::MY_CONST` or `self::class`
case ClassConstFetch::class:
// Traits can use `self` to get const values - but otherwise follow same
// logic as methods or properties.
if ($scope->isInTrait()) {
return [];
}
// static method call, e.g. `self::myMethod()`
case StaticCall::class:
// fetching a static property, e.g. `self::$my_property`
case StaticPropertyFetch::class:
if (!is_a($node->class, Name::class) || $node->class->toString() !== 'self') {
return [];
}
break;
// `self` as a type (for a property, argument, or return type)
case Name::class:
// Trait can use `self` for typehinting
if ($scope->isInTrait() || $node->toString() !== 'self') {
return [];
}
break;
// instantiating a new object from `self`, e.g. `new self()`
case New_::class:
if (!is_a($node->class, Name::class) || $node->class->toString() !== 'self') {
return [];
}
break;
default:
return [];
}

$actualClass = $scope->isInTrait()
? 'self::class'
: $scope->getClassReflection()->getNativeReflection()->getShortName();
return [
RuleErrorBuilder::message(
"Can't use keyword 'self'. Use '$actualClass' instead."
)->build()
];
}
}
2 changes: 2 additions & 0 deletions src/PHPStan/MethodAnnotationsRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
* Validates that `@method` annotations in DataObject and Extension classes are correct,
* according to the relations defined within those classes.
*
* See https://phpstan.org/developing-extensions/rules
*
* @implements Rule<Class_>
*/
class MethodAnnotationsRule implements Rule
Expand Down
69 changes: 69 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php

declare(strict_types=1);

namespace SilverStripe\Standards\Tests\PHPStan;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use SilverStripe\Standards\PHPStan\KeywordSelfRule;

/**
* @extends RuleTestCase<KeywordSelfRule>
*/
class KeywordSelfRuleTest extends RuleTestCase
{
protected function getRule(): Rule
{
return new KeywordSelfRule();
}

public function provideRule()
{
return [
'interface' => [
'filePaths' => [__DIR__ . '/KeywordSelfRuleTest/TestInterface.php'],
'errorMessage' => "Can't use keyword 'self'. Use 'TestInterface' instead.",
'errorLines' => [13, 18, 18],
],
'class' => [
'filePaths' => [__DIR__ . '/KeywordSelfRuleTest/TestClass.php'],
'errorMessage' => "Can't use keyword 'self'. Use 'TestClass' instead.",
'errorLines' => [9, 11, 16, 16, 18, 20, 21, 21, 25],
],
'enum' => [
'filePaths' => [__DIR__ . '/KeywordSelfRuleTest/TestEnum.php'],
'errorMessage' => "Can't use keyword 'self'. Use 'TestEnum' instead.",
'errorLines' => [9, 14, 14, 16, 17, 18, 20, 24],
],
'trait' => [
'filePaths' => [
__DIR__ . '/KeywordSelfRuleTest/TestTrait.php',
__DIR__ . '/KeywordSelfRuleTest/ClassUsesTrait.php',
],
'errorMessage' => "Can't use keyword 'self'. Use 'self::class' instead.",
'errorLines' => [17, 19, 20, 24],
],
'trait no errors' => [
'filePaths' => [
__DIR__ . '/KeywordSelfRuleTest/TestTraitCorrect.php',
__DIR__ . '/KeywordSelfRuleTest/ClassUsesTraitCorrect.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);
}
}
11 changes: 11 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest/ClassUsesTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace SilverStripe\Standards\Tests\PHPStan\KeywordSelfRuleTest;

/**
* PHPStan doesn't analyse traits unless there's a class that uses it
*/
class ClassUsesTrait
{
use TestTrait;
}
11 changes: 11 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest/ClassUsesTraitCorrect.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace SilverStripe\Standards\Tests\PHPStan\KeywordSelfRuleTest;

/**
* PHPStan doesn't analyse traits unless there's a class that uses it
*/
class ClassUsesTraitCorrect
{
use TestTraitCorrect;
}
33 changes: 33 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest/TestClass.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

namespace SilverStripe\Standards\Tests\PHPStan\KeywordSelfRuleTest;

class TestClass
{
private const MY_CONST = 'some value';

private static string $myProperty = self::MY_CONST;

private static self $mySecondProperty;

/**
* self::class is ignored here because this is a comment.
*/
public static function function1(self $someProperty): self
{
$self = new self();
$self::class; // $self:: isn't seen as self::
self::self();
self::$myProperty = self::class;
/* intentionally commented out to prove even multi-line comments are ignored.
self::$myProperty = self::class;
*/
return new self();
}

public static function self()
{
// doesn't need an implementation - the point of this method is just to show that "self" isn't just being
// searched for naively.
}
}
32 changes: 32 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest/TestEnum.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace SilverStripe\Standards\Tests\PHPStan\KeywordSelfRuleTest;

enum TestEnum: string
{
private const MY_CONST = 'some value';

case self = self::MY_CONST;

/**
* self::class is ignored here because this is a comment.
*/
public static function function1(self $someProperty): self
{
$selfEnumValue = self::self;
$selfName = self::class;
$self = new self();
$self::class; // $self:: isn't seen as self::
self::self();
/* intentionally commented out to prove even multi-line comments are ignored.
self::$myProperty = self::class;
*/
return new self('self');
}

public static function self()
{
// doesn't need an implementation - the point of this method is just to show that "self" isn't just being
// searched for naively.
}
}
21 changes: 21 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest/TestInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

namespace SilverStripe\Standards\Tests\PHPStan\KeywordSelfRuleTest;

/**
* Usage of `self` in an interface refers to the interface itself, unlike with traits.
* This means we should avoid using self, since it's not actually needed here.
*/
interface TestInterface
{
public const MY_CONST = 'some value';

public const MY_SECOND_CONST = self::MY_CONST;

/**
* self::class is ignored here because this is a comment.
*/
public static function function1(self $someProperty): self;

public static function self();
}
32 changes: 32 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest/TestTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

namespace SilverStripe\Standards\Tests\PHPStan\KeywordSelfRuleTest;

trait TestTrait
{
// Can't define a const on a trait, but the trait can access consts on the class that uses it
private static string $myProperty = self::MY_CONST;

private static self $mySecondProperty;

/**
* self::class is ignored here because this is a comment.
*/
public static function function1(self $someProperty): self
{
$self = new self();
$self::class; // $self:: isn't seen as self::
self::self();
self::$myProperty = self::class;
/* intentionally commented out to prove even multi-line comments are ignored.
self::$myProperty = self::class;
*/
return new self();
}

public static function self()
{
// doesn't need an implementation - the point of this method is just to show that "self" isn't just being
// searched for naively.
}
}
34 changes: 34 additions & 0 deletions tests/PHPStan/KeywordSelfRuleTest/TestTraitCorrect.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace SilverStripe\Standards\Tests\PHPStan\KeywordSelfRuleTest;

trait TestTraitCorrect
{
// Can't define a const on a trait, but the trait can access consts on the class that uses it
private static string $myProperty = self::MY_CONST;

private static self $mySecondProperty;

/**
* self::class is ignored here because this is a comment.
*/
public static function function1(self $someProperty): self
{
$self = new (self::class)();
$self::class; // $self:: isn't seen as self::
self::class::self();
self::class::$myProperty = self::class;
/* intentionally commented out to prove even multi-line comments are ignored.
self::$myProperty = self::class;
*/
// Alternative acceptable syntax for instantiating
$selfClass = self::class;
return new $selfClass();
}

public static function self()
{
// doesn't need an implementation - the point of this method is just to show that "self" isn't just being
// searched for naively.
}
}

0 comments on commit 8795b5f

Please sign in to comment.