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

Prevented crashes on array_map(...) #7321

Merged

Conversation

weirdan
Copy link
Collaborator

@weirdan weirdan commented Jan 6, 2022

Fixes #7305

@weirdan weirdan requested a review from orklah January 6, 2022 08:40
@weirdan weirdan added the release:fix The PR will be included in 'Fixes' section of the release notes label Jan 6, 2022
@weirdan weirdan changed the title Prevent crashes on array_map(...) Prevented crashes on array_map(...) Jan 6, 2022
@orklah
Copy link
Collaborator

orklah commented Jan 6, 2022

maybe add a TODO to signal that we're missing something at that place?

@weirdan
Copy link
Collaborator Author

weirdan commented Jan 6, 2022

Are we?

@orklah
Copy link
Collaborator

orklah commented Jan 6, 2022

I didn't look in details what the code here does but I'd expect first class callable to be replaced with a callable or something, no?

@weirdan
Copy link
Collaborator Author

weirdan commented Jan 6, 2022

With this patch applied, we already do the best we can short of supporting generic closures. For array_map(...) Psalm infers pure-Closure(callable|null, array<array-key, mixed>, array<array-key, mixed>...=):array<array-key, mixed>. I don't see how it could get any better (without generic closures, that is).

@orklah
Copy link
Collaborator

orklah commented Jan 6, 2022

Nevermind then 🙂 what do you call generic closures?

@weirdan
Copy link
Collaborator Author

weirdan commented Jan 6, 2022

E.g. this Closure<T1 of scalar, T2 of object>(T1, T2):T1|T2

@weirdan weirdan merged commit 46bcb62 into vimeo:4.x Jan 6, 2022
@weirdan weirdan deleted the dont-crash-on-array_map-first-class-callable branch January 6, 2022 12:11
@tm1000
Copy link
Contributor

tm1000 commented Jan 11, 2022

This actually results in a bug:

Call to undefined method PhpParser\Node\Expr\FuncCall::isFirstClassCallable()

That method isFirstClassCallable does not exist on PhpParser\Node\Expr\FuncCall

See: https://github.com/nikic/PHP-Parser/blob/master/lib/PhpParser/Node/Expr/FuncCall.php

@orklah
Copy link
Collaborator

orklah commented Jan 11, 2022

FuncCall extends CallLike which has isFirstClassCallable method. Mind posting the entire stack along with your installed version of PHP-Parser in a new issue?

@tm1000
Copy link
Contributor

tm1000 commented Jan 11, 2022

@orklah I missed the extends! Bad me! Looking through code. Probably a red-herring!

@tm1000
Copy link
Contributor

tm1000 commented Jan 11, 2022

@orklah figured it out: see: #7378

Thanks for pointing that out! sorry for the distractions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

First class callables causes crash in some contexts
3 participants