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

Weird behaviour with PHP 7.1 nullable types #452

Closed
lougreenwood opened this issue Jan 11, 2017 · 6 comments
Closed

Weird behaviour with PHP 7.1 nullable types #452

lougreenwood opened this issue Jan 11, 2017 · 6 comments
Labels

Comments

@lougreenwood
Copy link

lougreenwood commented Jan 11, 2017

PHP 7.1 introduces nullable Types to be used when type-hinting: https://wiki.php.net/rfc/nullable_types

I'm finding that sometimes, PHP-DI will throw an error when I have a class construct signature which makes use of nullable types, e.g:

public function __construct( User $user, Thing $thing, ?OptionalThing $optional_thing ){

It seems that adding = null, e.g ?OptionalThing $optional_thing = null is the only way to get past this error.

It seems that the behaviour is somewhat random, sometimes it occurs if the param is the last in the method signature, sometimes if other params in the method have default values, e.g: int $another_thing = 0, ?OptionalThing $optional_thing

Here's the error I see, with class names replaced:

Entry "\app\models\collections\SomeClass" cannot be resolved: Entry "app\models\collections\AnotherClass" cannot be resolved: Entry "app\models\User" cannot be resolved: Parameter $id of __construct() has no value defined or guessable
Full definition:
Object (
    class = app\models\User
    scope = singleton
    lazy = false
    __construct(
        $id = #UNDEFINED#
    )
)
Full definition:
Object (
    class = app\models\collections\AnotherClass
    scope = singleton
    lazy = false
    __construct(
        $user = get(app\models\User)
        $some_other_class = get(app\models\collections\SomeOtherClass)
        $collection_type = #UNDEFINED#
        $offset = (default value) 0
        $limit = (default value) 0
    )
)
Full definition:
Object (
    class = \app\models\collections\SomeClass
    scope = singleton
    lazy = false
    __construct(
        $another_class = get(app\models\collections\AnotherClass)
        $collection_type = #UNDEFINED#
        $offset = #UNDEFINED#
        $limit = #UNDEFINED#
        $thing = get(app\models\thing)
    )
)
@mnapoli mnapoli added the bug label Jan 11, 2017
@mnapoli
Copy link
Member

mnapoli commented Jan 11, 2017

Thanks for the report, so just to get this right: you get the error for parameters that are nullable with ?, and for which you would want null injected, right?

Maybe $parameter->isOptional() doesn't return true for nullable types?

@lougreenwood
Copy link
Author

lougreenwood commented Jan 11, 2017

yes, I get errors when the params are nullable, indicated by the ? char - = null is an alternate method of indicating a nullable param (as mentioned in the RFC) and this seems to work on the occasions that ? starts causing trouble.

and for which you would want null injected, right?

I'm only using PHP-DI using the ->make() method, so I can't comment on what I would want auto-injecting. But in my case, because I'm using make and passing params, I have been able to verify that the params passed are valid.

I hope that makes sense, it's a difficult issue to describe because it isn't easy to consistently reproduce.

@lougreenwood
Copy link
Author

If it's any help, I also came up against an issue in a separate library which was also triggered by the use of the new nullable param feature.

hanneskod/classtools#10

Perhaps there's a common library you share with that classtools which causes the issue, or similar?

@Taluu
Copy link

Taluu commented Jan 12, 2017

@mnapoli I think you need to use $parameter->getType()->allowsNull() if available (it is not optionnal per se, you * meed* to put null after all) : http://nl1.php.net/manual/fr/reflectiontype.allowsnull.php

@lougreenwood
Copy link
Author

I'll be able to provide some examples in a few days - I'm slammed with a deadline at the moment, but after that passes tomorrow or Tuesday, I'll put together examples that demonstrate the bug. :)

@mnapoli
Copy link
Member

mnapoli commented Dec 9, 2017

I've pushed a test to cover PHP 7.1 nullable types, I'll close this issue unless we hear about it again.

@mnapoli mnapoli closed this as completed Dec 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants