From cc22fb5a5177805a310af3e500544f533dc06c9a Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 13 Feb 2025 10:59:49 -0300 Subject: [PATCH 1/2] AbstractFunctionParameterSniff: fix first class callables and function imports The `AbstractFunctionParameterSniff` class was incorrectly flagging first class callables and function imports as a function call without parameters. This commit fixes this behavior by introducing the `AbstractFunctionParameterSniff::is_targetted_token()` method. This method calls the parent method and then performs two additional checks to make sniffs extending this class ignore first class callables and function imports. Since there are no tests for the abstract classes and the plan is to replace those classes with similar PHPCSUtils classes, I opted to add a few tests to the `I18nTextDomainFixer` tests that generate false positives before the changes implemented in this commit. --- WordPress/AbstractFunctionParameterSniff.php | 34 ++++++++++++++++++ .../Utils/I18nTextDomainFixerUnitTest.4.inc | 35 ++++++++++++++++++ .../I18nTextDomainFixerUnitTest.4.inc.fixed | 36 +++++++++++++++++++ .../Utils/I18nTextDomainFixerUnitTest.7.inc | 19 ++++++++++ .../Utils/I18nTextDomainFixerUnitTest.8.inc | 18 ++++++++++ .../Utils/I18nTextDomainFixerUnitTest.php | 8 +++++ 6 files changed, 150 insertions(+) create mode 100644 WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc create mode 100644 WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.8.inc diff --git a/WordPress/AbstractFunctionParameterSniff.php b/WordPress/AbstractFunctionParameterSniff.php index c6395a7014..e3b98acd8a 100644 --- a/WordPress/AbstractFunctionParameterSniff.php +++ b/WordPress/AbstractFunctionParameterSniff.php @@ -9,6 +9,7 @@ namespace WordPressCS\WordPress; +use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\PassedParameters; use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff; @@ -77,6 +78,39 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content } } + /** + * Verify if the current token is a function call. Behaves like the parent method, except that + * it returns false if there is no opening parenthesis after the function name (likely a + * function import) or if it is a first class callable. + * + * @param int $stackPtr The position of the current token in the stack. + * + * @return bool + */ + public function is_targetted_token( $stackPtr ) { + if ( ! parent::is_targetted_token( $stackPtr ) ) { + return false; + } + + $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + + if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next ]['code'] ) { + // Not a function call (likely a function import). + return false; + } + + // First class callable. + $firstNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $next + 1 ), null, true ); + if ( false !== $firstNonEmpty && \T_ELLIPSIS === $this->tokens[ $firstNonEmpty ]['code'] ) { + $secondNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $firstNonEmpty + 1 ), null, true ); + if ( false === $secondNonEmpty || \T_CLOSE_PARENTHESIS === $this->tokens[ $secondNonEmpty ]['code'] ) { + return false; + } + } + + return true; + } + /** * Process the parameters of a matched function. * diff --git a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc index 79dc82202d..d33d0a18e0 100644 --- a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc +++ b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc @@ -246,5 +246,40 @@ esc_html_x( context: $context, ); +/* + * Test that `AbstractFunctionParameterSniff::is_targetted_token()` does not treat first class + * callables and function imports as a function call without parameters. This test is added here as + * there are no dedicated tests for the WPCS abstract classes. The WPCS abstract classes will be + * replaced with PHPCSUtils similar classes in the future, so it is not worth creating dedicated + * tests at this point. + */ +use function __; +use function __ as my_function; +use function + __ /* comment */ + as /* comment */ + my_function; +use function + _n, // comment + _e, /* comment */ + __ as my_function; +add_action('my_action', __(...)); +add_action( + 'my_action', + __ /* comment */ + ( + /* comment */ ... /* comment */ + ) +); +// The tests below ensure that the AbstractFunctionParameterSniff does not incorrectly ignore +// function calls with variable unpacking. But they are also false positives in the context of the +// I18nTextDomainFixer sniff and will be addressed in a future update. +__(...$args); +__ ( + ... + /* comment */ + $args +); + // phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[] // phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false diff --git a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc.fixed b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc.fixed index bdc7909f22..f7fd2c6f4e 100644 --- a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc.fixed +++ b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc.fixed @@ -251,5 +251,41 @@ esc_html_x( context: $context, ); +/* + * Test that `AbstractFunctionParameterSniff::is_targetted_token()` does not treat first class + * callables and function imports as a function call without parameters. This test is added here as + * there are no dedicated tests for the WPCS abstract classes. The WPCS abstract classes will be + * replaced with PHPCSUtils similar classes in the future, so it is not worth creating dedicated + * tests at this point. + */ +use function __; +use function __ as my_function; +use function + __ /* comment */ + as /* comment */ + my_function; +use function + _n, // comment + _e, /* comment */ + __ as my_function; +add_action('my_action', __(...)); +add_action( + 'my_action', + __ /* comment */ + ( + /* comment */ ... /* comment */ + ) +); +// The tests below ensure that the AbstractFunctionParameterSniff does not incorrectly ignore +// function calls with variable unpacking. But they are also false positives in the context of the +// I18nTextDomainFixer sniff and will be addressed in a future update. +__(...$args, 'something-else'); +__ ( + ... + /* comment */ + $args, + 'something-else' +); + // phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[] // phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false diff --git a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc new file mode 100644 index 0000000000..66bf40bbac --- /dev/null +++ b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc @@ -0,0 +1,19 @@ + 1, 242 => 1, 245 => 1, + 277 => 1, + 278 => 1, ); default: @@ -193,6 +196,11 @@ public function getWarningList( $testFile = '' ) { 201 => 1, ); + case 'I18nTextDomainFixerUnitTest.7.inc': + return array( + 16 => 1, + ); + default: return array(); } From 25f1235225137f763ffa497493661a5a096281cd Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 24 Feb 2025 10:34:23 -0300 Subject: [PATCH 2/2] AbstractFunctionParameterSniff: fix handling of unfinished function calls Before this commit, the `AbstractFunctionParameterSniff` class would incorrectly treat an unfinished function call (missing closing parenthesis) as a function call without parameters. This behavior is now changed and the class will bail early when the closing parenthesis is missing and assume it is a syntax error or live coding. --- WordPress/AbstractFunctionParameterSniff.php | 9 +++++++-- .../Utils/I18nTextDomainFixerUnitTest.7.inc | 9 ++++----- .../Utils/I18nTextDomainFixerUnitTest.8.inc | 18 ------------------ .../Utils/I18nTextDomainFixerUnitTest.php | 5 ----- 4 files changed, 11 insertions(+), 30 deletions(-) delete mode 100644 WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.8.inc diff --git a/WordPress/AbstractFunctionParameterSniff.php b/WordPress/AbstractFunctionParameterSniff.php index e3b98acd8a..4f5f64e47d 100644 --- a/WordPress/AbstractFunctionParameterSniff.php +++ b/WordPress/AbstractFunctionParameterSniff.php @@ -99,11 +99,16 @@ public function is_targetted_token( $stackPtr ) { return false; } + if ( isset( $this->tokens[ $next ]['parenthesis_closer'] ) === false ) { + // Syntax error or live coding: missing closing parenthesis. + return false; + } + // First class callable. $firstNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $next + 1 ), null, true ); - if ( false !== $firstNonEmpty && \T_ELLIPSIS === $this->tokens[ $firstNonEmpty ]['code'] ) { + if ( \T_ELLIPSIS === $this->tokens[ $firstNonEmpty ]['code'] ) { $secondNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $firstNonEmpty + 1 ), null, true ); - if ( false === $secondNonEmpty || \T_CLOSE_PARENTHESIS === $this->tokens[ $secondNonEmpty ]['code'] ) { + if ( \T_CLOSE_PARENTHESIS === $this->tokens[ $secondNonEmpty ]['code'] ) { return false; } } diff --git a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc index 66bf40bbac..2b61bd578d 100644 --- a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc +++ b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc @@ -6,11 +6,10 @@ * Intentional parse error (nothing after opening parenthesis). * This should be the only test in this file. * - * Test to document that `AbstractFunctionParameterSniff::is_targetted_token()` returns true for a - * function call even when there is nothing after the opening parenthesis. This test is added here - * as there are no dedicated tests for the WPCS abstract classes. The WPCS abstract classes will be - * replaced with PHPCSUtils similar classes in the future, so it is not worth creating dedicated - * tests at this point. + * Test to document that `AbstractFunctionParameterSniff::is_targetted_token()` ignores unfinished + * function calls. This test is added here as there are no dedicated tests for the WPCS abstract + * classes. The WPCS abstract classes will be replaced with PHPCSUtils similar classes in the + * future, so it is not worth creating dedicated tests at this point. */ __( diff --git a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.8.inc b/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.8.inc deleted file mode 100644 index 7e1f33a39d..0000000000 --- a/WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.8.inc +++ /dev/null @@ -1,18 +0,0 @@ - 1, ); - case 'I18nTextDomainFixerUnitTest.7.inc': - return array( - 16 => 1, - ); - default: return array(); }