Correctly retrieve hasSideEffects metadata for inherited methods

[zonuexe]

Revert e97439c to fix phpstan/phpstan#13201

[zonuexe]

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.

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

Can you please debug why these lines are not sufficient?

phpstan-src/resources/functionMetadata.php

Lines 18 to 19 in 599ac2b

	'BackedEnum::from' => ['hasSideEffects' => false],
	'BackedEnum::tryFrom' => ['hasSideEffects' => false],

- they should be.

[zonuexe]

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.

[ondrejmirtes]

The code path that creates these methods should use the metadata too.

There's a chance it will fix more than just enums.

[zonuexe]

Here are some notes I've taken while looking at the code:

PHPStan\Reflection\ExtendedMethodReflection::isPure() is the authoritative source.

I checked it by injecting the following code into PHPStan\PhpDoc\PhpDocBlock::resolveMethodPhpDocBlockFromClass():

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;

ExtendedMethodReflection is degenerated by $classReflection = $parentReflection->getDeclaringClass() before being passed to the PhpDocBlock, so there's no way to get it out of here.

To bring out $isPure from here would require breaking backwards compatibility and extending PhpDocBlock or MethodReflection. But, PhpDocBlock::isPure() is unnatural and should not be extended.

Again, we need to pass an appropriate $isPure directly to PHPStan\Reflection\Php\PhpMethodReflection from PHPStan\Reflection\Php\PhpClassReflectionExtension::createUserlandMethodReflection().

Here's the problem: PhpDocBlock::resolveMethodPhpDocBlockFromClass() calls ClassReflection::getMethod() to get reflection of the parent class, which relies on PhpClassReflectionExtension contained in MethodsClassReflectionExtension[] $methodsClassReflectionExtensions.

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.

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

Perfect, thank you!