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

Preceeding slash confusing fully qualified class names #237

Open
M1ke opened this issue Nov 18, 2021 · 1 comment · May be fixed by #239
Open

Preceeding slash confusing fully qualified class names #237

M1ke opened this issue Nov 18, 2021 · 1 comment · May be fixed by #239

Comments

@M1ke
Copy link

M1ke commented Nov 18, 2021

This may be a known limitation, or even designed in, but we've stumbled across an interesting case of class naming as an issue:

// We might be using a string to run logic around loops/interpolation
$some_class = "\\Namespace\\ClassName";
$container->add($some_class);

Elsewhere

$container->get(ClassName::class)

This later access will not find the existing entry due to the leading slashes.

I wonder if there's a case for stripping leading slashes during name resolution - I can't see a situation where somebody would want "$key" and "\\$key" to resolve differently, and it avoids this being a hard-to-spot bug in a larger container setup.

Happy to PR for the feature if acceptable.

@philipobenito
Copy link
Member

Hi @M1ke

Yeah I'd be in favour of this, would need to normalise on strings used to add to the container as well as retrieving from the container so as to ensure it's not a breaking change for anyone who's taken account of this in their application code.

I'm not certain but there might be a gotcha where someone adds a class name string as alias and concrete while adding so will be worth having a test for that situation.

@M1ke M1ke linked a pull request Feb 1, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants