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

Add autowiring-support for PHP7.4 typed properties #708

Merged
merged 1 commit into from
Apr 6, 2020

Conversation

bzikarsky
Copy link

The current implementation prioritizes @var and the name property of the @Inject annotation over the type of the property. This allows to still inject custom-named entries into the properties even in the presence of types.

Fixes #707

@bzikarsky
Copy link
Author

I get a bunch of formatting errors on source files I did not touch, is this intentional?

@bzikarsky
Copy link
Author

It looks like, phpstan needs to be pushed to PHP7.4 only - I am not sure if you want me to do this in this branch? If you update master, I'll happily rebase.

@mnapoli
Copy link
Member

mnapoli commented Mar 19, 2020

That pull request is awesome! Thank you for covering all cases!

I will try to sort out master, hang on ^^

@mnapoli
Copy link
Member

mnapoli commented Mar 19, 2020

👍 I updated master, you can rebase/merge it in your branch.

@bzikarsky bzikarsky force-pushed the typed-property-support branch 5 times, most recently from 024740b to 22a6796 Compare March 20, 2020 10:58
@bzikarsky
Copy link
Author

phpstan pointed to me to two issues, I now fixed (and squashed):

  1. getName() only exists for ReflectionNamedType

  2. I didn't handle scalar-typed properties "correctly" (the same way @var does)
    This turned out to be a bit more complex and is not 100% fixed. The separate annotation-reader project has an internal list of ignored types which I couldn't access but also didn't want to replicate. So I decided that I ignore the type on the typed property if I cannot locate a class or interface with the name. It's probably close enough to the desired behaviour. WDYT?

@mnapoli
Copy link
Member

mnapoli commented Mar 20, 2020

Yes that looks good to me.

There is one last thing to fix regarding code formatting, I opened a code suggestion.

The current implementation prioritizes `@var` and the `name` property of
the `@Inject` annotation over the type of the property. This allows to
still inject custom-named entries into the properties even in the
presence of types.
@bzikarsky
Copy link
Author

Thanks, missed this. PR is updated with the squashed change.

@bzikarsky
Copy link
Author

Do we have an ETA on this being merged (and tagged incl. the other PR)? :)

@bzikarsky
Copy link
Author

Ping :) -- I have a bunch of developers eagerly waiting for this feature and I'd rather not switch our projects to our forked repo.

@mnapoli
Copy link
Member

mnapoli commented Apr 6, 2020

Sorry, I disable email notifications because I get so many of them every day, it creates a lot of anxiety :)

Let's give that a go!

@mnapoli mnapoli merged commit 69238bd into PHP-DI:master Apr 6, 2020
@bzikarsky
Copy link
Author

Awesome, thanks. I'll give it a go in prod today. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support PHP7.4 typed properties in combination with @var
2 participants