Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[DI] added possibility to define services with abstract arguments#35076
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
[DI] added possibility to define services with abstract arguments#35076
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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 YamlLoader and YamlDumper should also be able to deal with abstract arguments
and ContainerBuilder should throw an exception when it encounters one when creating a service.
src/Symfony/Component/DependencyInjection/Argument/AbstractArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedDec 26, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Before spending more time here: is it worth it? |
fu22ybear commentedDec 26, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
most part of changes are tests. |
fu22ybear commentedDec 29, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@nicolas-grekas, what if specific pass will be registered in removing passes list, like DefinitionErrorExceptionPass, to check an abstract argument existence? and this pass will throw an exception, if an abstract argument is not replaced yet if this behavior is suitable, do dumpers still have to able to deal with abstract arguments? |
example for yaml config
tags functionality looks suitable |
8ea71a0
todd1f2cb
Compare...Symfony/Component/DependencyInjection/Compiler/AbstractableArgumentPresenceExceptionPass.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
dd1f2cb
to767e456
CompareThere 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.
some final comments :)
src/Symfony/Component/DependencyInjection/Argument/AbstractableArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractableArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/DependencyInjection/Argument/AbstractableArgument.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
767e456
to5982780
Compare5982780
to62fefaa
CompareThere 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.
Looks good to me! (I made some minor tweak FYI)
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.
Interesting :)
@Islam93 Nice 👍 thank you having a look at this issue. |
Thank you@Islam93. |
…t arguments (Islam93)This PR was merged into the 5.1-dev branch.Discussion----------[DI] added possibility to define services with abstract arguments| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets |Fix#31769| License | MIT| Doc PR | n/afeature caused by rfc#31769 from issues listI hope, this PR will be useful Abstract argument have to replaced by one of compiler passes or exception will be thrown. Example: This service definition ```xml ... <service> <argument key="$a" type="abstract">should be defined by TestPass</argument> </service> ... ```or this for yaml```yaml App\Test\Test: arguments: $a: !abstract should be defined by TestPass``` causes exception like `Argument "$a" of service "App\Test\Test" is abstract (should be defined by TestPass), did you forget to define it?` if argument was not replaced by compiler pass ```php ... public function process(ContainerBuilder $container) { $test = $container->getDefinition(Test::class); $test->setArgument('$a', 'test'); } ... ```Commits-------62fefaa [DI] added possibility to define services with abstract arguments
Uh oh!
There was an error while loading.Please reload this page.
feature caused by rfc#31769 from issues list
I hope, this PR will be useful
Abstract argument have to replaced by one of compiler passes or exception will be thrown.
Example:
This service definition
or this for yaml
causes exception like
Argument "$a" of service "App\Test\Test" is abstract (should be defined by TestPass), did you forget to define it?
if argument was not replaced by compiler pass