Skip to content
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] Validate class-like service IDs #33111

Closed
wants to merge 1 commit into from

Conversation

@ro0NL
Copy link
Contributor

commented Aug 11, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #...
License MIT
Doc PR symfony/symfony-docs#...

Alternative approach of #33092 for 3.4 (enables to continue #33108 for 4.3)

cc @fabpot @nicolas-grekas

@ro0NL ro0NL changed the base branch from 4.4 to 3.4 Aug 11, 2019

@ro0NL

This comment has been minimized.

Copy link
Contributor Author

commented Aug 11, 2019

From the original issue, im not sure

Class “” used for service “\App\Some\Service” cannot be found.

happens in 3.4, i got the error at https://github.com/symfony/symfony/pull/33111/files#diff-81d8d6906c0de563850dc240f5a1d213R77

@@ -65,6 +65,14 @@ public function process(ContainerBuilder $container)
$id
));
}
if (preg_match('/^\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)++$/', $id)) {
throw new RuntimeException(sprintf(
'The definition for "%s" has no class attribute, and appears to reference a class or interface. '

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 12, 2019

Member

and appears to reference a class or interface

actually not, since the class doesn't exist
that + the fact that the message reported in #33092 is not the one just below, I'm not sure this PR is the correct answer.

This comment has been minimized.

Copy link
@ro0NL

ro0NL Aug 12, 2019

Author Contributor

you're right. Let's continue in #33108

AFAIK the if block at L51 is now dead code :/

@ro0NL ro0NL closed this Aug 12, 2019

@ro0NL ro0NL deleted the ro0NL:leading-slash branch Aug 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.