Skip to content

Commit 38c0039

Browse files
rodrigoprimorodrigoprimo
authored
AbstractFunctionParameterSniff: fix first class callables and function imports (#2518)
* 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. * 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.
1 parent b8d2089 commit 38c0039

File tree

5 files changed

+131
-0
lines changed

5 files changed

+131
-0
lines changed

‎WordPress/AbstractFunctionParameterSniff.php‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace WordPressCS\WordPress;
1111

12+
use PHP_CodeSniffer\Util\Tokens;
1213
use PHPCSUtils\Utils\PassedParameters;
1314
use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;
1415

@@ -77,6 +78,44 @@ public function process_matched_token( $stackPtr, $group_name, $matched_content
7778
}
7879
}
7980

81+
/**
82+
* Verify if the current token is a function call. Behaves like the parent method, except that
83+
* it returns false if there is no opening parenthesis after the function name (likely a
84+
* function import) or if it is a first class callable.
85+
*
86+
* @param int $stackPtr The position of the current token in the stack.
87+
*
88+
* @return bool
89+
*/
90+
public function is_targetted_token( $stackPtr ) {
91+
if ( ! parent::is_targetted_token( $stackPtr ) ) {
92+
return false;
93+
}
94+
95+
$next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true );
96+
97+
if ( \T_OPEN_PARENTHESIS !== $this->tokens[ $next ]['code'] ) {
98+
// Not a function call (likely a function import).
99+
return false;
100+
}
101+
102+
if ( isset( $this->tokens[ $next ]['parenthesis_closer'] ) === false ) {
103+
// Syntax error or live coding: missing closing parenthesis.
104+
return false;
105+
}
106+
107+
// First class callable.
108+
$firstNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $next + 1 ), null, true );
109+
if ( \T_ELLIPSIS === $this->tokens[ $firstNonEmpty ]['code'] ) {
110+
$secondNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $firstNonEmpty + 1 ), null, true );
111+
if ( \T_CLOSE_PARENTHESIS === $this->tokens[ $secondNonEmpty ]['code'] ) {
112+
return false;
113+
}
114+
}
115+
116+
return true;
117+
}
118+
80119
/**
81120
* Process the parameters of a matched function.
82121
*

‎WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,5 +246,40 @@ esc_html_x(
246246
context: $context,
247247
);
248248

249+
/*
250+
* Test that `AbstractFunctionParameterSniff::is_targetted_token()` does not treat first class
251+
* callables and function imports as a function call without parameters. This test is added here as
252+
* there are no dedicated tests for the WPCS abstract classes. The WPCS abstract classes will be
253+
* replaced with PHPCSUtils similar classes in the future, so it is not worth creating dedicated
254+
* tests at this point.
255+
*/
256+
use function __;
257+
use function __ as my_function;
258+
use function
259+
__ /* comment */
260+
as /* comment */
261+
my_function;
262+
use function
263+
_n, // comment
264+
_e, /* comment */
265+
__ as my_function;
266+
add_action('my_action', __(...));
267+
add_action(
268+
'my_action',
269+
__ /* comment */
270+
(
271+
/* comment */ ... /* comment */
272+
)
273+
);
274+
// The tests below ensure that the AbstractFunctionParameterSniff does not incorrectly ignore
275+
// function calls with variable unpacking. But they are also false positives in the context of the
276+
// I18nTextDomainFixer sniff and will be addressed in a future update.
277+
__(...$args);
278+
__ (
279+
...
280+
/* comment */
281+
$args
282+
);
283+
249284
// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[]
250285
// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false

‎WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc.fixed‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,5 +251,41 @@ esc_html_x(
251251
context: $context,
252252
);
253253

254+
/*
255+
* Test that `AbstractFunctionParameterSniff::is_targetted_token()` does not treat first class
256+
* callables and function imports as a function call without parameters. This test is added here as
257+
* there are no dedicated tests for the WPCS abstract classes. The WPCS abstract classes will be
258+
* replaced with PHPCSUtils similar classes in the future, so it is not worth creating dedicated
259+
* tests at this point.
260+
*/
261+
use function __;
262+
use function __ as my_function;
263+
use function
264+
__ /* comment */
265+
as /* comment */
266+
my_function;
267+
use function
268+
_n, // comment
269+
_e, /* comment */
270+
__ as my_function;
271+
add_action('my_action', __(...));
272+
add_action(
273+
'my_action',
274+
__ /* comment */
275+
(
276+
/* comment */ ... /* comment */
277+
)
278+
);
279+
// The tests below ensure that the AbstractFunctionParameterSniff does not incorrectly ignore
280+
// function calls with variable unpacking. But they are also false positives in the context of the
281+
// I18nTextDomainFixer sniff and will be addressed in a future update.
282+
__(...$args, 'something-else');
283+
__ (
284+
...
285+
/* comment */
286+
$args,
287+
'something-else'
288+
);
289+
254290
// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[]
255291
// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false

‎WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[] old-domain
3+
// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain something-else
4+
5+
/*
6+
* Intentional parse error (nothing after opening parenthesis).
7+
* This should be the only test in this file.
8+
*
9+
* Test to document that `AbstractFunctionParameterSniff::is_targetted_token()` ignores unfinished
10+
* function calls. This test is added here as there are no dedicated tests for the WPCS abstract
11+
* classes. The WPCS abstract classes will be replaced with PHPCSUtils similar classes in the
12+
* future, so it is not worth creating dedicated tests at this point.
13+
*/
14+
15+
__(
16+
17+
// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[]
18+
// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false

‎WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*
1818
* @since 1.2.0
1919
*
20+
* @covers \WordPressCS\WordPress\AbstractFunctionParameterSniff::is_targetted_token
2021
* @covers \WordPressCS\WordPress\Sniffs\Utils\I18nTextDomainFixerSniff
2122
*/
2223
final class I18nTextDomainFixerUnitTest extends AbstractSniffUnitTest {
@@ -148,6 +149,8 @@ public function getErrorList( $testFile = '' ) {
148149
241 => 1,
149150
242 => 1,
150151
245 => 1,
152+
277 => 1,
153+
278 => 1,
151154
);
152155

153156
default:

0 commit comments

Comments
 (0)