Skip to content

Commit 8009bbe

Browse files
committed
WP/I18n: fix SuperfluousDefaultTextDomain auto-fix code to allow for named params
The auto-fixer for the `SuperfluousDefaultTextDomain` warning attempts to remove a complete parameter, though it will bow out from auto-fixing when there is a comment in the tokens which would be removed. This auto-fixer code and the code to determine whether an auto-fix is possible, now needs adjusting to allow for function calls with named parameters as otherwise the fixer could cause parse errors. * The parameter label and the `:` also needs to be removed. * If the named parameter is the first parameter, there will not be a preceding comma, but we will need to remove the comma behind it. * The range of tokens which needs to be checked for comments to determine whether the error is auto-fixable needs to be expanded. This commit makes those changes. Includes additional unit tests to safeguard the fixes.
1 parent ffcfe6f commit 8009bbe

File tree

4 files changed

+52
-4
lines changed

4 files changed

+52
-4
lines changed

WordPress/Sniffs/WP/I18nSniff.php

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -578,8 +578,28 @@ private function check_textdomain_matches( $matched_content, $param_name, $param
578578

579579
$first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_info['start'], ( $param_info['end'] + 1 ), true );
580580

581-
$before_param = $this->phpcsFile->findPrevious( \T_WHITESPACE, ( $first_non_empty - 1 ), null, true );
582-
if ( \T_COMMA === $this->tokens[ $before_param ]['code'] ) {
581+
// Prevent removing comments when auto-fixing.
582+
$remove_from = ( $param_info['start'] - 1 );
583+
$remove_to = $first_non_empty;
584+
585+
if ( isset( $param_info['name_token'] ) ) {
586+
$remove_from = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $param_info['name_token'] - 1 ), null, true );
587+
if ( \T_OPEN_PARENTHESIS === $this->tokens[ $remove_from ]['code'] ) {
588+
++$remove_from; // Don't remove the open parenthesis.
589+
590+
/*
591+
* Named param as first param in the function call, if we fix this, we need to
592+
* remove the comma _after_ the parameter as well to prevent creating a parse error.
593+
*/
594+
$remove_to = $param_info['end'];
595+
if ( \T_COMMA === $this->tokens[ ( $param_info['end'] + 1 ) ]['code'] ) {
596+
++$remove_to; // Include the comma.
597+
}
598+
}
599+
}
600+
601+
// Now, make sure there are no comments in the tokens we want to remove.
602+
if ( $this->phpcsFile->findNext( Tokens::$commentTokens, $remove_from, ( $remove_to + 1 ) ) === false ) {
583603
$fixable = true;
584604
}
585605

@@ -590,9 +610,8 @@ private function check_textdomain_matches( $matched_content, $param_name, $param
590610

591611
$fix = $this->phpcsFile->addFixableWarning( $error, $first_non_empty, $error_code, $data );
592612
if ( true === $fix ) {
593-
// Remove preceeding comma, whitespace and the text domain token.
594613
$this->phpcsFile->fixer->beginChangeset();
595-
for ( $i = $before_param; $i <= $first_non_empty; $i++ ) {
614+
for ( $i = $remove_from; $i <= $remove_to; $i++ ) {
596615
$this->phpcsFile->fixer->replaceToken( $i, '' );
597616
}
598617
$this->phpcsFile->fixer->endChangeset();

WordPress/Tests/WP/I18nUnitTest.1.inc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,5 +290,16 @@ _nx( number: $i, domain: 'my-slug', plural: 'translate %s me', single: 'translat
290290
__( 'translate me', 'default', ); // Bad: superfluous domain (positional with trailing comma).
291291
__( 'translate me', 'default' /*comment*/, ); // Bad: superfluous domain (positional with trailing comma and comment).
292292

293+
// Safeguard handling of named params by the SuperfluousDefaultTextDomain fixer.
294+
__( text: 'translate me', domain: 'default' ); // Bad: superfluous domain.
295+
__( text: 'translate me', domain: 'default', ); // Bad: superfluous domain (with trailing comma).
296+
_n_noop( domain: 'default', singular: 'translate me', plural: 'translate me', ); // Bad: superfluous domain.
297+
_nx_noop( singular: 'translate me', domain: 'default' /*comment*/, plural: 'translate me', context: 'context', ); // Bad: superfluous domain.
298+
299+
// These should not be auto-fixable as we don't want to remove the comment.
300+
__( text: 'translate me', domain: /*comment*/ 'default' ); // Bad: superfluous domain.
301+
_n_noop( domain: 'default' /*comment*/, singular: 'translate me', plural: 'translate me', ); // Bad: superfluous domain.
302+
_nx_noop( singular: 'translate me', domain /*comment*/: 'default', plural: 'translate me', context: 'context', ); // Bad: superfluous domain.
303+
293304
// phpcs:set WordPress.WP.I18n text_domain[]
294305
// phpcs:set WordPress.WP.I18n check_translator_comments true

WordPress/Tests/WP/I18nUnitTest.1.inc.fixed

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,5 +290,16 @@ _nx( number: $i, domain: 'my-slug', plural: 'translate %s me', single: 'translat
290290
__( 'translate me', ); // Bad: superfluous domain (positional with trailing comma).
291291
__( 'translate me' /*comment*/, ); // Bad: superfluous domain (positional with trailing comma and comment).
292292

293+
// Safeguard handling of named params by the SuperfluousDefaultTextDomain fixer.
294+
__( text: 'translate me' ); // Bad: superfluous domain.
295+
__( text: 'translate me', ); // Bad: superfluous domain (with trailing comma).
296+
_n_noop( singular: 'translate me', plural: 'translate me', ); // Bad: superfluous domain.
297+
_nx_noop( singular: 'translate me' /*comment*/, plural: 'translate me', context: 'context', ); // Bad: superfluous domain.
298+
299+
// These should not be auto-fixable as we don't want to remove the comment.
300+
__( text: 'translate me', domain: /*comment*/ 'default' ); // Bad: superfluous domain.
301+
_n_noop( domain: 'default' /*comment*/, singular: 'translate me', plural: 'translate me', ); // Bad: superfluous domain.
302+
_nx_noop( singular: 'translate me', domain /*comment*/: 'default', plural: 'translate me', context: 'context', ); // Bad: superfluous domain.
303+
293304
// phpcs:set WordPress.WP.I18n text_domain[]
294305
// phpcs:set WordPress.WP.I18n check_translator_comments true

WordPress/Tests/WP/I18nUnitTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,13 @@ public function getWarningList( $testFile = '' ) {
203203
285 => 1,
204204
290 => 1,
205205
291 => 1,
206+
294 => 1,
207+
295 => 1,
208+
296 => 1,
209+
297 => 1,
210+
300 => 1,
211+
301 => 1,
212+
302 => 1,
206213
);
207214

208215
case 'I18nUnitTest.2.inc':

0 commit comments

Comments
 (0)