Conversation
|
This pull request has been marked as ready for review. |
| @@ -0,0 +1,22 @@ | |||
| <?php | |||
There was a problem hiding this comment.
I think you should log a regular bug for this example and use the bug number as file-name.
having a test file named type-specifier might come in our way with autocompletion or other IDE features when searching for the TypeSpecifier class while doing regular development on the codebase
There was a problem hiding this comment.
I renamed it Pr5445, there is no need to open a issue when it's already described in the pr
| { | ||
| if (empty($aggregation['field']) && $type !== 'filter') { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I would add 2 more assertions directly after the IF
assertType("array<string, mixed>", $aggregation);
assertType('non-falsy-string', $type);
| continue; | ||
| } | ||
|
|
||
| $scopeType = $scope->getType($expr); |
There was a problem hiding this comment.
do we need a similar fix in processBooleanNotSureConditionalTypes?
There was a problem hiding this comment.
Maybe, but I didn't yet succeed finding a non regression test for processBooleanNotSureConditionalTypes, so I thought about letting this to another PR.
processBooleanNotSureConditionalTypes use intersect rather than remove so it's kinda different.
There was a problem hiding this comment.
what do you think about https://phpstan.org/r/489d299c-d26b-4dfb-a518-0b793b775a5f ?
There was a problem hiding this comment.
I do agree your example is a bug but the similar changes in processBooleanNotSureConditionalTypes
$scopeType = $scope->getType($expr);
$conditionType = TypeCombinator::intersect($scopeType, $type);
if ($scopeType->equals($conditionType)) {
continue;
}
$conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes(
$expr,
$conditionType,
);
Does not solve this issue.
I would prefer open an issue about it an run the bot to find a dedicated fix.
This Pr will unlock #5386 which closes ~7 issues
See https://phpstan.org/r/0651d81a-d9d2-4a90-94cb-e82b48f72d63
Extracted form #5386