Respect phpstan level in InvalidKeyInArrayDimFetchRule #4166
Conversation
VincentLanglet
force-pushed
the
invalidKey
branch
2 times, most recently
from
b32f7fe to
2caa239
Compare
There was a problem hiding this comment.
If you're doing something to respect levels better, you need to add some cases to LevelsIntegrationTest. You can just add the "topic" php file and run the test, it will generate JSONs with the current result.
It'd actually be great to see the current behaviour in the first commit and then see how things get corrected in the second commit with the code changes.
The expectations are:
- level < 7 - report things that are always wrong
- level 7 - report things that might be wrong, like
int|stringonly whereintis expected - level 8 - same but for
int|null - level 9 - explicit mixed
- level 10 - implicit (untyped) mixed
|
The best way to get LevelsIntegrationTest results fast is to comment out all the other "topics" when you're working on a single topic. Otherwise it takes a long time for the tests to run. Also you could add some upload-artifact action call to |
VincentLanglet
force-pushed
the
invalidKey
branch
7 times, most recently
from
891baf8 to
38c316f
Compare
|
Ok, so here you have
Then last commit was just to fix the lint 7.4. |
| $varType = $this->ruleLevelHelper->findTypeToCheck( | ||
| $scope, | ||
| $node->var, | ||
| '', | ||
| static fn (Type $varType): bool => $varType->isArray()->no() || AllowedArrayKeysTypes::getType()->isSuperTypeOf($dimensionType)->yes(), | ||
| static fn (Type $varType): bool => $varType->isArray()->no(), |
There was a problem hiding this comment.
I'm still wondering about what's the effect of the $varType going through RuleLevelHelper. Does something change if we get rid of it only continue if $scope->getType($node->var)->isArray()->yes()?
It's weird to me that this rule checks maybe-arrays. It's okay for array|null for example, but I'm not sure if it ever should report something on mixed.
There was a problem hiding this comment.
I added more test case here with the commit 6f99f04
You can see that
- array|foo is checked
- explicit and implicit mixed are not checked (because transformed to ErrorType or to a
StrictMixedTypein whichisArray()return no.
But it was right to report I need to check the result for maybe array.
The error message was Invalid array key type DateTimeImmutable., I think it should be Possibly invalid array key type DateTimeImmutable. then. And I fix that in the commit 7e0f73b
VincentLanglet
force-pushed
the
invalidKey
branch
from
8630211 to
7571bdf
Compare
VincentLanglet
force-pushed
the
invalidKey
branch
4 times, most recently
from
5264f3d to
0f29634
Compare
VincentLanglet
force-pushed
the
invalidKey
branch
from
0f29634 to
6f99f04
Compare
VincentLanglet
force-pushed
the
invalidKey
branch
from
6325d1a to
7e0f73b
Compare
|
This pull request has been marked as ready for review. |
Extracted from #4012
It will be easier to review the CastArrayKeyRule after