Skip to content

Commit

Permalink
Add check for mismatched placeholders
Browse files Browse the repository at this point in the history
  • Loading branch information
Julian de Bhal committed Jun 12, 2016
1 parent 880e2fd commit b7e63aa
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 10 deletions.
48 changes: 48 additions & 0 deletions WordPress/Sniffs/WP/I18nSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,14 @@ public function process( PHP_CodeSniffer_File $phpcs_file, $stack_ptr ) {
}
call_user_func( array( $this, 'check_argument_tokens' ), $phpcs_file, $argument_assertion_context );
}

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

$this->compare_single_and_plural_arguments( $phpcs_file, $stack_ptr, $single_context, $plural_context );
}
}

/**
Expand All @@ -203,6 +211,7 @@ protected function check_argument_tokens( PHP_CodeSniffer_File $phpcs_file, $con
}
return false;
}

if ( count( $tokens ) > 1 ) {
$contents = '';
foreach ( $tokens as $token ) {
Expand Down Expand Up @@ -273,4 +282,43 @@ protected function check_argument_tokens( PHP_CodeSniffer_File $phpcs_file, $con
$phpcs_file->$method( 'The $%s arg should be single a string literal, not "%s".', $stack_ptr, $code, array( $arg_name, $content ) );
return false;
}

/**
* Check for inconsistencies between single and plural arguments.
*
* @param PHP_CodeSniffer_File $phpcs_file The file being scanned.
* @param array $single_context
* @param array $plural_context
* @return void
*/
protected function compare_single_and_plural_arguments( PHP_CodeSniffer_File $phpcs_file, $stack_ptr, $single_context, $plural_context ) {
$single_content = $single_context['tokens'][0]['content'];
$plural_content = $plural_context['tokens'][0]['content'];

// Regex copied from http://php.net/manual/en/function.sprintf.php#93552
$sprintf_placeholder_regex = '/(?:%%|(%(?:[0-9]+\$)?[+-]?(?:[ 0]|\'.)?-?[0-9]*(?:\.[0-9]+)?[bcdeufFos]))/';

preg_match_all( $sprintf_placeholder_regex, $single_content, $single_placeholders );
$single_placeholders = $single_placeholders[0];

preg_match_all( $sprintf_placeholder_regex, $plural_content, $plural_placeholders );
$plural_placeholders = $plural_placeholders[0];

// English conflates "singular" with "only one", described in the codex:
// https://codex.wordpress.org/I18n_for_WordPress_Developers#Plurals
if ( count( $single_placeholders ) < count( $plural_placeholders ) ) {
$error_string = 'Missing singular placeholder, needed for some languages. See https://codex.wordpress.org/I18n_for_WordPress_Developers#Plurals';
$single_index = $single_context['tokens'][0]['token_index'];

$phpcs_file->addError( $error_string, $single_index, 'MissingSingularPlaceholder' );
}

// Reordering is fine, but mismatched placeholders is probably wrong.
sort( $single_placeholders );
sort( $plural_placeholders );

if ( $single_placeholders !== $plural_placeholders ) {
$phpcs_file->addWarning( 'Mismatched placeholders is probably an error', $stack_ptr, 'MismatchedPlaceholders' );
}
}
}
17 changes: 12 additions & 5 deletions WordPress/Tests/WP/I18nUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,15 @@ _n_noop('I have %d cat.', 'I have %d cats.', 'my-slug'); // OK.
// Dollar sign in literal string is not interpolated, so OK.
_n_noop( 'I have %d cat.', 'I have %d cats literal-string-so-$variable-not-interpolated.', 'my-slug' ); // OK.

