Skip to content

Array after array_push / array_unshift call can still be empty#1431

Merged
ondrejmirtes merged 2 commits intophpstan:1.7.xfrom
herndlm:fix-7424
Jun 17, 2022
Merged

Array after array_push / array_unshift call can still be empty#1431
ondrejmirtes merged 2 commits intophpstan:1.7.xfrom
herndlm:fix-7424

Conversation

@herndlm
Copy link
Copy Markdown
Contributor

@herndlm herndlm commented Jun 16, 2022

Closes phpstan/phpstan#7424

The problem was that ArrayType::setOffsetValueType always makes arrays non-empty, which is not wanted in this case (if optional is true in the callback). And the solution here for simplicity reasons is to basically remove NonEmptyArrayType from the result again if either optional is false or the original array was already not iterable at least once before setting the offset.
An alternative might be to add an $optional arg to setOffsetValueType, but I'm not sure about the impact of that yet.

@herndlm herndlm marked this pull request as ready for review June 16, 2022 20:03
@herndlm herndlm marked this pull request as draft June 16, 2022 20:11
@herndlm herndlm marked this pull request as ready for review June 16, 2022 20:27
@herndlm herndlm marked this pull request as draft June 16, 2022 20:52
@ondrejmirtes
Copy link
Copy Markdown
Member

Yeah, I guess this is fine :) Any reason why it's still a draft?

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Jun 17, 2022

Wanted to see if there's a better solution, e.g. for removing non-empty-array or in general for setting optional offsets. But I guess it's fine them for now

@herndlm herndlm marked this pull request as ready for review June 17, 2022 08:04
@ondrejmirtes
Copy link
Copy Markdown
Member

There's always a cleaner solution by providing a new Type method on all implementations, but then you need to implement it on all Types obviously :D So this is fine :)

@ondrejmirtes ondrejmirtes merged commit 82dfce1 into phpstan:1.7.x Jun 17, 2022
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Jun 17, 2022

I remember there was only one failure yesterday. Rebased just in case this fixes something :)

@herndlm herndlm deleted the fix-7424 branch June 17, 2022 08:06
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.

PHP8 shows Variable $data in empty() always exists and is not falsy.

2 participants