-
Notifications
You must be signed in to change notification settings - Fork 686
feat(analyzer): add HiddenGeneratorReturn
issue
#11459
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
base: 6.x
Are you sure you want to change the base?
Conversation
closes vimeo#11458 Signed-off-by: azjezz <[email protected]>
} | ||
} | ||
|
||
$local_return_type = $generator_return_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.
this is the actual fix for #11458 which replaces the return type being compared against with TReturn
from a generator, or with mixed
if the current return type is not a generator.
Signed-off-by: azjezz <[email protected]>
Signed-off-by: azjezz <[email protected]>
public const ERROR_LEVEL = 4; | ||
public const SHORTCODE = 284; |
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.
not sure if those values are correct.
Signed-off-by: azjezz <[email protected]>
Is it safe to remove |
Signed-off-by: azjezz <[email protected]>
/** | ||
* @return Generator<int,int> | ||
* @psalm-suppress MixedReturnStatement | ||
*/ |
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.
the MixedReturnStatement
is correct, yield 1
results in mixed
, which is returned. previously psalm did not check return statements for generators, so this was never triggered.
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.
It could make sense to not report MixedReturnStatement
here actually, we do that in mago, the reason is the return type of the function is not mixed
but rather TReturn
on the Generator
type is, so it might be a good idea to either not report it, or have a different error code that tells user to have a better TReturn
for the generator other than mixed
🤔
/** | ||
* @return Generator<int,int, mixed, mixed> | ||
* @psalm-suppress MixedReturnStatement | ||
*/ |
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.
same here
closes #11458