Skip to content

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

Merged
ondrejmirtes merged 10 commits into
phpstan:2.1.xfrom
staabm:bug12847
Apr 15, 2025
Merged

ondrejmirtes merged 10 commits into
phpstan:2.1.xfrom
staabm:bug12847

Conversation

@staabm

@staabm staabm commented Apr 7, 2025 •

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

staabm commented Apr 14, 2025 •

Copy link
Copy Markdown
Contributor Author

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

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

staabm commented Apr 15, 2025

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

@staabm

staabm commented Apr 15, 2025 •

Copy link
Copy Markdown
Contributor Author

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
@ondrejmirtes

Copy link
Copy Markdown
Member

Thank you.

@ondrejmirtes

Copy link
Copy Markdown
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