Correctly retrieve hasSideEffects metadata for inherited methods#4079
Correctly retrieve hasSideEffects metadata for inherited methods#4079ondrejmirtes merged 8 commits intophpstan:2.1.xfrom
Conversation
98a9c9b to
17b4ef2
Compare
stubs/BackedEnum.stub
Outdated
| public static function from(int|string $value): static; | ||
|
|
||
| /** | ||
| * @return ?static |
There was a problem hiding this comment.
While debugging with @nsfisis, he pointed out that this DocBlock causes the return value to be a mixed type.
Although it is redundant, in the scope of this PR it makes sense to duplicate the type declarations in the PHPDoc.
|
This pull request has been marked as ready for review. |
ondrejmirtes
left a comment
There was a problem hiding this comment.
Can you please debug why these lines are not sufficient?
phpstan-src/resources/functionMetadata.php
Lines 18 to 19 in 599ac2b
|
I discovered that the functionMetadataMap wasn't being accessed in this process, so in this PR I only fixed the metadata. I agree that this should be the case, so I'll investigate the underlying issue. |
|
The code path that creates these methods should use the metadata too. There's a chance it will fix more than just enums. |
BackedEnum methods
Here are some notes I've taken while looking at the code:
I checked it by injecting the following code into modified src/PhpDoc/PhpDocBlock.php
@@ -343,6 +343,8 @@ final class PhpDocBlock
if ($parentReflection->isPrivate()) {
return null;
}
+ var_dump(["{$classReflection->getName()}::{$name}" => $parentReflection->isPure()->describe()]);
+ $isPure = $parentReflection->isPure()->maybe() ? null : $parentReflection->isPure()->yes();
$classReflection = $parentReflection->getDeclaringClass();
$traitReflection = null;
To bring out Again, we need to pass an appropriate Here's the problem: Therefore, injecting the following code will result in an infinite loop due to recursive calls. modified src/Reflection/Php/PhpClassReflectionExtension.php
@@ -839,7 +839,16 @@ final class PhpClassReflectionExtension
}
$isInternal = $resolvedPhpDoc->isInternal();
$isFinal = $resolvedPhpDoc->isFinal();
- $isPure = $resolvedPhpDoc->isPure();
+ $isPure = null;
+ if ($actualDeclaringClass->hasNativeMethod($methodReflection->getName())) {
+ var_dump("{$actualDeclaringClass->getName()}::{$methodReflection->getName()}");
+ $parentReflection = $actualDeclaringClass->getNativeMethod($methodReflection->getName());
+ if (!$parentReflection->isPrivate()) {
+ $isPure = $parentReflection->isPure()->maybe() ? null : $parentReflection->isPure()->yes();
+ }
+ }
+
+ $isPure ??= $resolvedPhpDoc->isPure();
$asserts = Assertions::createFromResolvedPhpDocBlock($resolvedPhpDoc);
$acceptsNamedArguments = $resolvedPhpDoc->acceptsNamedArguments();
$selfOutType = $resolvedPhpDoc->getSelfOutTag() !== null ? $resolvedPhpDoc->getSelfOutTag()->getType() : null;We can get the metadata directly as follows, but first we need to identify the parent class name: if ($this->signatureMapProvider->hasMethodMetadata($declaringClassName, $methodReflection->getName())) {
$hasSideEffects = TrinaryLogic::createFromBoolean($this->signatureMapProvider->getMethodMetadata($declaringClassName, $methodReflection->getName())['hasSideEffects']);
} else {
$hasSideEffects = TrinaryLogic::createMaybe();
}Checking for side effects from ancestor metadata. |
BackedEnum methodsBackedEnum methods
BackedEnum methods|
This pull request has been marked as ready for review. |
|
Perfect, thank you! |
Revert e97439c to fix phpstan/phpstan#13201