[TypeDeclaration] Handle infinite loop on array_reverse with index on AddMethodCallBasedStrictParamTypeRector on php8+ feature#3678
Conversation
|
Fixed with verify min or max of IntegerRangeType is null /cc @internalsystemerror |
d098a5b to
e6c95c1
Compare
|
Rebased. |
|
All checks have passed 🎉 @TomasVotruba I think it is ready. |
| if ($type->getMin() === null) { | ||
| return null; | ||
| } | ||
|
|
||
| if ($type->getMax() === null) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
min, max value null seems valid value https://phpstan.org/r/765b0694-8832-4b65-98b7-4944d2914bc3
getMin() === null printed as min
There was a problem hiding this comment.
all type applied with string type seems also valid https://phpstan.org/r/474c8c8b-1d17-4ba2-b6bf-c243b7390e19 for this type:
^ PHPStan\Type\UnionType^ {#23962
-sortedTypes: false
-cachedDescriptions: []
-types: array:3 [
0 => PHPStan\Type\IntegerRangeType^ {#22268
-min: null
-max: -1
}
1 => PHPStan\Type\IntegerRangeType^ {#22323
-min: 1
-max: null
}
2 => PHPStan\Type\StringType^ {#19486}
]
-normalized: false
}There was a problem hiding this comment.
The issue seems somehow used index as integer on this type:
// here index must be integer...
$this->someOtherMethod($index, $uri);
}
}
private function someOtherMethod(int $index, string $uri)
{
return sprintf('%d-%s', $index, $uri);
}
There was a problem hiding this comment.
I got it, it seems due to combination of UnionType + MixedType, then it should be skipped, see 42bcdaa#diff-42a5a059b66868f3ede960ea9a50a3591de610668457d76063ac2f6dc653dcc3
|
All checks have passed 🎉 @TomasVotruba I think it is ready. |
|
@TomasVotruba I am merging it ;) |
|
ty @samsonasik i was still figuring out the problem and you've got the fix already! |
| if ($totalTypes > 1 && $argumentStaticType instanceof UnionType) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
@internalsystemerror I just re-check that this verify is not valid, because multiple args no longer working now when one of them is UnionType, eg:
final class NarrowUnion
{
public function run(MethodCall $methodCall, StaticCall $staticCall, String_ $string)
{
$this->someExpr($methodCall, $staticCall);
$this->someExpr($staticCall, $staticCall);
$this->someExpr($string, $staticCall);
}
private function someExpr($expr, $staticCall)
{
}
}above is now completely skipped, which actually should be:
- private function someExpr($expr, $staticCall)
+ private function someExpr(\PhpParser\Node\Expr\MethodCall|\PhpParser\Node\Expr\StaticCall|\PhpParser\Node\Scalar\String_ $expr, \PhpParser\Node\Expr\StaticCall| $staticCall)
{
}I will add separate PR for that :)
… AddMethodCallBasedStrictParamTypeRector on php8+ feature (#3678) * [TypeDeclaration] Handle infinite loop on array_reverse with index * [ci-review] Rector Rectify * add fixture for php7.4 as well * fix namespace * [ci-review] Rector Rectify * Fix * [ci-review] Rector Rectify --------- Co-authored-by: GitHub Action <actions@github.com>
Closes rectorphp/rector#7896
Ref https://getrector.com/demo/fe336fa4-194f-4bd3-8922-2780d5141d13