ReflectionParameter::hasType() should assert return type of ReflectionParameter::getType()

[dbrekelmans]

If ReflectionParameter::hasType() returns true, then ReflectionParameter::getType() will always return ReflectionType.

If ReflectionParameter::hasType() returns false, then ReflectionParameter::getType() will always return null.

This is not currently understood by psalm.

https://psalm.dev/r/257f4e884d

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/257f4e884d

<?php

static function () : void {
    $method = new ReflectionMethod(stdClass::class);
    $parameters = $method->getParameters();
    
    foreach ($parameters as $parameter) {
        if (!$parameter->hasType()) {
            throw new RuntimeException('No type found.');
        }
        
        echo($parameter->getType()->__toString());
    }
};

Psalm output (using commit 3106635):

ERROR: PossiblyNullReference - 12:37 - Cannot call method __toString on possibly null value

[dbrekelmans]

@weirdan If you have some pointers for where to look.. I can attempt a PR myself ;)

[orklah]

Well, I don't think of any other way other than creating a new ReturnTypeProvider for this.

The complexity here is that the return type depends on the result of another method rather than the parameters of the method.

I looked in Psalm and in Psalm's plugin and it doesn't appear to have already been done anywhere.

However, I think we could exploit method memoization setting (MethodCallAnalyzer line 204) that store the result of previous methods in the context.

By retrieving $event->getContext()->vars_in_scope in the ReturnTypeProvider, you should be able to know the type returned by hasType when calling getType.

As I said, I don't think it has ever been done and it's exploiting a not totally relevant setting and feature to work. I would have no remorse doing that in one of my plugins, but I'm not sure it's good enough to be added in core.

Maybe instead of using memoization setting, Psalm could have a list of methods that should be memoized anyway for this kind of feature

[dbrekelmans]

Is this not do-able with a ReflectionParameter stub with an assertion annotation?

For example:

class ReflectionParameter {
    /*
     * @var ReflectionType|null
     */
    private $type;

    /*
     * @psalm-assert-if-true !null $type
     */
    public function hasType() : bool
    {
        return $this->type !== null;
    }

    public function getType() : ?ReflectionType
    {
        return $this->type;
    }
}

[orklah]

Nope, Psalm currently has no knowledge of the internal state of an object (except for it's type, and potential templates EDIT: also except array-like objects). That means you can't convey datas between calls on the same object. (it has been suggested and refused before)

That's why I suggested using data from the context that has called the methods.

[dbrekelmans]

Interesting. I will take a look later and see if I can produce a result. Thank you for your input @orklah

[weirdan]

Maybe instead of using memoization setting, Psalm could have a list of methods that should be memoized anyway for this kind of feature

Doesn't Psalm already do this for mutation-free methods? Actually ReflectionParameter as a whole looks immutable, and declaring it as such should enable memoization for all of its methods.

[orklah]

Doesn't Psalm already do this for mutation-free methods? Actually ReflectionParameter as a whole looks immutable, and declaring it as such should enable memoization for all of its methods.

I don't have much practice with purity in Psalm, but it looks right. There's some code that looks like that in MethodCallPurityAnalyzer line 144

[muglug]

It turns out Psalm already had pretty much everything necessary – the only blocker was type-insensitivity of assertions on $this->getType(), which is now fixed.