8000 [DI] confusing deprecation in TypedReference in 4.4 · Issue #35752 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[DI] confusing deprecation in TypedReference in 4.4 #35752

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
linaori opened this issue Feb 17, 2020 · 3 comments
Closed

[DI] confusing deprecation in TypedReference in 4.4 #35752

linaori opened this issue Feb 17, 2020 · 3 comments
Labels
DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony)

Comments

@linaori
Copy link
Contributor
linaori commented Feb 17, 2020 •
edited
Loading

In #26636 (4.1) the $requiringClass argument in TypedReference has been deprecated: https://github.com/symfony/symfony/pull/26636/files#diff-6561cf4254b86cdc43ada0985f87e4a4R29

While helping someone on Slack whom received this deprecation, we had a hard time figuring out what the actual issue was. As $requiringClass is removed, the deprecation is referencing to a non-existing argument. I was thinking about something shown below as alternative, though I'm not 100% sure about it.

public function __construct(string $id, string $type, /* $requiringClass, */ $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE, $name = null)

With as message:

The $requiringClass argument of "%s()" is deprecated since Symfony 4.1. All existing arguments should move 1 position to the left.

Edit: can make a PR based on this change: f473c24
@xabbuh xabbuh added DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) labels Feb 18, 2020
@nicolas-grekas
Copy link
Member

We don't document old signatures so that we shouldn't add the /* $requiringClass, */ part.
Not sure how to improve this. Isn't doing nothing the best option?
Please send a PR of course if you want (with the 1st statement in mind :) )

@linaori
Copy link
Contributor Author
linaori commented Feb 26, 2020

The problem is finding out what $requiringClass means as there are no argument references to it. Currently $invalidBehavior would act as $requiringClass, this means you have to fully understand argument shuffle that happens.

Do you have a suggestion how to find out what $requiringClass in the message is referring to?

@nicolas-grekas
Copy link
Member

Passing the requiring class as 3rd argument to ... is deprecated etc?

@linaori linaori mentioned this issue Feb 28, 2020
Merged
@fabpot fabpot closed this as completed Feb 29, 2020
fabpot added a commit that referenced this issue Feb 29, 2020
@fabpot
minor #35898 [DI] Clarified deprecation for TypedReference in 4.4 (li…
8ed2e66 
…naori)

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[DI] Clarified deprecation for TypedReference in 4.4

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #35752 <!-- prefix each issue number with "Fix #", if any -->
| License       | MIT
| Doc PR        |~ <!-- required for new features -->

Changes the deprecation message to indicate the argument has been removed and how to fix it.

Commits
-------

1c70048 [DI] Clarified deprecation for TypedReference in 4.4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @nicolas-grekas @linaori @xabbuh
0