From 1e6c4629aff2ae34ccc61e8012c918f7501284c8 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sun, 15 Feb 2026 23:01:55 +0100 Subject: [PATCH 1/5] Differenciate signaturePrototype and inheritancePrototype --- src/Rules/Methods/MethodPrototypeFinder.php | 62 ++++++++++++------- src/Rules/Methods/OverridingMethodRule.php | 24 ++++--- .../Methods/OverridingMethodRuleTest.php | 11 ++++ .../PHPStan/Rules/Methods/data/bug-11067.php | 46 ++++++++++++++ 4 files changed, 113 insertions(+), 30 deletions(-) create mode 100644 tests/PHPStan/Rules/Methods/data/bug-11067.php diff --git a/src/Rules/Methods/MethodPrototypeFinder.php b/src/Rules/Methods/MethodPrototypeFinder.php index 0ec7c9a1a6..0348a383b7 100644 --- a/src/Rules/Methods/MethodPrototypeFinder.php +++ b/src/Rules/Methods/MethodPrototypeFinder.php @@ -25,14 +25,19 @@ public function __construct( } /** - * @return array{ExtendedMethodReflection, ClassReflection, bool}|null + * Finds the prototype method that a class method should be validated against. + * Returns two prototypes with different purposes: + * - Signature prototype: Used for validating method signature (parameters, return type, ...). + * - Inheritance prototype: Used for validating inheritance rules (final keyword, override attribute, ...). + * + * @return array{ExtendedMethodReflection, ClassReflection, bool, ExtendedMethodReflection, ClassReflection}|null */ public function findPrototype(ClassReflection $classReflection, string $methodName): ?array { foreach ($classReflection->getImmediateInterfaces() as $immediateInterface) { if ($immediateInterface->hasNativeMethod($methodName)) { $method = $immediateInterface->getNativeMethod($methodName); - return [$method, $method->getDeclaringClass(), true]; + return [$method, $method->getDeclaringClass(), true, $method, $method->getDeclaringClass()]; } } @@ -47,15 +52,19 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa $isAbstract = $methodReflection->isAbstract(); if ($isAbstract) { $declaringTrait = $trait->getNativeMethod($methodName)->getDeclaringClass(); + $prototype = $this->phpClassReflectionExtension->createUserlandMethodReflection( + $trait, + $classReflection, + $methodReflection, + $declaringTrait->getName(), + ); + return [ - $this->phpClassReflectionExtension->createUserlandMethodReflection( - $trait, - $classReflection, - $methodReflection, - $declaringTrait->getName(), - ), + $prototype, $declaringTrait, false, + $prototype, + $declaringTrait, ]; } } @@ -76,25 +85,36 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa } $declaringClass = $method->getDeclaringClass(); - if ($declaringClass->hasConstructor()) { - if ($method->getName() === $declaringClass->getConstructor()->getName()) { - $prototype = $method->getPrototype(); - if ($prototype instanceof PhpMethodReflection || $prototype instanceof MethodPrototypeReflection || $prototype instanceof NativeMethodReflection) { - $abstract = $prototype->isAbstract(); - if (is_bool($abstract)) { - if (!$abstract) { - return null; - } - } elseif (!$abstract->yes()) { + if ($declaringClass->hasConstructor() && $method->getName() === $declaringClass->getConstructor()->getName()) { + $prototype = $method->getPrototype(); + if ($prototype instanceof PhpMethodReflection || $prototype instanceof MethodPrototypeReflection || $prototype instanceof NativeMethodReflection) { + $abstract = $prototype->isAbstract(); + if (is_bool($abstract)) { + if (!$abstract) { return null; } + } elseif (!$abstract->yes()) { + return null; + } + } + } + + $prototype = $method; + if (strtolower($method->getName()) === '__construct') { + foreach ($parentClass->getInterfaces() as $interface) { + if ($interface->hasNativeMethod($method->getName())) { + $prototype = $interface->getNativeMethod($method->getName()); } - } elseif (strtolower($methodName) === '__construct') { - return null; } } - return [$method, $method->getDeclaringClass(), true]; + return [ + $prototype, + $prototype->getDeclaringClass(), + true, + $method, + $declaringClass, + ]; } } diff --git a/src/Rules/Methods/OverridingMethodRule.php b/src/Rules/Methods/OverridingMethodRule.php index 3a568a95a8..f9023059fd 100644 --- a/src/Rules/Methods/OverridingMethodRule.php +++ b/src/Rules/Methods/OverridingMethodRule.php @@ -107,7 +107,13 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array return []; } - [$prototype, $prototypeDeclaringClass, $checkVisibility] = $prototypeData; + [ + $prototype, + $prototypeDeclaringClass, + $checkVisibility, + $inheritancePrototype, + $inheritancePrototypeDeclaringClass, + ] = $prototypeData; $messages = []; if ( @@ -119,8 +125,8 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array 'Method %s::%s() overrides method %s::%s() but is missing the #[\Override] attribute.', $method->getDeclaringClass()->getDisplayName(), $method->getName(), - $prototypeDeclaringClass->getDisplayName(true), - $prototype->getName(), + $inheritancePrototypeDeclaringClass->getDisplayName(true), + $inheritancePrototype->getName(), )) ->identifier('method.missingOverride') ->fixNode($node->getOriginalNode(), static function (Node\Stmt\ClassMethod $method) { @@ -132,24 +138,24 @@ public function processNode(Node $node, Scope&NodeCallbackInvoker $scope): array }) ->build(); } - if ($prototype->isFinalByKeyword()->yes()) { + if ($inheritancePrototype->isFinalByKeyword()->yes()) { $messages[] = RuleErrorBuilder::message(sprintf( 'Method %s::%s() overrides final method %s::%s().', $method->getDeclaringClass()->getDisplayName(), $method->getName(), - $prototypeDeclaringClass->getDisplayName(true), - $prototype->getName(), + $inheritancePrototypeDeclaringClass->getDisplayName(true), + $inheritancePrototype->getName(), )) ->nonIgnorable() ->identifier('method.parentMethodFinal') ->build(); - } elseif ($prototype->isFinal()->yes()) { + } elseif ($inheritancePrototype->isFinal()->yes()) { $messages[] = RuleErrorBuilder::message(sprintf( 'Method %s::%s() overrides @final method %s::%s().', $method->getDeclaringClass()->getDisplayName(), $method->getName(), - $prototypeDeclaringClass->getDisplayName(true), - $prototype->getName(), + $inheritancePrototypeDeclaringClass->getDisplayName(true), + $inheritancePrototype->getName(), ))->identifier('method.parentMethodFinalByPhpDoc') ->build(); } diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php index a309c79faa..da2ebf4925 100644 --- a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php +++ b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php @@ -803,6 +803,17 @@ public function testBug10165(): void $this->analyse([__DIR__ . '/data/bug-10165.php'], []); } + public function testBug11067(): void + { + $this->phpVersionId = PHP_VERSION_ID; + $this->analyse([__DIR__ . '/data/bug-11067.php'], [ + [ + 'Method Bug11067\BooleanBuilder2::__construct() overrides final method Bug11067\BaseBuilder2::__construct().', + 41, + ], + ]); + } + public function testBug9524(): void { $this->phpVersionId = PHP_VERSION_ID; diff --git a/tests/PHPStan/Rules/Methods/data/bug-11067.php b/tests/PHPStan/Rules/Methods/data/bug-11067.php new file mode 100644 index 0000000000..7245f5feca --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-11067.php @@ -0,0 +1,46 @@ + Date: Mon, 16 Feb 2026 08:43:50 +0100 Subject: [PATCH 2/5] Revert change --- src/Rules/Methods/MethodPrototypeFinder.php | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/Rules/Methods/MethodPrototypeFinder.php b/src/Rules/Methods/MethodPrototypeFinder.php index 0348a383b7..6f179bb711 100644 --- a/src/Rules/Methods/MethodPrototypeFinder.php +++ b/src/Rules/Methods/MethodPrototypeFinder.php @@ -85,17 +85,21 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa } $declaringClass = $method->getDeclaringClass(); - if ($declaringClass->hasConstructor() && $method->getName() === $declaringClass->getConstructor()->getName()) { - $prototype = $method->getPrototype(); - if ($prototype instanceof PhpMethodReflection || $prototype instanceof MethodPrototypeReflection || $prototype instanceof NativeMethodReflection) { - $abstract = $prototype->isAbstract(); - if (is_bool($abstract)) { - if (!$abstract) { + if ($declaringClass->hasConstructor()) { + if ($method->getName() === $declaringClass->getConstructor()->getName()) { + $prototype = $method->getPrototype(); + if ($prototype instanceof PhpMethodReflection || $prototype instanceof MethodPrototypeReflection || $prototype instanceof NativeMethodReflection) { + $abstract = $prototype->isAbstract(); + if (is_bool($abstract)) { + if (!$abstract) { + return null; + } + } elseif (!$abstract->yes()) { return null; } - } elseif (!$abstract->yes()) { - return null; } + } elseif (strtolower($methodName) === '__construct') { + return null; } } From 49734a5f1c0397c235d4756267a096f00fd25896 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Mon, 16 Feb 2026 08:50:02 +0100 Subject: [PATCH 3/5] Fix --- src/Rules/Methods/MethodPrototypeFinder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rules/Methods/MethodPrototypeFinder.php b/src/Rules/Methods/MethodPrototypeFinder.php index 6f179bb711..f776657187 100644 --- a/src/Rules/Methods/MethodPrototypeFinder.php +++ b/src/Rules/Methods/MethodPrototypeFinder.php @@ -108,6 +108,7 @@ public function findPrototype(ClassReflection $classReflection, string $methodNa foreach ($parentClass->getInterfaces() as $interface) { if ($interface->hasNativeMethod($method->getName())) { $prototype = $interface->getNativeMethod($method->getName()); + break; } } } From 1af9f78e674b3683785751e96c75446a7b10b506 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Mon, 16 Feb 2026 08:57:39 +0100 Subject: [PATCH 4/5] More non-regression tests --- .../Methods/OverridingMethodRuleTest.php | 12 +++++++ .../PHPStan/Rules/Methods/data/bug-12272.php | 32 +++++++++++++++++ .../PHPStan/Rules/Methods/data/bug-12830.php | 36 +++++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 tests/PHPStan/Rules/Methods/data/bug-12272.php create mode 100644 tests/PHPStan/Rules/Methods/data/bug-12830.php diff --git a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php index da2ebf4925..55979eeb77 100644 --- a/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php +++ b/tests/PHPStan/Rules/Methods/OverridingMethodRuleTest.php @@ -814,6 +814,18 @@ public function testBug11067(): void ]); } + public function testBug12272(): void + { + $this->phpVersionId = PHP_VERSION_ID; + $this->analyse([__DIR__ . '/data/bug-12272.php'], []); + } + + public function testBug12830(): void + { + $this->phpVersionId = PHP_VERSION_ID; + $this->analyse([__DIR__ . '/data/bug-12830.php'], []); + } + public function testBug9524(): void { $this->phpVersionId = PHP_VERSION_ID; diff --git a/tests/PHPStan/Rules/Methods/data/bug-12272.php b/tests/PHPStan/Rules/Methods/data/bug-12272.php new file mode 100644 index 0000000000..3c95602864 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-12272.php @@ -0,0 +1,32 @@ +MustBeString = $mustBeString; + $this->CanBeInt = $canBeInt; + } +} + +class B extends A +{ + public bool $CanBeBool; + + public function __construct(string $mustBeString, bool $canBeBool = false) + { + $this->MustBeString = $mustBeString; + $this->CanBeBool = $canBeBool; + } +} + +var_dump([ + new A('A', 1), + new B('B', true), +]); From 08fab17e7080af07a268cc32d445d560f0e447d1 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Mon, 16 Feb 2026 16:03:01 +0100 Subject: [PATCH 5/5] Add comment --- src/Rules/Methods/MethodPrototypeFinder.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Rules/Methods/MethodPrototypeFinder.php b/src/Rules/Methods/MethodPrototypeFinder.php index f776657187..e76d0b156a 100644 --- a/src/Rules/Methods/MethodPrototypeFinder.php +++ b/src/Rules/Methods/MethodPrototypeFinder.php @@ -29,6 +29,7 @@ public function __construct( * Returns two prototypes with different purposes: * - Signature prototype: Used for validating method signature (parameters, return type, ...). * - Inheritance prototype: Used for validating inheritance rules (final keyword, override attribute, ...). + * Also, return a bool to precise if the visibility of the prototype needs to be respected. * * @return array{ExtendedMethodReflection, ClassReflection, bool, ExtendedMethodReflection, ClassReflection}|null */