[DX] [DI] Throw more helpful error when shortcutting global classes #22153
Conversation
carsonbot
added
Status: Needs Review
DX
| @@ -48,6 +48,14 @@ public function process(ContainerBuilder $container) | |||
| if ($definition->getFactory()) { | |||
| throw new RuntimeException(sprintf('Please add the class to service "%s" even if it is constructed by a factory since we might need to add method calls based on compile-time checks.', $id)); | |||
| } | |||
| if (class_exists($id) || interface_exists($id)) { | |||
There was a problem hiding this comment.
false should be added as second argument to interface_exists
There was a problem hiding this comment.
Why that? We want to check whether the interface can be loaded by the project's autoloader, not whether it has been loaded during compilatiom.
There was a problem hiding this comment.
Because the class_exists will already have called the autoloader
There was a problem hiding this comment.
Valid point (had a few beers tonight ;) )
| throw new RuntimeException(sprintf( | ||
| 'Service "%s" appears to reference a class in the global namespace, and does ' | ||
| .'not have a class defined. Leaving out the class property is only allowed ' | ||
| .'for namespaced classes. Specify the class explicitly to get rid of this error.', |
There was a problem hiding this comment.
Full message could be eg.:
The definition for "%s" has no class. Leaving out the "class" attribute is only allowed for namespaced classes. Please specify this attribute explicitly to get rid of this error.
There was a problem hiding this comment.
That is imho less clear as the "appears to reference a class in the global namespace" part is key to the DX - it explains that what the developer is trying to do is not allowed, and then why. In your suggestion it's not emphasized that the global namespace is the issue.
I do use the word property instead of attribute. Is that convention?
…asses As discussed in symfony#22146 the error message received when trying to use a class in the global namespace as a service without defined class is confusing. Helpful information was added pointing out this current limitation.
|
Improved exception clarity and added parameter to |
|
Thank you @curry684. |
…bal classes (curry684) This PR was merged into the 3.3-dev branch. Discussion ---------- [DX] [DI] Throw more helpful error when shortcutting global classes | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? |no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22146 | License | MIT As discussed in #22146 the error message received when trying to use a class in the global namespace as a service without defined class is confusing. Helpful information was added pointing out this current limitation. Commits ------- b9e7b4f [DependencyInjection] Throw helpful error when shortcutting global classes
As discussed in #22146 the error message received when trying to use a class in the global
namespace as a service without defined class is confusing. Helpful information was added
pointing out this current limitation.