krakjoe / uopz

User Operations for Zend
Other
358 stars 47 forks source link

uopz.exit=0 and never return type #172

Open vjik opened 1 year ago

vjik commented 1 year ago

PHP code:

function x(): never { exit(0); };
x();

Throws error:

Fatal error: Uncaught TypeError: x(): never-returning function must not implicitly return in Command line code:1
Stack trace:
#0 Command line code(1): x()
#1 {main}
  thrown in Command line code on line 1
blueblakk commented 1 year ago

I'm getting a similar error. I cannot use PHPUnit because of this.

image

Only after I remove uopz from my docker environment does it work properly.

This was tested with PHP 8.2.8 in a Docker environment on Docker Engine v24.0.2, Windows 10.

I don't seem to have this issue running Docker on MacOS Ventura.

kornrunner commented 1 year ago

This is actually present since PHP 8.1 if you have configured uopz.exit=0.

I've submitted a PR which should sort it, but in case it doesn't get merged you could either set uopz.exit=1 or use my fork. Hope this helps, thanks.

cmb69 commented 3 months ago

This is a somewhat philosophical question: if exit does not stop program execution, is the never return type actually appropriate?

kornrunner commented 3 months ago

uopz_allow_exit was initially implemented to allow testing code that does exit() (otherwise hitting exit terminates test execution as well).

On the other hand, never return type (which appeared later) is there to let people know that script will exit after invocation, so - from that point it's appropriate and useful.

But again, while testing - if we disable exit in order to test code that might exit- we'll end up with errors if method also has (appropriate/expected) never return type, so ... thus this issue and my PR.

Perhaps another solution would be to use uopz to modify return types of specific methods / functions. I'm willing to update the PR if there are any issues with it. Thanks!

cmb69 commented 3 months ago

I understand what this ticket is about, but I'm still not sure if we should try to "fix" the issue, for the following reasons:

My point is that I don't like to add yet another feature, that might break in the future (or might even not work under all conditions right now), especially if that feature might not be that useful. I mean, if uopz.exit=1 solves the problem, there is no issue.

Note that I'm not strictly against this change.

@remicollet, what do you think?