Skip to content
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

check "never" return type more strictly #8599

Closed
wants to merge 3 commits into from

Conversation

kkmuffme
Copy link
Contributor

@kkmuffme kkmuffme commented Oct 17, 2022

@kkmuffme kkmuffme force-pushed the check-never-return-more-strictly branch 2 times, most recently from 3b3cf85 to aa9f4dc Compare October 17, 2022 21:13
@kkmuffme kkmuffme marked this pull request as ready for review October 17, 2022 21:21
@kkmuffme
Copy link
Contributor Author

kkmuffme commented Oct 17, 2022

I'm sure this will cause some followup issues to be created at some point in the future with PHP 8.1+, but I'll fix those once they actually pop up (and not for some synthetic examples we make up now)

@kkmuffme kkmuffme marked this pull request as draft October 18, 2022 08:42
@kkmuffme kkmuffme force-pushed the check-never-return-more-strictly branch 3 times, most recently from 975dbb6 to c64c4ac Compare October 18, 2022 08:56
@kkmuffme kkmuffme marked this pull request as ready for review October 18, 2022 09:14
@orklah
Copy link
Collaborator

orklah commented Oct 20, 2022

I'm not sure I followed the actual changes yet. However, a few points first:

  • I'll need tests for that
  • This will have to be pushed to master for two reasons: we removed TEmpty and we made the Union immutable. This PR will break both. (I'll start to refuse almost any PR to 4.x soon anyway)

@kkmuffme
Copy link
Contributor Author

I need to wait until v5 is released before I can provide a PR on master/v5 branch, so we'll just put it on hold until then I guess

@kkmuffme kkmuffme marked this pull request as draft October 22, 2022 09:59
@danog
Copy link
Collaborator

danog commented Oct 22, 2022

You can already retarget it on master right now :)

@kkmuffme
Copy link
Contributor Author

I can't bc we use this PR internally in our CI

* require explicit "never" return type when function always exits
* error if function does not exit, but return type explicitly contains "never"
* Fix: vimeo#8175
previous PHP versions could give a fatal error in some cases, e.g. https://3v4l.org/ZRZZE
as this function needs a complete overhaul at some point in v5 - not doing this now, as it will just create conflicts with v5
@kkmuffme kkmuffme force-pushed the check-never-return-more-strictly branch from 520d9d0 to 81804bb Compare November 7, 2022 11:25
@kkmuffme
Copy link
Contributor Author

v5 see #8624

@kkmuffme kkmuffme closed this Nov 10, 2022
@kkmuffme kkmuffme deleted the check-never-return-more-strictly branch December 1, 2022 18:29
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.

InvalidReturnType due to void and null not treated the same Missing InvalidReturnType check when exit;
3 participants