Skip to content

Convert "true" typehint to "bool"#497

Merged
staabm merged 1 commit into
thecodingmachine:masterfrom
shish:pr497
Dec 3, 2024
Merged

Convert "true" typehint to "bool"#497
staabm merged 1 commit into
thecodingmachine:masterfrom
shish:pr497

Conversation

@shish

@shish shish commented Dec 3, 2024

Copy link
Copy Markdown
Collaborator

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)

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)
Comment thread generated/stream.php
*
*/
function stream_context_set_options($context, array $options): true
function stream_context_set_options($context, array $options): void

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why void?

@shish shish Dec 3, 2024

Copy link
Copy Markdown
Collaborator Author

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)

@staabm staabm Dec 3, 2024

Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Collaborator Author

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makese sense, I guess

@staabm staabm merged commit c046795 into thecodingmachine:master Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants