forked from silverstripe/silverstripe-standards
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
NEW Add rule to avoid use of "new" keyword
- Loading branch information
1 parent
62b0416
commit 8795b5f
Showing
12 changed files
with
322 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() | ||
]; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
11
tests/PHPStan/KeywordSelfRuleTest/ClassUsesTraitCorrect.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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. | ||
} | ||
} |