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

Specify never for array_combine with different number of elements #2303

Merged
merged 1 commit into from Apr 17, 2023

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Mar 26, 2023

@staabm
Copy link
Contributor

staabm commented Mar 27, 2023

Thanks for working on it.

I think we need another test which shows a actual useful rule error.

The never type alone might not create a useful error

@herndlm herndlm force-pushed the array-combine-never branch 2 times, most recently from d5e1be7 to 7984aea Compare March 29, 2023 17:26
@herndlm
Copy link
Contributor Author

herndlm commented Mar 30, 2023

not sure if this really needs another rule test? It feels not much related to me, but what were you thinking of, a rule that reports that this always throws an exception or smth like that? is this even existing already? Everything else I can think of is maybe not really related to array_combine, not sure 🤔

@staabm
Copy link
Contributor

staabm commented Mar 30, 2023

my thinking is that a in-detail change like this is not really useful, when we don't know how it fits in the bigger picture (so which errors are generated because of this change and whether these are useful for a particular use-case)

@herndlm
Copy link
Contributor Author

herndlm commented Apr 17, 2023

I think this one's a tiny change that fixes a bug and is good to go, or does anybody have any better ideas about more tests? @ondrejmirtes
It's essentially also doing the same return-type-wise as we did in some other extensions and I just did recently via filter_input*

@ondrejmirtes ondrejmirtes merged commit 83664ad into phpstan:1.10.x Apr 17, 2023
376 of 377 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

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