-
Notifications
You must be signed in to change notification settings - Fork 506
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
Set offset on list keeps list if there's HasOffsetType for all preceeding offsets #3909
Conversation
@@ -800,8 +800,30 @@ public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $uni | |||
|
|||
$result = $this->intersectTypes(static fn (Type $type): Type => $type->setOffsetValueType($offsetType, $valueType, $unionValues)); | |||
|
|||
if ($offsetType !== null && $this->isList()->yes() && $this->isIterableAtLeastOnce()->yes() && (new ConstantIntegerType(1))->isSuperTypeOf($offsetType)->yes()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this previously existing logic, which was implemented for phpstan/phpstan#12131 - only worked for index 1
. the new logic works for the generic case
//cc @herndlm |
This pull request has been marked as ready for review. |
Thanks! We need something similar in NodeScopeResolver in AST-based logic. If we have a $list and we know that $list[$i] exists based on Scope::hasExpressionType (because $i isn't narrow enough, it's just an int), assigning $list[$i] should keep the list-ness of the variable. |
did you see #3908 or do you mean someting different? |
I think I understand now, and you mean: https://phpstan.org/r/323b3300-934b-46f3-bdd4-b428d2e8a9c1 |
Yeah, I think it's #3908 actually :D |
taking the from #3905 and put it onto setOffsetValueType, to preserve list-state