Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DependencyInjection] Tweaked factories#12065
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
562b870 to4264436CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sorry about that but PHP-CS-Fixer is buggy here. The previous code was "better". Can you revert this one and the other one as well. Thanks.
4264436 tocf905b7Compareunkind commentedSep 28, 2014
if ($definition->isSynthetic()) {$return[] ='@throws RuntimeException always since this service is expected to be injected dynamically'; }elseif ($class =$definition->getClass()) {$return[] =sprintf("@return %s A %s instance.",0 ===strpos($class,'%') ?'object' :"\\".$class,$class); }elseif ($definition->getFactory()) {$factory =$definition->getFactory();if (is_string($factory)) {$return[] =sprintf('@return object An instance returned by %s().',$factory); }elseif (is_array($factory) && (is_string($factory[0]) ||$factory[0]instanceof Definition ||$factory[0]instanceof Reference)) {if (is_string($factory[0]) ||$factory[0]instanceof Reference) {$return[] =sprintf('@return object An instance returned by %s::%s().', (string)$factory[0],$factory[1]); }elseif ($factory[0]instanceof Definition) {$return[] =sprintf('@return object An instance returned by %s::%s().',$factory[0]->getClass(),$factory[1]); } } }elseif ($definition->getFactoryClass()) {$return[] =sprintf('@return object An instance returned by %s::%s().',$definition->getFactoryClass(),$definition->getFactoryMethod()); }elseif ($definition->getFactoryService()) {$return[] =sprintf('@return object An instance returned by %s::%s().',$definition->getFactoryService(),$definition->getFactoryMethod()); } I thought it is not allowed to define service without |
unkind commentedSep 28, 2014
I'll fix tests later. |
nicolas-grekas commentedSep 29, 2014
👍 once tests are fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
the factory should win over the factory method, to be consistent with the code generated for dumped containers
stof commentedSep 29, 2014
Please also add a test instantiating a service with a factory in the ContainerBuilder (well, several to cover the different cases) |
b6ee1f8 tod9f2720Compareunkind commentedOct 1, 2014
Sorry for delay, tests have been updated. |
d9f2720 toee82392Compareunkind commentedOct 2, 2014
fabpot commentedOct 3, 2014
Looks good to me@unkind. This is merged now. Thanks. |
This PR was merged into the 2.6-dev branch.Discussion----------[DependencyInjection] Tweaked factories| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#12008| License | MIT| Doc PR | n/aThere are some issues with new service factories: * `ContainerBuilder` cannot instantiate service from factory (i.e. currently it works for dumped code only) * Dumped code sometimes is invalid (anonymous services as factories, factories without arguments)Commits-------ee82392 [DependencyInjection] Tweaked factories
There are some issues with new service factories:
ContainerBuildercannot instantiate service from factory (i.e. currently it works for dumped code only)