Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[DX][DependencyInjection] Suggest to write an implementation if the interface cannot be autowired#25547

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@sroze
Copy link
Contributor

@srozesroze commentedDec 19, 2017•
edited
Loading

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsø
LicenseMIT
Doc PRø

This would add a hint for the developers when the interface cannot be wired. This suggests creating the implementation of the interface.

Note: this is 3.4 because I believe DX should be treated as bugs.
Note 2: fabbot issue is false positive

Koc and TomasVotruba reacted with thumbs up emoji
@carsonbotcarsonbot added Status: Needs Review DXDX = Developer eXperience (anything that improves the experience of using Symfony) DependencyInjection Bug labelsDec 19, 2017
@srozesrozeforce-pushed thedx-suggest-to-write-an-implementation branch from50da469 tocb333cdCompareDecember 19, 2017 11:24
@nicolas-grekasnicolas-grekas added this to the3.4 milestoneDec 19, 2017
@nicolas-grekas
Copy link
Member

Isn't this just stating the obvious?

chalasr reacted with thumbs up emoji

@sroze
Copy link
ContributorAuthor

sroze commentedDec 19, 2017•
edited
Loading

@nicolas-grekas well, I'd say so as well, but I just saw a developer having this issue; and such message would have helped him. It doesn't cost us anything to phrase it a bit differently and benefits is it helps some :)

Simperfit and TomasVotruba reacted with thumbs up emoji

@Simperfit
Copy link
Contributor

I agree with@sroze people could have this kinds of errors, and doing this help them to develop.

@weaverryan
Copy link
Member

weaverryan commentedDec 20, 2017•
edited by nicolas-grekas
Loading

I like this step. In 3.4, we already create interface aliases if you have a service that implements that interface. So, indeed, it seems likemost of the time, the solutionwould indeed be to simply create a class that implements this interface (then the interface alias would be created and it would autowire).And this would be true even if the interface were from a 3rd-party library (your implementation would still create that alias).

So, talking this through, yea, I think that this recommendation isalways correct. So why not?

$message =sprintf('references %s "%s" but %s.%s',$r->isInterface() ?'interface' :'class',$type,$message,$this->createTypeAlternatives($reference));

if ($r->isInterface()) {
$message .=' Did you write an implementation of this interface?';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

What about:

Did you create a class that implements this interface?

Or:

You may need to create a class that implements this interface.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I like "Did you create a class that implements this interface?"

xabbuh reacted with thumbs up emoji
@stof
Copy link
Member

And this would be true even if the interface were from a 3rd-party library (your implementation would still create that alias).

not true, as the alias is created automatically only if the implementationand the interface are both discovered by the PSR-4 discovery. A third-party interface will not be discovered insrc.

nicolas-grekas reacted with thumbs up emoji

@srozesrozeforce-pushed thedx-suggest-to-write-an-implementation branch fromcb333cd to961e3e7CompareDecember 24, 2017 10:30
@sroze
Copy link
ContributorAuthor

Updated the message based on@weaverryan's 1st suggestion 👍

@nicolas-grekasnicolas-grekas merged commit961e3e7 intosymfony:3.4Dec 29, 2017
nicolas-grekas added a commit that referenced this pull requestDec 29, 2017
…on if the interface cannot be autowired (sroze)This PR was merged into the 3.4 branch.Discussion----------[DX][DependencyInjection] Suggest to write an implementation if the interface cannot be autowired| 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        | øThis would add a hint for the developers when the interface cannot be wired. This suggests creating the implementation of the interface.**Note:** this is 3.4 because I believe DX should be treated as bugs.**Note 2:** fabbot issue is false positiveCommits-------961e3e7 Suggest to write an implementation if the interface cannot be autowired
This was referencedJan 5, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@weaverryanweaverryanweaverryan approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@xabbuhxabbuhxabbuh approved these changes

+2 more reviewers

@ro0NLro0NLro0NL approved these changes

@SimperfitSimperfitSimperfit approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

BugDependencyInjectionDXDX = Developer eXperience (anything that improves the experience of using Symfony)Status: Reviewed

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

8 participants

@sroze@nicolas-grekas@Simperfit@weaverryan@stof@ro0NL@xabbuh@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp