diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e0d0208..da57d57b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), For a full diff see [`2.13.1...main`][2.13.1...main]. +### Added + +- Added `Constants\NoAccessToConstantViaInstanceRule`, `Methods\NoAccessToStaticMethodViaInstanceRule`, and `Properties\NoAccessToStaticPropertyViaInstanceRule`, which report an error when a static constant, method, or property is accessed via an instance instead of a class name ([#1020]), by [@localheinz] + ## [`2.13.1`][2.13.1] For a full diff see [`2.13.0...2.13.1`][2.13.0...2.13.1]. diff --git a/README.md b/README.md index afe6464a..fd1c96a3 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,7 @@ This package provides the following rules for use with [`phpstan/phpstan`](https - [`Ergebnis\PHPStan\Rules\Closures\NoParameterPassedByReferenceRule`](https://github.com/ergebnis/phpstan-rules#closuresnoparameterpassedbyreferencerule) - [`Ergebnis\PHPStan\Rules\Closures\NoParameterWithNullableTypeDeclarationRule`](https://github.com/ergebnis/phpstan-rules#closuresnoparameterwithnullabletypedeclarationrule) - [`Ergebnis\PHPStan\Rules\Closures\NoParameterWithNullDefaultValueRule`](https://github.com/ergebnis/phpstan-rules#closuresnoparameterwithnulldefaultvaluerule) +- [`Ergebnis\PHPStan\Rules\Constants\NoAccessToConstantViaInstanceRule`](https://github.com/ergebnis/phpstan-rules#constantsnoaccesstoconstantviainstancerule) - [`Ergebnis\PHPStan\Rules\Expressions\NoAssignByReferenceRule`](https://github.com/ergebnis/phpstan-rules#expressionsnoassignbyreferencerule) - [`Ergebnis\PHPStan\Rules\Expressions\NoCompactRule`](https://github.com/ergebnis/phpstan-rules#expressionsnocompactrule) - [`Ergebnis\PHPStan\Rules\Expressions\NoErrorSuppressionRule`](https://github.com/ergebnis/phpstan-rules#expressionsnoerrorsuppressionrule) @@ -66,6 +67,7 @@ This package provides the following rules for use with [`phpstan/phpstan`](https - [`Ergebnis\PHPStan\Rules\Functions\NoReturnByReferenceRule`](https://github.com/ergebnis/phpstan-rules#functionsnoreturnbyreferencerule) - [`Ergebnis\PHPStan\Rules\Methods\FinalInAbstractClassRule`](https://github.com/ergebnis/phpstan-rules#methodsfinalinabstractclassrule) - [`Ergebnis\PHPStan\Rules\Methods\InvokeParentHookMethodRule`](https://github.com/ergebnis/phpstan-rules#methodsinvokeparenthookmethodrule) +- [`Ergebnis\PHPStan\Rules\Methods\NoAccessToStaticMethodViaInstanceRule`](https://github.com/ergebnis/phpstan-rules#methodsnoaccesstostaticmethodviainstancerule) - [`Ergebnis\PHPStan\Rules\Methods\NoConstructorParameterWithDefaultValueRule`](https://github.com/ergebnis/phpstan-rules#methodsnoconstructorparameterwithdefaultvaluerule) - [`Ergebnis\PHPStan\Rules\Methods\NoNullableReturnTypeDeclarationRule`](https://github.com/ergebnis/phpstan-rules#methodsnonullablereturntypedeclarationrule) - [`Ergebnis\PHPStan\Rules\Methods\NoParameterPassedByReferenceRule`](https://github.com/ergebnis/phpstan-rules#methodsnoparameterpassedbyreferencerule) @@ -74,6 +76,7 @@ This package provides the following rules for use with [`phpstan/phpstan`](https - [`Ergebnis\PHPStan\Rules\Methods\NoParameterWithNullDefaultValueRule`](https://github.com/ergebnis/phpstan-rules#methodsnoparameterwithnulldefaultvaluerule) - [`Ergebnis\PHPStan\Rules\Methods\NoReturnByReferenceRule`](https://github.com/ergebnis/phpstan-rules#methodsnoreturnbyreferencerule) - [`Ergebnis\PHPStan\Rules\Methods\PrivateInFinalClassRule`](https://github.com/ergebnis/phpstan-rules#methodsprivateinfinalclassrule) +- [`Ergebnis\PHPStan\Rules\Properties\NoAccessToStaticPropertyViaInstanceRule`](https://github.com/ergebnis/phpstan-rules#propertiesnoaccesstostaticpropertyviainstancerule) - [`Ergebnis\PHPStan\Rules\Statements\NoSwitchRule`](https://github.com/ergebnis/phpstan-rules#statementsnoswitchrule) @@ -255,6 +258,25 @@ parameters: enabled: false ``` +### Constants + +#### `Constants\NoAccessToConstantViaInstanceRule` + +This rule reports an error when a class constant is accessed via an instance (e.g., `$foo::BAZ`) instead of via a class name (e.g., `Foo::BAZ`, `self::BAZ`, `static::BAZ`, or `parent::BAZ`). + +:bulb: This rule does not report an error for `$foo::class`, which is an idiomatic way to get the runtime class of an object. + +##### Disabling the rule + +You can set the `enabled` parameter to `false` to disable this rule. + +```neon +parameters: + ergebnis: + noAccessToStaticMemberViaInstance: + enabled: false +``` + ### Expressions #### `Expressions\NoAssignByReferenceRule` @@ -520,6 +542,21 @@ parameters: - `hasContent`: one of `"yes"`, `"no"`, `"maybe"` - `invocation`: one of `"any"` (needs to be invoked), `"first"` (needs to be invoked before all other statements in the overriding hook method, `"last"` (needs to be invoked after all other statements in the overriding hook method) +#### `Methods\NoAccessToStaticMethodViaInstanceRule` + +This rule reports an error when a static method is called via an instance (e.g., `$foo::bar()`) instead of via a class name (e.g., `Foo::bar()`, `self::bar()`, `static::bar()`, or `parent::bar()`). + +##### Disabling the rule + +You can set the `enabled` parameter to `false` to disable this rule. + +```neon +parameters: + ergebnis: + noAccessToStaticMemberViaInstance: + enabled: false +``` + #### `Methods\NoConstructorParameterWithDefaultValueRule` This rule reports an error when a constructor declared in @@ -696,6 +733,23 @@ parameters: enabled: false ``` +### Properties + +#### `Properties\NoAccessToStaticPropertyViaInstanceRule` + +This rule reports an error when a static property is accessed via an instance (e.g., `$foo::$BAR`) instead of via a class name (e.g., `Foo::$BAR`, `self::$BAR`, `static::$BAR`, or `parent::$BAR`). + +##### Disabling the rule + +You can set the `enabled` parameter to `false` to disable this rule. + +```neon +parameters: + ergebnis: + noAccessToStaticMemberViaInstance: + enabled: false +``` + ### Statements #### `Statements\NoSwitchRule` diff --git a/rules.neon b/rules.neon index e1287c54..b7e49f35 100644 --- a/rules.neon +++ b/rules.neon @@ -23,6 +23,12 @@ conditionalTags: phpstan.rules.rule: %ergebnis.noEval.enabled% Ergebnis\PHPStan\Rules\Expressions\NoIssetRule: phpstan.rules.rule: %ergebnis.noIsset.enabled% + Ergebnis\PHPStan\Rules\Constants\NoAccessToConstantViaInstanceRule: + phpstan.rules.rule: %ergebnis.noAccessToStaticMemberViaInstance.enabled% + Ergebnis\PHPStan\Rules\Methods\NoAccessToStaticMethodViaInstanceRule: + phpstan.rules.rule: %ergebnis.noAccessToStaticMemberViaInstance.enabled% + Ergebnis\PHPStan\Rules\Properties\NoAccessToStaticPropertyViaInstanceRule: + phpstan.rules.rule: %ergebnis.noAccessToStaticMemberViaInstance.enabled% Ergebnis\PHPStan\Rules\Files\DeclareStrictTypesRule: phpstan.rules.rule: %ergebnis.declareStrictTypes.enabled% Ergebnis\PHPStan\Rules\Files\NoPhpstanIgnoreRule: @@ -108,6 +114,8 @@ parameters: enabled: %ergebnis.allRules% noReturnByReference: enabled: %ergebnis.allRules% + noAccessToStaticMemberViaInstance: + enabled: %ergebnis.allRules% noSwitch: enabled: %ergebnis.allRules% privateInFinalClass: @@ -186,6 +194,9 @@ parametersSchema: noReturnByReference: structure([ enabled: bool(), ]) + noAccessToStaticMemberViaInstance: structure([ + enabled: bool(), + ]) noSwitch: structure([ enabled: bool(), ]) @@ -298,5 +309,14 @@ services: - class: Ergebnis\PHPStan\Rules\Methods\PrivateInFinalClassRule + - + class: Ergebnis\PHPStan\Rules\Constants\NoAccessToConstantViaInstanceRule + + - + class: Ergebnis\PHPStan\Rules\Methods\NoAccessToStaticMethodViaInstanceRule + + - + class: Ergebnis\PHPStan\Rules\Properties\NoAccessToStaticPropertyViaInstanceRule + - class: Ergebnis\PHPStan\Rules\Statements\NoSwitchRule diff --git a/src/Constants/NoAccessToConstantViaInstanceRule.php b/src/Constants/NoAccessToConstantViaInstanceRule.php new file mode 100644 index 00000000..3d0e26a1 --- /dev/null +++ b/src/Constants/NoAccessToConstantViaInstanceRule.php @@ -0,0 +1,95 @@ + + */ +final class NoAccessToConstantViaInstanceRule implements Rules\Rule +{ + private Reflection\ReflectionProvider $reflectionProvider; + + public function __construct(Reflection\ReflectionProvider $reflectionProvider) + { + $this->reflectionProvider = $reflectionProvider; + } + + public function getNodeType(): string + { + return Node\Expr\ClassConstFetch::class; + } + + public function processNode( + Node $node, + Analyser\Scope $scope + ): array { + if ($node->class instanceof Node\Name) { + return []; + } + + if (!$node->name instanceof Node\Identifier) { + return []; + } + + if ('class' === $node->name->toLowerString()) { + return []; + } + + $classNames = $scope->getType($node->class)->getObjectClassNames(); + + if ([] === $classNames) { + return []; + } + + if ( + !$node->class instanceof Node\Expr\Variable + || 'this' !== $node->class->name + ) { + if ($this->isAnonymousClass(...$classNames)) { + return []; + } + } + + $message = \sprintf( + 'Static constant %s should be accessed via class name, not via instance.', + $node->name->toString(), + ); + + return [ + Rules\RuleErrorBuilder::message($message) + ->identifier(ErrorIdentifier::noAccessToStaticMemberViaInstance()->toString()) + ->build(), + ]; + } + + private function isAnonymousClass(string ...$classNames): bool + { + foreach ($classNames as $className) { + if ( + $this->reflectionProvider->hasClass($className) + && $this->reflectionProvider->getClass($className)->isAnonymous() + ) { + return true; + } + } + + return false; + } +} diff --git a/src/ErrorIdentifier.php b/src/ErrorIdentifier.php index 8f9d6f68..911bbdb3 100644 --- a/src/ErrorIdentifier.php +++ b/src/ErrorIdentifier.php @@ -120,6 +120,11 @@ public static function noReturnByReference(): self return new self('noReturnByReference'); } + public static function noAccessToStaticMemberViaInstance(): self + { + return new self('noAccessToStaticMemberViaInstance'); + } + public static function noSwitch(): self { return new self('noSwitch'); diff --git a/src/Methods/NoAccessToStaticMethodViaInstanceRule.php b/src/Methods/NoAccessToStaticMethodViaInstanceRule.php new file mode 100644 index 00000000..0d2779f8 --- /dev/null +++ b/src/Methods/NoAccessToStaticMethodViaInstanceRule.php @@ -0,0 +1,91 @@ + + */ +final class NoAccessToStaticMethodViaInstanceRule implements Rules\Rule +{ + private Reflection\ReflectionProvider $reflectionProvider; + + public function __construct(Reflection\ReflectionProvider $reflectionProvider) + { + $this->reflectionProvider = $reflectionProvider; + } + + public function getNodeType(): string + { + return Node\Expr\StaticCall::class; + } + + public function processNode( + Node $node, + Analyser\Scope $scope + ): array { + if ($node->class instanceof Node\Name) { + return []; + } + + if (!$node->name instanceof Node\Identifier) { + return []; + } + + $classNames = $scope->getType($node->class)->getObjectClassNames(); + + if ([] === $classNames) { + return []; + } + + if ( + !$node->class instanceof Node\Expr\Variable + || 'this' !== $node->class->name + ) { + if ($this->isAnonymousClass(...$classNames)) { + return []; + } + } + + $message = \sprintf( + 'Static method %s() should be called via class name, not via instance.', + $node->name->toString(), + ); + + return [ + Rules\RuleErrorBuilder::message($message) + ->identifier(ErrorIdentifier::noAccessToStaticMemberViaInstance()->toString()) + ->build(), + ]; + } + + private function isAnonymousClass(string ...$classNames): bool + { + foreach ($classNames as $className) { + if ( + $this->reflectionProvider->hasClass($className) + && $this->reflectionProvider->getClass($className)->isAnonymous() + ) { + return true; + } + } + + return false; + } +} diff --git a/src/Properties/NoAccessToStaticPropertyViaInstanceRule.php b/src/Properties/NoAccessToStaticPropertyViaInstanceRule.php new file mode 100644 index 00000000..94583c48 --- /dev/null +++ b/src/Properties/NoAccessToStaticPropertyViaInstanceRule.php @@ -0,0 +1,91 @@ + + */ +final class NoAccessToStaticPropertyViaInstanceRule implements Rules\Rule +{ + private Reflection\ReflectionProvider $reflectionProvider; + + public function __construct(Reflection\ReflectionProvider $reflectionProvider) + { + $this->reflectionProvider = $reflectionProvider; + } + + public function getNodeType(): string + { + return Node\Expr\StaticPropertyFetch::class; + } + + public function processNode( + Node $node, + Analyser\Scope $scope + ): array { + if ($node->class instanceof Node\Name) { + return []; + } + + if (!$node->name instanceof Node\VarLikeIdentifier) { + return []; + } + + $classNames = $scope->getType($node->class)->getObjectClassNames(); + + if ([] === $classNames) { + return []; + } + + if ( + !$node->class instanceof Node\Expr\Variable + || 'this' !== $node->class->name + ) { + if ($this->isAnonymousClass(...$classNames)) { + return []; + } + } + + $message = \sprintf( + 'Static property $%s should be accessed via class name, not via instance.', + $node->name->toString(), + ); + + return [ + Rules\RuleErrorBuilder::message($message) + ->identifier(ErrorIdentifier::noAccessToStaticMemberViaInstance()->toString()) + ->build(), + ]; + } + + private function isAnonymousClass(string ...$classNames): bool + { + foreach ($classNames as $className) { + if ( + $this->reflectionProvider->hasClass($className) + && $this->reflectionProvider->getClass($className)->isAnonymous() + ) { + return true; + } + } + + return false; + } +} diff --git a/test/Fixture/Constants/NoAccessToConstantViaInstanceRule/ChildClassAccessingStaticConstantViaKeyword.php b/test/Fixture/Constants/NoAccessToConstantViaInstanceRule/ChildClassAccessingStaticConstantViaKeyword.php new file mode 100644 index 00000000..bf604f7d --- /dev/null +++ b/test/Fixture/Constants/NoAccessToConstantViaInstanceRule/ChildClassAccessingStaticConstantViaKeyword.php @@ -0,0 +1,23 @@ + $className */ +$className = ClassWithConstant::class; +$className::BAZ; + +$instance::BAZ; + +$instance::class; diff --git a/test/Fixture/Methods/NoAccessToStaticMethodViaInstanceRule/ChildClassAccessingStaticMethodViaKeyword.php b/test/Fixture/Methods/NoAccessToStaticMethodViaInstanceRule/ChildClassAccessingStaticMethodViaKeyword.php new file mode 100644 index 00000000..711fc0fb --- /dev/null +++ b/test/Fixture/Methods/NoAccessToStaticMethodViaInstanceRule/ChildClassAccessingStaticMethodViaKeyword.php @@ -0,0 +1,23 @@ + $className */ +$className = ClassWithStaticMethod::class; +$className::bar(); + +$instance::bar(); diff --git a/test/Fixture/Properties/NoAccessToStaticPropertyViaInstanceRule/ChildClassAccessingStaticPropertyViaKeyword.php b/test/Fixture/Properties/NoAccessToStaticPropertyViaInstanceRule/ChildClassAccessingStaticPropertyViaKeyword.php new file mode 100644 index 00000000..39f09b15 --- /dev/null +++ b/test/Fixture/Properties/NoAccessToStaticPropertyViaInstanceRule/ChildClassAccessingStaticPropertyViaKeyword.php @@ -0,0 +1,23 @@ + $className */ +$className = ClassWithStaticProperty::class; +$className::$BAR; + +$instance::$BAR; diff --git a/test/Integration/Constants/NoAccessToConstantViaInstanceRuleTest.php b/test/Integration/Constants/NoAccessToConstantViaInstanceRuleTest.php new file mode 100644 index 00000000..96351862 --- /dev/null +++ b/test/Integration/Constants/NoAccessToConstantViaInstanceRuleTest.php @@ -0,0 +1,67 @@ + + */ +final class NoAccessToConstantViaInstanceRuleTest extends Testing\RuleTestCase +{ + use Test\Util\Helper; + + public function testNoAccessToConstantViaInstanceRule(): void + { + $this->analyse( + self::phpFilesIn(__DIR__ . '/../../Fixture/Constants/NoAccessToConstantViaInstanceRule'), + [ + [ + 'Static constant BAZ should be accessed via class name, not via instance.', + 7, + ], + [ + 'Static constant QUX should be accessed via class name, not via instance.', + 12, + ], + [ + 'Static constant BAZ should be accessed via class name, not via instance.', + 13, + ], + [ + 'Static constant BAZ should be accessed via class name, not via instance.', + 14, + ], + [ + 'Static constant BAZ should be accessed via class name, not via instance.', + 15, + ], + ], + ); + } + + protected function getRule(): Rules\Rule + { + return new Constants\NoAccessToConstantViaInstanceRule( + self::createReflectionProvider(), + ); + } +} diff --git a/test/Integration/Methods/NoAccessToStaticMethodViaInstanceRuleTest.php b/test/Integration/Methods/NoAccessToStaticMethodViaInstanceRuleTest.php new file mode 100644 index 00000000..5fb5b327 --- /dev/null +++ b/test/Integration/Methods/NoAccessToStaticMethodViaInstanceRuleTest.php @@ -0,0 +1,63 @@ + + */ +final class NoAccessToStaticMethodViaInstanceRuleTest extends Testing\RuleTestCase +{ + use Test\Util\Helper; + + public function testNoAccessToStaticMethodViaInstanceRule(): void + { + $this->analyse( + self::phpFilesIn(__DIR__ . '/../../Fixture/Methods/NoAccessToStaticMethodViaInstanceRule'), + [ + [ + 'Static method bar() should be called via class name, not via instance.', + 7, + ], + [ + 'Static method bar() should be called via class name, not via instance.', + 15, + ], + [ + 'Static method bar() should be called via class name, not via instance.', + 16, + ], + [ + 'Static method bar() should be called via class name, not via instance.', + 17, + ], + ], + ); + } + + protected function getRule(): Rules\Rule + { + return new Methods\NoAccessToStaticMethodViaInstanceRule( + self::createReflectionProvider(), + ); + } +} diff --git a/test/Integration/Properties/NoAccessToStaticPropertyViaInstanceRuleTest.php b/test/Integration/Properties/NoAccessToStaticPropertyViaInstanceRuleTest.php new file mode 100644 index 00000000..273ffd89 --- /dev/null +++ b/test/Integration/Properties/NoAccessToStaticPropertyViaInstanceRuleTest.php @@ -0,0 +1,63 @@ + + */ +final class NoAccessToStaticPropertyViaInstanceRuleTest extends Testing\RuleTestCase +{ + use Test\Util\Helper; + + public function testNoAccessToStaticPropertyViaInstanceRule(): void + { + $this->analyse( + self::phpFilesIn(__DIR__ . '/../../Fixture/Properties/NoAccessToStaticPropertyViaInstanceRule'), + [ + [ + 'Static property $BAR should be accessed via class name, not via instance.', + 7, + ], + [ + 'Static property $BAR should be accessed via class name, not via instance.', + 13, + ], + [ + 'Static property $BAR should be accessed via class name, not via instance.', + 14, + ], + [ + 'Static property $BAR should be accessed via class name, not via instance.', + 15, + ], + ], + ); + } + + protected function getRule(): Rules\Rule + { + return new Properties\NoAccessToStaticPropertyViaInstanceRule( + self::createReflectionProvider(), + ); + } +}