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

Prevent PropertyNotSetInConstructor #142

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Punk-UnDeaD
Copy link
Contributor

@Punk-UnDeaD Punk-UnDeaD commented Feb 12, 2021

Support @reqired annotation and #[Required] attribute for properties.
Prevent PropertyNotSetInConstructor when ContainerAwareTrait used.

for #141 and #140

@seferov
Copy link
Member

seferov commented Feb 26, 2021

@Punk-UnDeaD thank you for your contribution. Could you please rebase your branch or merge with up-to-date master to pass the tests?

@Punk-UnDeaD Punk-UnDeaD force-pushed the master branch 4 times, most recently from 25380ac to aa267ec Compare March 1, 2021 17:33

final class MyServiceB {
/** @required */
public MyServiceA $a;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately this is not working for < PHP 7.4

@Punk-UnDeaD Punk-UnDeaD force-pushed the master branch 7 times, most recently from 9f89da0 to 19dba20 Compare March 24, 2021 08:43
Prevent PropertyNotSetInConstructor when ContainerAwareTrait used.
@ostrolucky
Copy link
Contributor

What's the status here, you need some help or anything? CI is failing, so it can't really be expected to be merged in this state.

@Punk-UnDeaD
Copy link
Contributor Author

@ostrolucky i fixed all my issues, but tests failed on 'Class 'Symfony\Component\BrowserKit\AbstractBrowser' not found' and i don't know how to skip this

@VincentLanglet
Copy link
Contributor

I fixed all the tests issue in #242, maybe this PR could be rebased after the merge of mine @seferov @Punk-UnDeaD ?

@MisatoTremor
Copy link

Just a heads up: There already is a handler for the annotation in src/Handler/RequiredSetterHandler.php. Shouldn't that be removed in favor for this solution?

@aurimasniekis
Copy link

Any progress on this?

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.

None yet

6 participants