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

wp_die() not exclusively an early terminating function #200

Closed
IanDelMar opened this issue Oct 14, 2023 · 4 comments · Fixed by #201
Closed

wp_die() not exclusively an early terminating function #200

IanDelMar opened this issue Oct 14, 2023 · 4 comments · Fixed by #201

Comments

@IanDelMar
Copy link
Contributor

wp_die() is treated as if it terminated script execution early:

earlyTerminatingFunctionCalls:
- wp_die

But for wp_die('', '', ['exit' => false]) the script execution will not be terminated in any of WP's wp_die handlers.

if (somethingIsTrue()) {
	$foo = true;
} elseif (orSomethingElseIsTrue()) {
	$foo = false;
} else {
	wp_die('', '', ['exit' => false])
}

doFoo($foo); // won't catch possibly undefined variable $foo
@IanDelMar IanDelMar changed the title wp_die() not exclusively a early terminating function wp_die() not exclusively an early terminating function Oct 14, 2023
@szepeviktor
Copy link
Owner

szepeviktor commented Oct 14, 2023

WordPress is so weird PHPStan cannot follow.
There seems to be no setEarlyTerminating method in PHPStan.
https://github.com/search?q=repo%3Aphpstan%2Fphpstan-src%20earlyTerminating&type=code

What is your plan?
Never use ['exit' => false] again? 🙃

@IanDelMar
Copy link
Contributor Author

IanDelMar commented Oct 14, 2023

What is your plan? Never use ['exit' => false] again? 🙃

😆

Don't know. Maybe putting 'wp_die' => ['($args is not non-empty-array ? never : void)'], into the functionMap.php of WordPress stubs?

Or adding an extension checking for the exit key in $args and then either use the never or the void type.

@szepeviktor
Copy link
Owner

szepeviktor commented Oct 14, 2023

Okay!
I am in!
Please add that and add a few (2) tests too.

Thank you.

@szepeviktor
Copy link
Owner

Related php-stubs/wordpress-stubs#36

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 a pull request may close this issue.

2 participants