support removing class-strings from GenericClassStringType#1590
support removing class-strings from GenericClassStringType#1590ondrejmirtes merged 6 commits intophpstan:1.8.xfrom
class-strings from GenericClassStringType#1590Conversation
|
Please also add a regression test for the match expression, there's also some example with $obj::class I think. |
|
Here is a smaller example: https://phpstan.org/r/e4476215-99ea-434f-adae-3564ef93a5a5 If this works I really have to take a look how the semantics work, I'm not sure I understand the NeverType there. |
|
Not sure if this is related in this case, but have another test case there: phpstan/phpstan#7721 |
|
just added the match-test. its still failling though. I think Dgame is right, that there is a missing piece in |
|
ok, after sleeping a night over it, I know understood the basic building blocks of the arm analysis. we more or less rewrite the AST in phpstan-src/src/Rules/Comparison/MatchExpressionRule.php Lines 54 to 57 in 1537424 which adds similar IF-identical expressions we already have in https://github.com/phpstan/phpstan-src/pull/1590/files#diff-9ca94a9a28f6d393c869ef34bde57e4bb0d53dcc9a8cb5fc865f074ef0df5348R30-R32 with this knowledge I was able to fix the test with commit 21e5bcf. still, I think the match rule contains a bug which makes it only work when the match condition is a variable. works: $t = new Test(new A());
$class = $t->value::class;
echo match ($class) {
A::class => 'A',
B::class => 'B'
};doesn't work: $t = new Test(new A());
echo match ($t->value::class) {
A::class => 'A',
B::class => 'B'
};I am still figuring out the details. |
|
I put my finding into a new issue, as I think this one can be merged as is? |
|
Thanks @staabm! That a good step forward. 🙂 Could we call |
|
issue phpstan/phpstan#7746 is not related to the MatchRule. its a more general problem, see e.g. https://phpstan.org/r/be1b13ce-b8d9-445e-b6ed-dc9e87f2879b |
|
@staabm Here is a more reduced example which does not work: https://phpstan.org/r/0a06314d-d25e-4435-8aa1-d60bb41b8fda |
|
phpstan.org/try does not yet contain the patch, as the PR is not yet merged. |
| if ($typeToRemove instanceof ConstantStringType && $typeToRemove->isClassString()) { | ||
| $generic = $this->getGenericType(); | ||
|
|
||
| if ($generic instanceof ObjectType) { |
There was a problem hiding this comment.
I tried that while PR creation and it made some tests fail.
Therefore I went back to ObjectType
There was a problem hiding this comment.
That's a red flag, we should investigate that
There was a problem hiding this comment.
ok - seems to work now. maybe it was some intermediate step which made it fail…
|
Thank you. |
|
Thanks a lot @staabm ! |
refs phpstan/phpstan#7698 (comment)