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

Split InvalidArrayAccess into PossiblyInvalidArrayAccess? #285

Closed
TysonAndre opened this issue Nov 11, 2017 · 2 comments
Closed

Split InvalidArrayAccess into PossiblyInvalidArrayAccess? #285

TysonAndre opened this issue Nov 11, 2017 · 2 comments

Comments

@TysonAndre
Copy link
Contributor

TysonAndre commented Nov 11, 2017

E.g. InvalidArrayAccess: Cannot access array value on non-array variable $preferences of type array<mixed, mixed>|string|float|int|bool|null could be PossiblyInvalidArrayAccess because at least one type is array.

This makes InvalidArrayAccess less likely to be a false positive.

InvalidReturnType could have something similar (T1 vs T1|T2 could have one message, T1 vs string could have a different message)

  • E.g. for InvalidReturnType: The declared return type 'string' for a::method is incorrect, got 'string|false' could be BroaderInferredReturnType?
@muglug
Copy link
Collaborator

muglug commented Nov 11, 2017

For InvalidReturnType, I want to keep that simple – if the thing can return more than the suggested thing, it's just a wrong return type.

For the special case where B extends A and we return A with an @return B, I have MoreSpecificReturnType.

@muglug
Copy link
Collaborator

muglug commented May 31, 2018

InvalidReturnType could have something similar (T1 vs T1|T2 could have one message, T1 vs string could have a different message)

E.g. for InvalidReturnType: The declared return type 'string' for a::method is incorrect, got 'string|false' could be BroaderInferredReturnType?

Actually I like this idea, especially give we already have InvalidFalsableReturnStatement. There should be PossiblyInvalidReturnType and PossiblyInvalidReturnStatement issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants