[BE] TooWideMethodReturnTypehintRule - always report for final methods

Author: ondrejmirtes

changelog-2.0.md

@@ -71,7 +71,6 @@ Bleeding edge (TODO move to other sections)
 * Report narrowing `PHPStan\Type\Type` interface via `@var` (https://github.com/phpstan/phpstan-src/commit/713b98fb107213c28e3d8c8b4b43c5f5fc47c144), https://github.com/nunomaduro/larastan/issues/1567#issuecomment-1460445389
 * Check invalid PHPDocs in previously unchecked statement types (https://github.com/phpstan/phpstan-src/commit/9780d352f3264aac09ac7954f691de1877db8e01)
 * InvalidPHPStanDocTagRule in StubValidator (https://github.com/phpstan/phpstan-src/commit/9c2552b7e744926d1a74c1ba8fd32c64079eed61)
-* TooWideMethodReturnTypehintRule - always report for final methods (https://github.com/phpstan/phpstan-src/commit/c30e9a484c8245b8126cd63444607ca74d2af761)
 * Report unused results of `and` and `or` (https://github.com/phpstan/phpstan-src/commit/1d8fff637d70a9e9ed3f11dee5d61b9f796cbf1a)
 * Report unused result of ternary (https://github.com/phpstan/phpstan-src/commit/9664f7a9d2223c07e750f0dfc949c3accfa6b65e)
 * Report unused results of `&&` and `||` (https://github.com/phpstan/phpstan-src/commit/cf2c8bbd9ebd2ebe300dbd310e136ad603d7def3)
@@ -128,6 +127,7 @@ Improvements 🔧
 * Empty `skipCheckGenericClasses` (https://github.com/phpstan/phpstan-src/commit/28c2c79b16cac6ba6b01f1b4d211541dd49d8a77)
 * Report unnecessary nullsafe property fetch inside `??` / `isset` / `empty` with different message ([#1253](https://github.com/phpstan/phpstan-src/pull/1253)), thanks @rajyan!
 * Specify explicit mixed array type via `is_array` ([#1191](https://github.com/phpstan/phpstan-src/pull/1191)), thanks @herndlm!
+* TooWideMethodReturnTypehintRule - always report for final methods (https://github.com/phpstan/phpstan-src/commit/c30e9a484c8245b8126cd63444607ca74d2af761)
 
 Bugfixes 🐛
 =====================

conf/bleedingEdge.neon

@@ -8,7 +8,6 @@ parameters:
 		arrayValues: true
 		looseComparison: true
 		readOnlyByPhpDoc: true
-		alwaysCheckTooWideReturnTypeFinalMethods: true
 		alwaysTrueAlwaysReported: true
 		disableUnreachableBranchesRules: true
 		pure: true

conf/config.level4.neon

@@ -282,7 +282,6 @@ services:
 		class: PHPStan\Rules\TooWideTypehints\TooWideMethodReturnTypehintRule
 		arguments:
 			checkProtectedAndPublicMethods: %checkTooWideReturnTypesInProtectedAndPublicMethods%
-			alwaysCheckFinal: %featureToggles.alwaysCheckTooWideReturnTypeFinalMethods%
 		tags:
 			- phpstan.rules.rule
 

conf/config.neon

@@ -29,7 +29,6 @@ parameters:
 		illegalConstructorMethodCall: false
 		looseComparison: false
 		readOnlyByPhpDoc: false
-		alwaysCheckTooWideReturnTypeFinalMethods: false
 		alwaysTrueAlwaysReported: false
 		disableUnreachableBranchesRules: false
 		stricterFunctionMap: false

conf/parametersSchema.neon

@@ -35,7 +35,6 @@ parametersSchema:
 		illegalConstructorMethodCall: bool(),
 		looseComparison: bool(),
 		readOnlyByPhpDoc: bool()
-		alwaysCheckTooWideReturnTypeFinalMethods: bool()
 		alwaysTrueAlwaysReported: bool()
 		disableUnreachableBranchesRules: bool()
 		stricterFunctionMap: bool()

src/Rules/TooWideTypehints/TooWideMethodReturnTypehintRule.php

@@ -22,7 +22,7 @@
 final class TooWideMethodReturnTypehintRule implements Rule
 {
 
-	public function __construct(private bool $checkProtectedAndPublicMethods, private bool $alwaysCheckFinal)
+	public function __construct(private bool $checkProtectedAndPublicMethods)
 	{
 	}
 
@@ -39,20 +39,14 @@ public function processNode(Node $node, Scope $scope): array
 		$method = $node->getMethodReflection();
 		$isFirstDeclaration = $method->getPrototype()->getDeclaringClass() === $method->getDeclaringClass();
 		if (!$method->isPrivate()) {
-			if ($this->alwaysCheckFinal) {
-				if (!$method->getDeclaringClass()->isFinal() && !$method->isFinal()->yes()) {
-					if (!$this->checkProtectedAndPublicMethods) {
-						return [];
-					}
+			if (!$method->getDeclaringClass()->isFinal() && !$method->isFinal()->yes()) {
+				if (!$this->checkProtectedAndPublicMethods) {
+					return [];
+				}
 
-					if ($isFirstDeclaration) {
-						return [];
-					}
+				if ($isFirstDeclaration) {
+					return [];
 				}
-			} elseif (!$this->checkProtectedAndPublicMethods) {
-				return [];
-			} elseif ($isFirstDeclaration && !$method->getDeclaringClass()->isFinal() && !$method->isFinal()->yes()) {
-				return [];
 			}
 		}
 

tests/PHPStan/Rules/TooWideTypehints/TooWideMethodReturnTypehintRuleTest.php

@@ -14,11 +14,9 @@ class TooWideMethodReturnTypehintRuleTest extends RuleTestCase
 
 	private bool $checkProtectedAndPublicMethods = true;
 
-	private bool $alwaysCheckFinal = false;
-
 	protected function getRule(): Rule
 	{
-		return new TooWideMethodReturnTypehintRule($this->checkProtectedAndPublicMethods, $this->alwaysCheckFinal);
+		return new TooWideMethodReturnTypehintRule($this->checkProtectedAndPublicMethods);
 	}
 
 	public function testPrivate(): void
@@ -113,26 +111,6 @@ public function testBug6175(): void
 	public function dataAlwaysCheckFinal(): iterable
 	{
 		yield [
-			false,
-			false,
-			[
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\Foo::test() never returns null so it can be removed from the return type.',
-					8,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FinalFoo::test() never returns null so it can be removed from the return type.',
-					28,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FooFinalMethods::test() never returns null so it can be removed from the return type.',
-					48,
-				],
-			],
-		];
-
-		yield [
-			true,
 			false,
 			[
 				[
@@ -167,42 +145,6 @@ public function dataAlwaysCheckFinal(): iterable
 		];
 
 		yield [
-			false,
-			true,
-			[
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\Foo::test() never returns null so it can be removed from the return type.',
-					8,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FinalFoo::test() never returns null so it can be removed from the return type.',
-					28,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FinalFoo::test2() never returns null so it can be removed from the return type.',
-					33,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FinalFoo::test3() never returns null so it can be removed from the return type.',
-					38,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FooFinalMethods::test() never returns null so it can be removed from the return type.',
-					48,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FooFinalMethods::test2() never returns null so it can be removed from the return type.',
-					53,
-				],
-				[
-					'Method MethodTooWideReturnAlwaysCheckFinal\FooFinalMethods::test3() never returns null so it can be removed from the return type.',
-					58,
-				],
-			],
-		];
-
-		yield [
-			true,
 			true,
 			[
 				[
@@ -241,10 +183,9 @@ public function dataAlwaysCheckFinal(): iterable
 	 * @dataProvider dataAlwaysCheckFinal
 	 * @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrors
 	 */
-	public function testAlwaysCheckFinal(bool $checkProtectedAndPublicMethods, bool $alwaysCheckFinal, array $expectedErrors): void
+	public function testAlwaysCheckFinal(bool $checkProtectedAndPublicMethods, array $expectedErrors): void
 	{
 		$this->checkProtectedAndPublicMethods = $checkProtectedAndPublicMethods;
-		$this->alwaysCheckFinal = $alwaysCheckFinal;
 		$this->analyse([__DIR__ . '/data/method-too-wide-return-always-check-final.php'], $expectedErrors);
 	}