-
Notifications
You must be signed in to change notification settings - Fork 507
Make NonEmptyArrayType::toArray()
return $this
#2349
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
You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x. |
@@ -12,7 +12,7 @@ class Foo | |||
*/ | |||
public function nonEmpty(array $a): void | |||
{ | |||
assertType('array', array_slice($a, 1)); | |||
assertType('non-empty-array', array_slice($a, 1)); |
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.
array_slice()
can return empty arrays. https://3v4l.org/8VGrv
This pull request has been marked as ready for review. |
@@ -50,7 +51,7 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection, | |||
} | |||
|
|||
if ($valueType->isIterableAtLeastOnce()->yes()) { | |||
return $valueType->toArray(); | |||
return TypeCombinator::union($valueType->toArray(), new NonEmptyArrayType()); |
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.
intersect, not union
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.
I'm trying to make a non-empty-array&array<T>
into an array<T>
. Isn't it a union?
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.
So you mean that you want an array that’s non-empty to no longer be non-empty? I’m afraid you need to recreate it from scratch by doing new ArrayType.
function f(int|array $id_or_ids): array | ||
{ | ||
if (is_array($id_or_ids)) { | ||
\PHPStan\Testing\assertType('non-empty-list<int>', (array)$id_or_ids); |
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 file needs to have namespace Bug9208
.
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 file isn't referenced in NodeScopeResolverTest
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.
Please use PHPStan\Testing\assertType
in a use function
statement.
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.
fixed
{ | ||
assertType('array', array_slice($a, 1)); | ||
assertType('list', array_slice($b, 1)); |
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.
seems this assertion fails atm in CI
assertType('list', array_slice($b, 1)); | |
assertType('list<mixed>', array_slice($b, 1)); |
This pull request has been marked as ready for review. |
@ondrejmirtes Could you please review this PR? |
Ah interesting, this is the same problem I found independently via #2451 (comment) but with a different solution that not just only removes that one accessory |
There's a much easier solution to make a non-empty array possibly empty again :) 9e84671 |
Thank you! |
Oh wow, so you actually planted it in my brain two months ago :D Well I'm a bit embarassed now :D |
close phpstan/phpstan#9208