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.I18n: Fix false positive on method calls. #1267

Merged
merged 2 commits into from
Dec 27, 2017
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
91 changes: 66 additions & 25 deletions WordPress/Sniffs/WP/I18nSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace WordPress\Sniffs\WP;

use WordPress\Sniff;
use WordPress\AbstractFunctionRestrictionsSniff;
use WordPress\PHPCSHelper;
use PHP_CodeSniffer_Tokens as Tokens;

Expand All @@ -27,8 +27,11 @@
* as a comma-delimited list.
* `phpcs --runtime-set text_domain my-slug,default`
* @since 0.13.0 Class name changed: this class is now namespaced.
* @since 1.0.0 This class now extends the AbstractFunctionRestrictionSniff.
* The parent `exclude` property is, however, disabled as it
* would disable the whole sniff.
*/
class I18nSniff extends Sniff {
class I18nSniff extends AbstractFunctionRestrictionsSniff {

/**
* These Regexes copied from http://php.net/manual/en/function.sprintf.php#93552
Expand Down Expand Up @@ -144,25 +147,43 @@ class I18nSniff extends Sniff {
private $text_domain_is_default = false;

/**
* Returns an array of tokens this test wants to listen for.
* Groups of functions to restrict.
*
* Example: groups => array(
* 'lambda' => array(
* 'type' => 'error' | 'warning',
* 'message' => 'Use anonymous functions instead please!',
* 'functions' => array( 'file_get_contents', 'create_function' ),
* )
* )
*
* @return array
*/
public function register() {
public function getGroups() {
return array(
T_STRING,
'i18n' => array(
'functions' => array_keys( $this->i18n_functions ),
),
'typos' => array(
'functions' => array(
'_',
),
),
);
}
} // End getGroups().

/**
* Processes this test, when one of its tokens is encountered.
*
* @since 1.0.0 Defers to the abstractFunctionRestriction sniff for determining
* whether something is a function call. The logic after that has
* been split off to the `process_matched_token()` method.
*
* @param int $stack_ptr The position of the current token in the stack.
*
* @return void
*/
public function process_token( $stack_ptr ) {
$token = $this->tokens[ $stack_ptr ];

// Reset defaults.
$this->text_domain_contains_default = false;
Expand All @@ -185,22 +206,42 @@ public function process_token( $stack_ptr ) {
}
}

if ( '_' === $token['content'] ) {
$this->phpcsFile->addError( 'Found single-underscore "_()" function when double-underscore expected.', $stack_ptr, 'SingleUnderscoreGetTextFunction' );
}
// Prevent exclusion of the i18n group.
$this->exclude = '';

parent::process_token( $stack_ptr );
}

if ( ! isset( $this->i18n_functions[ $token['content'] ] ) ) {
/**
* Process a matched token.
*
* @since 1.0.0 Logic split off from the `process_token()` method.
*
* @param int $stack_ptr The position of the current token in the stack.
* @param string $group_name The name of the group which was matched.
* @param string $matched_content The token content (function name) which was matched.
*
* @return int|void Integer stack pointer to skip forward or void to continue
* normal file processing.
*/
public function process_matched_token( $stack_ptr, $group_name, $matched_content ) {

$func_open_paren_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stack_ptr + 1 ), null, true );
if ( false === $func_open_paren_token
|| T_OPEN_PARENTHESIS !== $this->tokens[ $func_open_paren_token ]['code']
|| ! isset( $this->tokens[ $func_open_paren_token ]['parenthesis_closer'] )
) {
// Live coding, parse error or not a function call.
return;
}
$translation_function = $token['content'];

if ( in_array( $translation_function, array( 'translate', 'translate_with_gettext_context' ), true ) ) {
$this->phpcsFile->addWarning( 'Use of the "%s()" function is reserved for low-level API usage.', $stack_ptr, 'LowLevelTranslationFunction', array( $translation_function ) );
if ( 'typos' === $group_name && '_' === $matched_content ) {
$this->phpcsFile->addError( 'Found single-underscore "_()" function when double-underscore expected.', $stack_ptr, 'SingleUnderscoreGetTextFunction' );
return;
}

$func_open_paren_token = $this->phpcsFile->findNext( T_WHITESPACE, ( $stack_ptr + 1 ), null, true );
if ( false === $func_open_paren_token || T_OPEN_PARENTHESIS !== $this->tokens[ $func_open_paren_token ]['code'] ) {
return;
if ( in_array( $matched_content, array( 'translate', 'translate_with_gettext_context' ), true ) ) {
$this->phpcsFile->addWarning( 'Use of the "%s()" function is reserved for low-level API usage.', $stack_ptr, 'LowLevelTranslationFunction', array( $matched_content ) );
}

$arguments_tokens = array();
Expand Down Expand Up @@ -249,7 +290,7 @@ public function process_token( $stack_ptr ) {
unset( $argument_tokens );

$argument_assertions = array();
if ( 'simple' === $this->i18n_functions[ $translation_function ] ) {
if ( 'simple' === $this->i18n_functions[ $matched_content ] ) {
$argument_assertions[] = array(
'arg_name' => 'text',
'tokens' => array_shift( $arguments_tokens ),
Expand All @@ -258,7 +299,7 @@ public function process_token( $stack_ptr ) {
'arg_name' => 'domain',
'tokens' => array_shift( $arguments_tokens ),
);
} elseif ( 'context' === $this->i18n_functions[ $translation_function ] ) {
} elseif ( 'context' === $this->i18n_functions[ $matched_content ] ) {
$argument_assertions[] = array(
'arg_name' => 'text',
'tokens' => array_shift( $arguments_tokens ),
Expand All @@ -271,7 +312,7 @@ public function process_token( $stack_ptr ) {
'arg_name' => 'domain',
'tokens' => array_shift( $arguments_tokens ),
);
} elseif ( 'number' === $this->i18n_functions[ $translation_function ] ) {
} elseif ( 'number' === $this->i18n_functions[ $matched_content ] ) {
$argument_assertions[] = array(
'arg_name' => 'single',
'tokens' => array_shift( $arguments_tokens ),
Expand All @@ -285,7 +326,7 @@ public function process_token( $stack_ptr ) {
'arg_name' => 'domain',
'tokens' => array_shift( $arguments_tokens ),
);
} elseif ( 'number_context' === $this->i18n_functions[ $translation_function ] ) {
} elseif ( 'number_context' === $this->i18n_functions[ $matched_content ] ) {
$argument_assertions[] = array(
'arg_name' => 'single',
'tokens' => array_shift( $arguments_tokens ),
Expand All @@ -303,7 +344,7 @@ public function process_token( $stack_ptr ) {
'arg_name' => 'domain',
'tokens' => array_shift( $arguments_tokens ),
);
} elseif ( 'noopnumber' === $this->i18n_functions[ $translation_function ] ) {
} elseif ( 'noopnumber' === $this->i18n_functions[ $matched_content ] ) {
$argument_assertions[] = array(
'arg_name' => 'single',
'tokens' => array_shift( $arguments_tokens ),
Expand All @@ -316,7 +357,7 @@ public function process_token( $stack_ptr ) {
'arg_name' => 'domain',
'tokens' => array_shift( $arguments_tokens ),
);
} elseif ( 'noopnumber_context' === $this->i18n_functions[ $translation_function ] ) {
} elseif ( 'noopnumber_context' === $this->i18n_functions[ $matched_content ] ) {
$argument_assertions[] = array(
'arg_name' => 'single',
'tokens' => array_shift( $arguments_tokens ),
Expand All @@ -336,7 +377,7 @@ public function process_token( $stack_ptr ) {
}

if ( ! empty( $arguments_tokens ) ) {
$this->phpcsFile->addError( 'Too many arguments for function "%s".', $func_open_paren_token, 'TooManyFunctionArgs', array( $translation_function ) );
$this->phpcsFile->addError( 'Too many arguments for function "%s".', $func_open_paren_token, 'TooManyFunctionArgs', array( $matched_content ) );
}

foreach ( $argument_assertions as $argument_assertion_context ) {
Expand All @@ -349,7 +390,7 @@ public function process_token( $stack_ptr ) {
}

// For _n*() calls, compare the singular and plural strings.
if ( false !== strpos( $this->i18n_functions[ $translation_function ], 'number' ) ) {
if ( false !== strpos( $this->i18n_functions[ $matched_content ], 'number' ) ) {
$single_context = $argument_assertions[0];
$plural_context = $argument_assertions[1];

Expand Down
6 changes: 6 additions & 0 deletions WordPress/Tests/WP/I18nUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,9 @@ __( 'String default text domain.' ); // Ok because default domain is 'default' a

// @codingStandardsChangeSetting WordPress.WP.I18n text_domain false
// @codingStandardsChangeSetting WordPress.WP.I18n check_translator_comments true

// Issue #1266.
// @codingStandardsChangeSetting WordPress.WP.I18n text_domain my-slug
$mo->translate( $string ); // OK, not a function, but a method call.
Something\esc_html_e( $string ); // OK, not the WP function, but namespaced function call.
// @codingStandardsChangeSetting WordPress.WP.I18n text_domain false
6 changes: 6 additions & 0 deletions WordPress/Tests/WP/I18nUnitTest.1.inc.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -161,3 +161,9 @@ __( 'String default text domain.' ); // Ok because default domain is 'default' a

// @codingStandardsChangeSetting WordPress.WP.I18n text_domain false
// @codingStandardsChangeSetting WordPress.WP.I18n check_translator_comments true

// Issue #1266.
// @codingStandardsChangeSetting WordPress.WP.I18n text_domain my-slug
$mo->translate( $string ); // OK, not a function, but a method call.
Something\esc_html_e( $string ); // OK, not the WP function, but namespaced function call.
// @codingStandardsChangeSetting WordPress.WP.I18n text_domain false