EscapeOutput: false negatives when checking fully qualified `exit()` and `die()` calls

[rodrigoprimo]

Bug Description

PHP 8.4 transformed exit() and its alias die() from a language construct into a standard function. This means that both can now accept named parameters and be called using a fully qualified name.

When those two functions are called with a fully qualified name, they are not tokenized as T_EXIT. Instead, they are tokenized as T_NS_SEPARATOR + T_STRING (or T_NAME_FULLY_QUALIFIED in PHPCS 4.0+).

The sniff WordPress.Security.EscapeOutput searches for the T_EXIT token to see if the parameter passed to this function is escaped. The sniff needs to be updated to account for the fact that a fully qualified call to exit()/die() will be tokenized differently.

At the moment, the sniff is unable to check for fully qualified calls to exit()/die(), resulting in false positives in some cases.

Minimal Code Snippet

The issue happens when running this command:

vendor/bin/phpcs --standard=WordPress --sniffs=WordPress.Security.EscapeOutput test.php

... over a file containing this code:

<?php

\exit( $foo );
\DiE( $foo );

PHPCS doesn't return any errors, while it should return a OutputNotEscaped error.

Environment

Question	Answer
PHP version	8.3.22
PHP_CodeSniffer version	3.13.2
WordPressCS version	develop
PHPCSUtils version	1.1.0
PHPCSExtra version	1.4.0
WordPressCS install type	git clone
IDE (if relevant)	N/A

Additional Context (optional)

I still need to investigate this a bit more, especially to see if there are unwanted side effects, but I believe that simply adding exit and die to the list defined in PrintingFunctionsTrait::$printingFunctions might be all that is needed to fix this issue. What do you think of this approach, @jrfnl?

Tested Against develop Branch?

• I have verified the issue still exists in the develop branch of WordPressCS.

[jrfnl]

I still need to investigate this a bit more, especially to see if there are unwanted side effects, but I believe that simply adding exit and die to the list defined in PrintingFunctionsTrait::$printingFunctions might be all that is needed to fix this issue.

I believe that may work, but yes, please double-check that wouldn't have unintentional side-effects.
Keep in mind, that when exit() or die() is encountered - even in fully qualified form, the program execution will be halted completely, which is different from other "printing functions".

Note: fixing this is low priority as I expect it will be very rare for any WP related code to have a PHP 8.4 minimum at this time. Will become higher priority as time goes by.

[rodrigoprimo]

Closing this issue, as it no longer occurs after PHPCS 3.13.3 changed how fully qualified \exit() and \die() calls are tokenized via PHPCSStandards/PHP_CodeSniffer#1206. They are now tokenized as T_NS_SEPARATOR + T_EXIT in PHPCS >= 3.13.3 and as T_EXIT in PHPCS >= 4.0.0.

WPCS now requires PHPCS >= 3.13.4.

[jrfnl]

In case anyone is looking for the full details on the tokenization changes, see: PHPCSStandards/PHP_CodeSniffer#1201