Skip to content

Commit

Permalink
Merge pull request #64 from wp-cli/61-audit
Browse files Browse the repository at this point in the history 
Check for more mistakes in translatable strings
  • Loading branch information
@schlessera
schlessera authored Aug 6, 2018
2 parents eb8d68f + 03a4872 commit 7762b19
Show file tree
Hide file tree
Showing 2 changed files with 289 additions and 14 deletions.
136 changes: 132 additions & 4 deletions features/makepot.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -605,9 +605,6 @@ Feature: Generate a POT file of a WordPress project
/* translators: Translators 1! */
__( 'Hello World', 'foo-plugin' );
/* translators: Translators 1! */
__( 'Hello World', 'foo-plugin' );
/* Translators: Translators 2! */
__( 'Hello World', 'foo-plugin' );
"""
Expand All @@ -620,7 +617,138 @@ Feature: Generate a POT file of a WordPress project
"""
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: Prints a warning for strings without translatable content
Given an empty foo-plugin directory
And a foo-plugin/foo-plugin.php file:
"""
<?php
/**
* Plugin Name: Plugin name
*/
sprintf( __( '"%s"', 'foo-plugin' ), $some_variable );
"""

When I try `wp i18n make-pot foo-plugin --debug`
Then STDOUT should be:
"""
Plugin file detected.
Success: POT file successfully generated!
"""
And STDERR should contain:
"""
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:
"""
<?php
/**
* Plugin Name: Plugin name
*/
sprintf( __( 'Hello, %s', 'foo-plugin' ), $some_variable );
"""

When I try `wp i18n make-pot foo-plugin --debug`
Then STDOUT should be:
"""
Plugin file detected.
Success: POT file successfully generated!
"""
And STDERR should contain:
"""
Warning: The string "Hello, %s" contains placeholders but has no "translators:" comment to clarify their meaning. (foo-plugin.php:6)
"""

Scenario: Prints a warning for missing singular placeholder
Given an empty foo-plugin directory
And a foo-plugin/foo-plugin.php file:
"""
<?php
/**
* Plugin Name: Plugin name
*/
sprintf(
_n( 'One Comment', '%s Comments', $number, 'foo-plugin' ),
$number
);
"""

When I try `wp i18n make-pot foo-plugin --debug`
Then STDOUT should be:
"""
Plugin file detected.
Success: POT file successfully generated!
"""
And STDERR should contain:
"""
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 mismatched placeholders
Given an empty foo-plugin directory
And a foo-plugin/foo-plugin.php file:
"""
<?php
/**
* Plugin Name: Plugin name
*/
sprintf(
_n( '%1$s Comment (%2$d)', '%2$s Comments (%1$s)', $number, 'foo-plugin' ),
$number,
$another_variable
);
"""

When I try `wp i18n make-pot foo-plugin --debug`
Then STDOUT should be:
"""
Plugin file detected.
Success: POT file successfully generated!
"""
And STDERR should contain:
"""
Mismatched placeholders for singular and plural string. (foo-plugin.php:7)
"""

Scenario: Prints a warning for multiple unordered placeholders
Given an empty foo-plugin directory
And a foo-plugin/foo-plugin.php file:
"""
<?php
/**
* Plugin Name: Plugin name
*/
sprintf(
__( 'Hello %s %s', 'foo-plugin' ),
$a_variable,
$another_variable
);
"""

When I try `wp i18n make-pot foo-plugin --debug`
Then STDOUT should be:
"""
Plugin file detected.
Success: POT file successfully generated!
"""
And STDERR should contain:
"""
Multiple placeholders should be ordered. (foo-plugin.php:7)
"""

