Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NEW Add rule to avoid use of "new" keyword #9

Merged
merged 1 commit into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
},
"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
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
{
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') {
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
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 [];
}
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
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') {
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
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.
}
}
Loading