Check for more mistakes in translatable strings

features/makepot.feature

@@ -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' );
       """
@@ -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

src/MakePotCommand.php

@@ -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.
 	 *
@@ -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' );
@@ -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 );
@@ -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() {
@@ -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() {
@@ -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;
 	}
 
 	/**
@@ -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();