-
-
Notifications
You must be signed in to change notification settings - Fork 155
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
Convert "true" typehint to "bool" #497
Conversation
php 8.1 doesn't support "true" as a typehint (The generator will then normally see "function returns bool, except it never returns false because we convert false to exception, which means it only ever returns true, which means the return value is meaningless, so let's convert the return value to void" - which is why this change to the generator results in "returns void" for the generated output)
@@ -20,14 +20,13 @@ | |||
* @throws StreamException | |||
* | |||
*/ | |||
function stream_context_set_options($context, array $options): true | |||
function stream_context_set_options($context, array $options): void |
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.
why void
?
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.
As well as translating "return false" into "throw exception", the generator also translates "return true" into "return void" (presumably because true on success / false on error
and void on success / Exception on error
are relatively-standard patterns whereas true on success / Exception on error
is weird and misleading)
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 guess this breaks the types and analysis with PHPStan and similar?
"always returns true" and "does not return anything" are very different things
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'm not sure, but this is how all the other functions are being generated already, and it seems to work -- this PR doesn't change the behaviour of "return bool" functions, it only turns "return true" into "return bool" and leaves the rest of the process untouched
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.
makese sense, I guess
php 8.1 doesn't support "true" as a typehint
(The generator will then normally see "function returns bool, except it never returns false because we convert false to exception, which means it only ever returns true, which means the return value is meaningless, so let's convert the return value to void" - which is why this change to the generator results in "returns void" for the generated output)