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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DI] Fix autowire error for inlined services #22857

Closed
wants to merge 2 commits into
base: 3.3
from

Conversation

Projects
None yet
4 participants
@weaverryan
Member

weaverryan commented May 22, 2017

Q A
Branch? 3.3
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #22848
License MIT
Doc PR n/a

The AutowirePass defers autowiring exceptions until later so that we don't throw autowiring exceptions for services that are ultimately removed. But, if a service is inlined, then it appears to be removed, and so we don't throw the exception. This fixes that.

It's an easy fix - but it's a bit ugly. We're adding a bit more "state" to the passes... simply because there is some information that needs to be shared through the compiler process. There might be a better way of doing this in the future (e.g. storing some metadata on the Compiler), but this does work well.

new AnalyzeServiceReferencesPass(),
new RemoveUnusedDefinitionsPass(),
)),
new AutowireExceptionPass($autowirePass),
new AutowireExceptionPass($autowirePass, $inlinedServicePass),

This comment has been minimized.

@ro0NL

ro0NL May 22, 2017

Contributor

perhaps pass $inlinedServicePass->getInlinedServiceIds() directly?

@ro0NL

ro0NL May 22, 2017

Contributor

perhaps pass $inlinedServicePass->getInlinedServiceIds() directly?

This comment has been minimized.

@weaverryan

weaverryan May 22, 2017

Member

I'd love to, but the passes haven't run yet at this point

@weaverryan

weaverryan May 22, 2017

Member

I'd love to, but the passes haven't run yet at this point

This comment has been minimized.

@nicolas-grekas

nicolas-grekas May 22, 2017

Member

the pass did not run yet so this would just return an empty array

@nicolas-grekas

nicolas-grekas May 22, 2017

Member

the pass did not run yet so this would just return an empty array

This comment has been minimized.

@ro0NL

ro0NL May 22, 2017

Contributor

aargh.. right :)

@ro0NL

ro0NL May 22, 2017

Contributor

aargh.. right :)

*
* @return array Service id strings
*/
public function getInlinedServiceIds()

This comment has been minimized.

@ro0NL

ro0NL May 22, 2017

Contributor

@internal?

@ro0NL

ro0NL May 22, 2017

Contributor

@internal?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas May 22, 2017

Member

I don't think so

@nicolas-grekas

nicolas-grekas May 22, 2017

Member

I don't think so

@nicolas-grekas

👍 (after fabbot fix)

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas
Member

nicolas-grekas commented May 23, 2017

Thank you @weaverryan.

nicolas-grekas added a commit that referenced this pull request May 23, 2017

bug #22857 [DI] Fix autowire error for inlined services (weaverryan)
This PR was squashed before being merged into the 3.3 branch (closes #22857).

Discussion
----------

[DI] Fix autowire error for inlined services

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

The `AutowirePass` defers autowiring exceptions until later so that we don't throw autowiring exceptions for services that are ultimately removed. But, if a service is *inlined*, then it appears to be removed, and so we don't throw the exception. This fixes that.

It's an easy fix - but it's a bit ugly. We're adding a bit more "state" to the passes... simply because there is some information that needs to be shared through the compiler process. There might be a better way of doing this in the future (e.g. storing some metadata on the `Compiler`), but this *does* work well.

Commits
-------

4bcef3d [DI] Fix autowire error for inlined services

@weaverryan weaverryan deleted the weaverryan:fix-autowire-inlined-error branch May 23, 2017

@fabpot fabpot referenced this pull request May 29, 2017

Merged

Release v3.3.0 #22949

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment