8000 [DI] Cannot declare class ...\DefinitionDecorator, because the name is already in use · Issue #21369 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] Cannot declare class ...\DefinitionDecorator, because the name is already in use #21369

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
wouterj opened this issue Jan 21, 2017 · 12 comments
Closed

[DI] Cannot declare class ...\DefinitionDecorator, because the name is already in use #21369

wouterj opened this issue Jan 21, 2017 · 12 comments
Labels
Bug DependencyInjection Status: Reviewed Status: Waiting feedback

Comments

@wouterj
Copy link
Member
wouterj commented Jan 21, 2017 •
edited
Loading
Q A
Bug report? yes
Feature request? no
BC Break report? yes
RFC? no
Symfony version 3.3-dev

When using DefinitionDecorator in a unit test, the error above will be triggered. Seems like the renaming is not done properly.

Test output

There was 1 error:

1) Symfony\Cmf\SeoBundle\Tests\Unit\DependencyInjection\CmfSeoExtensionTest::testErrorHandlingPHPCR
Cannot declare class Symfony\Component\DependencyInjection\DefinitionDecorator, because the name is already in use

/home/travis/build/symfony-cmf/seo-bundle/.phpunit/phpunit-5.3/vendor/symfony/phpunit-bridge/DeprecationErrorHandler.php:73
/home/travis/build/symfony-cmf/seo-bundle/vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/ChildDefinition.php:226
/home/travis/build/symfony-cmf/seo-bundle/vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php:16
/home/travis/build/symfony-cmf/seo-bundle/src/DependencyInjection/CmfSeoExtension.php:254
/home/travis/build/symfony-cmf/seo-bundle/src/DependencyInjection/CmfSeoExtension.php:240
/home/travis/build/symfony-cmf/seo-bundle/src/DependencyInjection/CmfSeoExtension.php:83
/home/travis/build/symfony-cmf/seo-bundle/vendor/matthiasnoback/symfony-dependency-injection-test/PhpUnit/AbstractExtensionTestCase.php:54
/home/travis/build/symfony-cmf/seo-bundle/tests/Unit/DependencyInjection/CmfSeoExtensionTest.php:192
@Simperfit
Copy link
Contributor
8000

Isn't it because of the class_alias on line 226 in ChildDefinition ?

@xabbuh
Copy link
Member
xabbuh commented Jan 23, 2017

@wouterj How can I reproduce this issue?

@chalasr
Copy link
Member
chalasr commented May 1, 2017

@wouterj Is this issue still relevant? I checked out the bundle and the test suite runs fine on my side.

@scaytrase
Copy link
Contributor
scaytrase commented May 6, 2017 •
edited
Loading

I've faced this issue today with 3.3@dev. Also I see a little problem with DefinitionDecorator is now just a clone of Definition, and all the decorating logic is missed. Is it a BC?

@scaytrase
Copy link
Contributor

The problem occures when doing like new DefinitionDecorator('some.service.id'). Does the mocking logic works properly? maybe the DefinitionDecorator should just extend ChildDefinition instead?

@ogizanagi
Copy link
Contributor
ogizanagi commented May 7, 2017 •
edited
Loading

@scaytrase: Could you provide a reproducer?

Also I see a little problem with DefinitionDecorator is now just a clone of Definition, and all the decorating logic is missed. Is it a BC?

IIRC, the idea was to trigger the autoload of ChildDefinition to create the DefinitionDecorator as an alias (see end of ChildDefinition.php file). The DefinitionDecorator class signature is only here to help IDEs and composer. But it's (supposed to be) never reached thanks to the return.

BTW, I tried on symfony-demo with dev-master, everything works, but I don't get the deprecation. Probably because ChildDefinition is used first and the class_alias makes the trigger_error in DefinitionDecorator.php being never reached.

@scaytrase
Copy link
Contributor

@ogizanagi I'll try to make one

scaytrase added a commit to scaytrase/symfony-21369 that referenced this issue May 7, 2017
@scaytrase
Reproducer for symfony/symfony#21369
c5469de
@scaytrase
Copy link
Contributor

@ogizanagi https://github.com/scaytrase/symfony-21369

Here it is. I cannot make a travis build for you (they have some issues ATM), but you can manually switch to symfony/symfony:3.3@dev or minimum-stability:dev ( the easiest way) to make this repo failing

@scaytrase
Copy link
Contributor

Stacktrace is following

Cannot declare class Symfony\Component\DependencyInjection\DefinitionDecorator, because the name is already in use
 C:\Work\Projects\scaytrase\symfony-21369\vendor\symfony\http-kernel\Kernel.php:546
 C:\Work\Projects\scaytrase\symfony-21369\vendor\symfony\dependency-injection\ChildDefinition.php:131
 C:\Work\Projects\scaytrase\symfony-21369\vendor\symfony\dependency-injection\DefinitionDecorator.php:16
 C:\Work\Projects\scaytrase\symfony-21369\DependencyInjection\Issue21369Extension.php:19

@scaytrase
Copy link
Contributor
scaytrase commented May 7, 2017 •
edited
Loading

https://travis-ci.org/scaytrase/symfony-21369/builds/229627906 Here is the travis. sf@3.1 build are failed for another reason (looks like phpunit 6 incompatibility)

3.3@dev failed for reason above

@ogizanagi ogizanagi mentioned this issue May 7, 2017
Merged
@ogizanagi
Copy link
Contributor
ogizanagi commented May 7, 2017 •
edited
Loading

@scaytrase : Thanks a lot for your help and for the reproducer. It seems the return trick isn't good enough when the DefinitionDecorator class is used first. Instead, I suggested another trick in #22657 which seems to work. Could you confirm it please?

Status: reviewed

@scaytrase
Copy link
Contributor

Locally applying your patch to vendor makes test pass. You have a travis build, so I think there is nothing more I can confirm. Thank you

fabpot added a commit that referenced this issue May 7, 2017
@fabpot
bug #22657 [DI] Fix Cannot declare class ...\DefinitionDecorator, bec…
5b6928d 
…ause the name is already in use (ogizanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Fix Cannot declare class ...\DefinitionDecorator, because the name is already in use

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21369
| License       | MIT
| Doc PR        | N/A

The `return` trick doesn't seem to work, and php is still trying to declare the `DefinitionDecorator` class, which causes the "Cannot declare class ...\DefinitionDecorator, because the name is already in use" error because of the `class_alias` previously declared in `ChildDefinition.php`.

This never happens as soon as the `ChildDefinition` class is used first, as the alias will take hand, but their are some situations, like in some unit test cases it can happen apparently, because `DefinitionDecorator` is used first.

Commits
-------

530849e [DI] Fix Cannot declare class ...\DefinitionDecorator, because the name is already in use
@fabpot fabpot closed this as completed May 7, 2017
@ogizanagi ogizanagi mentioned this issue May 7, 2017
Merged
@dpfaffenbauer dpfaffenbauer mentioned this issue May 9, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug DependencyInjection Status: Reviewed Status: Waiting feedback
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@fabpot @wouterj @xabbuh @ogizanagi @Simperfit @scaytrase @chalasr @carsonbot
0