Respect phpstan level in ParameterCastableToNumberRule#4873
Respect phpstan level in ParameterCastableToNumberRule#4873VincentLanglet wants to merge 3 commits intophpstan:2.1.xfrom
Conversation
VincentLanglet
force-pushed
the
fixParameterCastable
branch
from
bb32d03 to
5dbfff4
Compare
| $scope, | ||
| $parameter->value, | ||
| '', | ||
| static fn (Type $type): bool => $type->isArray()->yes() && !$castFn($type->getIterableValueType()) instanceof ErrorType, |
There was a problem hiding this comment.
The following change follows the same idea than https://github.com/phpstan/phpstan-src/pull/4166/changes for instance.
&& and || doesn't work always very well in UnionTypeCriteriaCallback it's sometimes better to split the check into multiple findTypeToCheck calls.
Here this allows to transform the iterable value type $type->getIterableValueType() into a StrictMixed when it's a Mixed for instance or array<mixed> is never reported at any level.
| $castFn = $this->phpVersion->supportsObjectsInArraySumProduct() | ||
| ? static fn (Type $t) => $t->toNumber() | ||
| : static fn (Type $t) => !$t->isObject()->no() ? new ErrorType() : $t->toNumber(); | ||
| : static fn (Type $t) => $t instanceof MixedType || $t->isObject()->no() ? $t->toNumber() : new ErrorType(); |
There was a problem hiding this comment.
MixedType should not be transform automatically to ErrorType, we will rely on the ruleLevelHelper inside the parameterCastableToStringCheck to decide.
|
This pull request has been marked as ready for review. |
|
Some errors are new on Compile PHAR / integration-tests / PMMP Tests (pull_request) I need to check Edit: This is expected ! They are using the level 9 |
staabm
left a comment
staabm
left a comment
There was a problem hiding this comment.
I would prefer @ondrejmirtes reviewing this one, as my experience with RuleLevelHelper is very limited
VincentLanglet
force-pushed
the
fixParameterCastable
branch
2 times, most recently
from
e966f31 to
3ff9877
Compare
| ! $typeResult->getType()->isArray()->yes() | ||
| || !$castFn($typeResult->getType()->getIterableValueType()) instanceof ErrorType | ||
| ) { | ||
| if (!$castFn($typeResult->getType()) instanceof ErrorType) { |
There was a problem hiding this comment.
If you're not sure about how to work with RuleLevelHelper::findTypeToCheck, write thorough test for LevelsIntegrationTest. (The test regenerates the JSON files with expectations so 2nd run should always succeed.)
Before level 7, it should only complain about a type that is never castable to number.
On level 7+, it should complain about unions where only some types are castable to number.
On level 8+, it should complain about a nullable type (but maybe since null is castable to number, there will be no difference?)
On level 9+, it should complain about explicit mixed.
On level 10, it should complain about implicit mixed (without typehint).
Also I suspect we'll need also a separate test file for PHP 8.3+ because of supportsObjectsInArraySumProduct.
There was a problem hiding this comment.
I added tests in LevelsIntegrationTest to show the result.
I don't think it's worth using separate test file for PHP 8.3+ or - ; because the only difference is for some specific object type which are already tested.
VincentLanglet
force-pushed
the
fixParameterCastable
branch
from
675f260 to
99b0fb1
Compare
4 participants
Complete #4867