Skip to content

non-empty-array<mixed> does not accept array with hasOffsetValue #3924

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 10 commits into from
Apr 15, 2025

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Apr 7, 2025

closes phpstan/phpstan#12847

edit: the underlying issue was fixed with #3937

@staabm staabm marked this pull request as ready for review April 7, 2025 19:35
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm staabm changed the title non-empty-array<mixed> does not except array with hasOffsetValue non-empty-array<mixed> does not accept array with hasOffsetValue Apr 10, 2025
@staabm
Copy link
Contributor Author

staabm commented Apr 14, 2025

I see we have more todo here.

a union of 2 non-empty-arrays with has-offset accessory atm is just array, could be at least non-empty-array

https://phpstan.org/r/cfad9fa9-2279-4f97-b9df-580aab7aa844

edit: fixed in #3937

@@ -122,6 +122,10 @@ public function isSubTypeOf(Type $otherType): IsSuperTypeOfResult

public function isAcceptedBy(Type $acceptingType, bool $strictTypes): AcceptsResult
{
if ($acceptingType->isArray()->yes() && $acceptingType->isIterableAtLeastOnce()->yes()) {
return AcceptsResult::createYes();
Copy link
Member

Choose a reason for hiding this comment

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

It should check the iterable value type against $this->valueType.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't check the iteratable-value-type and the non-emptiness in a single IF because $acceptingType is invoked once for each element of the intersection.

this means I get the ArrayType which I could check against the value-type, but not against non-emptiness
and on the 2nd call I get the NonEmptyArray type, which I can check only against the non-emptiness, but not the value type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will think more about it.. maybe we need a fix directly on IntersectionType then

@staabm staabm marked this pull request as draft April 15, 2025 14:57
@staabm
Copy link
Contributor Author

staabm commented Apr 15, 2025

ohh lol, I did not realize that my recently merged PR fixed the issue

@staabm staabm closed this Apr 15, 2025
@staabm staabm deleted the bug12847 branch April 15, 2025 14:59
@staabm staabm restored the bug12847 branch April 15, 2025 14:59
@staabm staabm reopened this Apr 15, 2025
@staabm staabm marked this pull request as ready for review April 15, 2025 15:00
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm
Copy link
Contributor Author

staabm commented Apr 15, 2025

lets use this PR to just land the regression test. its fixed

@ondrejmirtes ondrejmirtes merged commit 3cdaab0 into phpstan:2.1.x Apr 15, 2025
252 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

@ondrejmirtes
Copy link
Member

BTW it'd still be beneficial to write some actual unit tests for the types in question, maybe we realize the fix is still needed for some situations where the types don't even go through TypeCombinator first.

@staabm staabm deleted the bug12847 branch April 15, 2025 15:31
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.

non-empty-array<mixed> does not except array with hasOffsetValue
3 participants