From b7e63aaa35688f4414f52860cf754f1d81cb6713 Mon Sep 17 00:00:00 2001 From: Julian de Bhal Date: Thu, 9 Jun 2016 12:51:08 +1000 Subject: [PATCH] Add check for mismatched placeholders --- WordPress/Sniffs/WP/I18nSniff.php | 48 +++++++++++++++++++++++ WordPress/Tests/WP/I18nUnitTest.inc | 17 +++++--- WordPress/Tests/WP/I18nUnitTest.inc.fixed | 17 +++++--- WordPress/Tests/WP/I18nUnitTest.php | 8 ++++ 4 files changed, 80 insertions(+), 10 deletions(-) diff --git a/WordPress/Sniffs/WP/I18nSniff.php b/WordPress/Sniffs/WP/I18nSniff.php index 34de3aab6f..2172b8e9e7 100644 --- a/WordPress/Sniffs/WP/I18nSniff.php +++ b/WordPress/Sniffs/WP/I18nSniff.php @@ -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 ); + } } /** @@ -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 ) { @@ -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' ); + } + } } diff --git a/WordPress/Tests/WP/I18nUnitTest.inc b/WordPress/Tests/WP/I18nUnitTest.inc index 72f8292002..d5929c916a 100644 --- a/WordPress/Tests/WP/I18nUnitTest.inc +++ b/WordPress/Tests/WP/I18nUnitTest.inc @@ -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. diff --git a/WordPress/Tests/WP/I18nUnitTest.inc.fixed b/WordPress/Tests/WP/I18nUnitTest.inc.fixed index 7df3b8ff9b..0618202aec 100644 --- a/WordPress/Tests/WP/I18nUnitTest.inc.fixed +++ b/WordPress/Tests/WP/I18nUnitTest.inc.fixed @@ -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. diff --git a/WordPress/Tests/WP/I18nUnitTest.php b/WordPress/Tests/WP/I18nUnitTest.php index 35a66ba909..a5faa22d51 100644 --- a/WordPress/Tests/WP/I18nUnitTest.php +++ b/WordPress/Tests/WP/I18nUnitTest.php @@ -68,6 +68,10 @@ public function getErrorList() { 78 => 1, 93 => 1, 95 => 2, + 100 => 1, + 101 => 1, + 102 => 1, + 103 => 1, ); } @@ -83,6 +87,10 @@ public function getWarningList() { return array( 69 => 1, 70 => 1, + 100 => 1, + 101 => 1, + 102 => 1, + 103 => 1, ); } }