Skip to content

Fix expr string cast #1999

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

Merged
merged 2 commits into from
Nov 18, 2022
Merged

Conversation

rajyan
Copy link
Contributor

@rajyan rajyan commented Nov 14, 2022

I was wondering why we need string casts for exprs at all, because array key string which causes an conversion to integer is a LNumber and should be skipped by

$expr instanceof Node\Scalar || $expr instanceof Array_

The culprit was LNumber with UnaryMinus:smile:

@rajyan rajyan force-pushed the fix-expr-string-cast branch from 5982104 to 7fd3efb Compare November 14, 2022 22:54
@rajyan rajyan marked this pull request as ready for review November 14, 2022 22:59
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@rajyan rajyan marked this pull request as draft November 14, 2022 23:03
@rajyan
Copy link
Contributor Author

rajyan commented Nov 14, 2022

Wait, found an edge case
https://3v4l.org/bMGnT

Maybe we should make type specifier don't specify scalar types.

@rajyan
Copy link
Contributor Author

rajyan commented Nov 14, 2022

On second thought, this was fine, because it wouldn't converted to integer if used as array keys.
https://3v4l.org/aj358

@rajyan rajyan marked this pull request as ready for review November 14, 2022 23:07
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes ondrejmirtes merged commit 42412e1 into phpstan:1.9.x Nov 18, 2022
@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
Development

Successfully merging this pull request may close these issues.

3 participants