[DependencyInjection] Create an util class to determine services implementing a FQCN #20940
Conversation
| $services = $this->typeHelper->getOfType($type); | ||
| if (1 === count($services)) { | ||
| return $services[0]; | ||
| } elseif (1 < count($services)) { |
|
|
||
| try { | ||
| $name = (new \ReflectionClass($class))->name; | ||
| } catch (\ReflectionException $e) { |
There was a problem hiding this comment.
this works fine only if you have a fallback autoloader throwing a ReflectionException as a last resort to avoid the fatal error. As you extracted the logic, you need to handle this requirement in your new service
There was a problem hiding this comment.
This is needed in the AutowirePass because it fetches parameters type hint but this helper doesn't use anything else than the class name, is it still needed?
There was a problem hiding this comment.
the class may extend a non-existent parent class (if the service relies on an optional dependency which is not installed)
There was a problem hiding this comment.
You're right, it's updated in 98481b9.
| */ | ||
| final class ServiceTypeHelper | ||
| { | ||
| private static $classesName; |
| * | ||
| * @param Definition $definition | ||
| * | ||
| * @return string|false |
There was a problem hiding this comment.
I would use string|null (a nullable string type) instead of this mixed type
There was a problem hiding this comment.
I changed it but I'm not sure that's worth it (I used ?: to not change the isset to a array_key_exists).
|
Does it work fine when the compiler pass registers new autowired services ? The compiler pass currently updates it type map for them, but you removed lots of the logic. |
| $name = false; | ||
| } | ||
|
|
||
| return self::$classNames[$class] = $name ?: null; |
There was a problem hiding this comment.
Should I put brackets around it, use false or use array_key_exists?
|
Generally speaking, types should not be used for wiring things. We added some features recently to make it possible (ie #21530 ) but there is a big difference: there, the type is used on a very local scope only (the config file that uses it). Here, it opens the door to global type-related side effects. |
|
@nicolas-grekas this proposal was mostly intended for the DunglasActionBundle but now that you implemented |
Reopening of #18300.
This PR creates a new util class: the
ServiceTypeHelper. It extracts some of theAutowirePasslogic usable like this (only during the container compilation):This could be useful in the DunglasActionBundle, to detect when a class is already registered in the container.
Edit: one new advantage of using
autowireTypes: the types map is no longer built if they are sufficient!