Scenario: Skips excluded folders
Expand Down
167 changes: 157 additions & 10 deletions src/MakePotCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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 = '/(?:
(?<!%) # Don\'t match a literal % (%%).
(
% # Start of placeholder.
(?:[0-9]+\$)? # Optional ordering of the placeholders.
[+-]? # Optional sign specifier.
(?:
(?:0|\'.)? # Optional padding specifier - excluding the space.
-? # Optional alignment specifier.
[0-9]* # Optional width specifier.
(?:\.(?:[ 0]|\'.)?[0-9]+)? # Optional precision specifier with optional padding character.
| # Only recognize the space as padding in combination with a width specifier.
(?:[ ])? # Optional space padding specifier.
-? # Optional alignment specifier.
[0-9]+ # Width specifier.
(?:\.(?:[ 0]|\'.)?[0-9]+)? # Optional precision specifier with optional padding character.
)
[bcdeEfFgGosuxX] # Type specifier.
)
)/x';

/**
* "Unordered" means there's no position specifier: '%s', not '%2$s'.
*/
const UNORDERED_SPRINTF_PLACEHOLDER_REGEX = '/(?:
(?<!%) # Don\'t match a literal % (%%).
% # Start of placeholder.
[+-]? # Optional sign specifier.
(?:
(?:0|\'.)? # Optional padding specifier - excluding the space.
-? # Optional alignment specifier.
[0-9]* # Optional width specifier.
(?:\.(?:[ 0]|\'.)?[0-9]+)? # Optional precision specifier with optional padding character.
| # Only recognize the space as padding in combination with a width specifier.
(?:[ ])? # Optional space padding specifier.
-? # Optional alignment specifier.
[0-9]+ # Width specifier.
(?:\.(?:[ 0]|\'.)?[0-9]+)? # Optional precision specifier with optional padding character.
)
[bcdeEfFgGosuxX] # Type specifier.
)/x';

/**
* Create a POT file for a WordPress plugin or theme.
*
Expand Down Expand Up @@ -103,27 +149,40 @@ class MakePotCommand extends WP_CLI_Command {
* $ wp i18n make-pot . languages/my-plugin.pot
*
* @when before_wp_load
*
* @throws WP_CLI\ExitException
*/
public function __invoke( $args, $assoc_args ) {
$this->handle_arguments( $args, $assoc_args );

$translations = $this->extract_strings();

if ( ! $translations ) {
WP_CLI::warning( 'No translation found' );
}

$translations_count = count( $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' );
}

if ( ! PotGenerator::toFile( $translations, $this->destination ) ) {
WP_CLI::error( 'Could not generate a POT file!' );
}

WP_CLI::success( 'POT file successfully generated!' );
}

/**
* Process arguments from command-line in a reusable way.
*
* @throws WP_CLI\ExitException
*
* @param array $args Command arguments.
* @param array $assoc_args Associative arguments.
*/
public function handle_arguments( $args, $assoc_args ) {
$array_arguments = array( 'headers' );
Expand Down Expand Up @@ -193,7 +252,7 @@ public function handle_arguments( $args, $assoc_args ) {
$this->merge = $assoc_args['merge'];
}

if ( isset( $this->merge ) && ! file_exists( $this->merge ) ) {
if ( null !== $this->merge && ! file_exists( $this->merge ) ) {
WP_CLI::warning( sprintf( 'Invalid file provided to --merge: %s', $this->merge ) );

unset( $this->merge );
Expand All @@ -220,6 +279,8 @@ protected function unslashit( $string ) {
/**
* Retrieves the main file data of the plugin or theme.
*
* @throws WP_CLI\ExitException
*
* @return void
*/
protected function retrieve_main_file_data() {
Expand Down Expand Up @@ -316,6 +377,8 @@ protected function get_main_file_data() {
/**
* Creates a POT file and stores it on disk.
*
* @throws WP_CLI\ExitException
*
* @return Translations A Translation set.
*/
protected function extract_strings() {
Expand Down Expand Up @@ -385,23 +448,105 @@ protected function extract_strings() {
WP_CLI::error( $e->getMessage() );
}

$this->audit_strings( $translations );

return $translations;
}

/**
* Audits strings.
*
* Goes through all extracted strings to find possible mistakes.
*
* @param Translations $translations Translations object.
*/
protected function audit_strings( $translations ) {
foreach( $translations as $translation ) {
if ( ! $translation->hasExtractedComments() ) {
continue;
}
/** @var Translation $translation */

$references = $translation->getReferences();

$comments = $translation->getExtractedComments();
$comments_count = count( $comments );
// File headers don't have any file references.
$location = $translation->hasReferences() ? '(' . implode( ':', array_shift( $references ) ) . ')' : '';

if ( $comments_count > 1 ) {
// 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" has %2$d different translator comments.',
'The string "%1$s" contains placeholders but has no "translators:" comment to clarify their meaning. %2$s',
$translation->getOriginal(),
$comments_count
$location
) );
}

// Check 2: Flag strings with different translator comments.
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
) );
}
}

$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',
$location
) );
}

// 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];

if ( $unordered_matches_count >= 2 ) {
WP_CLI::warning( sprintf(
'Multiple placeholders should be ordered. %s',
$location
) );
}

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 ) ) {
// 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
) );
} else {
// Reordering is fine, but mismatched placeholders is probably wrong.
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',
$location
) );
}
}
}
}
return $translations;
}

/**
Expand Down Expand Up @@ -448,6 +593,8 @@ protected function get_meta_data() {

/**
* Sets default POT file headers for the project.
*
* @param Translations $translations Translations object.
*/
protected function set_default_headers( $translations ) {
$meta = $this->get_meta_data();
Expand Down

0 comments on commit 7762b19

Please sign in to comment.