Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
May 1, 2020

Conversation

Khartir
Copy link
Contributor

@Khartir Khartir commented Apr 29, 2020

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:

  • Yii::createObject('\Foo') will return an instance of \Foo
  • Yii::createObject(\Foo::class) will return an instance of \Foo
  • Yii::createObject('alias') will return an instance of alias. More on this later.
  • Yii::createObject(['class' => '\Foo']) will return an instance of \Foo
  • Yii::createObject(function: \Foo {}) will return an instance of \Foo

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
Copy link
Contributor

akondas commented Apr 29, 2020

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

@Khartir
Copy link
Contributor Author

Khartir commented Apr 30, 2020

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

@marmichalski
Copy link
Contributor

Thanks

@marmichalski marmichalski merged commit d50c2a2 into proget-hq:master May 1, 2020
@Khartir Khartir deleted the creatObject-stub branch May 26, 2020 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants