proget-hq / phpstan-yii2

Yii2 extension for PHPStan
MIT License
52 stars 18 forks source link

Attempt at a better type resolution on Yii::createObject #28

Closed Khartir closed 4 years ago

Khartir commented 4 years ago

This PR attempts to provide more flexibility in the way Yii::createObject can be used, by using generic typehints instead of an extension. The current implementation of the extension only allows for a few ways to use Yii::createObject:

Anything else will cause the extension to fail so that phpstan will only report an internal error. This can be something as simple as this:

$className = \Foo::class;
Yii::createObject($className);

By using a stub with generic typehints phpstan can properly resolve variables, method calls, etc.

There is however one problem. As mentioned above, currently it is possible to do something like this: Yii::createObject('alias') This works in Yii, because Yii::createObject() is just a wrapper for Yii::$container->get() when called with a string as the first argument. As far as i can tell, this is not documented behavior: https://www.yiiframework.com/doc/api/2.0/yii-baseyii#createObject()-detail But it works. This extension currently assumes that the return value of Yii::createObject('alias') is an instance of a class called alias. This can easily be corrected by properly typehinting at the calling site:

/** @var \Foo $foo */
$foo = Yii::createObject('alias');

However with generics phpstan attempts to resolve the classname and will report an error: Parameter #1 $type of static method yii\BaseYii::createObject() expects array('class' => class-string<alias>)|(callable(): alias)|class-string<alias>, 'alias' given.

This can not be solved by typehinting on the calling site, however it is possible to just use Yii::$container->get() instead. This has the added benefit of resolving properly to the correct return type, if the config file is properly set up.

What do you think?

akondas commented 4 years ago

For me looks good :+1:, either way, the test must pass so that we can talk further. @marmichalski?

Khartir commented 4 years ago

Those were some dumb mistakes I made... Anyway fixed now.

marmichalski commented 4 years ago

Thanks