WP/I18n: new error: passing empty string as text domain

Issue 1948 flagged an issue which, while rare, should still be flagged.

The default value for the `$domain` parameter for nearly all translation functions is (currently) `'default'`, with the exceptions being `_n_noop()` and `_nx_noop()`, where the default value is `null`.

If a function call would pass an empty string as the `$domain`, this would previously already be flagged if the end-user had set one or more valid `text_domain`s in their ruleset.

However, if no `text_domain` was set in the ruleset, this would not be flagged by the sniff and as an empty string would overrule the default value for the functions, in effect, the text string would become untranslatable as there is no text domain and the functions would no longer fall back to the WP native `'default'` text domain.

I haven't checked if/when these default values were changed in Core, but that shouldn't really matter anyway as, as things currently are, passing an empty text string as the `$domain` is problematic and should be flagged.

Fixed now.
The behaviour for when a  valid `text_domain` was set in the ruleset remains unchanged.

Includes unit test.

Fixes 1948

WordPress/Sniffs/WP/I18nSniff.php

@@ -361,7 +361,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p
 				continue;
 			}
 
-			if ( 'domain' === $param_name && ! empty( $this->text_domain ) ) {
+			if ( 'domain' === $param_name ) {
 				$this->check_textdomain_matches( $matched_content, $param_name, $param_info );
 			}
 
@@ -544,6 +544,21 @@ private function check_argument_is_string_literal( $stackPtr, $matched_content,
 	private function check_textdomain_matches( $matched_content, $param_name, $param_info ) {
 		$stripped_content = TextStrings::stripQuotes( $param_info['clean'] );
 
+		if ( empty( $this->text_domain ) && '' === $stripped_content ) {
+			$first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_info['start'], ( $param_info['end'] + 1 ), true );
+
+			$this->phpcsFile->addError(
+				'The passed $domain should never be an empty string. Either pass a text domain or remove the parameter.',
+				$first_non_empty,
+				'EmptyTextDomain'
+			);
+		}
+
+		if ( empty( $this->text_domain ) ) {
+			// Nothing more to do, the other checks all depend on a text domain being known.
+			return;
+		}
+
 		if ( ! \in_array( $stripped_content, $this->text_domain, true ) ) {
 			$first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_info['start'], ( $param_info['end'] + 1 ), true );
 

WordPress/Tests/WP/I18nUnitTest.1.inc

@@ -307,5 +307,11 @@ _nx( 'I have
 %d cats and %d dogs.', $number, 'Not
 really.' ); // Bad, multiple arguments should be numbered.
 
+// Show an error when the text domain is an empty string.
+esc_html_e( 'foo', '' ); // Bad: text domain mismatch.
+
+// Issue #1948 - Show an error when the text domain is an empty string and no text domain has been passed.
 // phpcs:set WordPress.WP.I18n text_domain[]
+esc_html_e( 'foo', '' ); // Bad: text-domain can not be empty.
+
 // phpcs:enable WordPress.WP.I18n.MissingTranslatorsComment

WordPress/Tests/WP/I18nUnitTest.1.inc.fixed

@@ -307,5 +307,11 @@ _nx( 'I have
 %1$d cats and %2$d dogs.', $number, 'Not
 really.' ); // Bad, multiple arguments should be numbered.
 
+// Show an error when the text domain is an empty string.
+esc_html_e( 'foo', '' ); // Bad: text domain mismatch.
+
+// Issue #1948 - Show an error when the text domain is an empty string and no text domain has been passed.
 // phpcs:set WordPress.WP.I18n text_domain[]
+esc_html_e( 'foo', '' ); // Bad: text-domain can not be empty.
+
 // phpcs:enable WordPress.WP.I18n.MissingTranslatorsComment

WordPress/Tests/WP/I18nUnitTest.php

@@ -144,6 +144,8 @@ public function getErrorList( $testFile = '' ) {
 					284 => 1,
 					305 => 1,
 					306 => 1,
+					311 => 1,
+					315 => 1,
 				);
 
 			case 'I18nUnitTest.2.inc':