Fix phpstan/phpstan#4860: Error „should return static(Test) but returns Test“#5190
Fix phpstan/phpstan#4860: Error „should return static(Test) but returns Test“#5190phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
- StaticType::getClassStringType() was delegating to getStaticObjectType() which returned the base ObjectType, losing the static/this type information - Changed it to return GenericClassStringType($this) directly, preserving the static type so get_class($this) correctly returns class-string<static(T)> - This fixes false positives when passing get_class($this) to template methods expecting class-string<T> and returning T, where the return should be static - New regression test in tests/PHPStan/Analyser/nsrt/bug-4860.php - Updated existing test expectations to reflect the more precise type
VincentLanglet
force-pushed
the
create-pull-request/patch-maf6i34
branch
from
098e97a to
a06cd93
Compare
|
|
||
| public function getClassStringType(): Type | ||
| { | ||
| return $this->getStaticObjectType()->getClassStringType(); |
There was a problem hiding this comment.
The method was initially implemented in #4456 where I just copied the strategy from other methods.
There was a problem hiding this comment.
Confirmed - the RawErrorFormatterTest failure is pre-existing and completely unrelated to our changes. Our PR changes are limited to StaticType.php and test expectations.
Summary: The review comment from VincentLanglet is purely informational context - they noted that the original getClassStringType() implementation in PR #4456 just copied the delegation pattern from other methods (which is what caused the bug). No code changes are requested. The branch is clean, static analysis passes with no errors, and all tests related to our changes pass. The one failing test (RawErrorFormatterTest) is a pre-existing issue on the repository unrelated to this PR.
The branch is already pushed and up to date with origin/create-pull-request/patch-maf6i34, so there's nothing further to push.
|
Everything is already up to date. The branch is pushed and the PR has been approved by VincentLanglet. Summary:
|
2 participants
Summary
get_class($this)was returningclass-string<ClassName>instead ofclass-string<static(ClassName)>, causing false positive errors when the result was passed to a template method that should preserve thestatictype. For example, acopy(): staticmethod calling$this->copyTo(get_class($this))wherecopyTohas@template Tand@param class-string<T>/@return Twould incorrectly report "should return static(Test) but returns Test".Changes
StaticType::getClassStringType()insrc/Type/StaticType.phpto returnnew GenericClassStringType($this)instead of delegating togetStaticObjectType()->getClassStringType(), which was losing the static type informationtests/PHPStan/Analyser/data/bug-7391b.php,tests/PHPStan/Analyser/nsrt/get-parent-class.php, andtests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.phpto reflect the more precise type descriptionsRoot cause
StaticType::getClassStringType()was delegating togetStaticObjectType()which resolves theStaticTypeto a plainObjectType, losing thestatic/$thisinformation. This meantget_class($this)returnedclass-string<Foo>instead ofclass-string<$this(Foo)>. When this was passed to a template method with@param class-string<T> @return T, the templateTresolved toFooinstead ofstatic(Foo), causing the false positive.Test
Added
tests/PHPStan/Analyser/nsrt/bug-4860.phpwhich verifies thatget_class($this)returnsclass-string<$this(ClassName)>and that passing it to a template method correctly preserves the static type.Fixes phpstan/phpstan#4860