Skip to content

AbstractFunctionParameterSniff: fix first class callables and function imports #2518

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions WordPress/AbstractFunctionParameterSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

namespace WordPressCS\WordPress;

use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Utils\PassedParameters;
use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff;

Expand Down Expand Up @@ -77,6 +78,44 @@ 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;
}

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 ( \T_ELLIPSIS === $this->tokens[ $firstNonEmpty ]['code'] ) {
$secondNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $firstNonEmpty + 1 ), null, true );
if ( \T_CLOSE_PARENTHESIS === $this->tokens[ $secondNonEmpty ]['code'] ) {
return false;
}
}

return true;
}

/**
* Process the parameters of a matched function.
*
Expand Down
35 changes: 35 additions & 0 deletions WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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
36 changes: 36 additions & 0 deletions WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.4.inc.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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
18 changes: 18 additions & 0 deletions WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.7.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php
// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[] old-domain
// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain something-else

/*
* Intentional parse error (nothing after opening parenthesis).
* This should be the only test in this file.
*
* 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.
*/

__(

// phpcs:set WordPress.Utils.I18nTextDomainFixer old_text_domain[]
// phpcs:set WordPress.Utils.I18nTextDomainFixer new_text_domain false
3 changes: 3 additions & 0 deletions WordPress/Tests/Utils/I18nTextDomainFixerUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*
* @since 1.2.0
*
* @covers \WordPressCS\WordPress\AbstractFunctionParameterSniff::is_targetted_token
* @covers \WordPressCS\WordPress\Sniffs\Utils\I18nTextDomainFixerSniff
*/
final class I18nTextDomainFixerUnitTest extends AbstractSniffUnitTest {
Expand Down Expand Up @@ -148,6 +149,8 @@ public function getErrorList( $testFile = '' ) {
241 => 1,
242 => 1,
245 => 1,
277 => 1,
278 => 1,
);

default:
Expand Down
Loading