Skip to content

[DependencyInjection] Dumping out container may produce syntax error code #8030

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
tgabi333 opened this issue May 13, 2013 · 1 comment
Closed

[DependencyInjection] Dumping out container may produce syntax error code #8030

tgabi333 opened this issue May 13, 2013 · 1 comment
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required)

Comments

@tgabi333
Copy link
Contributor

Create a service where name contains "-" and there will be a syntax error.

I think the root of the problem is that PhpDumper uses Container::camelize to generate function names which doesn't comply with PHP labels.

In my opinion, PhpDumper should use an internal function to generate PHP label-safe identifiers from service names, it would be more general solution than restrict service names.

If you agree i would like to push this change.
@GromNaN GromNaN mentioned this issue Jul 16, 2013
Merged
@GromNaN
Copy link
Member

GromNaN commented Jul 18, 2013

There is 2 places where service ids are converted to PHP method names:

They must be compatible.

This was referenced Jul 19, 2013
Closed
Merged
fabpot added a commit that referenced this issue Jul 20, 2013
@fabpot
merged branch GromNaN/di-dump-exception (PR #8494)
4970770 
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
fabpot added a commit that referenced this issue Jul 20, 2013
@fabpot
merged branch GromNaN/2.3-di-dump-exception (PR #8524)
017c44d 
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
@fabpot fabpot closed this as completed Jul 20, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fabpot @tgabi333 @GromNaN