Skip to content

Ignore negative keys on checking whether a ConstantArray is a list #3870

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 4 commits into from
Mar 11, 2025

Conversation

watasuke102
Copy link
Contributor

In ConstantArrayTypeBuilder, setOffsetValueType() uses ConstantIntegerType of offsetType for checking whether a ConstantArray is a list, but doesn't check whether it is positive. Because of that, a ConstantArray that has negative integer keys is treated as list.

This PR resolves phpstan/phpstan#12708.

@watasuke102 watasuke102 marked this pull request as draft March 11, 2025 02:21
@watasuke102 watasuke102 marked this pull request as ready for review March 11, 2025 02:48
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A similar fix needs to be done in ArrayType::isList() - see https://phpstan.org/r/47372278-5264-4f01-a12c-dce00a50539f

@ondrejmirtes
Copy link
Member

Sorry, I'm wrong.

@ondrejmirtes
Copy link
Member

Disregard my review there.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please only check this review. Thanks.

@watasuke102
Copy link
Contributor Author

Oh I requested review by mistake, sorry

@watasuke102
Copy link
Contributor Author

It's ready for review, thank you!

@ondrejmirtes ondrejmirtes merged commit 1b58626 into phpstan:2.1.x Mar 11, 2025
416 of 417 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

@zonuexe
Copy link
Contributor

zonuexe commented Mar 11, 2025

Probably not a problem with ArrayType, but I found a problem with ConstantArrayType::isList() by modifying Ondřej's case.

https://phpstan.org/r/78b134e0-88b0-4bbf-be11-032bc267650c

@ondrejmirtes
Copy link
Member

@zonuexe Array shapes are not currently sealed by default. The most upvoted feature request about it is here on GitHub.

@zonuexe
Copy link
Contributor

zonuexe commented Mar 11, 2025

This problem is also related to sealed array, but that alone doesn't prevent input like [1 => 'x', 0 => 'y'], so we might need a type like array{0: string, 1: string}&list

@ondrejmirtes
Copy link
Member

I get it, it's complicated 😊 Maybe we could enforce the order of keys along with true sealed-ness of arrays.

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.

Negative Keys Not Validated for @var list<T>
4 participants