Skip to content

Fix resolving mixed + array#1445

Closed
rvanvelzen wants to merge 1 commit intophpstan:1.7.xfrom
rvanvelzen:bug-7492
Closed

Fix resolving mixed + array#1445
rvanvelzen wants to merge 1 commit intophpstan:1.7.xfrom
rvanvelzen:bug-7492

Conversation

@rvanvelzen
Copy link
Copy Markdown
Contributor

@ondrejmirtes
Copy link
Copy Markdown
Member

The failing test is obviously wrong. But we should preserve that for example int + array is ErrorType.

@rvanvelzen rvanvelzen marked this pull request as ready for review June 20, 2022 09:19

if (
($leftType->isArray()->yes() || $rightType->isArray()->yes())
&& ($leftType instanceof MixedType || $rightType instanceof MixedType)
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.

My suspicion is that this has a number of problems. Doing instanceof *Type is rarely right so it's not hard to poke holes into the logic - this handles the mixed scenario but what about something else that "may be" an array or int? What if we do int|array + int|array? What piece of code will handle that?

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.

Also this condition will catch TemplateMixedType, maybe there's a situation which will cause generics to be lost.

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.

Also - MixedType is subtractable, meaning there can be a mixed~array|int type which means "this is mixed but it's definitely not int nor array". My point is - doing isSuperTypeOf() is much safer and usually more correct than instanceof :) More about it here: https://phpstan.org/developing-extensions/type-system#querying-a-specific-type

@rvanvelzen rvanvelzen closed this Aug 29, 2022
@rvanvelzen rvanvelzen deleted the bug-7492 branch August 29, 2022 12:50
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.

ternary and "+" operator

3 participants