NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[DependencyInjection] Ignore argument type check in CheckTypeDeclarationsPass if it's a Definition with a factory#44879

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

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:4.4fromfancyweb:di/ignore-factory-arg-lint

Jan 9, 2022

Merged

[DependencyInjection] Ignore argument type check in CheckTypeDeclarationsPass if it's a Definition with a factory#44879

fabpot merged 1 commit intosymfony:4.4fromfancyweb:di/ignore-factory-arg-lint

Jan 9, 2022

Conversation

Copy link

Contributor

fancyweb commentedDec 31, 2021

Q	A
Branch?	4.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	#35599,#44515
License	MIT
Doc PR	-

When a definition uses a factory, we don't know what it returns.

carsonbot added Status: Needs Review Bug DependencyInjection labels

Dec 31, 2021

carsonbot added this to the4.4 milestone

Dec 31, 2021

[DependencyInjection] Ignore argument type check in CheckTypeDeclarat…

b9095e6

…ionsPass if it's a Definition with a factory

fancyweb force-pushed thedi/ignore-factory-arg-lint branch from45f1e38 tob9095e6Compare

December 31, 2021 15:45

Copy link

Member

chalasr commentedDec 31, 2021

What about using the factory return type when it has one?

Copy link

ContributorAuthor

fancyweb commentedDec 31, 2021

Sure, but that would be a feature isn'it? Because right now, it relies only on the definition class and either it's good or not. Fall backing on the return type is a new behavior.

Copy link

Member

nicolas-grekas commentedDec 31, 2021

The class is supposed to be used for that purpose. Can't we fix the class of the definition instead?

Copy link

ContributorAuthor

fancyweb commentedDec 31, 2021 via email

I don’t think so because we are talking aboutAbstractAdapter::createConnection from the Cache component which can returndifferent values depending of the passed dsn. The factory class should beset to a sensible value but it can’t always be the case I guess.

…

On Fri 31 Dec 2021 at 17:29, Nicolas Grekas ***@***.***> wrote: The class is supposed to be used for that purpose. Can't we fix the class of the definition instead? — Reply to this email directly, view it on GitHub <#44879 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA35DB56HNRQVAWGYFZX733UTXK6DANCNFSM5LBN37MA> . You are receiving this because you authored the thread.Message ID: ***@***.***>