// Multiple placeholders should have orderable placeholders
__( 'There are %d monkeys in the %s', 'my-slug' ); // Multiple arguments should be numbered
__( 'There are %1$d monkeys in the %2$s', 'my-slug' ); // OK
_n( 'There is %d monkey in the %s', 'There are %d monkeys in the %s', $number, 'my-slug' ); // Multiple arguments should be numbered
_n( 'There is %1$d monkey in the %2$s', 'In the %2$s are There are %1$d monkeys', $number, 'my-slug' ); // OK
// Multiple placeholders should have orderable placeholders.
__( 'There are %d monkeys in the %s', 'my-slug' ); // Multiple arguments should be numbered.
__( 'There are %1$d monkeys in the %2$s', 'my-slug' ); // OK.
_n( 'There is %d monkey in the %s', 'There are %d monkeys in the %s', $number, 'my-slug' ); // Multiple arguments should be numbered.
_n( 'There is %1$d monkey in the %2$s', 'In the %2$s there are %1$d monkeys', $number, 'my-slug' ); // OK.

// The singular form should use placeholders if the plural does.
// https://codex.wordpress.org/I18n_for_WordPress_Developers#Plurals
_n( 'I have a cat.', 'I have %d cats.', $number, 'my-slug' ); // Bad, singular should have placeholder.
_n_noop( 'I have a cat.', 'I have %d cats.', 'my-slug' ); // Bad, singular should have placeholder.
_nx( 'I have a cat.', 'I have %d cats.', $number, 'Not really.', 'my-slug' ); // Bad, singular should have placeholder.
_nx_noop( 'I have a cat.', 'I have %d cats.', 'Not really.', 'my-slug' ); // Bad, singular should have placeholder.
17 changes: 12 additions & 5 deletions WordPress/Tests/WP/I18nUnitTest.inc.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,15 @@ _n_noop('I have %d cat.', 'I have %d cats.', 'my-slug'); // OK.
// Dollar sign in literal string is not interpolated, so OK.
_n_noop( 'I have %d cat.', 'I have %d cats literal-string-so-$variable-not-interpolated.', 'my-slug' ); // OK.

// Multiple placeholders should have orderable placeholders
__( 'There are %1$d monkeys in the %2$s', 'my-slug' ); // Multiple arguments should be numbered
__( 'There are %1$d monkeys in the %2$s', 'my-slug' ); // OK
_n( 'There is %1$d monkey in the %2$s', 'There are %1$d monkeys in the %2$s', $number, 'my-slug' ); // Multiple arguments should be numbered
_n( 'There is %1$d monkey in the %2$s', 'In the %2$s are There are %1$d monkeys', $number, 'my-slug' ); // OK
// Multiple placeholders should have orderable placeholders.
__( 'There are %1$d monkeys in the %2$s', 'my-slug' ); // Multiple arguments should be numbered.
__( 'There are %1$d monkeys in the %2$s', 'my-slug' ); // OK.
_n( 'There is %1$d monkey in the %2$s', 'There are %1$d monkeys in the %2$s', $number, 'my-slug' ); // Multiple arguments should be numbered.
_n( 'There is %1$d monkey in the %2$s', 'In the %2$s there are %1$d monkeys', $number, 'my-slug' ); // OK.

// The singular form should use placeholders if the plural does.
// https://codex.wordpress.org/I18n_for_WordPress_Developers#Plurals
_n( 'I have a cat.', 'I have %d cats.', $number, 'my-slug' ); // Bad, singular should have placeholder.
_n_noop( 'I have a cat.', 'I have %d cats.', 'my-slug' ); // Bad, singular should have placeholder.
_nx( 'I have a cat.', 'I have %d cats.', $number, 'Not really.', 'my-slug' ); // Bad, singular should have placeholder.
_nx_noop( 'I have a cat.', 'I have %d cats.', 'Not really.', 'my-slug' ); // Bad, singular should have placeholder.
8 changes: 8 additions & 0 deletions WordPress/Tests/WP/I18nUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public function getErrorList() {
78 => 1,
93 => 1,
95 => 2,
100 => 1,
101 => 1,
102 => 1,
103 => 1,
);
}

Expand All @@ -83,6 +87,10 @@ public function getWarningList() {
return array(
69 => 1,
70 => 1,
100 => 1,
101 => 1,
102 => 1,
103 => 1,
);
}
}

0 comments on commit b7e63aa

Please sign in to comment.