Merge pull request #560 from WordPress-Coding-Standards/feature/i18n-…

…text-domain-check Validate $domain arg in WordPress.WP.I18n
WordPress · May 2, 2016 · ec69684 · ec69684
2 parents 6749e4e + d21367b
commit ec69684
Show file tree

Hide file tree

Showing 4 changed files with 96 additions and 55 deletions.
diff --git a/WordPress-Core/ruleset.xml b/WordPress-Core/ruleset.xml
@@ -101,5 +101,6 @@
 	<rule ref="WordPress.WhiteSpace.OperatorSpacing"/>
 	<rule ref="WordPress.WhiteSpace.CastStructureSpacing"/>
 	<rule ref="WordPress.PHP.YodaConditions"/>
+	<rule ref="WordPress.WP.I18n"/>
 
 </ruleset>
diff --git a/WordPress/Sniffs/WP/I18nSniff.php b/WordPress/Sniffs/WP/I18nSniff.php
@@ -10,6 +10,25 @@
  */
 class WordPress_Sniffs_WP_I18nSniff extends WordPress_Sniff {
 
+	/**
+	 * Text domain.
+	 *
+	 * @todo Eventually this should be able to be auto-supplied via looking at $phpcs_file->getFilename()
+	 * @link https://youtrack.jetbrains.com/issue/WI-17740
+	 *
+	 * @var string
+	 */
+	public $text_domain;
+
+	/**
+	 * Allow unit tests to override the supplied text_domain.
+	 *
+	 * @todo While it doesn't work, ideally this should be able to be done in \WordPress_Tests_WP_I18nUnitTest::setUp()
+	 *
+	 * @var string
+	 */
+	static $text_domain_override;
+
 	public $i18n_functions = array(
 		'translate'                      => 'simple',
 		'__'                             => 'simple',
@@ -53,6 +72,10 @@ public function process( PHP_CodeSniffer_File $phpcs_file, $stack_ptr ) {
 		$tokens = $phpcs_file->getTokens();
 		$token  = $tokens[ $stack_ptr ];
 
+		if ( ! empty( self::$text_domain_override ) ) {
+			$this->text_domain = self::$text_domain_override;
+		}
+
 		if ( '_' === $token['content'] ) {
 			$phpcs_file->addError( 'Found single-underscore "_()" function when double-underscore expected.', $stack_ptr, 'SingleUnderscoreGetTextFunction' );
 		}
@@ -117,31 +140,31 @@ public function process( PHP_CodeSniffer_File $phpcs_file, $stack_ptr ) {
 		$argument_assertions = array();
 		if ( 'simple' === $this->i18n_functions[ $translation_function ] ) {
 			$argument_assertions[] = array( 'arg_name' => 'text',    'tokens' => array_shift( $arguments_tokens ) );
-			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ), 'warning' => true );
+			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ) );
 		} elseif ( 'context' === $this->i18n_functions[ $translation_function ] ) {
 			$argument_assertions[] = array( 'arg_name' => 'text',    'tokens' => array_shift( $arguments_tokens ) );
 			$argument_assertions[] = array( 'arg_name' => 'context', 'tokens' => array_shift( $arguments_tokens ) );
-			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ), 'warning' => true );
+			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ) );
 		} elseif ( 'number' === $this->i18n_functions[ $translation_function ] ) {
 			$argument_assertions[] = array( 'arg_name' => 'single',  'tokens' => array_shift( $arguments_tokens ) );
 			$argument_assertions[] = array( 'arg_name' => 'plural',  'tokens' => array_shift( $arguments_tokens ) );
 			array_shift( $arguments_tokens );
-			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ), 'warning' => true );
+			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ) );
 		} elseif ( 'number_context' === $this->i18n_functions[ $translation_function ] ) {
 			$argument_assertions[] = array( 'arg_name' => 'single',  'tokens' => array_shift( $arguments_tokens ) );
 			$argument_assertions[] = array( 'arg_name' => 'plural',  'tokens' => array_shift( $arguments_tokens ) );
 			array_shift( $arguments_tokens );
 			$argument_assertions[] = array( 'arg_name' => 'context', 'tokens' => array_shift( $arguments_tokens ) );
-			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ), 'warning' => true );
+			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ) );
 		} elseif ( 'noopnumber' === $this->i18n_functions[ $translation_function ] ) {
 			$argument_assertions[] = array( 'arg_name' => 'single',  'tokens' => array_shift( $arguments_tokens ) );
 			$argument_assertions[] = array( 'arg_name' => 'plural',  'tokens' => array_shift( $arguments_tokens ) );
-			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ), 'warning' => true );
+			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ) );
 		} elseif ( 'noopnumber_context' === $this->i18n_functions[ $translation_function ] ) {
 			$argument_assertions[] = array( 'arg_name' => 'single',  'tokens' => array_shift( $arguments_tokens ) );
 			$argument_assertions[] = array( 'arg_name' => 'plural',  'tokens' => array_shift( $arguments_tokens ) );
 			$argument_assertions[] = array( 'arg_name' => 'context', 'tokens' => array_shift( $arguments_tokens ) );
-			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ), 'warning' => true );
+			$argument_assertions[] = array( 'arg_name' => 'domain',  'tokens' => array_shift( $arguments_tokens ) );
 		}
 
 		if ( ! empty( $arguments_tokens ) ) {
@@ -173,7 +196,9 @@ protected function check_argument_tokens( PHP_CodeSniffer_File $phpcs_file, $con
 
 		if ( 0 === count( $tokens ) ) {
 			$code = 'MissingArg' . ucfirst( $arg_name );
-			$phpcs_file->$method( 'Missing $%s arg.', $stack_ptr, $code, array( $arg_name ) );
+			if ( 'domain' !== $arg_name || ! empty( $this->text_domain ) ) {
+				$phpcs_file->$method( 'Missing $%s arg.', $stack_ptr, $code, array( $arg_name ) );
+			}
 			return false;
 		}
 		if ( count( $tokens ) > 1 ) {
@@ -186,6 +211,10 @@ protected function check_argument_tokens( PHP_CodeSniffer_File $phpcs_file, $con
 			return false;
 		}
 		if ( T_CONSTANT_ENCAPSED_STRING === $tokens[0]['code'] ) {
+			if ( 'domain' === $arg_name && ! empty( $this->text_domain ) && trim( $tokens[0]['content'], '\'""' ) !== $this->text_domain ) {
+				$phpcs_file->$method( 'Mismatch text domain. Expected \'%s\' but got %s.', $stack_ptr, 'TextDomainMismatch', array( $this->text_domain, $tokens[0]['content'] ) );
+				return false;
+			}
 			return true;
 		}
 		if ( T_DOUBLE_QUOTED_STRING === $tokens[0]['code'] ) {
@@ -194,7 +223,14 @@ protected function check_argument_tokens( PHP_CodeSniffer_File $phpcs_file, $con
 				$code = 'InterpolatedVariable' . ucfirst( $arg_name );
 				$phpcs_file->$method( 'The $%s arg must not contain interpolated variables. Found "$%s".', $stack_ptr, $code, array( $arg_name, $interpolated_variable ) );
 			}
-			return false;
+			if ( ! empty( $interpolated_variables ) ) {
+				return false;
+			}
+			if ( 'domain' === $arg_name && ! empty( $this->text_domain ) && trim( $tokens[0]['content'], '\'""' ) !== $this->text_domain ) {
+				$phpcs_file->$method( 'Mismatch text domain. Expected \'%s\' but got %s.', $stack_ptr, 'TextDomainMismatch', array( $this->text_domain, $tokens[0]['content'] ) );
+				return false;
+			}
+			return true;
 		}
 
 		$code = 'NonSingularStringLiteral' . ucfirst( $arg_name );

diff --git a/WordPress/Tests/WP/I18nUnitTest.inc b/WordPress/Tests/WP/I18nUnitTest.inc
@@ -1,46 +1,46 @@
 <?php
 
-__( "hell$varo", 'domain' ); // Bad, shouldn't use a string with variables
+__( "hell$varo", 'my-slug' ); // Bad, shouldn't use a string with variables
 
-__( "hell\$varo", 'domain' ); // OK, Variable is not interpolated.
-__( "hell\\$varo", 'domain' ); // Bad, is interpolated.
-__( "hell\\\$varo", 'domain' ); // OK, variable is escaped.
+__( "hell\$varo", 'my-slug' ); // OK, Variable is not interpolated.
+__( "hell\\$varo", 'my-slug' ); // Bad, is interpolated.
+__( "hell\\\$varo", 'my-slug' ); // OK, variable is escaped.
 
-__( $var, 'domain' ); // Bad, shouldn't use variables
+__( $var, 'my-slug' ); // Bad, shouldn't use variables
 
 __( 'string', SOMETHING ); // Bad, shouldn't use CONSTANTS
 
-__( 'string' . $var, 'domain' ); // Bad, shouldn't use variable for string
+__( 'string' . $var, 'my-slug' ); // Bad, shouldn't use variable for string
 
-__( $var . 'string', 'domain' ); // Bad, shouldn't use variable for string
+__( $var . 'string', 'my-slug' ); // Bad, shouldn't use variable for string
 
-__( SOMETHING, 'domain' ); // Bad, shouldn't use constant for string
+__( SOMETHING, 'my-slug' ); // Bad, shouldn't use constant for string
 
-__( 'string' . SOMETHING, 'domain' ); // Bad, shouldn't use constant for string
+__( 'string' . SOMETHING, 'my-slug' ); // Bad, shouldn't use constant for string
 
-__( SOMETHING . 'string', 'domain' ); // Bad, shouldn't use variable for string
+__( SOMETHING . 'string', 'my-slug' ); // Bad, shouldn't use variable for string
 
 __( 'string', $domain ); // Bad, shouldn't use variable for domain
 
 __( 'string', 'my' . $domain ); // Bad, shouldn't use variable for domain
 
-__( 'string', $domain . 'domain' ); // Bad, shouldn't use variable for domain
+__( 'string', $domain . 'my-slug' ); // Bad, shouldn't use variable for domain
 
-__( 'string', 'domain' ); // Good
+__( 'string', 'my-slug' ); // Good
 
-_x( 'string', 'context', 'domain' ); // Good
+_x( 'string', 'context', 'my-slug' ); // Good
 
-_x( 'string', $var, 'domain' ); // Bad, shouldn't use variable for context
+_x( 'string', $var, 'my-slug' ); // Bad, shouldn't use variable for context
 
-_x( 'string', 'context' . $var, 'domain' ); // Bad, shouldn't use variable for context
+_x( 'string', 'context' . $var, 'my-slug' ); // Bad, shouldn't use variable for context
 
-_x( 'string', $var . 'context', 'domain' ); // Bad, shouldn't use variable for context
+_x( 'string', $var . 'context', 'my-slug' ); // Bad, shouldn't use variable for context
 
-_x( 'string', SOMETHING, 'domain' ); // Bad, shouldn't use constant for context
+_x( 'string', SOMETHING, 'my-slug' ); // Bad, shouldn't use constant for context
 
-_x( 'string', SOMETHING . 'context', 'domain' ); // Bad, shouldn't use constant for context
+_x( 'string', SOMETHING . 'context', 'my-slug' ); // Bad, shouldn't use constant for context
 
-_x( 'string', 'context' . SOMETHING, 'domain' ); // Bad, shouldn't use constant for context
+_x( 'string', 'context' . SOMETHING, 'my-slug' ); // Bad, shouldn't use constant for context
 
 _n( 'I have %d cat.', 'I have %d cats.', $number ); // Bad, no text domain.
 _n( 'I have %d cat.', 'I have %d cats.', $number, 'my-slug' ); // OK.
@@ -66,13 +66,13 @@ _nx_noop( 'I have %d cat.', 'I have %d cats.', $context, 'my-slug' ); // Bad.
 _nx_noop( 'I have %d cat.', 'I have %d cats.', 'Not really.', "illegal $string" ); // Bad.
 _nx_noop( 'I have %d cat.', 'I have %d cats.', 'Not really.', SOMETHING ); // Bad.
 
-translate( 'foo', 'bar' ); // Bad, low-level API function.
-translate_with_gettext_context( 'foo', 'bar', 'baz' ); // Bad, low-level API function.
+translate( 'foo', 'my-slug' ); // Bad, low-level API function.
+translate_with_gettext_context( 'foo', 'bar', 'my-slug' ); // Bad, low-level API function.
 
-_( 'foo', 'bar' ); // Bad.
+_( 'foo', 'my-slug' ); // Bad.
 
-__( 'foo', 'bar', 'too-many-args' ); // Bad.
-_x( 'string', 'context', 'domain', 'too-many-args' ); // Bad
+__( 'foo', 'my-slug', 'too-many-args' ); // Bad.
+_x( 'string', 'context', 'my-slug', 'too-many-args' ); // Bad
 _n( 'I have %d cat.', 'I have %d cats.', $number, 'my-slug', 'too-many-args' ); // Bad
 _n_noop( 'I have %d cat.', 'I have %d cats.', 'my-slug', 'too-many-args' ); // Bad.
 _nx_noop( 'I have %d cat.', 'I have %d cats.', 'Not really.', 'my-slug', 'too-many-args' ); // Bad.
@@ -81,11 +81,10 @@ _nx_noop( 'I have %d cat.', 'I have %d cats.', 'Not really.', 'my-slug', 'too-ma
 _nx( 'I have
 %d cat.', 'I have
 %d cats.', $number, 'Not
-really.', 'my-
-slug' ); // OK.
+really.', 'my-slug' ); // OK.
 
 // Ensure lack of spaces doesn't cause i18n error.
 _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' ); // OK.
+_n_noop( 'I have %d cat.', 'I have %d cats literal-string-so-$variable-not-interpolated.', 'my-slug' ); // OK.
diff --git a/WordPress/Tests/WP/I18nUnitTest.php b/WordPress/Tests/WP/I18nUnitTest.php
@@ -10,6 +10,12 @@
  */
 class WordPress_Tests_WP_I18nUnitTest extends AbstractSniffUnitTest {
 
+	protected function setUp() {
+		// @todo Should be possible via self::$phpcs->setSniffProperty( 'WordPress_Sniffs_WP_I18nSniff', 'text_domain', 'my-slug' );
+		WordPress_Sniffs_WP_I18nSniff::$text_domain_override = 'my-slug';
+		parent::setUp();
+	}
+
 	/**
 	 * Returns the lines where errors should occur.
 	 *
@@ -23,27 +29,44 @@ public function getErrorList() {
 			3 => 1,
 			6 => 1,
 			9 => 1,
+			11 => 1,
 			13 => 1,
 			15 => 1,
 			17 => 1,
 			19 => 1,
 			21 => 1,
+			23 => 1,
+			25 => 1,
+			27 => 1,
 			33 => 1,
 			35 => 1,
 			37 => 1,
 			39 => 1,
 			41 => 1,
 			43 => 1,
-			56 => 1,
+			45 => 1,
+			47 => 1,
+			48 => 1,
+			50 => 1,
+			52 => 1,
+			53 => 1,
+			55 => 1,
+			56 => 2,
 			58 => 1,
-			63 => 1,
+			59 => 1,
+			60 => 1,
+			62 => 1,
+			63 => 2,
 			65 => 1,
+			66 => 1,
+			67 => 1,
 			72 => 1,
 			74 => 1,
 			75 => 1,
 			76 => 1,
 			77 => 1,
 			78 => 1,
+
 		);
 	}
 
@@ -57,24 +80,6 @@ public function getErrorList() {
 	 */
 	public function getWarningList() {
 		return array(
-			11 => 1,
-			23 => 1,
-			25 => 1,
-			27 => 1,
-			45 => 1,
-			47 => 1,
-			48 => 1,
-			50 => 1,
-			52 => 1,
-			53 => 1,
-			55 => 1,
-			56 => 1,
-			59 => 1,
-			60 => 1,
-			62 => 1,
-			63 => 1,
-			66 => 1,
-			67 => 1,
 			69 => 1,
 			70 => 1,
 		);