DynamicMethodReturnTypeExtension does not seem to work for static method calls

[asprega]

First of all, thanks for this amazing static analyzer.

Is it possible that DynamicMethodReturnTypeExtension does not work for static method calls?

I am using mockery in my test code and this is the code for creating a mock: \Mockery::mock(SomeClass::class). The return value of this is a \Mockery\MockInterface which I'd like to also "union" with the mocked type, in order to remove the errors PHPStan reports.

So, based on your example, I built this extension (NOTE: this does not yet provide the union type, but it should at least "convert" the mock type to the specific class type being mocked):

public static function getClass(): string
{
    return \Mockery::class;
}

public function isMethodSupported(MethodReflection $methodReflection): bool
{
    return $methodReflection->getName() === 'mock';
}

public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): Type
{
    if (count($methodCall->args) === 0) {
        return $methodReflection->getReturnType();
    }
    $arg = $methodCall->args[0]->value;
    $type = $scope->getType($arg);
    if ($type->getClass() !== null) {
        return $type;
    }

    return $methodReflection->getReturnType();
}

and registered it. I then executed phpstan over a file with plenty of \Mockery::mock calls and I noticed only getClass() was being called. PHPStan never asked for isMethodSupported from my extension, and the errors are still there. I suspect the extension system does not work with static method calls.

Is there anything I'm missing?

[ondrejmirtes]

You're not mising anything, it didn't occur to me yet that it should work for static methods too 😊 I'll fix it for the next version and your code will work. Thank you for the report.

…

On Sat, 7 Jan 2017 at 17:56, Andrea Sprega ***@***.***> wrote: First of all, thanks for this amazing static analyzer. Is it possible that DynamicMethodReturnTypeExtension does not work for static method calls? I am using mockery in my test code and this is the code for creating a mock: \Mockery::mock(SomeClass::class). The return value of this is a \Mockery\MockInterface which I'd like to also "union" with the mocked type, in order to remove the errors PHPStan reports. So, based on your example, I built this extension: public static function getClass(): string { return \Mockery::class; } public function isMethodSupported(MethodReflection $methodReflection): bool { return $methodReflection->getName() === 'mock'; } public function getTypeFromMethodCall(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope): Type { if (count($methodCall->args) === 0) { return $methodReflection->getReturnType(); } $arg = $methodCall->args[0]->value; $type = $scope->getType($arg); if ($type->getClass() !== null) { return $type; } return $methodReflection->getReturnType(); } and registered it. I then executed phpstan over a file with plenty of \Mockery::mock calls and I noticed only getClass() was being called. PHPStan never asked for isMethodSupported from my extension, and the errors are still there. Is there anything I'm missing? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#71>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGZuLWIj8exxZuvkJHETSsnqwgbe754ks5rP8PGgaJpZM4Ldd2R> .

[ondrejmirtes]

Implemented in e665b74. I actually had to add new interface DynamicStaticMethodReturnTypeExtension and service tag phpstan.broker.dynamicStaticMethodReturnTypeExtension to be able to mix these two interfaces in a single class which is something someone might want.

I know that DynamicStaticMethodReturnTypeExtension is quite a retarded name - the "dynamic" part is for the return type and "static" belongs to method.

Feel free to test it if it works like you want it to. I will add it to the nearest 0.5.2 release which will be out soon.

[asprega]

Hi @ondrejmirtes! Thanks for the very quick fix. I still have an issue and I'd like to understand whether it's my fault or not.

Again, this is my extension:

<?php

namespace MyApp\PHPStan;

use PhpParser\Node\Expr\StaticCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\DynamicStaticMethodReturnTypeExtension;
use PHPStan\Type\Type;

/**
 * Tells PHPStan to consider the mockery return type as dynamic (same as the mocked class).
 */
class MockeryDynamicReturnTypeExtension implements DynamicStaticMethodReturnTypeExtension
{
    public static function getClass() : string
    {
        return \Mockery::class;
    }

    public function getTypeFromStaticMethodCall(
        MethodReflection $methodReflection,
        StaticCall $methodCall,
        Scope $scope
    ) : Type {
        if (count($methodCall->args) === 0) {
            return $methodReflection->getReturnType();
        }

        $arg = $methodCall->args[0]->value;
        $type = $scope->getType($arg);
        if ($type->getClass() !== null) {
            return $type;
        }

        return $methodReflection->getReturnType();
    }

    public function isStaticMethodSupported(MethodReflection $methodReflection) : bool
    {
        return $methodReflection->getName() === 'mock';
    }
}

and this is how I register it:

services:
    - class: MyApp\PHPStan\MockeryDynamicReturnTypeExtension
      tags:
          - phpstan.broker.dynamicStaticMethodReturnTypeExtension

Now the extension method getTypeFromStaticMethodCall gets called, but it seems to always return at the last line.
Specifically, $arg does contain the correct type, example:

PhpParser\Node\Expr\ClassConstFetch::__set_state(array(
   'class' => 
  PhpParser\Node\Name\FullyQualified::__set_state(array(
     'parts' => 
    array (
      0 => 'MyApp',
      1 => 'SomeNamespace',
      2 => 'SomeClass',
    ),
     'attributes' => 
    array (
      'startLine' => 45,
      'endLine' => 45,
    ),
  )),
   'name' => 'class',
   'attributes' => 
  array (
    'startLine' => 45,
    'endLine' => 45,
  ),

I am sure \MyApp\SomeNamespace\SomeClass can be loaded from the autoloader.

$type, however, is always returned as an instance of PHPStan\Type\MixedType, so it doesn't have a class and the extension never returns the type I intend to (defaulting to the called function's actual type, which is \Mockery\MockInterface).

Sorry if I wasn't clear enough, let me know if I can provide you with more info. Thanks!

[ondrejmirtes]

Scope::getType() is for getting a type of an expression, like:

new Foo() // ObjectType (with class Foo)
1 // IntegerType
$this->foo() // type of the returned value from foo()

In your case, you need to write code if $arg is an instanceof ClassConstFetch, with property name === class.

[asprega]

So it was my fault ;) For future reference, I changed the implementation to:

public function getTypeFromStaticMethodCall(
        MethodReflection $methodReflection,
        StaticCall $methodCall,
        Scope $scope
    ) : Type {
        if (count($methodCall->args) === 0) {
            return $methodReflection->getReturnType();
        }

        $arg = $methodCall->args[0]->value;

        // Return a union type with \Mocker\MockInterface and the mocked class.
        if ($arg instanceof ClassConstFetch) {
            return new CommonUnionType(
                [
                    $methodReflection->getReturnType(),
                    new ObjectType((string) $arg->class, false),
                ],
                false
            );
        }

        return $methodReflection->getReturnType();
    }

and now everything works flawlessly. Thanks a lot for your help, much appreciated!

[ondrejmirtes]

You're missing one if. Imagine this:

\Mockery::mock(\Foo::SOME_CONSTANT);

In that case, your mock would return Foo. But that's not \Foo::class! You need to check if $arg->name is class.

[asprega]

Good catch :) thank you again!

[ondrejmirtes]

@asprega Actually, thanks to your original mistake, I realized that Foo::class can be detected as StringType and does not have to default to mixed. I think it would be also less confusing to you and you could have realized the difference on your own :)

The fix is in master.

[asprega]

right, I did not realize it before (I'm still getting acquainted with PHPParser) but it makes total sense now.