[DependencyInjection] Improving autowiring error messages #17877
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…* something cannot be autowired
58b86a2 to
6fe97f9
Compare
|
👍 This will help a lot of developers |
|
👍 nice DX improvement. |
|
Thank you @weaverryan. |
fabpot
added a commit
that referenced
this pull request
Feb 22, 2016
…ges (weaverryan) This PR was merged into the 3.1-dev branch. Discussion ---------- [DependencyInjection] Improving autowiring error messages | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | not needed This does not change any behavior, but changes error messages to be more descriptive. **Before** > Unable to autowire argument of type "Doctrine\ORM\EntityManager" > for the service "my_cool_service". **After** > Unable to autowire argument of type "Doctrine\ORM\EntityManager" > for the service "my_cool_service". Multiple services exist for this class > (doctrine.orm.default_entity_manager, doctrine_orm.other_entity_manager). Commits ------- 6fe97f9 [DependencyInjection] Improving autowiring error messages to say *why* something cannot be autowired
|
Wouldn't it be good to do this in 2.8 too (I mean we already improved exception messages several times in patch releases in the past, didn't we?)? |
|
We made simple changes, not larger ones like this one. We need incentives for people to upgrade :) |
fabpot
added a commit
that referenced
this pull request
Mar 10, 2016
This PR was merged into the 3.1-dev branch. Discussion ---------- Fixing autowiring collision failure | Q | A | ------------- | --- | Branch | master | Bug fix? | yes (bug introduced in master) | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | n/a In #17877, I introduced a bug: https://github.com/symfony/symfony/pull/17877/files#diff-62df969ae028c559d33ffd256de1ac49L200. Namely, if some class cannot be autowired because there is an *odd* number of matching services, then it *would* autowire the last service found, instead of throwing an exception. The tests only tested even numbers, which is how it was missed. This fixes that. Thanks! Commits ------- 2aea337 Fixing a bug where an odd number of type collisions would incorrectly autowire (instead of an error)
fabpot
added a commit
that referenced
this pull request
May 13, 2016
…nem) This PR was merged into the 3.1-dev branch. Discussion ---------- Make failed autowiring error messages more explicit | Q | A | ------------- | --- | Branch? | master | Bug fix? | no (better DX integration) | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18658 | License | MIT | Doc PR | N/A @nicolas-grekas This is a port from #18691 (which was for **2.8**). It looks like the original PR has been already mostly implemented in **master** by @weaverryan (in #17877). IMO, this PR is still improving two use cases: * In case a class cannot be autowired because no service can be found AND a service cannot be registered automatically, the error message now mentions both conditions (instead of simply saying no service has been found) * In case a class with no service attached to it can be automatically instantiated but cannot be autowired because of further problems, an exception mentioning this is now thrown instead of the lower level exception, making the situation easier to debug. (The lower level exception is still available through `getPrevious()`) Hence, I still think this PR provides useful additional DX features and is worth to be merged. Commits ------- 6894e03 Make failed autowiring error messages more explicit
Merged
nicolas-grekas
added a commit
that referenced
this pull request
May 19, 2016
…e future friendly (lemoinem) This PR was merged into the 2.8 branch. Discussion ---------- [DX][DependencyInjection] Make Autowiring exceptions more future friendly | Q | A | ------------- | --- | Branch? | 2.8, 3.0 | Bug fix? | no (forward compatibility) | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A This is a follow-up on #18766 (3.1, master) and #18691 (2.8, 3.0). This PR changes the exception messages to the ones introduced in #18766 and #17877. I also unified the tests so they are exactly the same. This way the error messages will be the same in 2.8 onward. Commits ------- 834f550 [DX][DI] Make Autowiring exceptions more future friendly
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does not change any behavior, but changes error messages to be more descriptive.
Before
After