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
Improve ConstantArrayType::unsetOffset
#1537
Improve ConstantArrayType::unsetOffset
#1537
Conversation
2b02425
to
7af6056
Compare
namespace failures from my side
has been fixed by Ondrej in the tagged unions preparations
also been fixed by Ondrej |
irrelevant, since it has been fixed by Ondrej as mentioned in the previous comment. But I extracted a test case based on that and put it here just to be sure. |
1a7cf21
to
eda95ea
Compare
} | ||
} | ||
|
||
assertType('array{}|array{a?: bool, b?: numeric-string, c?: int<-1, 1>, d?: int<0, 1>}', $breakdowns); |
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 was array<'a'|'b'|'c'|'d', bool|int<-1, 1>|numeric-string>
before. See https://phpstan.org/r/6c90923e-abf5-492d-8bf6-c5f95a6627e6
I added it because something very similar to this was failing in prestashop while developing. It has been unblocked/fixed by Ondrej's recent changes (IIRC there was a non-empty-array intersection before), but I want to avoid it failing again. And it is a nice example I think.
The union with an empty array could be optimised away. If this isn't already done in the tagged unions branch anyway.. :)
@@ -802,8 +802,6 @@ public function testBug7094(): void | |||
|
|||
$this->assertSame('Parameter #1 $attr of method Bug7094\Foo::setAttributes() expects array{foo?: string, bar?: 5|6|7, baz?: bool}, non-empty-array<string, 5|6|7|bool|string> given.', $errors[5]->getMessage()); | |||
$this->assertSame(29, $errors[5]->getLine()); | |||
$this->assertSame('Parameter #1 $attr of method Bug7094\Foo::setAttributes() expects array{foo?: string, bar?: 5|6|7, baz?: bool}, array<\'bar\'|\'baz\'|\'foo\', 5|6|7|bool|string> given.', $errors[6]->getMessage()); | |||
$this->assertSame(49, $errors[6]->getLine()); |
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 extracted case is basically https://phpstan.org/r/4ca3ec8a-9f81-4b87-a321-c62a7e58b768
this should be fine IMO, phpstan got a tiny bit smarter here.
9ef1528
to
a31a0f1
Compare
a31a0f1
to
41637f4
Compare
this is now ready. has been unblocked by recent changes basically. |
Really nice, thank you :) |
Basically adds support for union offsets.
Previously it generalized too early to a generic array IMO.