-
Notifications
You must be signed in to change notification settings - Fork 523
Add regression test #3873
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
Add regression test #3873
Conversation
{ | ||
$this->analyse([__DIR__ . '/data/bug-1311.php'], [ | ||
[ | ||
'Property Bug1311\HelloWorld::$list (array<int, string>) does not accept array<int, int>.', |
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 is still here in a different format, because the test basically has strict rules enabled and the phpdoc is essentially wrong. not fully sure how to deal with this. see also
https://phpstan.org/r/29982739-327a-494d-bc97-06021030576b vs https://phpstan.org/r/a780f206-b942-4d5e-8be2-e3a79a911964
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 don't get it. How does this test have strict-rules enabled? And which parameter from strict-rules is causing this error to appear?
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.
tbh I was hoping you would tell me 😅 looks like it's coming from the $strictTypes here:
phpstan-src/src/Type/StringType.php
Lines 128 to 130 in c8833df
if ($thatClassNames === [] || $strictTypes) { | |
return AcceptsResult::createNo(); | |
} |
the string array values are not accepting the int values.
but not sure how that is related to strict-rules..
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.
aha on the playground the treatPhpDocTypesAsCertain is also somehow involved apparently. I obviously did not invest carefully now but it's confusing :)
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.
My guess is one of the pollute options set to false, that would explain the playground, but it doesn't explain the rule test.
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.
you were right, setting polluteScopeWithLoopInitialAssignments
to true
makes the error go away in the test (it is false
there). in general, by default it is true
and strict-rules sets this to false
in the playground which makes the error appear.
I changed it (it surprised me), so please rebase and update this branch 1b56b0c |
6316359
to
887da99
Compare
done. looking good now 👍 |
Thank you. |
For the loop/count case of phpstan/phpstan#1311