-
-
Notifications
You must be signed in to change notification settings - Fork 359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DeadCode] Handle crash on valid conditional type on RemoveUselessReturnTagRector #6475
Conversation
Fixed 🎉 /cc @ruudk |
All checks have passed 🎉 @TomasVotruba I am merging it ;) |
🔥 🥷 ⚡ |
@@ -147,7 +147,7 @@ private function processAddArrayReturnType( | |||
ClassMethod|Function_|Closure $node, | |||
Type $returnType | |||
): ClassMethod|Function_|Closure|null { | |||
if (! $returnType->isArray()->yes()) { | |||
if (! $returnType instanceof ArrayType && ! $returnType instanceof ConstantArrayType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this stop working on a union of arrays?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or on non-empty-array, which is a intersection type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked with @TomasVotruba about this, and more and more we used PHPStan method from Type, it just cause more bugs, so we decide to use instanceof if the variable of call is used next, that's allow for improvement for more types later, rather than cause crash because of method not exists in target PHPStan type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, as soon as you use methods which are not part of the Type interface you hurt PHPStan guarantees.
It would be great if we could get rid of calls to methods which only exist on certain types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see #6478
Fix crash:
Fixes rectorphp/rector#8907