From f521f3f0d4d155dddc58f25a94010963de389416 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Tue, 31 Jul 2018 17:02:09 +0200 Subject: [PATCH 01/10] Check for more mistakes in translatable strings See #61. --- src/MakePotCommand.php | 126 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 116 insertions(+), 10 deletions(-) diff --git a/src/MakePotCommand.php b/src/MakePotCommand.php index 02c01858..4ff04324 100644 --- a/src/MakePotCommand.php +++ b/src/MakePotCommand.php @@ -63,6 +63,52 @@ class MakePotCommand extends WP_CLI_Command { */ protected $domain; + /** + * These Regexes copied from http://php.net/manual/en/function.sprintf.php#93552 + * and adjusted for better precision and updated specs. + */ + const SPRINTF_PLACEHOLDER_REGEX = '/(?: + (?getMessage() ); } + $this->audit_strings(); + + return PotGenerator::toFile( $this->translations, $this->destination ); + } + + /** + * Audits strings. + * + * Goes through all extracted strings to find possible mistakes. + */ + protected function audit_strings() { foreach( $this->translations as $translation ) { - if ( ! $translation->hasExtractedComments() ) { - continue; + /** @var Translation $translation */ + + $references = $translation->getReferences(); + $location = implode( ':', array_unshift( $references ) ); + + if ( $translation->hasExtractedComments() ) { + $comments = $translation->getExtractedComments(); + $comments_count = count( $comments ); + + if ( $comments_count > 1 ) { + WP_CLI::warning( sprintf( + 'The string "%1$s" has %2$d different translator comments. (%3$s)', + $translation->getOriginal(), + $comments_count, + $location + ) ); + } } - $comments = $translation->getExtractedComments(); - $comments_count = count( $comments ); + $non_placeholder_content = trim( preg_replace( '`^([\'"])(.*)\1$`Ds', '$2', $translation->getOriginal() ) ); + $non_placeholder_content = preg_replace( self::SPRINTF_PLACEHOLDER_REGEX, '', $non_placeholder_content ); - if ( $comments_count > 1 ) { + if ( '' === $non_placeholder_content ) { WP_CLI::warning( sprintf( - 'The string "%1$s" has %2$d different translator comments.', - $translation->getOriginal(), - $comments_count + 'Found string without translatable content (%s)', + $location ) ); } - } - return PotGenerator::toFile( $this->translations, $this->destination ); + if ( $translation->hasPlural() ) { + preg_match_all( self::SPRINTF_PLACEHOLDER_REGEX, $translation->getOriginal(), $single_placeholders ); + $single_placeholders = $single_placeholders[0]; + + preg_match_all( self::SPRINTF_PLACEHOLDER_REGEX, $translation->getPlural(), $plural_placeholders ); + $plural_placeholders = $plural_placeholders[0]; + + // see https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals + if ( count( $single_placeholders ) < \count( $plural_placeholders ) ) { + WP_CLI::warning( sprintf( + 'Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals (%s)', + $location + ) ); + } else { + sort( $single_placeholders ); + sort( $plural_placeholders ); + + if ( $single_placeholders !== $plural_placeholders ) { + WP_CLI::warning( sprintf( + 'Singular and plural placeholder appear in different order (%s)', + $location + ) ); + } + } + + // UnorderedPlaceholders: Check for multiple unordered placeholders. + $unordered_matches_count = preg_match_all( self::UNORDERED_SPRINTF_PLACEHOLDER_REGEX, $translation->getOriginal(), $unordered_matches ); + $unordered_matches = $unordered_matches[0]; + + if ( $unordered_matches_count >= 2 ) { + WP_CLI::warning( sprintf( + 'Multiple placeholders should be ordered (%s)', + $location + ) ); + } + } + } } /** From 5c5c6a85d8f71365e11502fb2ead102d7d046048 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Tue, 31 Jul 2018 17:25:27 +0200 Subject: [PATCH 02/10] unshift -> shift --- src/MakePotCommand.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/MakePotCommand.php b/src/MakePotCommand.php index 4ff04324..8bce14cf 100644 --- a/src/MakePotCommand.php +++ b/src/MakePotCommand.php @@ -421,7 +421,7 @@ protected function audit_strings() { /** @var Translation $translation */ $references = $translation->getReferences(); - $location = implode( ':', array_unshift( $references ) ); + $location = implode( ':', array_shift( $references ) ); if ( $translation->hasExtractedComments() ) { $comments = $translation->getExtractedComments(); From 3c8f91f1553e9d501f1fefab4d37ccc51f9fa266 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Tue, 31 Jul 2018 17:54:44 +0200 Subject: [PATCH 03/10] update file reference check --- features/makepot.feature | 5 +---- src/MakePotCommand.php | 10 ++++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/features/makepot.feature b/features/makepot.feature index b689682c..e69f7723 100644 --- a/features/makepot.feature +++ b/features/makepot.feature @@ -605,9 +605,6 @@ Feature: Generate a POT file of a WordPress plugin /* translators: Translators 1! */ __( 'Hello World', 'foo-plugin' ); - /* translators: Translators 1! */ - __( 'Hello World', 'foo-plugin' ); - /* Translators: Translators 2! */ __( 'Hello World', 'foo-plugin' ); """ @@ -620,7 +617,7 @@ Feature: Generate a POT file of a WordPress plugin """ And STDERR should contain: """ - Warning: The string "Hello World" has 2 different translator comments. + Warning: The string "Hello World" has 2 different translator comments. (foo-plugin.php:7) """ Scenario: Skips excluded folders diff --git a/src/MakePotCommand.php b/src/MakePotCommand.php index 8bce14cf..0ea0c242 100644 --- a/src/MakePotCommand.php +++ b/src/MakePotCommand.php @@ -421,7 +421,9 @@ protected function audit_strings() { /** @var Translation $translation */ $references = $translation->getReferences(); - $location = implode( ':', array_shift( $references ) ); + + // File headers don't have any file references. + $location = $translation->hasReferences() ? '(' . implode( ':', array_shift( $references ) ) . ')' : ''; if ( $translation->hasExtractedComments() ) { $comments = $translation->getExtractedComments(); @@ -442,7 +444,7 @@ protected function audit_strings() { if ( '' === $non_placeholder_content ) { WP_CLI::warning( sprintf( - 'Found string without translatable content (%s)', + 'Found string without translatable content %s', $location ) ); } @@ -466,7 +468,7 @@ protected function audit_strings() { if ( $single_placeholders !== $plural_placeholders ) { WP_CLI::warning( sprintf( - 'Singular and plural placeholder appear in different order (%s)', + 'Singular and plural placeholder appear in different order %s', $location ) ); } @@ -478,7 +480,7 @@ protected function audit_strings() { if ( $unordered_matches_count >= 2 ) { WP_CLI::warning( sprintf( - 'Multiple placeholders should be ordered (%s)', + 'Multiple placeholders should be ordered %s', $location ) ); } From c976d232d37cb5467f990f7f16627f89cc5ed274 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Fri, 3 Aug 2018 12:24:31 +0200 Subject: [PATCH 04/10] Remove duplicate parentheses --- src/MakePotCommand.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/MakePotCommand.php b/src/MakePotCommand.php index 0ea0c242..e4ca0024 100644 --- a/src/MakePotCommand.php +++ b/src/MakePotCommand.php @@ -431,7 +431,7 @@ protected function audit_strings() { if ( $comments_count > 1 ) { WP_CLI::warning( sprintf( - 'The string "%1$s" has %2$d different translator comments. (%3$s)', + 'The string "%1$s" has %2$d different translator comments. %3$s', $translation->getOriginal(), $comments_count, $location @@ -459,7 +459,7 @@ protected function audit_strings() { // see https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals if ( count( $single_placeholders ) < \count( $plural_placeholders ) ) { WP_CLI::warning( sprintf( - 'Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals (%s)', + 'Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals %s', $location ) ); } else { From 369dcd92b3ed101dea2b56c38349e7ae244cd2d0 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Fri, 3 Aug 2018 15:01:59 +0200 Subject: [PATCH 05/10] Add some more tests --- features/makepot.feature | 107 +++++++++++++++++++++++++++++++++++++++ src/MakePotCommand.php | 26 +++++----- 2 files changed, 120 insertions(+), 13 deletions(-) diff --git a/features/makepot.feature b/features/makepot.feature index e69f7723..ec8a4350 100644 --- a/features/makepot.feature +++ b/features/makepot.feature @@ -620,6 +620,113 @@ Feature: Generate a POT file of a WordPress plugin Warning: The string "Hello World" has 2 different translator comments. (foo-plugin.php:7) """ + Scenario: Prints a warning for strings without translatable content + Given an empty foo-plugin directory + And a foo-plugin/foo-plugin.php file: + """ + getOriginal(), $unordered_matches ); + $unordered_matches = $unordered_matches[0]; + + if ( $unordered_matches_count >= 2 ) { + WP_CLI::warning( sprintf( + 'Multiple placeholders should be ordered. %s', $location ) ); } @@ -468,22 +479,11 @@ protected function audit_strings() { if ( $single_placeholders !== $plural_placeholders ) { WP_CLI::warning( sprintf( - 'Singular and plural placeholder appear in different order %s', + 'Singular and plural placeholder appear in different order. %s', $location ) ); } } - - // UnorderedPlaceholders: Check for multiple unordered placeholders. - $unordered_matches_count = preg_match_all( self::UNORDERED_SPRINTF_PLACEHOLDER_REGEX, $translation->getOriginal(), $unordered_matches ); - $unordered_matches = $unordered_matches[0]; - - if ( $unordered_matches_count >= 2 ) { - WP_CLI::warning( sprintf( - 'Multiple placeholders should be ordered %s', - $location - ) ); - } } } } From a8830ed894b9e14561881388f035b7c6d7b67ea8 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Sun, 5 Aug 2018 15:45:48 +0200 Subject: [PATCH 06/10] Use pluralize utility function --- src/MakePotCommand.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/MakePotCommand.php b/src/MakePotCommand.php index e389307a..7aa181cb 100644 --- a/src/MakePotCommand.php +++ b/src/MakePotCommand.php @@ -12,6 +12,8 @@ use DirectoryIterator; use IteratorIterator; +use function WP_CLI\Utils\pluralize; + class MakePotCommand extends WP_CLI_Command { /** * @var Translations @@ -433,11 +435,14 @@ protected function makepot() { $translations_count = count( $this->translations ); - if ( 1 === $translations_count ) { - WP_CLI::debug( sprintf( 'Extracted %d string', $translations_count ), 'make-pot' ); - } else { - WP_CLI::debug( sprintf( 'Extracted %d strings', $translations_count ), 'make-pot' ); - } + WP_CLI::debug( + sprintf( + 'Extracted %d %s', + $translations_count, + pluralize( 'string', $translations_count ) + ), + 'make-pot' + ); return $result; } From f4173365aaa2a9cd6435805a055342b11f329c6a Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Sun, 5 Aug 2018 15:51:16 +0200 Subject: [PATCH 07/10] PHP compat --- src/MakePotCommand.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/MakePotCommand.php b/src/MakePotCommand.php index 7aa181cb..5c6b8886 100644 --- a/src/MakePotCommand.php +++ b/src/MakePotCommand.php @@ -12,8 +12,6 @@ use DirectoryIterator; use IteratorIterator; -use function WP_CLI\Utils\pluralize; - class MakePotCommand extends WP_CLI_Command { /** * @var Translations @@ -439,7 +437,7 @@ protected function makepot() { sprintf( 'Extracted %d %s', $translations_count, - pluralize( 'string', $translations_count ) + WP_CLI\Utils\pluralize( 'string', $translations_count ) ), 'make-pot' ); From cbebdac7287e1558b4fe47d8a2206ef66a746451 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Sun, 5 Aug 2018 17:16:40 +0200 Subject: [PATCH 08/10] run -> try --- features/makepot.feature | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/makepot.feature b/features/makepot.feature index 9ef70627..9b7cc1bf 100644 --- a/features/makepot.feature +++ b/features/makepot.feature @@ -660,7 +660,7 @@ Feature: Generate a POT file of a WordPress project """ - When I run `wp i18n make-pot foo-plugin --debug` + When I try `wp i18n make-pot foo-plugin --debug` Then STDOUT should be: """ Plugin file detected. @@ -688,7 +688,7 @@ Feature: Generate a POT file of a WordPress project """ - When I run `wp i18n make-pot foo-plugin --debug` + When I try `wp i18n make-pot foo-plugin --debug` Then STDOUT should be: """ Plugin file detected. @@ -716,7 +716,7 @@ Feature: Generate a POT file of a WordPress project """ - When I run `wp i18n make-pot foo-plugin --debug` + When I try `wp i18n make-pot foo-plugin --debug` Then STDOUT should be: """ Plugin file detected. From b58b69ebb8af741c93e78f485db6814d57c45daa Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Sun, 5 Aug 2018 19:56:05 +0200 Subject: [PATCH 09/10] Fixes after code review --- features/makepot.feature | 6 +++--- src/MakePotCommand.php | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/features/makepot.feature b/features/makepot.feature index 9b7cc1bf..d0905560 100644 --- a/features/makepot.feature +++ b/features/makepot.feature @@ -671,7 +671,7 @@ Feature: Generate a POT file of a WordPress project Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals (foo-plugin.php:7) """ - Scenario: Prints a warning for placeholders in different order + Scenario: Prints a warning for mismatched placeholders Given an empty foo-plugin directory And a foo-plugin/foo-plugin.php file: """ @@ -681,7 +681,7 @@ Feature: Generate a POT file of a WordPress project */ sprintf( - _n( '%1$s Comment (%2$s)', '%2$$s Comments (%1$s)', $number, 'foo-plugin' ), + _n( '%1$s Comment (%2$d)', '%2$s Comments (%1$s)', $number, 'foo-plugin' ), $number, $another_variable ); @@ -696,7 +696,7 @@ Feature: Generate a POT file of a WordPress project """ And STDERR should contain: """ - Singular and plural placeholder appear in different order. (foo-plugin.php:7) + Mismatched placeholders for singular and plural string. (foo-plugin.php:7) """ Scenario: Prints a warning for multiple unordered placeholders diff --git a/src/MakePotCommand.php b/src/MakePotCommand.php index 5c6b8886..8e1a0b16 100644 --- a/src/MakePotCommand.php +++ b/src/MakePotCommand.php @@ -502,18 +502,19 @@ protected function audit_strings() { $plural_placeholders = $plural_placeholders[0]; // see https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals - if ( count( $single_placeholders ) < \count( $plural_placeholders ) ) { + if ( count( $single_placeholders ) < count( $plural_placeholders ) ) { WP_CLI::warning( sprintf( 'Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals %s', $location ) ); } else { + // Reordering is fine, but mismatched placeholders is probably wrong. sort( $single_placeholders ); sort( $plural_placeholders ); if ( $single_placeholders !== $plural_placeholders ) { WP_CLI::warning( sprintf( - 'Singular and plural placeholder appear in different order. %s', + 'Mismatched placeholders for singular and plural string. %s', $location ) ); } From 03a4872a23dcbaa4930b4af36f2e23d95f03e839 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Mon, 6 Aug 2018 19:28:24 +0200 Subject: [PATCH 10/10] Flag strings with placeholders that should have translator comments --- features/makepot.feature | 26 +++++++++++++++++++++++++- src/MakePotCommand.php | 20 ++++++++++++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/features/makepot.feature b/features/makepot.feature index d0905560..75fb84e6 100644 --- a/features/makepot.feature +++ b/features/makepot.feature @@ -629,7 +629,7 @@ Feature: Generate a POT file of a WordPress project * Plugin Name: Plugin name */ - sprintf( __( '"%s"', 'foo-plugin' ) ); + sprintf( __( '"%s"', 'foo-plugin' ), $some_variable ); """ @@ -644,6 +644,30 @@ Feature: Generate a POT file of a WordPress project Warning: Found string without translatable content. (foo-plugin.php:6) """ + Scenario: Prints a warning for a string with missing translator comment + Given an empty foo-plugin directory + And a foo-plugin/foo-plugin.php file: + """ + getReferences(); // File headers don't have any file references. - $location = $translation->hasReferences() ? '(' . implode( ':', array_shift( $references ) ) . ')' : ''; + $location = $translation->hasReferences() ? '(' . implode( ':', array_shift( $references ) ) . ')' : ''; + // Check 1: Flag strings with placeholders that should have translator comments. + if ( + ! $translation->hasExtractedComments() && + preg_match( self::SPRINTF_PLACEHOLDER_REGEX, $translation->getOriginal(), $placeholders ) >= 1 + ) { + WP_CLI::warning( sprintf( + 'The string "%1$s" contains placeholders but has no "translators:" comment to clarify their meaning. %2$s', + $translation->getOriginal(), + $location + ) ); + } + + // Check 2: Flag strings with different translator comments. if ( $translation->hasExtractedComments() ) { $comments = $translation->getExtractedComments(); $comments_count = count( $comments ); @@ -486,6 +499,7 @@ protected function audit_strings( $translations ) { $non_placeholder_content = trim( preg_replace( '`^([\'"])(.*)\1$`Ds', '$2', $translation->getOriginal() ) ); $non_placeholder_content = preg_replace( self::SPRINTF_PLACEHOLDER_REGEX, '', $non_placeholder_content ); + // Check 3: Flag empty strings without any translatable content. if ( '' === $non_placeholder_content ) { WP_CLI::warning( sprintf( 'Found string without translatable content. %s', @@ -493,7 +507,7 @@ protected function audit_strings( $translations ) { ) ); } - // UnorderedPlaceholders: Check for multiple unordered placeholders. + // Check 4: Flag strings with multiple unordered placeholders (%s %s %s vs. %1$s %2$s %3$s). $unordered_matches_count = preg_match_all( self::UNORDERED_SPRINTF_PLACEHOLDER_REGEX, $translation->getOriginal(), $unordered_matches ); $unordered_matches = $unordered_matches[0]; @@ -513,6 +527,7 @@ protected function audit_strings( $translations ) { // see https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals if ( count( $single_placeholders ) < count( $plural_placeholders ) ) { + // Check 5: Flag things like _n( 'One comment', '%s Comments' ) WP_CLI::warning( sprintf( 'Missing singular placeholder, needed for some languages. See https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#plurals %s', $location @@ -522,6 +537,7 @@ protected function audit_strings( $translations ) { sort( $single_placeholders ); sort( $plural_placeholders ); + // Check 6: Flag things like _n( '%s Comment (%d)', '%s Comments (%s)' ) if ( $single_placeholders !== $plural_placeholders ) { WP_CLI::warning( sprintf( 'Mismatched placeholders for singular and plural string. %s',