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() doesn't end script anymore #203

Closed
Screenfeed opened this issue Oct 16, 2023 · 24 comments · Fixed by #204
Closed

wp_die() doesn't end script anymore #203

Screenfeed opened this issue Oct 16, 2023 · 24 comments · Fixed by #204

Comments

@Screenfeed
Copy link

Screenfeed commented Oct 16, 2023

Hello,

Sorry to bring bad news. Unless I'm missing something, it seems that something went wrong with #201.

/**
 * @phpstan-return never
 */
function foo() {
	wp_die();
}

This ends with the following message:

Function foo() should always throw an exception or terminate script execution but doesn't do that.

Versions used:

php-stubs/wordpress-stubs v6.3.2
szepeviktor/phpstan-wordpress v1.3.1
@szepeviktor
Copy link
Owner

@IanDelMar Please hotfix this 🔥

@szepeviktor
Copy link
Owner

Seems like @return never is does not mean termination.

@szepeviktor
Copy link
Owner

@IanDelMar Wait! wp_die got no @return never in #110!

@johnbillion
Copy link
Sponsor Contributor

Looks like a test is needed for assertType('*NEVER*', wp_die('')); too (no second parameter).

@IanDelMar
Copy link
Contributor

Was at work. Will have a look at it now.

@IanDelMar
Copy link
Contributor

I added tests for wp_die() and wp_die(''). Both tests pass, because the extension returns "never" if the number of arguments is less than 3:

// Called without $args parameter
if (count($args) < 3) {
return new NeverType();
}

@szepeviktor
Copy link
Owner

szepeviktor commented Oct 16, 2023

Okay.
What could make the above code snippet fail?

@szepeviktor
Copy link
Owner

@Screenfeed What is you phpstan-wordpress and wp-stubs version?

@Screenfeed
Copy link
Author

@szepeviktor Sorry I should have mentioned that in the OP.

php-stubs/wordpress-stubs v6.3.2
szepeviktor/phpstan-wordpress v1.3.1

@IanDelMar
Copy link
Contributor

IanDelMar commented Oct 16, 2023

Does PHPStan know about the dynamic return type of a function when that function is called by another function?

@szepeviktor
Copy link
Owner

Does PHPStan know about the dynamic return type of a function when that function is called by another function?

Yes.

@szepeviktor
Copy link
Owner

szepeviktor commented Oct 16, 2023

\PHPStan\dumpType(wp_die()); says NEVER.

@IanDelMar Adding back wp_die to earlyTerminatingFunctionCalls resolves the problem.
#203 (comment)

@szepeviktor
Copy link
Owner

szepeviktor commented Oct 16, 2023

@ondrejmirtes Could you help us?
Is there a programmatic way to mark a function early terminating?
wp_die sometimes exits, other times not.

@IanDelMar
Copy link
Contributor

Look at this:

/**
 * @phpstan-return never
 */
function foo() {
	\PHPStan\dumpType(wp_die());
	wp_die();
}
\PHPStan\dumpType(foo());
:51    Dumped type: *NEVER*
:52    Function foo() should always throw an exception or terminate script execution but doesn't do that.
:54    Dumped type: never

@szepeviktor
Copy link
Owner

😲

@IanDelMar
Copy link
Contributor

There is a NonAcceptingNeverType, is that the correct type to return in the extension?

@szepeviktor
Copy link
Owner

Yess please!

@IanDelMar
Copy link
Contributor

IanDelMar commented Oct 16, 2023

I was wondering why it's '*NEVER*' not just 'never', but was not aware that there is another never type (NonAcceptingNeverType) and thought PHPStan will know that '*NEVER*' is 'never'.

@johnbillion
Copy link
Sponsor Contributor

WordPress will *NEVER* die()

@ondrejmirtes
Copy link
Contributor

Yeah, use new NeverType(true) or new NonAcceptingNeverType(). The NeverType needs to be explicit: https://github.com/phpstan/phpstan-src/blob/485922f5c2734af5af15230250340e6c1cc76451/src/Analyser/NodeScopeResolver.php#L1846-L1848

@szepeviktor
Copy link
Owner

Thank you.

@szepeviktor
Copy link
Owner

@Screenfeed This release is for you.
https://github.com/szepeviktor/phpstan-wordpress/releases/tag/v1.3.2

@manooweb
Copy link

@Screenfeed This release is for you. https://github.com/szepeviktor/phpstan-wordpress/releases/tag/v1.3.2

Yes! It's ok for us 👍 Thank you so much 😉

@Screenfeed
Copy link
Author

@szepeviktor Perfect. Thanks a lot to everyone! ❤️

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.

6 participants