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

Fix autowiring null #1081

Merged
merged 6 commits into from Aug 6, 2018

Conversation

Projects
None yet
3 participants
@Taluu
Copy link
Contributor

commented Sep 24, 2017

Ref #1080, but not sure it really fixes it, as we still have a problem if the class is not loadable (because it doesn't exist). But IMO this is not the concern of the autowirer, but more the responsability of the organizers...

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented Sep 25, 2017

Nevermind, solved the problem with the last commit, a try... catch ReflectionException was all it needed. :}

@everzet

This comment has been minimized.

Copy link
Member

commented Nov 20, 2017

@Taluu you used slightly wrong assertion class. Can you update and I'll merge?

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented Nov 20, 2017

You mean using the namespaced one instead ? Sure, I'll rebase while I'm at it then.

@Taluu Taluu force-pushed the Taluu:fix-autowiring-null branch from fd7a5ab to 2223007 Nov 23, 2017

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented Nov 23, 2017

Rebased and fixed the correct class. :}

@Taluu Taluu force-pushed the Taluu:fix-autowiring-null branch from 2223007 to 73b749e Nov 27, 2017

@everzet

This comment has been minimized.

Copy link
Member

commented Nov 30, 2017

@Taluu top one, man :)

@Taluu Taluu force-pushed the Taluu:fix-autowiring-null branch from 6c1125e to d635738 Nov 30, 2017

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented Nov 30, 2017

You mean in the Autowirer ? Looks good to me, It should not autowire if the key / argument exists in the index or the arguments or if the class can't be loaded ?

Unless I'm reading it wrong.

@Taluu Taluu force-pushed the Taluu:fix-autowiring-null branch from d635738 to aeeaeba Jan 29, 2018

@Taluu Taluu force-pushed the Taluu:fix-autowiring-null branch from aeeaeba to 784583e May 9, 2018

@Taluu

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2018

A shot in the dark : Ping ? @ciaranmcnulty @stof @everzet

(branch updated and rebased)

@Taluu Taluu force-pushed the Taluu:fix-autowiring-null branch 3 times, most recently from 0828c95 to 923812f May 10, 2018

Taluu added some commits Sep 24, 2017

Check with array_key_exists for autowiring arguments
isset() returns false for `null` value, so if we put a `null` value
in the list arguments, it will be ignored and "something else" can
be used instead.

This is particularly bothersome if the type-hinted class does not
exist, even though a `null` value has been specifically used.

@Taluu Taluu force-pushed the Taluu:fix-autowiring-null branch from 495503f to 80e7340 Jun 2, 2018

@ciaranmcnulty ciaranmcnulty merged commit 8ccd676 into Behat:master Aug 6, 2018

3 checks passed

Scrutinizer Analysis: No new issues – Tests: passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ciaranmcnulty

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2018

Thanks, @Taluu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.