-
Notifications
You must be signed in to change notification settings - Fork 548
Add Type::spliceArray(), improve splice_array() array type narrowing
#3952
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
Conversation
c7df375 to
4ebff11
Compare
| $offset *= -1; | ||
| $reversedLength = min($length, $offset); | ||
| $reversedOffset = $offset - $reversedLength; |
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.
these operations look like they could be expressed on the type-level directly.
but as you said, maybe this would be too much and we leave it less abstract for now and see whether people report new issues
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.
Can we do such calculations too, can you show me an example maybe? I'm not too confident that I can make it work, but might be also a good follow-up.
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.
maybe
| if (count($argType->getConstantArrays()) > 0) { | |
| foreach ($argType->getConstantArrays() as $constantArray) { | |
| $node = new Int_(0); | |
| foreach ($constantArray->getValueTypes() as $i => $type) { | |
| if ($constantArray->isOptionalKey($i)) { | |
| $node = new Plus($node, new TypeExpr(TypeCombinator::union($type, new ConstantIntegerType(0)))); | |
| } else { | |
| $node = new Plus($node, new TypeExpr($type)); | |
| } | |
| } | |
| $resultTypes[] = $scope->getType($node); | |
| } | |
| } else { | |
| $itemType = $argType->getIterableValueType(); | |
| $mulNode = new Mul(new TypeExpr($itemType), new TypeExpr(IntegerRangeType::fromInterval(0, null))); | |
| $resultTypes[] = $scope->getType(new Plus(new TypeExpr($itemType), $mulNode)); | |
| } |
or
InitializerExprTypeResolver::get(Minus|Plus|...)Type
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.
can't figure out how to do this now unfortunately, there's also no access to scope in the type. but it's way simpler because non-optional keys don't have to be considered and there's a chance it might work somehow I suppose. maybe still a good follow-up :)
116c71e to
e7afdd9
Compare
|
This pull request has been marked as ready for review. |
7721222 to
a76dc4a
Compare
|
This pull request has been marked as ready for review. |
| assertType('non-empty-array<0|1|2|3|4, 0|1|2|3|4>', $items); | ||
| assertType('non-empty-list<int<min, 4>>', $items); | ||
| $batch = array_splice($items, 0, 2); | ||
| assertType('array<0|1|2|3|4, 0|1|2|3|4>', $items); | ||
| assertType('non-empty-list<0|1|2|3|4>', $batch); | ||
| assertType('list<int<min, 4>>', $items); | ||
| assertType('non-empty-list<int<min, 4>>', $batch); |
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.
the changes here confused me briefly, but it seems to be coming from loop generalization and there's not much I can do about it
|
Meh, I'll do one more improvement and also add tests for the return types into the same file. Surely can't hurt and hopefully does not discover slice issues... |
e1204ea to
5b4de14
Compare
|
Ok, so I figured out a way to get correct types for |
|
This pull request has been marked as ready for review. |
2ba876f to
a2978d1
Compare
81677fb to
4ed36ef
Compare
|
Thank you! |
Needs #3959 firstDONECloses phpstan/phpstan#11917
This is very similar to
Type:: sliceArray()and it is used inNodeScopeResolverin a similar fashion asType::popArray()is. Moving logic toTypedeals obviously with all accessory types which were not working properly before too like e.g. list or non-empty-array.First commit is a tiny refactor I didn't want to cause more noise and dependencies with another PR..