[DependencyInjection] Add exception for service name not dumpable in PHP #8494
Conversation
| $name = Container::camelize($id); | ||
|
|
||
| if (!preg_match('/^[a-zA-Z0-9_\x7f-\xff]+$/', $name)) { | ||
| throw new InvalidArgumentException(sprintf('"%s" service name cannot converted to a valid PHP method name.', $id)); |
There was a problem hiding this comment.
please use service id instead of service name in the message
|
I've submitted this PR to 2.2, but it will not merge well into 2.3 as the method name is generated in many more places. Should I resubmit the PR on 2.3 or master ? |
|
If there are significant changes, submitting a PR for 2.3 too makes sense. Thanks. |
This PR was merged into the 2.2 branch. Discussion ---------- [DependencyInjection] Add exception for service name not dumpable in PHP | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #8485 #8030 | License | MIT | Doc PR | n/a Throws an exception when the DIC is dumped to PHP, before generating invalid PHP. The regex comes from the PHP doc: http://www.php.net/manual/en/language.oop5.basic.php Commits ------- 242b318 [DependencyInjection] Add exception for service name not dumpable in PHP
This PR was merged into the 2.3 branch. Discussion ---------- [DependencyInjection][2.3] Add exception for service name not dumpable in PHP Same as #8494 for branch 2.3 since the DI component has been refactored (bb797ee, f1c2ab7) | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #8485 #8030 | License | MIT | Doc PR | n/a Throws an exception when the DIC is dumped to PHP, before generating invalid PHP. The regex comes from the PHP doc: http://www.php.net/manual/en/language.oop5.basic.php Commits ------- 9ac3556 [DependencyInjection] Add exception for service name not dumpable in PHP
|
This is a serious BC break. We have a lot of services with "" in their names - they reflect their classnames. In the past, the bug of generating invalid PHP code was fixable by performing search&replace on the generated container code. Now we get this exception from PhpDumper. This could be easily solved if the "camelize" method was overridable. |
|
Why do you use "" in your service names ? |
|
Most of our services match their classes' names, which simplifies autowiring and prevents the class name duplication in YAML "class:" parameter (with our custom container pass). |
…on (ondrejmirtes) This PR was submitted for the 2.4 branch but it was merged into the 2.3 branch instead (closes #9610). Discussion ---------- Container::camelize also takes backslashes into consideration I [complained](#8494) about unreported and unnoticed BC break. Service names in our DI container contain backslashes and that pull request rendered newer versions of Symfony unusable for us. This commit fixes this - it normalizes backslashes in service names into underscores. Commits ------- 0f9bb0a Container::camelize also takes backslashes into consideration
Throws an exception when the DIC is dumped to PHP, before generating invalid PHP.
The regex comes from the PHP doc: http://www.php.net/manual/en/language.oop5.basic.php