Conversation
ondrejmirtes
left a comment
There was a problem hiding this comment.
Would be nice to actually test this somewhere :) Pick a rule when there's already checkExplicitMixed tested, and add a case for the implicit mixed.
Both changes in RuleLevelHelper should actually have a test. Thanks!
1ad51d2 to
c21c278
Compare
|
Also, the test should show that |
|
I tried to find some test where checkExplicitMixed is tested but all I found is just tests for specific bugs so I was not sure how this test should look like. Should I add new testCheckImplicitMixed() with some minimal input? Or copy some basic method like with same input and just modify expected errors? |
|
Don't copy this huge test, that's a maintenance nightmare :) Just add a small one, to show the differences. |
|
OK I will do it tomorrow ;-) |
c21c278 to
fc17ab7
Compare
|
Test added. Based on explicit mixed test I found in CallMethodsRuleTest. |
|
I have another suggestion. I would welcome option that would allow casting implicit/explicit mixed. It is currently not allowed when checkXMixed is used. Maybe add new option allowCastMixed or two options allowCastXMixed? I think it would be enough to check it in findTypeToCheck of RuleLevelHelper right? |
|
I really like how you made these two options independent :) Thank you very much! |
|
About your suggestion - I don't get it, can you give me a code sample? |
|
At level 9 casting explicit mixed to string is not allowed. With implicit mixed option it would not be allowed for implicit mixed too. |
|
If you want you can just ignore this with a regex. But IMHO it's a valid error. |
|
That is what we do now. I just think it would be nice to have built-in support for ignoring such errors because it is often used. BUt I can live with just ignoring it. |
|
I just found out problems with checkImplicitMixed option. See https://phpstan.org/r/fbdaf0db-cf77-4cce-827f-63ca9facfaee It seems that access lelel-2 key access to array with undefined structure always raises error even ehoen used in context where it is valid like in isset or setting array value. But I do not know how to fix that. |
|
@ondrejmirtes I am not sure what to do now. I can prepare failing test but I do not know where to even start with fixing it. |
|
It's expected, and I think it's not specific to implicit mixed. You can see the same thing with explicit mixed: https://phpstan.org/r/50a04edc-93f4-4871-981a-8d1147500d02 I think this error is desirable. You can fatal-crash PHP with some types and this construct: https://3v4l.org/oPMeC |
|
But here it is known it is an array and it works without errors on level 9. |
|
Because |
|
Oh I see. Problem is that you do not know if value of static is not set to someting completely different later and picked up in next call. |
|
Please send a documentation update here: https://phpstan.org/config-reference#stricter-analysis Editing the page: https://github.com/phpstan/phpstan/edit/1.8.x/website/src/config-reference.md |
|
Ok I will write it |
|
Please open a PR :) |
Draft of checkImplicitMixed option as discussed in phpstan/phpstan#7835