From e1f463e7185f9b161535e549b3431c11f05b7429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20M=C3=B6ller?= Date: Mon, 17 Feb 2025 20:49:21 +0100 Subject: [PATCH] Enhancement: Implement NoReturnByReferenceRule --- CHANGELOG.md | 2 + README.md | 32 +++++++++ rules.neon | 15 ++++ src/ErrorIdentifier.php | 5 ++ src/Functions/NoReturnByReferenceRule.php | 50 +++++++++++++ src/Methods/NoReturnByReferenceRule.php | 72 +++++++++++++++++++ .../NoReturnByReferenceRule/script.php | 14 ++++ .../NoReturnByReferenceRule/MethodInClass.php | 13 ++++ .../MethodInClassReturningByReference.php | 13 ++++ .../MethodInInterface.php | 10 +++ .../MethodInInterfaceReturningByReference.php | 10 +++ .../NoReturnByReferenceRule/MethodInTrait.php | 13 ++++ .../MethodInTraitReturningByReference.php | 13 ++++ .../NoReturnByReferenceRule/script.php | 25 +++++++ .../Functions/NoReturnByReferenceRuleTest.php | 49 +++++++++++++ .../Methods/NoReturnByReferenceRuleTest.php | 63 ++++++++++++++++ test/Unit/ErrorIdentifierTest.php | 7 ++ 17 files changed, 406 insertions(+) create mode 100644 src/Functions/NoReturnByReferenceRule.php create mode 100644 src/Methods/NoReturnByReferenceRule.php create mode 100644 test/Fixture/Functions/NoReturnByReferenceRule/script.php create mode 100644 test/Fixture/Methods/NoReturnByReferenceRule/MethodInClass.php create mode 100644 test/Fixture/Methods/NoReturnByReferenceRule/MethodInClassReturningByReference.php create mode 100644 test/Fixture/Methods/NoReturnByReferenceRule/MethodInInterface.php create mode 100644 test/Fixture/Methods/NoReturnByReferenceRule/MethodInInterfaceReturningByReference.php create mode 100644 test/Fixture/Methods/NoReturnByReferenceRule/MethodInTrait.php create mode 100644 test/Fixture/Methods/NoReturnByReferenceRule/MethodInTraitReturningByReference.php create mode 100644 test/Fixture/Methods/NoReturnByReferenceRule/script.php create mode 100644 test/Integration/Functions/NoReturnByReferenceRuleTest.php create mode 100644 test/Integration/Methods/NoReturnByReferenceRuleTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 754eb31c..cb428e42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ For a full diff see [`2.6.1...main`][2.6.1...main]. ### Added - Added `Closures\NoParameterPassedByReferenceRule`, `Functions\NoParameterPassedByReferenceRule`, `Methods\NoParameterPassedByReferenceRule`, which report an error when a closure, a function, or a method has a parameter that is passed by reference ([#911]), by [@localheinz] +- Added `Functions\NoReturnByReferenceRule` and `Methods\NoReturnByReferenceRule`, which report an error when a function or a method returns by reference ([#912]), by [@localheinz] ## [`2.6.1`][2.6.1] @@ -604,6 +605,7 @@ For a full diff see [`362c7ea...0.1.0`][362c7ea...0.1.0]. [#897]: https://github.com/ergebnis/phpstan-rules/pull/897 [#902]: https://github.com/ergebnis/phpstan-rules/pull/902 [#911]: https://github.com/ergebnis/phpstan-rules/pull/911 +[#912]: https://github.com/ergebnis/phpstan-rules/pull/912 [@cosmastech]: https://github.com/cosmastech [@enumag]: https://github.com/enumag diff --git a/README.md b/README.md index 1fce8c9d..57aa860a 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,7 @@ This package provides the following rules for use with [`phpstan/phpstan`](https - [`Ergebnis\PHPStan\Rules\Functions\NoParameterPassedByReferenceRule`](https://github.com/ergebnis/phpstan-rules#functionsnoparameterpassedbyreferencerule) - [`Ergebnis\PHPStan\Rules\Functions\NoParameterWithNullableTypeDeclarationRule`](https://github.com/ergebnis/phpstan-rules#functionsnoparameterwithnullabletypedeclarationrule) - [`Ergebnis\PHPStan\Rules\Functions\NoParameterWithNullDefaultValueRule`](https://github.com/ergebnis/phpstan-rules#functionsnoparameterwithnulldefaultvaluerule) +- [`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\NoConstructorParameterWithDefaultValueRule`](https://github.com/ergebnis/phpstan-rules#methodsnoconstructorparameterwithdefaultvaluerule) - [`Ergebnis\PHPStan\Rules\Methods\NoNullableReturnTypeDeclarationRule`](https://github.com/ergebnis/phpstan-rules#methodsnonullablereturntypedeclarationrule) @@ -67,6 +68,7 @@ This package provides the following rules for use with [`phpstan/phpstan`](https - [`Ergebnis\PHPStan\Rules\Methods\NoParameterWithContainerTypeDeclarationRule`](https://github.com/ergebnis/phpstan-rules#methodsnoparameterwithcontainertypedeclarationrule) - [`Ergebnis\PHPStan\Rules\Methods\NoParameterWithNullableTypeDeclarationRule`](https://github.com/ergebnis/phpstan-rules#methodsnoparameterwithnullabletypedeclarationrule) - [`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\Statements\NoSwitchRule`](https://github.com/ergebnis/phpstan-rules#statementsnoswitchrule) @@ -372,6 +374,21 @@ parameters: enabled: false ``` +#### `Functions\NoReturnByReferenceRule` + +This rule reports an error when a function [returns by reference](https://www.php.net/manual/en/language.references.return.php). + +##### Disabling the rule + +You can set the `enabled` parameter to `false` to disable this rule. + +```neon +parameters: + ergebnis: + noReturnByReference: + enabled: false +``` + ### Methods #### `Methods\FinalInAbstractClassRule` @@ -540,6 +557,21 @@ parameters: enabled: false ``` +#### `Functions\NoReturnByReferenceRule` + +This rule reports an error when a method [returns by reference](https://www.php.net/manual/en/language.references.return.php). + +##### Disabling the rule + +You can set the `enabled` parameter to `false` to disable this rule. + +```neon +parameters: + ergebnis: + noReturnByReference: + enabled: false +``` + #### `Methods\PrivateInFinalClassRule` This rule reports an error when a method in a `final` class is `protected` but could be `private`. diff --git a/rules.neon b/rules.neon index 994a1feb..b2865b52 100644 --- a/rules.neon +++ b/rules.neon @@ -29,6 +29,8 @@ conditionalTags: phpstan.rules.rule: %ergebnis.noParameterWithNullableTypeDeclaration.enabled% Ergebnis\PHPStan\Rules\Functions\NoParameterWithNullDefaultValueRule: phpstan.rules.rule: %ergebnis.noParameterWithNullDefaultValue.enabled% + Ergebnis\PHPStan\Rules\Functions\NoReturnByReferenceRule: + phpstan.rules.rule: %ergebnis.noReturnByReference.enabled% Ergebnis\PHPStan\Rules\Methods\FinalInAbstractClassRule: phpstan.rules.rule: %ergebnis.finalInAbstractClass.enabled% Ergebnis\PHPStan\Rules\Methods\NoConstructorParameterWithDefaultValueRule: @@ -43,6 +45,8 @@ conditionalTags: phpstan.rules.rule: %ergebnis.noParameterWithNullableTypeDeclaration.enabled% Ergebnis\PHPStan\Rules\Methods\NoParameterWithNullDefaultValueRule: phpstan.rules.rule: %ergebnis.noParameterWithNullDefaultValue.enabled% + Ergebnis\PHPStan\Rules\Methods\NoReturnByReferenceRule: + phpstan.rules.rule: %ergebnis.noReturnByReference.enabled% Ergebnis\PHPStan\Rules\Methods\PrivateInFinalClassRule: phpstan.rules.rule: %ergebnis.privateInFinalClass.enabled% Ergebnis\PHPStan\Rules\Statements\NoSwitchRule: @@ -84,6 +88,8 @@ parameters: enabled: true noParameterWithNullDefaultValue: enabled: true + noReturnByReference: + enabled: true noSwitch: enabled: true privateInFinalClass: @@ -140,6 +146,9 @@ parametersSchema: noParameterWithNullDefaultValue: structure([ enabled: bool(), ]) + noReturnByReference: structure([ + enabled: bool(), + ]) noSwitch: structure([ enabled: bool(), ]) @@ -205,6 +214,9 @@ services: - class: Ergebnis\PHPStan\Rules\Functions\NoParameterWithNullDefaultValueRule + - + class: Ergebnis\PHPStan\Rules\Functions\NoReturnByReferenceRule + - class: Ergebnis\PHPStan\Rules\Methods\FinalInAbstractClassRule @@ -229,6 +241,9 @@ services: - class: Ergebnis\PHPStan\Rules\Methods\NoParameterWithNullDefaultValueRule + - + class: Ergebnis\PHPStan\Rules\Methods\NoReturnByReferenceRule + - class: Ergebnis\PHPStan\Rules\Methods\PrivateInFinalClassRule diff --git a/src/ErrorIdentifier.php b/src/ErrorIdentifier.php index a0a67429..6b4912b8 100644 --- a/src/ErrorIdentifier.php +++ b/src/ErrorIdentifier.php @@ -95,6 +95,11 @@ public static function noNullableReturnTypeDeclaration(): self return new self('noNullableReturnTypeDeclaration'); } + public static function noReturnByReference(): self + { + return new self('noReturnByReference'); + } + public static function noSwitch(): self { return new self('noSwitch'); diff --git a/src/Functions/NoReturnByReferenceRule.php b/src/Functions/NoReturnByReferenceRule.php new file mode 100644 index 00000000..0efb7008 --- /dev/null +++ b/src/Functions/NoReturnByReferenceRule.php @@ -0,0 +1,50 @@ + + */ +final class NoReturnByReferenceRule implements Rules\Rule +{ + public function getNodeType(): string + { + return Node\Stmt\Function_::class; + } + + public function processNode( + Node $node, + Analyser\Scope $scope + ): array { + if (false === $node->byRef) { + return []; + } + + $message = \sprintf( + 'Function %s() returns by reference.', + $node->namespacedName, + ); + + return [ + Rules\RuleErrorBuilder::message($message) + ->identifier(ErrorIdentifier::noReturnByReference()->toString()) + ->build(), + ]; + } +} diff --git a/src/Methods/NoReturnByReferenceRule.php b/src/Methods/NoReturnByReferenceRule.php new file mode 100644 index 00000000..a1bf87ff --- /dev/null +++ b/src/Methods/NoReturnByReferenceRule.php @@ -0,0 +1,72 @@ + + */ +final class NoReturnByReferenceRule implements Rules\Rule +{ + public function getNodeType(): string + { + return Node\Stmt\ClassMethod::class; + } + + public function processNode( + Node $node, + Analyser\Scope $scope + ): array { + if (false === $node->byRef) { + return []; + } + + $methodName = $node->name->toString(); + + /** @var Reflection\ClassReflection $classReflection */ + $classReflection = $scope->getClassReflection(); + + if ($classReflection->isAnonymous()) { + $message = \sprintf( + 'Method %s() in anonymous class returns by reference.', + $methodName, + ); + + return [ + Rules\RuleErrorBuilder::message($message) + ->identifier(ErrorIdentifier::noReturnByReference()->toString()) + ->build(), + ]; + } + + $className = $classReflection->getName(); + + $message = \sprintf( + 'Method %s::%s() returns by reference.', + $className, + $methodName, + ); + + return [ + Rules\RuleErrorBuilder::message($message) + ->identifier(ErrorIdentifier::noReturnByReference()->toString()) + ->build(), + ]; + } +} diff --git a/test/Fixture/Functions/NoReturnByReferenceRule/script.php b/test/Fixture/Functions/NoReturnByReferenceRule/script.php new file mode 100644 index 00000000..ed5e6253 --- /dev/null +++ b/test/Fixture/Functions/NoReturnByReferenceRule/script.php @@ -0,0 +1,14 @@ + + */ +final class NoReturnByReferenceRuleTest extends Testing\RuleTestCase +{ + use Test\Util\Helper; + + public function testNoReturnByReferenceRule(): void + { + $this->analyse( + self::phpFilesIn(__DIR__ . '/../../Fixture/Functions/NoReturnByReferenceRule'), + [ + [ + 'Function Ergebnis\PHPStan\Rules\Test\Fixture\Functions\NoReturnByReferenceRule\bar() returns by reference.', + 11, + ], + ], + ); + } + + protected function getRule(): Rules\Rule + { + return new Functions\NoReturnByReferenceRule(); + } +} diff --git a/test/Integration/Methods/NoReturnByReferenceRuleTest.php b/test/Integration/Methods/NoReturnByReferenceRuleTest.php new file mode 100644 index 00000000..baa13a92 --- /dev/null +++ b/test/Integration/Methods/NoReturnByReferenceRuleTest.php @@ -0,0 +1,63 @@ + + */ +final class NoReturnByReferenceRuleTest extends Testing\RuleTestCase +{ + use Test\Util\Helper; + + public function testNoReturnByReferenceRule(): void + { + $this->analyse( + self::phpFilesIn(__DIR__ . '/../../Fixture/Methods/NoReturnByReferenceRule'), + [ + [ + \sprintf( + 'Method %s::foo() returns by reference.', + Test\Fixture\Methods\NoReturnByReferenceRule\MethodInClassReturningByReference::class, + ), + 9, + ], + [ + \sprintf( + 'Method %s::foo() returns by reference.', + Test\Fixture\Methods\NoReturnByReferenceRule\MethodInInterfaceReturningByReference::class, + ), + 9, + ], + [ + 'Method foo() in anonymous class returns by reference.', + 21, + ], + ], + ); + } + + protected function getRule(): Rules\Rule + { + return new Methods\NoReturnByReferenceRule(); + } +} diff --git a/test/Unit/ErrorIdentifierTest.php b/test/Unit/ErrorIdentifierTest.php index 2045bdfe..be69cb78 100644 --- a/test/Unit/ErrorIdentifierTest.php +++ b/test/Unit/ErrorIdentifierTest.php @@ -119,6 +119,13 @@ public function testNoParameterWithNullableTypeDeclarationReturnsErrorIdentifier self::assertSame('ergebnis.noParameterWithNullableTypeDeclaration', $errorIdentifier->toString()); } + public function testNoReturnByReferenceErrorIdentifier(): void + { + $errorIdentifier = ErrorIdentifier::noReturnByReference(); + + self::assertSame('ergebnis.noReturnByReference', $errorIdentifier->toString()); + } + public function testNoSwitchReturnsErrorIdentifier(): void { $errorIdentifier = ErrorIdentifier::noSwitch();