From e8656b962f63fd5665d6f6b2f99a33ce6fe119e3 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 28 Feb 2020 18:01:33 +0100
Subject: [PATCH 01/41] Add new ESLint rule to validate text domains

---
 packages/eslint-plugin/CHANGELOG.md           |   4 +
 packages/eslint-plugin/configs/custom.js      |   1 +
 .../docs/rules/valid-text-domain.md           |  27 ++++
 .../rules/__tests__/valid-text-domain.js      | 126 +++++++++++++++
 .../eslint-plugin/rules/valid-text-domain.js  | 143 ++++++++++++++++++
 5 files changed, 301 insertions(+)
 create mode 100644 packages/eslint-plugin/docs/rules/valid-text-domain.md
 create mode 100644 packages/eslint-plugin/rules/__tests__/valid-text-domain.js
 create mode 100644 packages/eslint-plugin/rules/valid-text-domain.js

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index a38e783e1adb9f..7f7effb4d1b4c4 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -1,5 +1,9 @@
 ## Master
 
+### New Features
+
+- New Rule: [`@wordpress/valid-text-domain`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/valid-text-domain.md)
+
 ## 4.0.0 (2020-02-10)
 
 ### Breaking Changes
diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index 8949b63f94d8dc..9fce23934c8d3e 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -5,6 +5,7 @@ module.exports = {
 		'@wordpress/valid-sprintf': 'error',
 		'@wordpress/no-base-control-with-label-without-id': 'error',
 		'@wordpress/no-unguarded-get-range-at': 'error',
+		'@wordpress/valid-text-domain': 'error',
 		'no-restricted-syntax': [
 			'error',
 			{
diff --git a/packages/eslint-plugin/docs/rules/valid-text-domain.md b/packages/eslint-plugin/docs/rules/valid-text-domain.md
new file mode 100644
index 00000000000000..c4691d20652525
--- /dev/null
+++ b/packages/eslint-plugin/docs/rules/valid-text-domain.md
@@ -0,0 +1,27 @@
+# Enforce passing valid text domains (valid-text-domain)
+
+[Translation functions](https://github.com/WordPress/gutenberg/blob/master/packages/i18n/README.md#api) must be called with a valid string literal as the text domain.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```js
+__( 'Hello World' ); // unless allowDefault is set.
+__( 'Hello World', 'default' ); // with allowDefault set.
+__( 'Hello World', foo );
+```
+
+Examples of **correct** code for this rule:
+
+```js
+__( 'Hello World' ); // with allowDefault set.
+__( 'Hello World', 'foo-bar' ); // with allowedTextDomains set
+```
+
+## Options
+
+This rule accepts two options:
+
+- Set the `allowDefault` boolean to allow omitting the text domain and using its default value (`'default'`).
+- Set `allowedTextDomains` to specify the list of allowed text domains, e.g. `'foo-bar'`.
diff --git a/packages/eslint-plugin/rules/__tests__/valid-text-domain.js b/packages/eslint-plugin/rules/__tests__/valid-text-domain.js
new file mode 100644
index 00000000000000..c5bd15d92474e5
--- /dev/null
+++ b/packages/eslint-plugin/rules/__tests__/valid-text-domain.js
@@ -0,0 +1,126 @@
+/**
+ * External dependencies
+ */
+import { RuleTester } from 'eslint';
+
+/**
+ * Internal dependencies
+ */
+import rule from '../valid-text-domain';
+
+const ruleTester = new RuleTester( {
+	parserOptions: {
+		ecmaVersion: 6,
+	},
+} );
+
+ruleTester.run( 'valid-text-domain', rule, {
+	valid: [
+		{
+			code: `__('Hello World')`,
+			options: [ { allowDefault: true } ],
+		},
+		{
+			code: `_x('Hello World', 'context')`,
+			options: [ { allowDefault: true } ],
+		},
+		{
+			code: `var number = ''; _n('Singular', 'Plural', number)`,
+			options: [ { allowDefault: true } ],
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
+			options: [ { allowDefault: true } ],
+		},
+		{
+			code: `__('Hello World', 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+		},
+		{
+			code: `_x('Hello World', 'context', 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+		},
+		{
+			code: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+		},
+	],
+	invalid: [
+		{
+			code: `__('Hello World')`,
+			errors: [ { message: 'Missing text domain' } ],
+		},
+		{
+			code: `_x('Hello World', 'context')`,
+			errors: [ { message: 'Missing text domain' } ],
+		},
+		{
+			code: `var number = ''; _n('Singular', 'Plural', number)`,
+			errors: [ { message: 'Missing text domain' } ],
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
+			errors: [ { message: 'Missing text domain' } ],
+		},
+		{
+			code: `__('Hello World', 'bar')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { message: 'Invalid text domain: bar' } ],
+		},
+		{
+			code: `_x('Hello World', 'context', 'bar')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { message: 'Invalid text domain: bar' } ],
+		},
+		{
+			code: `var number = ''; _n('Singular', 'Plural', number, 'bar')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { message: 'Invalid text domain: bar' } ],
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'bar')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { message: 'Invalid text domain: bar' } ],
+		},
+		{
+			code: `var value = ''; __('Hello World', value)`,
+			errors: [ { message: 'Text domain is not a string literal' } ],
+		},
+		{
+			code: `var value = ''; _x('Hello World', 'context', value)`,
+			errors: [ { message: 'Text domain is not a string literal' } ],
+		},
+		{
+			code: `var value = ''; var number = ''; _n('Singular', 'Plural', number, value)`,
+			errors: [ { message: 'Text domain is not a string literal' } ],
+		},
+		{
+			code: `var value = ''; var number = ''; _nx('Singular', 'Plural', number, 'context', value)`,
+			errors: [ { message: 'Text domain is not a string literal' } ],
+		},
+		{
+			code: `__('Hello World', 'default')`,
+			options: [ { allowDefault: true } ],
+			errors: [ { message: 'Unnecessary default text domain' } ],
+		},
+		{
+			code: `_x('Hello World', 'context', 'default')`,
+			options: [ { allowDefault: true } ],
+			errors: [ { message: 'Unnecessary default text domain' } ],
+		},
+		{
+			code: `var number = ''; _n('Singular', 'Plural', number, 'default')`,
+			options: [ { allowDefault: true } ],
+			errors: [ { message: 'Unnecessary default text domain' } ],
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'default')`,
+			options: [ { allowDefault: true } ],
+			errors: [ { message: 'Unnecessary default text domain' } ],
+		},
+	],
+} );
diff --git a/packages/eslint-plugin/rules/valid-text-domain.js b/packages/eslint-plugin/rules/valid-text-domain.js
new file mode 100644
index 00000000000000..bf3ad6d8ad0b61
--- /dev/null
+++ b/packages/eslint-plugin/rules/valid-text-domain.js
@@ -0,0 +1,143 @@
+const STATUS = {
+	MISSING: 'missing',
+	INVALID_VALUE: 'invalid-domain',
+	INVALID_TYPE: 'invalid-type',
+	VALID: 'valid',
+};
+
+function validateTextDomain( functionName, args, allowedTextDomains ) {
+	switch ( functionName ) {
+		case '__':
+			if ( args.length < 2 ) {
+				return STATUS.MISSING;
+			}
+			break;
+		case '_x':
+			if ( args.length < 3 ) {
+				return STATUS.MISSING;
+			}
+
+			break;
+		case '_n':
+			if ( args.length < 4 ) {
+				return STATUS.MISSING;
+			}
+
+			break;
+		case '_nx':
+			if ( args.length < 5 ) {
+				return STATUS.MISSING;
+			}
+
+			break;
+		default:
+			break;
+	}
+
+	const textDomain = getTextDomain( functionName, args );
+
+	if ( ! textDomain ) {
+		return STATUS.INVALID_TYPE;
+	}
+
+	const { type, value } = textDomain;
+
+	if ( type !== 'Literal' ) {
+		return STATUS.INVALID_TYPE;
+	}
+
+	if ( ! allowedTextDomains.includes( value ) ) {
+		return STATUS.INVALID_VALUE;
+	}
+
+	return STATUS.VALID;
+}
+
+function getTextDomain( functionName, args ) {
+	switch ( functionName ) {
+		case '__':
+			return args[ 1 ];
+		case '_x':
+			return args[ 2 ];
+		case '_n':
+			return args[ 3 ];
+		case '_nx':
+			return args[ 4 ];
+		default:
+			return {};
+	}
+}
+
+const TRANSLATION_FUNCTIONS = [ '__', '_x', '_n', '_nx' ];
+
+module.exports = {
+	meta: {
+		type: 'problem',
+		schema: [
+			{
+				type: 'object',
+				properties: {
+					allowDefault: {
+						type: 'boolean',
+						default: false,
+					},
+					allowedTextDomains: {
+						type: 'array',
+						items: {
+							type: 'string',
+						},
+					},
+				},
+				additionalProperties: false,
+			},
+		],
+	},
+	create( context ) {
+		const options = context.options[ 0 ] || {};
+		const { allowDefault, allowedTextDomains = [] } = options;
+
+		return {
+			CallExpression( node ) {
+				const { callee, arguments: args } = node;
+				if ( ! TRANSLATION_FUNCTIONS.includes( callee.name ) ) {
+					return;
+				}
+
+				const status = validateTextDomain(
+					callee.name,
+					args,
+					allowedTextDomains
+				);
+
+				switch ( status ) {
+					case STATUS.MISSING:
+						if ( ! allowDefault ) {
+							context.report( node, 'Missing text domain' );
+						}
+						break;
+					case STATUS.INVALID_TYPE:
+						context.report(
+							node,
+							'Text domain is not a string literal'
+						);
+						break;
+					case STATUS.INVALID_VALUE:
+						const { value } = getTextDomain( callee.name, args );
+
+						if ( 'default' === value && allowDefault ) {
+							context.report(
+								node,
+								'Unnecessary default text domain'
+							);
+							break;
+						}
+
+						context.report( node, 'Invalid text domain: ' + value );
+						break;
+					default:
+						break;
+				}
+			},
+		};
+	},
+};

From 80f6b8bf95ac583fb50081fa9b7e3a3d7c2394f1 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 28 Feb 2020 18:26:02 +0100
Subject: [PATCH 02/41] Enforce `@wordpress/valid-text-domain` rule for
 Gutenberg code base

---
 .eslintrc.js                    | 6 ++++++
 packages/i18n/src/test/index.js | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/.eslintrc.js b/.eslintrc.js
index 9f7da03674a7df..9c9e18fcf7ec7f 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -42,6 +42,12 @@ module.exports = {
 		'@wordpress/dependency-group': 'error',
 		'@wordpress/gutenberg-phase': 'error',
 		'@wordpress/react-no-unsafe-timeout': 'error',
+		'@wordpress/valid-text-domain': [
+			'error',
+			{
+				allowDefault: true,
+			},
+		],
 		'no-restricted-syntax': [
 			'error',
 			// NOTE: We can't include the forward slash in our regex or
diff --git a/packages/i18n/src/test/index.js b/packages/i18n/src/test/index.js
index 5246bc8f3659b1..e464e7f02e03bf 100644
--- a/packages/i18n/src/test/index.js
+++ b/packages/i18n/src/test/index.js
@@ -1,3 +1,5 @@
+/* eslint-disable @wordpress/valid-text-domain */
+
 // Mock memoization as identity function. Inline since Jest errors on out-of-
 // scope references in a mock callback.
 jest.mock( 'memize', () => ( fn ) => fn );
@@ -185,3 +187,5 @@ describe( 'i18n', () => {
 function setDefaultLocalData() {
 	setLocaleData( localeData, 'test_domain' );
 }
+
+/* eslint-enable @wordpress/valid-text-domain */

From 656ec12b26e28154dea52c930e6a999f606d1dcd Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Sat, 29 Feb 2020 12:36:02 +0100
Subject: [PATCH 03/41] Use messageId and make fixable

---
 .../rules/__tests__/valid-text-domain.js      | 48 ++++++++-----
 .../eslint-plugin/rules/valid-text-domain.js  | 72 ++++++++++++++++---
 2 files changed, 95 insertions(+), 25 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/valid-text-domain.js b/packages/eslint-plugin/rules/__tests__/valid-text-domain.js
index c5bd15d92474e5..d08a23b92a292e 100644
--- a/packages/eslint-plugin/rules/__tests__/valid-text-domain.js
+++ b/packages/eslint-plugin/rules/__tests__/valid-text-domain.js
@@ -52,75 +52,91 @@ ruleTester.run( 'valid-text-domain', rule, {
 	invalid: [
 		{
 			code: `__('Hello World')`,
-			errors: [ { message: 'Missing text domain' } ],
+			output: `__('Hello World', 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `_x('Hello World', 'context')`,
-			errors: [ { message: 'Missing text domain' } ],
+			output: `_x('Hello World', 'context', 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `var number = ''; _n('Singular', 'Plural', number)`,
-			errors: [ { message: 'Missing text domain' } ],
+			output: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
-			errors: [ { message: 'Missing text domain' } ],
+			output: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `__('Hello World', 'bar')`,
+			output: `__('Hello World', 'foo')`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
-			errors: [ { message: 'Invalid text domain: bar' } ],
+			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
 			code: `_x('Hello World', 'context', 'bar')`,
+			output: `_x('Hello World', 'context', 'foo')`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
-			errors: [ { message: 'Invalid text domain: bar' } ],
+			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
 			code: `var number = ''; _n('Singular', 'Plural', number, 'bar')`,
+			output: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
-			errors: [ { message: 'Invalid text domain: bar' } ],
+			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
 			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'bar')`,
+			output: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
-			errors: [ { message: 'Invalid text domain: bar' } ],
+			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
 			code: `var value = ''; __('Hello World', value)`,
-			errors: [ { message: 'Text domain is not a string literal' } ],
+			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
 			code: `var value = ''; _x('Hello World', 'context', value)`,
-			errors: [ { message: 'Text domain is not a string literal' } ],
+			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
 			code: `var value = ''; var number = ''; _n('Singular', 'Plural', number, value)`,
-			errors: [ { message: 'Text domain is not a string literal' } ],
+			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
 			code: `var value = ''; var number = ''; _nx('Singular', 'Plural', number, 'context', value)`,
-			errors: [ { message: 'Text domain is not a string literal' } ],
+			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
 			code: `__('Hello World', 'default')`,
+			output: `__('Hello World')`,
 			options: [ { allowDefault: true } ],
-			errors: [ { message: 'Unnecessary default text domain' } ],
+			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `_x('Hello World', 'context', 'default')`,
+			output: `_x('Hello World', 'context')`,
 			options: [ { allowDefault: true } ],
-			errors: [ { message: 'Unnecessary default text domain' } ],
+			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `var number = ''; _n('Singular', 'Plural', number, 'default')`,
+			output: `var number = ''; _n('Singular', 'Plural', number)`,
 			options: [ { allowDefault: true } ],
-			errors: [ { message: 'Unnecessary default text domain' } ],
+			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'default')`,
+			output: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
 			options: [ { allowDefault: true } ],
-			errors: [ { message: 'Unnecessary default text domain' } ],
+			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 	],
 } );
diff --git a/packages/eslint-plugin/rules/valid-text-domain.js b/packages/eslint-plugin/rules/valid-text-domain.js
index bf3ad6d8ad0b61..163a3f641fe0da 100644
--- a/packages/eslint-plugin/rules/valid-text-domain.js
+++ b/packages/eslint-plugin/rules/valid-text-domain.js
@@ -86,11 +86,21 @@ module.exports = {
 						items: {
 							type: 'string',
 						},
+						uniqueItems: true,
 					},
 				},
 				additionalProperties: false,
 			},
 		],
+		messages: {
+			invalidValue: "Invalid text domain '{{ textDomain }}'",
+			invalidType: 'Text domain is not a string literal',
+			unnecessaryDefault: 'Unnecessary default text domain',
+			missing: 'Missing text domain',
+			useAllowedValue:
+				'Use one of the whitelisted text domains: {{ textDomains }}',
+		},
+		fixable: 'code',
 	},
 	create( context ) {
 		const options = context.options[ 0 ] || {};
@@ -112,27 +122,71 @@ module.exports = {
 				switch ( status ) {
 					case STATUS.MISSING:
 						if ( ! allowDefault ) {
-							context.report( node, 'Missing text domain' );
+							const lastArg = args[ args.length - 1 ];
+
+							context.report( {
+								node,
+								messageId: 'missing',
+								fix:
+									allowedTextDomains.length === 1
+										? ( fixer ) => {
+												return fixer.insertTextAfter(
+													lastArg,
+													`, '${ allowedTextDomains[ 0 ] }'`
+												);
+										  }
+										: null,
+							} );
 						}
 						break;
 					case STATUS.INVALID_TYPE:
-						context.report(
+						context.report( {
 							node,
-							'Text domain is not a string literal'
-						);
+							messageId: 'invalidType',
+						} );
 						break;
 					case STATUS.INVALID_VALUE:
-						const { value } = getTextDomain( callee.name, args );
+						const textDomain = getTextDomain( callee.name, args );
+						const { value, range, parent } = textDomain;
+
+						const previousArg = [ ...parent.arguments ] // avoids reverse() modifying the AST.
+							.reverse()
+							.find( ( arg ) => arg.range[ 1 ] < range[ 0 ] );
 
 						if ( 'default' === value && allowDefault ) {
-							context.report(
+							context.report( {
 								node,
-								'Unnecessary default text domain'
-							);
+								messageId: 'unnecessaryDefault',
+								fix: ( fixer ) => {
+									return fixer.removeRange( [
+										previousArg.range[ 1 ],
+										range[ 1 ],
+									] );
+								},
+							} );
 							break;
 						}
 
-						context.report( node, 'Invalid text domain: ' + value );
+						context.report( {
+							node,
+							messageId: 'invalidValue',
+							data: {
+								textDomain: value,
+							},
+							fix:
+								allowedTextDomains.length === 1
+									? ( fixer ) => {
+											return fixer.replaceTextRange(
+												// account for quotes.
+												[
+													range[ 0 ] + 1,
+													range[ 1 ] - 1,
+												],
+												allowedTextDomains[ 0 ]
+											);
+									  }
+									: null,
+						} );
 						break;
 					default:
 						break;

From 6beedd1eb3e28d7b1b0d48e68e50a7ce3501f3be Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Sat, 29 Feb 2020 13:29:04 +0100
Subject: [PATCH 04/41] Add new no-missing-translator-comments rule

---
 packages/eslint-plugin/configs/custom.js      |  1 +
 .../no-missing-translator-comments.js         | 59 ++++++++++++
 .../rules/no-missing-translator-comments.js   | 90 +++++++++++++++++++
 packages/eslint-plugin/rules/valid-sprintf.js | 38 +-------
 .../eslint-plugin/rules/valid-text-domain.js  |  7 +-
 packages/eslint-plugin/util/index.js          | 46 ++++++++++
 6 files changed, 203 insertions(+), 38 deletions(-)
 create mode 100644 packages/eslint-plugin/rules/__tests__/no-missing-translator-comments.js
 create mode 100644 packages/eslint-plugin/rules/no-missing-translator-comments.js
 create mode 100644 packages/eslint-plugin/util/index.js

diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index 9fce23934c8d3e..807ca56dcd6417 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -5,6 +5,7 @@ module.exports = {
 		'@wordpress/valid-sprintf': 'error',
 		'@wordpress/no-base-control-with-label-without-id': 'error',
 		'@wordpress/no-unguarded-get-range-at': 'error',
+		'@wordpress/no-missing-translator-comments': 'error',
 		'@wordpress/valid-text-domain': 'error',
 		'no-restricted-syntax': [
 			'error',
diff --git a/packages/eslint-plugin/rules/__tests__/no-missing-translator-comments.js b/packages/eslint-plugin/rules/__tests__/no-missing-translator-comments.js
new file mode 100644
index 00000000000000..e82d6aaf45bf35
--- /dev/null
+++ b/packages/eslint-plugin/rules/__tests__/no-missing-translator-comments.js
@@ -0,0 +1,59 @@
+/**
+ * External dependencies
+ */
+import { RuleTester } from 'eslint';
+
+/**
+ * Internal dependencies
+ */
+import rule from '../no-missing-translator-comments';
+
+const ruleTester = new RuleTester( {
+	parserOptions: {
+		ecmaVersion: 6,
+	},
+} );
+
+ruleTester.run( 'no-missing-translator-comments', rule, {
+	valid: [
+		{
+			code: `
+var color = '';
+// translators: %s: Color
+sprintf( __( 'Color: %s' ), color );`,
+		},
+		{
+			code: `
+var address = '';
+sprintf(
+	// translators: %s: Address.
+	__( 'Address: %s' ),
+	address
+);`,
+		},
+	],
+	invalid: [
+		{
+			code: `
+var color = '';
+sprintf( __( 'Color: %s' ), color );`,
+			errors: [ { messageId: 'missing' } ],
+		},
+		{
+			code: `
+var address = '';
+sprintf(
+	__( 'Address: %s' ),
+	address
+);`,
+			errors: [ { messageId: 'missing' } ],
+		},
+		{
+			code: `
+// translators: %s: Name
+var name = '';
+sprintf( __( 'Name: %s' ), name );`,
+			errors: [ { messageId: 'missing' } ],
+		},
+	],
+} );
diff --git a/packages/eslint-plugin/rules/no-missing-translator-comments.js b/packages/eslint-plugin/rules/no-missing-translator-comments.js
new file mode 100644
index 00000000000000..271466943cd612
--- /dev/null
+++ b/packages/eslint-plugin/rules/no-missing-translator-comments.js
@@ -0,0 +1,90 @@
+/**
+ * Internal dependencies
+ */
+const {
+	TRANSLATION_FUNCTIONS,
+	REGEXP_PLACEHOLDER,
+	getTranslateStrings,
+} = require( '../util' );
+
+module.exports = {
+	meta: {
+		type: 'problem',
+		messages: {
+			missing:
+				'Translation function is missing preceding translator comment',
+		},
+	},
+	create( context ) {
+		return {
+			CallExpression( node ) {
+				const {
+					callee,
+					loc: {
+						start: { line: currentLine },
+					},
+					parent,
+					arguments: args,
+				} = node;
+				if ( ! TRANSLATION_FUNCTIONS.includes( callee.name ) ) {
+					return;
+				}
+
+				const candidates = getTranslateStrings( callee.name, args );
+
+				if ( ! candidates ) {
+					return;
+				}
+
+				let hasPlaceholders = false;
+
+				for ( const candidate of candidates ) {
+					if ( candidate.match( REGEXP_PLACEHOLDER ) ) {
+						hasPlaceholders = true;
+						break;
+					}
+				}
+
+				if ( ! hasPlaceholders ) {
+					return;
+				}
+
+				const comments = [];
+
+				comments.push( ...context.getCommentsBefore( node ) );
+
+				let parentNode = parent;
+
+				while (
+					parentNode &&
+					Math.abs( parentNode.loc.start.line - currentLine ) <= 1
+				) {
+					comments.push( ...context.getCommentsBefore( parentNode ) );
+					parentNode = parentNode.parent;
+				}
+
+				for ( const comment of comments.filter( Boolean ) ) {
+					const {
+						value: commentText,
+						loc: {
+							start: { line: commentLine },
+						},
+					} = comment;
+
+					if ( Math.abs( commentLine - currentLine ) > 1 ) {
+						continue;
+					}
+
+					if ( commentText.trim().match( /translators: /i ) ) {
+						return;
+					}
+				}
+
+				context.report( {
+					node,
+					messageId: 'missing',
+				} );
+			},
+		};
+	},
+};
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index 73101d12b7d676..c95be7fedfa162 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -1,41 +1,7 @@
 /**
- * Regular expression matching the presence of a printf format string
- * placeholder. This naive pattern which does not validate the format.
- *
- * @type {RegExp}
+ * Internal dependencies
  */
-const REGEXP_PLACEHOLDER = /%[^%]/g;
-
-/**
- * Given a function name and array of argument Node values, returns all
- * possible string results from the corresponding translate function, or
- * undefined if the function is not a translate function.
- *
- * @param {string}        functionName Function name.
- * @param {espree.Node[]} args         Espree argument Node objects.
- *
- * @return {?Array<string>} All possible translate function string results.
- */
-function getTranslateStrings( functionName, args ) {
-	switch ( functionName ) {
-		case '__':
-		case '_x':
-			args = args.slice( 0, 1 );
-			break;
-
-		case '_n':
-		case '_nx':
-			args = args.slice( 0, 2 );
-			break;
-
-		default:
-			return;
-	}
-
-	return args
-		.filter( ( arg ) => arg.type === 'Literal' )
-		.map( ( arg ) => arg.value );
-}
+const { REGEXP_PLACEHOLDER, getTranslateStrings } = require( '../util' );
 
 module.exports = {
 	meta: {
diff --git a/packages/eslint-plugin/rules/valid-text-domain.js b/packages/eslint-plugin/rules/valid-text-domain.js
index 163a3f641fe0da..d71bd446b918d2 100644
--- a/packages/eslint-plugin/rules/valid-text-domain.js
+++ b/packages/eslint-plugin/rules/valid-text-domain.js
@@ -1,3 +1,8 @@
+/**
+ * Internal dependencies
+ */
+const { TRANSLATION_FUNCTIONS } = require( '../util' );
+
 const STATUS = {
 	MISSING: 'missing',
 	INVALID_VALUE: 'invalid-domain',
@@ -68,8 +73,6 @@ function getTextDomain( functionName, args ) {
 	}
 }
 
-const TRANSLATION_FUNCTIONS = [ '__', '_x', '_n', '_nx' ];
-
 module.exports = {
 	meta: {
 		type: 'problem',
diff --git a/packages/eslint-plugin/util/index.js b/packages/eslint-plugin/util/index.js
new file mode 100644
index 00000000000000..ee74b4b27087c3
--- /dev/null
+++ b/packages/eslint-plugin/util/index.js
@@ -0,0 +1,46 @@
+const TRANSLATION_FUNCTIONS = [ '__', '_x', '_n', '_nx' ];
+
+/**
+ * Regular expression matching the presence of a printf format string
+ * placeholder. This naive pattern which does not validate the format.
+ *
+ * @type {RegExp}
+ */
+const REGEXP_PLACEHOLDER = /%[^%]/g;
+
+/**
+ * Given a function name and array of argument Node values, returns all
+ * possible string results from the corresponding translate function, or
+ * undefined if the function is not a translate function.
+ *
+ * @param {string}        functionName Function name.
+ * @param {espree.Node[]} args         Espree argument Node objects.
+ *
+ * @return {?Array<string>} All possible translate function string results.
+ */
+function getTranslateStrings( functionName, args ) {
+	switch ( functionName ) {
+		case '__':
+		case '_x':
+			args = args.slice( 0, 1 );
+			break;
+
+		case '_n':
+		case '_nx':
+			args = args.slice( 0, 2 );
+			break;
+
+		default:
+			return;
+	}
+
+	return args
+		.filter( ( arg ) => arg.type === 'Literal' )
+		.map( ( arg ) => arg.value );
+}
+
+module.exports = {
+	TRANSLATION_FUNCTIONS,
+	REGEXP_PLACEHOLDER,
+	getTranslateStrings,
+};

From d366d590c735a702a44652979fece960cd4153d9 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Sat, 29 Feb 2020 13:53:24 +0100
Subject: [PATCH 05/41] Enforce `@wordpress/no-missing-translator-comments`
 rule for Gutenberg code base

---
 .eslintrc.js                                         |  1 +
 .../src/components/block-ratings/index.js            | 10 ++++------
 .../src/components/block-ratings/stars.js            |  8 +++++++-
 .../downloadable-block-author-info/index.js          | 11 ++++++++---
 .../components/downloadable-block-header/index.js    |  7 +++++--
 .../src/components/downloadable-block-info/index.js  |  1 +
 .../components/downloadable-blocks-panel/index.js    |  1 +
 .../block-editor/src/components/block-mover/index.js |  4 ++--
 .../src/components/block-mover/mover-description.js  |  2 +-
 .../src/components/block-patterns/index.js           |  6 +++++-
 .../src/components/block-switcher/index.js           |  1 +
 .../src/components/button-block-appender/index.js    |  2 +-
 .../block-editor/src/components/inserter/index.js    |  4 ++--
 .../block-editor/src/components/inserter/menu.js     |  3 ++-
 .../src/components/link-control/index.js             |  6 +++++-
 .../components/link-control/search-create-button.js  |  1 +
 .../components/multi-selection-inspector/index.js    | 11 +++++++----
 .../src/components/responsive-block-control/index.js | 12 +++++++-----
 .../src/components/responsive-block-control/label.js |  1 +
 .../block-editor/src/components/url-input/index.js   |  1 +
 packages/block-editor/src/store/effects.js           |  2 +-
 packages/block-library/src/embed/embed-preview.js    |  6 +++---
 packages/block-library/src/gallery/gallery.js        |  2 +-
 packages/block-library/src/gallery/gallery.native.js |  2 +-
 packages/block-library/src/image/edit.js             |  1 +
 packages/block-library/src/missing/edit.js           |  2 ++
 packages/block-library/src/missing/edit.native.js    |  7 ++++---
 packages/block-library/src/post-author/edit.js       |  8 +++++++-
 packages/block-library/src/social-link/edit.js       |  6 +++++-
 packages/block-library/src/video/edit.js             |  1 +
 packages/components/src/autocomplete/index.js        |  1 +
 packages/components/src/form-token-field/index.js    |  1 +
 packages/components/src/guide/page-control.js        |  2 +-
 .../src/mobile/bottom-sheet/range-cell.native.js     |  1 +
 .../src/components/manage-blocks-modal/manager.js    |  5 +++--
 .../src/components/post-last-revision/index.js       |  1 +
 .../post-publish-panel/maybe-post-format-panel.js    |  1 +
 .../components/post-taxonomies/flat-term-selector.js |  3 +++
 .../post-taxonomies/hierarchical-term-selector.js    |  2 ++
 packages/i18n/src/test/index.js                      |  2 ++
 .../server-side-render/src/server-side-render.js     |  2 +-
 41 files changed, 106 insertions(+), 45 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index 9c9e18fcf7ec7f..1720803321df52 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -48,6 +48,7 @@ module.exports = {
 				allowDefault: true,
 			},
 		],
+		'@wordpress/no-missing-translator-comments': 'error',
 		'no-restricted-syntax': [
 			'error',
 			// NOTE: We can't include the forward slash in our regex or
diff --git a/packages/block-directory/src/components/block-ratings/index.js b/packages/block-directory/src/components/block-ratings/index.js
index c55cace300028c..c04f6c0fd7c5cd 100644
--- a/packages/block-directory/src/components/block-ratings/index.js
+++ b/packages/block-directory/src/components/block-ratings/index.js
@@ -13,13 +13,11 @@ export const BlockRatings = ( { rating, ratingCount } ) => (
 		<Stars rating={ rating } />
 		<span
 			className="block-directory-block-ratings__rating-count"
-			aria-label={
+			aria-label={ sprintf(
 				// translators: %d: number of ratings (number).
-				sprintf(
-					_n( '%d total rating', '%d total ratings', ratingCount ),
-					ratingCount
-				)
-			}
+				_n( '%d total rating', '%d total ratings', ratingCount ),
+				ratingCount
+			) }
 		>
 			({ ratingCount })
 		</span>
diff --git a/packages/block-directory/src/components/block-ratings/stars.js b/packages/block-directory/src/components/block-ratings/stars.js
index 843fca4385ce2a..a2d13b0aacb51b 100644
--- a/packages/block-directory/src/components/block-ratings/stars.js
+++ b/packages/block-directory/src/components/block-ratings/stars.js
@@ -17,7 +17,13 @@ function Stars( { rating } ) {
 	const emptyStarCount = 5 - ( fullStarCount + halfStarCount );
 
 	return (
-		<div aria-label={ sprintf( __( '%s out of 5 stars' ), stars ) }>
+		<div
+			aria-label={ sprintf(
+				/* translators: %s: number of stars. */
+				__( '%s out of 5 stars' ),
+				stars
+			) }
+		>
 			{ times( fullStarCount, ( i ) => (
 				<Icon
 					key={ `full_stars_${ i }` }
diff --git a/packages/block-directory/src/components/downloadable-block-author-info/index.js b/packages/block-directory/src/components/downloadable-block-author-info/index.js
index 5c27144430e109..d848a6dc8fd310 100644
--- a/packages/block-directory/src/components/downloadable-block-author-info/index.js
+++ b/packages/block-directory/src/components/downloadable-block-author-info/index.js
@@ -12,13 +12,18 @@ function DownloadableBlockAuthorInfo( {
 	return (
 		<Fragment>
 			<span className="block-directory-downloadable-block-author-info__content-author">
-				{ sprintf( __( 'Authored by %s' ), author ) }
+				{ sprintf(
+					/* translators: %s: author name. */
+					__( 'Authored by %s' ),
+					author
+				) }
 			</span>
 			<span className="block-directory-downloadable-block-author-info__content">
 				{ sprintf(
+					/* translators: 1: number of blocks. 2: average rating. */
 					_n(
-						'This author has %d block, with an average rating of %d.',
-						'This author has %d blocks, with an average rating of %d.',
+						'This author has %1$d block, with an average rating of %2$d.',
+						'This author has %1$d blocks, with an average rating of %2$d.',
 						authorBlockCount
 					),
 					authorBlockCount,
diff --git a/packages/block-directory/src/components/downloadable-block-header/index.js b/packages/block-directory/src/components/downloadable-block-header/index.js
index 78ab2962e60ce8..dc835ed3295399 100644
--- a/packages/block-directory/src/components/downloadable-block-header/index.js
+++ b/packages/block-directory/src/components/downloadable-block-header/index.js
@@ -20,10 +20,13 @@ function DownloadableBlockHeader( {
 	return (
 		<div className="block-directory-downloadable-block-header__row">
 			{ icon.match( /\.(jpeg|jpg|gif|png)(?:\?.*)?$/ ) !== null ? (
-				// translators: %s: Name of the plugin e.g: "Akismet".
 				<img
 					src={ icon }
-					alt={ sprintf( __( '%s block icon' ), title ) }
+					alt={ sprintf(
+						// translators: %s: Name of the plugin e.g: "Akismet".
+						__( '%s block icon' ),
+						title
+					) }
 				/>
 			) : (
 				<span>
diff --git a/packages/block-directory/src/components/downloadable-block-info/index.js b/packages/block-directory/src/components/downloadable-block-info/index.js
index b6865a5c9197e9..4c65e639554a9d 100644
--- a/packages/block-directory/src/components/downloadable-block-info/index.js
+++ b/packages/block-directory/src/components/downloadable-block-info/index.js
@@ -19,6 +19,7 @@ function DownloadableBlockInfo( {
 				<div className="block-directory-downloadable-block-info__column">
 					<Icon icon={ chartLine }></Icon>
 					{ sprintf(
+						/* translators: %s: number of active installations. */
 						_n(
 							'%d active installation',
 							'%d active installations',
diff --git a/packages/block-directory/src/components/downloadable-blocks-panel/index.js b/packages/block-directory/src/components/downloadable-blocks-panel/index.js
index 29d7cbb29ecac0..08d346d6774a58 100644
--- a/packages/block-directory/src/components/downloadable-blocks-panel/index.js
+++ b/packages/block-directory/src/components/downloadable-blocks-panel/index.js
@@ -55,6 +55,7 @@ function DownloadableBlocksPanel( {
 	}
 
 	const resultsFoundMessage = sprintf(
+		/* translators: %s: number of available blocks. */
 		_n(
 			'No blocks found in your library. We did find %d block available for download.',
 			'No blocks found in your library. We did find %d blocks available for download.',
diff --git a/packages/block-editor/src/components/block-mover/index.js b/packages/block-editor/src/components/block-mover/index.js
index 46889a13537976..d33fe6b2e38ff9 100644
--- a/packages/block-editor/src/components/block-mover/index.js
+++ b/packages/block-editor/src/components/block-mover/index.js
@@ -118,8 +118,8 @@ export class BlockMover extends Component {
 								className="block-editor-block-mover__control block-editor-block-mover__control-up"
 								onClick={ isFirst ? null : onMoveUp }
 								icon={ getArrowIcon( 'up' ) }
-								// translators: %s: Horizontal direction of block movement ( left, right )
 								label={ sprintf(
+									// translators: %s: Horizontal direction of block movement ( left, right )
 									__( 'Move %s' ),
 									getMovementDirection( 'up' )
 								) }
@@ -133,8 +133,8 @@ export class BlockMover extends Component {
 								className="block-editor-block-mover__control block-editor-block-mover__control-down"
 								onClick={ isLast ? null : onMoveDown }
 								icon={ getArrowIcon( 'down' ) }
-								// translators: %s: Horizontal direction of block movement ( left, right )
 								label={ sprintf(
+									// translators: %s: Horizontal direction of block movement ( left, right )
 									__( 'Move %s' ),
 									getMovementDirection( 'down' )
 								) }
diff --git a/packages/block-editor/src/components/block-mover/mover-description.js b/packages/block-editor/src/components/block-mover/mover-description.js
index 88e67e5ff4a715..fbdc10cb256138 100644
--- a/packages/block-editor/src/components/block-mover/mover-description.js
+++ b/packages/block-editor/src/components/block-mover/mover-description.js
@@ -58,8 +58,8 @@ export function getBlockMoverDescription(
 	}
 
 	if ( isFirst && isLast ) {
-		// translators: %s: Type of block (i.e. Text, Image etc)
 		return sprintf(
+			// translators: %s: Type of block (i.e. Text, Image etc)
 			__( 'Block %s is the only block, and cannot be moved' ),
 			type
 		);
diff --git a/packages/block-editor/src/components/block-patterns/index.js b/packages/block-editor/src/components/block-patterns/index.js
index c265548cf6e82d..c52daf83b6dfc5 100644
--- a/packages/block-editor/src/components/block-patterns/index.js
+++ b/packages/block-editor/src/components/block-patterns/index.js
@@ -56,7 +56,11 @@ function BlockPatterns( { patterns } ) {
 			false
 		);
 		createSuccessNotice(
-			sprintf( __( 'Pattern "%s" inserted' ), pattern.title ),
+			sprintf(
+				/* translators: %s: block pattern title. */
+				__( 'Pattern "%s" inserted' ),
+				pattern.title
+			),
 			{
 				type: 'snackbar',
 			}
diff --git a/packages/block-editor/src/components/block-switcher/index.js b/packages/block-editor/src/components/block-switcher/index.js
index e74cfc9ad94b76..95794f8f935731 100644
--- a/packages/block-editor/src/components/block-switcher/index.js
+++ b/packages/block-editor/src/components/block-switcher/index.js
@@ -114,6 +114,7 @@ export class BlockSwitcher extends Component {
 						1 === blocks.length
 							? __( 'Change block type or style' )
 							: sprintf(
+									/* translators: %s: number of blocks. */
 									_n(
 										'Change type of %d block',
 										'Change type of %d blocks',
diff --git a/packages/block-editor/src/components/button-block-appender/index.js b/packages/block-editor/src/components/button-block-appender/index.js
index 3991b5bb530f35..df2a3cef0b2b4a 100644
--- a/packages/block-editor/src/components/button-block-appender/index.js
+++ b/packages/block-editor/src/components/button-block-appender/index.js
@@ -33,8 +33,8 @@ function ButtonBlockAppender( {
 			} ) => {
 				let label;
 				if ( hasSingleBlockType ) {
-					// translators: %s: the name of the block when there is only one
 					label = sprintf(
+						// translators: %s: the name of the block when there is only one
 						_x( 'Add %s', 'directly add the only allowed block' ),
 						blockTitle
 					);
diff --git a/packages/block-editor/src/components/inserter/index.js b/packages/block-editor/src/components/inserter/index.js
index 2c434fae89edc0..11bc69d23ac5c0 100644
--- a/packages/block-editor/src/components/inserter/index.js
+++ b/packages/block-editor/src/components/inserter/index.js
@@ -29,8 +29,8 @@ const defaultRenderToggle = ( {
 } ) => {
 	let label;
 	if ( hasSingleBlockType ) {
-		// translators: %s: the name of the block when there is only one
 		label = sprintf(
+			// translators: %s: the name of the block when there is only one
 			_x( 'Add %s', 'directly add the only allowed block' ),
 			blockTitle
 		);
@@ -236,8 +236,8 @@ export default compose( [
 				);
 
 				if ( ! selectBlockOnInsert ) {
-					// translators: %s: the name of the block that has been added
 					const message = sprintf(
+						// translators: %s: the name of the block that has been added
 						__( '%s block added' ),
 						allowedBlockType.title
 					);
diff --git a/packages/block-editor/src/components/inserter/menu.js b/packages/block-editor/src/components/inserter/menu.js
index 72f071bf29b4cf..ec42eb8d8ada6d 100644
--- a/packages/block-editor/src/components/inserter/menu.js
+++ b/packages/block-editor/src/components/inserter/menu.js
@@ -256,6 +256,7 @@ export class InserterMenu extends Component {
 		);
 
 		const resultsFoundMessage = sprintf(
+			/* translators: %d: number of results. */
 			_n( '%d result found.', '%d results found.', resultCount ),
 			resultCount
 		);
@@ -666,8 +667,8 @@ export default compose(
 					);
 
 					if ( ! selectBlockOnInsert ) {
-						// translators: %s: the name of the block that has been added
 						const message = sprintf(
+							// translators: %s: the name of the block that has been added
 							__( '%s block added' ),
 							title
 						);
diff --git a/packages/block-editor/src/components/link-control/index.js b/packages/block-editor/src/components/link-control/index.js
index 1a47f3283550f8..2e70b76811288a 100644
--- a/packages/block-editor/src/components/link-control/index.js
+++ b/packages/block-editor/src/components/link-control/index.js
@@ -434,7 +434,11 @@ function LinkControl( {
 		const searchResultsLabelId = `block-editor-link-control-search-results-label-${ instanceId }`;
 		const labelText = isInitialSuggestions
 			? __( 'Recently updated' )
-			: sprintf( __( 'Search results for "%s"' ), inputValue );
+			: sprintf(
+					/* translators: %s: search term. */
+					__( 'Search results for "%s"' ),
+					inputValue
+			  );
 
 		// VisuallyHidden rightly doesn't accept custom classNames
 		// so we conditionally render it as a wrapper to visually hide the label
diff --git a/packages/block-editor/src/components/link-control/search-create-button.js b/packages/block-editor/src/components/link-control/search-create-button.js
index 0b9f48394107ce..b4e66bd762c990 100644
--- a/packages/block-editor/src/components/link-control/search-create-button.js
+++ b/packages/block-editor/src/components/link-control/search-create-button.js
@@ -40,6 +40,7 @@ export const LinkControlSearchCreate = ( {
 				<span className="block-editor-link-control__search-item-title">
 					{ __experimentalCreateInterpolateElement(
 						sprintf(
+							/* translators: %s: search term. */
 							__( 'New page: <mark>%s</mark>' ),
 							searchTerm
 						),
diff --git a/packages/block-editor/src/components/multi-selection-inspector/index.js b/packages/block-editor/src/components/multi-selection-inspector/index.js
index d501e1247d71ac..3109834295d559 100644
--- a/packages/block-editor/src/components/multi-selection-inspector/index.js
+++ b/packages/block-editor/src/components/multi-selection-inspector/index.js
@@ -27,15 +27,18 @@ function MultiSelectionInspector( { blocks } ) {
 			/>
 			<div className="block-editor-multi-selection-inspector__card-content">
 				<div className="block-editor-multi-selection-inspector__card-title">
-					{ /* translators: %d: number of blocks */
-					sprintf(
+					{ sprintf(
+						/* translators: %d: number of blocks */
 						_n( '%d block', '%d blocks', blocks.length ),
 						blocks.length
 					) }
 				</div>
 				<div className="block-editor-multi-selection-inspector__card-description">
-					{ /* translators: %d: number of words */
-					sprintf( _n( '%d word', '%d words', words ), words ) }
+					{ sprintf(
+						/* translators: %d: number of words */
+						_n( '%d word', '%d words', words ),
+						words
+					) }
 				</div>
 			</div>
 		</div>
diff --git a/packages/block-editor/src/components/responsive-block-control/index.js b/packages/block-editor/src/components/responsive-block-control/index.js
index 8a3d3eef08145e..53854980a1070d 100644
--- a/packages/block-editor/src/components/responsive-block-control/index.js
+++ b/packages/block-editor/src/components/responsive-block-control/index.js
@@ -26,9 +26,8 @@ function ResponsiveBlockControl( props ) {
 		isResponsive = false,
 		defaultLabel = {
 			id: 'all',
-			label: __(
-				'All'
-			) /* translators: 'Label. Used to signify a layout property (eg: margin, padding) will apply uniformly to all screensizes.' */,
+			/* translators: 'Label. Used to signify a layout property (eg: margin, padding) will apply uniformly to all screensizes.' */
+			label: __( 'All' ),
 		},
 		viewports = [
 			{
@@ -50,10 +49,13 @@ function ResponsiveBlockControl( props ) {
 		return null;
 	}
 
-	/* translators: 'Toggle control label. Should the property be the same across all screen sizes or unique per screen size.'. %s property value for the control (eg: margin, padding...etc) */
 	const toggleControlLabel =
 		toggleLabel ||
-		sprintf( __( 'Use the same %s on all screensizes.' ), property );
+		sprintf(
+			/* translators: 'Toggle control label. Should the property be the same across all screen sizes or unique per screen size.'. %s property value for the control (eg: margin, padding...etc) */
+			__( 'Use the same %s on all screensizes.' ),
+			property
+		);
 
 	/* translators: 'Help text for the responsive mode toggle control.' */
 	const toggleHelpText = __(
diff --git a/packages/block-editor/src/components/responsive-block-control/label.js b/packages/block-editor/src/components/responsive-block-control/label.js
index dba8fe9e310c74..6a4ce0c4c3a180 100644
--- a/packages/block-editor/src/components/responsive-block-control/label.js
+++ b/packages/block-editor/src/components/responsive-block-control/label.js
@@ -14,6 +14,7 @@ export default function ResponsiveBlockControlLabel( {
 	const accessibleLabel =
 		desc ||
 		sprintf(
+			/* translators: 1: property name. 2: viewport name. */
 			_x(
 				'Controls the %1$s property for %2$s viewports.',
 				'Text labelling a interface as controlling a given layout property (eg: margin) for a given screen size.'
diff --git a/packages/block-editor/src/components/url-input/index.js b/packages/block-editor/src/components/url-input/index.js
index f9c6eeb4b94e66..68f495e29adf89 100644
--- a/packages/block-editor/src/components/url-input/index.js
+++ b/packages/block-editor/src/components/url-input/index.js
@@ -178,6 +178,7 @@ class URLInput extends Component {
 				if ( !! suggestions.length ) {
 					this.props.debouncedSpeak(
 						sprintf(
+							/* translators: %s: number of results. */
 							_n(
 								'%d result found, use up and down arrow keys to navigate.',
 								'%d results found, use up and down arrow keys to navigate.',
diff --git a/packages/block-editor/src/store/effects.js b/packages/block-editor/src/store/effects.js
index b01e37b1e76dd8..5142020ffffa1e 100644
--- a/packages/block-editor/src/store/effects.js
+++ b/packages/block-editor/src/store/effects.js
@@ -223,9 +223,9 @@ export default {
 	MULTI_SELECT: ( action, { getState } ) => {
 		const blockCount = getSelectedBlockCount( getState() );
 
-		/* translators: %s: number of selected blocks */
 		speak(
 			sprintf(
+				/* translators: %s: number of selected blocks */
 				_n( '%s block selected.', '%s blocks selected.', blockCount ),
 				blockCount
 			),
diff --git a/packages/block-library/src/embed/embed-preview.js b/packages/block-library/src/embed/embed-preview.js
index db139e5292b7bd..c7dba11188d1f6 100644
--- a/packages/block-library/src/embed/embed-preview.js
+++ b/packages/block-library/src/embed/embed-preview.js
@@ -74,8 +74,8 @@ class EmbedPreview extends Component {
 			.splice( parsedHost.length - 2, parsedHost.length - 1 )
 			.join( '.' );
 		const cannotPreview = includes( HOSTS_NO_PREVIEWS, parsedHostBaseUrl );
-		// translators: %s: host providing embed content e.g: www.youtube.com
 		const iframeTitle = sprintf(
+			// translators: %s: host providing embed content e.g: www.youtube.com
 			__( 'Embedded content from %s' ),
 			parsedHostBaseUrl
 		);
@@ -126,8 +126,8 @@ class EmbedPreview extends Component {
 							<a href={ url }>{ url }</a>
 						</p>
 						<p className="components-placeholder__error">
-							{ /* translators: %s: host providing embed content e.g: www.youtube.com */
-							sprintf(
+							{ sprintf(
+								/* translators: %s: host providing embed content e.g: www.youtube.com */
 								__(
 									"Embedded content from %s can't be previewed in the editor."
 								),
diff --git a/packages/block-library/src/gallery/gallery.js b/packages/block-library/src/gallery/gallery.js
index 94eb4b16f6d93e..7b7f2c12a7a778 100644
--- a/packages/block-library/src/gallery/gallery.js
+++ b/packages/block-library/src/gallery/gallery.js
@@ -54,8 +54,8 @@ export const Gallery = ( props ) => {
 		>
 			<ul className="blocks-gallery-grid">
 				{ images.map( ( img, index ) => {
-					/* translators: %1$d is the order number of the image, %2$d is the total number of images. */
 					const ariaLabel = sprintf(
+						/* translators: 1: the order number of the image. 2: the total number of images. */
 						__( 'image %1$d of %2$d in gallery' ),
 						index + 1,
 						images.length
diff --git a/packages/block-library/src/gallery/gallery.native.js b/packages/block-library/src/gallery/gallery.native.js
index 27dc15df623e9f..f6c2c3abebce61 100644
--- a/packages/block-library/src/gallery/gallery.native.js
+++ b/packages/block-library/src/gallery/gallery.native.js
@@ -88,8 +88,8 @@ export const Gallery = ( props ) => {
 				}
 			>
 				{ images.map( ( img, index ) => {
-					/* translators: %1$d is the order number of the image, %2$d is the total number of images. */
 					const ariaLabel = sprintf(
+						/* translators: 1: the order number of the image. 2: the total number of images. */
 						__( 'image %1$d of %2$d in gallery' ),
 						index + 1,
 						images.length
diff --git a/packages/block-library/src/image/edit.js b/packages/block-library/src/image/edit.js
index e844c14227e9e5..f9167c4f4fb964 100644
--- a/packages/block-library/src/image/edit.js
+++ b/packages/block-library/src/image/edit.js
@@ -518,6 +518,7 @@ export class ImageEdit extends Component {
 								defaultedAlt = alt;
 							} else if ( filename ) {
 								defaultedAlt = sprintf(
+									/* translators: %s: file name */
 									__(
 										'This image has an empty alt attribute; its file name is %s'
 									),
diff --git a/packages/block-library/src/missing/edit.js b/packages/block-library/src/missing/edit.js
index b9caa890a5b950..3f1d39f11edc01 100644
--- a/packages/block-library/src/missing/edit.js
+++ b/packages/block-library/src/missing/edit.js
@@ -17,6 +17,7 @@ function MissingBlockWarning( { attributes, convertToHTML } ) {
 	let messageHTML;
 	if ( hasContent && hasHTMLBlock ) {
 		messageHTML = sprintf(
+			/* translators: %s: block name */
 			__(
 				'Your site doesn’t include support for the "%s" block. You can leave this block intact, convert its content to a Custom HTML block, or remove it entirely.'
 			),
@@ -29,6 +30,7 @@ function MissingBlockWarning( { attributes, convertToHTML } ) {
 		);
 	} else {
 		messageHTML = sprintf(
+			/* translators: %s: block name */
 			__(
 				'Your site doesn’t include support for the "%s" block. You can leave this block intact or remove it entirely.'
 			),
diff --git a/packages/block-library/src/missing/edit.native.js b/packages/block-library/src/missing/edit.native.js
index f77e29ce9c94a3..f4072ca42000db 100644
--- a/packages/block-library/src/missing/edit.native.js
+++ b/packages/block-library/src/missing/edit.native.js
@@ -75,11 +75,12 @@ export class UnsupportedBlockEdit extends Component {
 			styles.infoSheetIconDark
 		);
 
-		// translators: %s: Name of the block
 		const titleFormat =
 			Platform.OS === 'android'
-				? __( "'%s' isn't yet supported on WordPress for Android" )
-				: __( "'%s' isn't yet supported on WordPress for iOS" );
+				? // translators: %s: Name of the block
+				  __( "'%s' isn't yet supported on WordPress for Android" )
+				: // translators: %s: Name of the block
+				  __( "'%s' isn't yet supported on WordPress for iOS" );
 		const infoTitle = sprintf( titleFormat, title );
 
 		return (
diff --git a/packages/block-library/src/post-author/edit.js b/packages/block-library/src/post-author/edit.js
index 9ca968c5679f70..05ab61f3ef92a7 100644
--- a/packages/block-library/src/post-author/edit.js
+++ b/packages/block-library/src/post-author/edit.js
@@ -13,7 +13,13 @@ function PostAuthorDisplay() {
 		[ authorId ]
 	);
 	return author ? (
-		<address>{ sprintf( __( 'By %s' ), author.name ) }</address>
+		<address>
+			{ sprintf(
+				/* translators: %s: author name. */
+				__( 'By %s' ),
+				author.name
+			) }
+		</address>
 	) : null;
 }
 
diff --git a/packages/block-library/src/social-link/edit.js b/packages/block-library/src/social-link/edit.js
index 2dd4e747638c28..99ac2ac636e144 100644
--- a/packages/block-library/src/social-link/edit.js
+++ b/packages/block-library/src/social-link/edit.js
@@ -41,7 +41,11 @@ const SocialLinkEdit = ( { attributes, setAttributes, isSelected } ) => {
 		<Fragment>
 			<InspectorControls>
 				<PanelBody
-					title={ sprintf( __( '%s label' ), socialLinkName ) }
+					title={ sprintf(
+						/* translators: %s: name of the social service. */
+						__( '%s label' ),
+						socialLinkName
+					) }
 					initialOpen={ false }
 				>
 					<PanelRow>
diff --git a/packages/block-library/src/video/edit.js b/packages/block-library/src/video/edit.js
index 05e3bc0d9d3f6d..584d1072e0e152 100644
--- a/packages/block-library/src/video/edit.js
+++ b/packages/block-library/src/video/edit.js
@@ -200,6 +200,7 @@ class VideoEdit extends Component {
 								<p id={ videoPosterDescription } hidden>
 									{ this.props.attributes.poster
 										? sprintf(
+												/* translators: %s: poster image URL. */
 												__(
 													'The current poster image url is %s'
 												),
diff --git a/packages/components/src/autocomplete/index.js b/packages/components/src/autocomplete/index.js
index 6466328cabd836..8517c7bfc83e0b 100644
--- a/packages/components/src/autocomplete/index.js
+++ b/packages/components/src/autocomplete/index.js
@@ -227,6 +227,7 @@ export class Autocomplete extends Component {
 		if ( !! filteredOptions.length ) {
 			debouncedSpeak(
 				sprintf(
+					/* translators: %d: number of results. */
 					_n(
 						'%d result found, use up and down arrow keys to navigate.',
 						'%d results found, use up and down arrow keys to navigate.',
diff --git a/packages/components/src/form-token-field/index.js b/packages/components/src/form-token-field/index.js
index b2c81977a50fbc..0641a327c64734 100644
--- a/packages/components/src/form-token-field/index.js
+++ b/packages/components/src/form-token-field/index.js
@@ -514,6 +514,7 @@ class FormTokenField extends Component {
 
 			const message = hasMatchingSuggestions
 				? sprintf(
+						/* translators: %d: number of results. */
 						_n(
 							'%d result found, use up and down arrow keys to navigate.',
 							'%d results found, use up and down arrow keys to navigate.',
diff --git a/packages/components/src/guide/page-control.js b/packages/components/src/guide/page-control.js
index 837a0b31e7d5ad..1a39fdd1426c8e 100644
--- a/packages/components/src/guide/page-control.js
+++ b/packages/components/src/guide/page-control.js
@@ -37,8 +37,8 @@ export default function PageControl( {
 								isSelected={ page === currentPage }
 							/>
 						}
-						/* translators: %1$d: current page number %2$d: total number of pages */
 						aria-label={ sprintf(
+							/* translators: 1: current page number 2: total number of pages */
 							__( 'Page %1$d of %2$d' ),
 							page + 1,
 							numberOfPages
diff --git a/packages/components/src/mobile/bottom-sheet/range-cell.native.js b/packages/components/src/mobile/bottom-sheet/range-cell.native.js
index 717ab84232a097..fa361e39429424 100644
--- a/packages/components/src/mobile/bottom-sheet/range-cell.native.js
+++ b/packages/components/src/mobile/bottom-sheet/range-cell.native.js
@@ -133,6 +133,7 @@ class BottomSheetRangeCell extends Component {
 	}
 
 	announceCurrentValue( value ) {
+		/* translators: %s: current cell value. */
 		const announcement = sprintf( __( 'Current value is %s' ), value );
 		AccessibilityInfo.announceForAccessibility( announcement );
 	}
diff --git a/packages/edit-post/src/components/manage-blocks-modal/manager.js b/packages/edit-post/src/components/manage-blocks-modal/manager.js
index bdb8349be888f7..41eba0b4f97e9a 100644
--- a/packages/edit-post/src/components/manage-blocks-modal/manager.js
+++ b/packages/edit-post/src/components/manage-blocks-modal/manager.js
@@ -52,9 +52,10 @@ function BlockManager( {
 			{ !! numberOfHiddenBlocks && (
 				<div className="edit-post-manage-blocks-modal__disabled-blocks-count">
 					{ sprintf(
+						/* translators: %d: number of blocks. */
 						_n(
-							'%1$d block is disabled.',
-							'%1$d blocks are disabled.',
+							'%d block is disabled.',
+							'%d blocks are disabled.',
 							numberOfHiddenBlocks
 						),
 						numberOfHiddenBlocks
diff --git a/packages/editor/src/components/post-last-revision/index.js b/packages/editor/src/components/post-last-revision/index.js
index b24f0f3ac0c1cc..643b6c1a4700a6 100644
--- a/packages/editor/src/components/post-last-revision/index.js
+++ b/packages/editor/src/components/post-last-revision/index.js
@@ -24,6 +24,7 @@ function LastRevision( { lastRevisionId, revisionsCount } ) {
 				icon={ backup }
 			>
 				{ sprintf(
+					/* translators: %d: number of revisions */
 					_n( '%d Revision', '%d Revisions', revisionsCount ),
 					revisionsCount
 				) }
diff --git a/packages/editor/src/components/post-publish-panel/maybe-post-format-panel.js b/packages/editor/src/components/post-publish-panel/maybe-post-format-panel.js
index fede6a0a33186d..5d173e4f3c6807 100644
--- a/packages/editor/src/components/post-publish-panel/maybe-post-format-panel.js
+++ b/packages/editor/src/components/post-publish-panel/maybe-post-format-panel.js
@@ -46,6 +46,7 @@ const PostFormatPanel = ( { suggestion, onUpdatePostFormat } ) => {
 					onUpdatePostFormat={ onUpdatePostFormat }
 					suggestedPostFormat={ suggestion.id }
 					suggestionText={ sprintf(
+						/* translators: %s: post format */
 						__( 'Apply the "%1$s" format.' ),
 						suggestion.caption
 					) }
diff --git a/packages/editor/src/components/post-taxonomies/flat-term-selector.js b/packages/editor/src/components/post-taxonomies/flat-term-selector.js
index 299245a5e45aad..c6dd5af6db8530 100644
--- a/packages/editor/src/components/post-taxonomies/flat-term-selector.js
+++ b/packages/editor/src/components/post-taxonomies/flat-term-selector.js
@@ -245,14 +245,17 @@ class FlatTermSelector extends Component {
 			slug === 'post_tag' ? __( 'Tag' ) : __( 'Term' )
 		);
 		const termAddedLabel = sprintf(
+			/* translators: %s: term name. */
 			_x( '%s added', 'term' ),
 			singularName
 		);
 		const termRemovedLabel = sprintf(
+			/* translators: %s: term name. */
 			_x( '%s removed', 'term' ),
 			singularName
 		);
 		const removeTermLabel = sprintf(
+			/* translators: %s: term name. */
 			_x( 'Remove %s', 'term' ),
 			singularName
 		);
diff --git a/packages/editor/src/components/post-taxonomies/hierarchical-term-selector.js b/packages/editor/src/components/post-taxonomies/hierarchical-term-selector.js
index c8897753492f13..234b5f25e8f6ac 100644
--- a/packages/editor/src/components/post-taxonomies/hierarchical-term-selector.js
+++ b/packages/editor/src/components/post-taxonomies/hierarchical-term-selector.js
@@ -171,6 +171,7 @@ class HierarchicalTermSelector extends Component {
 					? this.state.availableTerms
 					: [ term, ...this.state.availableTerms ];
 				const termAddedMessage = sprintf(
+					/* translators: %s: taxonomy name */
 					_x( '%s added', 'term' ),
 					get(
 						this.props.taxonomy,
@@ -319,6 +320,7 @@ class HierarchicalTermSelector extends Component {
 
 		const resultCount = getResultCount( filteredTermsTree );
 		const resultsFoundMessage = sprintf(
+			/* translators: %d: number of results */
 			_n( '%d result found.', '%d results found.', resultCount ),
 			resultCount
 		);
diff --git a/packages/i18n/src/test/index.js b/packages/i18n/src/test/index.js
index e464e7f02e03bf..c8ae40cb23f072 100644
--- a/packages/i18n/src/test/index.js
+++ b/packages/i18n/src/test/index.js
@@ -1,4 +1,5 @@
 /* eslint-disable @wordpress/valid-text-domain */
+/* eslint-disable @wordpress/no-missing-translator-comments */
 
 // Mock memoization as identity function. Inline since Jest errors on out-of-
 // scope references in a mock callback.
@@ -188,4 +189,5 @@ function setDefaultLocalData() {
 	setLocaleData( localeData, 'test_domain' );
 }
 
+/* eslint-enable @wordpress/no-missing-translator-comments */
 /* eslint-enable @wordpress/valid-text-domain */
diff --git a/packages/server-side-render/src/server-side-render.js b/packages/server-side-render/src/server-side-render.js
index 8e2f93446095d4..f428adf8f4495b 100644
--- a/packages/server-side-render/src/server-side-render.js
+++ b/packages/server-side-render/src/server-side-render.js
@@ -131,8 +131,8 @@ ServerSideRender.defaultProps = {
 		</Placeholder>
 	),
 	ErrorResponsePlaceholder: ( { response, className } ) => {
-		// translators: %s: error message describing the problem
 		const errorMessage = sprintf(
+			// translators: %s: error message describing the problem
 			__( 'Error loading block: %s' ),
 			response.errorMsg
 		);

From 6c4e07c5ce5e502af07cdbba61f52e535a426029 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Sat, 29 Feb 2020 13:57:09 +0100
Subject: [PATCH 06/41] Add docs

---
 packages/eslint-plugin/CHANGELOG.md           |  1 +
 .../rules/no-missing-translator-comments.md   | 38 +++++++++++++++++++
 2 files changed, 39 insertions(+)
 create mode 100644 packages/eslint-plugin/docs/rules/no-missing-translator-comments.md

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 7f7effb4d1b4c4..44c41ddb0bddcf 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -3,6 +3,7 @@
 ### New Features
 
 - New Rule: [`@wordpress/valid-text-domain`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/valid-text-domain.md)
+- New Rule: [`@wordpress/no-missing-translator-comments`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/no-missing-translator-comments.md)
 
 ## 4.0.0 (2020-02-10)
 
diff --git a/packages/eslint-plugin/docs/rules/no-missing-translator-comments.md b/packages/eslint-plugin/docs/rules/no-missing-translator-comments.md
new file mode 100644
index 00000000000000..e1c7b8bc1455ea
--- /dev/null
+++ b/packages/eslint-plugin/docs/rules/no-missing-translator-comments.md
@@ -0,0 +1,38 @@
+# Enforce adding translator comments (no-missing-translator-comments)
+
+If using [translation functions](https://github.com/WordPress/gutenberg/blob/master/packages/i18n/README.md#api) with placeholders in them,
+they need accompanying translator comments.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```js
+var color = '';
+sprintf( __( 'Color: %s' ), color );
+
+var address = '';
+sprintf(
+	__( 'Address: %s' ),
+	address
+);
+
+// translators: %s: Name
+var name = '';
+sprintf( __( 'Name: %s' ), name );
+```
+
+Examples of **correct** code for this rule:
+
+```js
+var color = '';
+// translators: %s: Color
+sprintf( __( 'Color: %s' ), color );
+
+var address = '';
+sprintf(
+	// translators: %s: Address.
+	__( 'Address: %s' ),
+	address,
+);
+```

From 05ee583f851e7ae7d2c75883b8fa91f3a147747a Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 11 Mar 2020 13:58:18 +0100
Subject: [PATCH 07/41] Implement feedback from code review

---
 .../__tests__/i18n-translator-comments.js     |  23 ++-
 .../eslint-plugin/rules/i18n-text-domain.js   | 191 +++++++-----------
 .../rules/i18n-translator-comments.js         |  42 ++--
 packages/eslint-plugin/rules/valid-sprintf.js |   2 +-
 .../eslint-plugin/{util => utils}/index.js    |   2 +-
 5 files changed, 116 insertions(+), 144 deletions(-)
 rename packages/eslint-plugin/{util => utils}/index.js (92%)

diff --git a/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js b/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js
index 2906056831ed56..10ef70023862fe 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js
@@ -18,13 +18,11 @@ ruleTester.run( 'i18n-translator-comments', rule, {
 	valid: [
 		{
 			code: `
-var color = '';
 // translators: %s: Color
 sprintf( __( 'Color: %s' ), color );`,
 		},
 		{
 			code: `
-var address = '';
 sprintf(
 	// translators: %s: Address.
 	__( 'Address: %s' ),
@@ -35,13 +33,11 @@ sprintf(
 	invalid: [
 		{
 			code: `
-var color = '';
 sprintf( __( 'Color: %s' ), color );`,
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `
-var address = '';
 sprintf(
 	__( 'Address: %s' ),
 	address
@@ -55,5 +51,24 @@ var name = '';
 sprintf( __( 'Name: %s' ), name );`,
 			errors: [ { messageId: 'missing' } ],
 		},
+		{
+			code: `
+// translators: %s: Surname
+console.log(
+	sprintf( __( 'Surname: %s' ), name )
+);`,
+			errors: [ { messageId: 'missing' } ],
+		},
+		{
+			code: `
+// translators: %s: Preference
+console.log(
+	sprintf(
+		__( 'Preference: %s' ),
+		preference
+	)
+);`,
+			errors: [ { messageId: 'missing' } ],
+		},
 	],
 } );
diff --git a/packages/eslint-plugin/rules/i18n-text-domain.js b/packages/eslint-plugin/rules/i18n-text-domain.js
index d71bd446b918d2..a9862ad89836d1 100644
--- a/packages/eslint-plugin/rules/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/i18n-text-domain.js
@@ -1,63 +1,15 @@
 /**
  * Internal dependencies
  */
-const { TRANSLATION_FUNCTIONS } = require( '../util' );
-
-const STATUS = {
-	MISSING: 'missing',
-	INVALID_VALUE: 'invalid-domain',
-	INVALID_TYPE: 'invalid-type',
-	VALID: 'valid',
-};
-
-function validateTextDomain( functionName, args, allowedTextDomains ) {
-	switch ( functionName ) {
-		case '__':
-			if ( args.length < 2 ) {
-				return STATUS.MISSING;
-			}
-			break;
-		case '_x':
-			if ( args.length < 3 ) {
-				return STATUS.MISSING;
-			}
-
-			break;
-		case '_n':
-			if ( args.length < 4 ) {
-				return STATUS.MISSING;
-			}
-
-			break;
-		case '_nx':
-			if ( args.length < 5 ) {
-				return STATUS.MISSING;
-			}
-
-			break;
-		default:
-			break;
-	}
-
-	const textDomain = getTextDomain( functionName, args );
-
-	if ( ! textDomain ) {
-		return STATUS.INVALID_TYPE;
-	}
-
-	const { type, value } = textDomain;
-
-	if ( type !== 'Literal' ) {
-		return STATUS.INVALID_TYPE;
-	}
-
-	if ( ! allowedTextDomains.includes( value ) ) {
-		return STATUS.INVALID_VALUE;
-	}
-
-	return STATUS.VALID;
-}
+const { TRANSLATION_FUNCTIONS } = require( '../utils' );
 
+/**
+ * Returns the text domain passed to the given translation function.
+ *
+ * @param {string} functionName Translation function name.
+ * @param {Array} args Function arguments.
+ * @return {undefined|*} Text domain argument.
+ */
 function getTextDomain( functionName, args ) {
 	switch ( functionName ) {
 		case '__':
@@ -69,7 +21,7 @@ function getTextDomain( functionName, args ) {
 		case '_nx':
 			return args[ 4 ];
 		default:
-			return {};
+			return undefined;
 	}
 }
 
@@ -108,6 +60,7 @@ module.exports = {
 	create( context ) {
 		const options = context.options[ 0 ] || {};
 		const { allowDefault, allowedTextDomains = [] } = options;
+		const canFixTextDomain = allowedTextDomains.length === 1;
 
 		return {
 			CallExpression( node ) {
@@ -116,83 +69,75 @@ module.exports = {
 					return;
 				}
 
-				const status = validateTextDomain(
-					callee.name,
-					args,
-					allowedTextDomains
-				);
+				const textDomain = getTextDomain( callee.name, args );
 
-				switch ( status ) {
-					case STATUS.MISSING:
-						if ( ! allowDefault ) {
-							const lastArg = args[ args.length - 1 ];
+				if ( textDomain === undefined ) {
+					if ( ! allowDefault ) {
+						const lastArg = args[ args.length - 1 ];
+						const addMissingTextDomain = ( fixer ) => {
+							return fixer.insertTextAfter(
+								lastArg,
+								`, '${ allowedTextDomains[ 0 ] }'`
+							);
+						};
 
-							context.report( {
-								node,
-								messageId: 'missing',
-								fix:
-									allowedTextDomains.length === 1
-										? ( fixer ) => {
-												return fixer.insertTextAfter(
-													lastArg,
-													`, '${ allowedTextDomains[ 0 ] }'`
-												);
-										  }
-										: null,
-							} );
-						}
-						break;
-					case STATUS.INVALID_TYPE:
 						context.report( {
 							node,
-							messageId: 'invalidType',
+							messageId: 'missing',
+							fix: canFixTextDomain ? addMissingTextDomain : null,
 						} );
-						break;
-					case STATUS.INVALID_VALUE:
-						const textDomain = getTextDomain( callee.name, args );
-						const { value, range, parent } = textDomain;
+					}
+					return;
+				}
+
+				const { type, value, range, parent } = textDomain;
 
-						const previousArg = [ ...parent.arguments ] // avoids reverse() modifying the AST.
-							.reverse()
-							.find( ( arg ) => arg.range[ 1 ] < range[ 0 ] );
+				if ( type !== 'Literal' ) {
+					context.report( {
+						node,
+						messageId: 'invalidType',
+					} );
+					return;
+				}
 
-						if ( 'default' === value && allowDefault ) {
-							context.report( {
-								node,
-								messageId: 'unnecessaryDefault',
-								fix: ( fixer ) => {
-									return fixer.removeRange( [
-										previousArg.range[ 1 ],
-										range[ 1 ],
-									] );
-								},
-							} );
-							break;
-						}
+				if ( ! allowedTextDomains.includes( value ) ) {
+					// avoids reverse() modifying the AST.
+					const previousArg = [ ...parent.arguments ]
+						.reverse()
+						.find( ( arg ) => arg.range[ 1 ] < range[ 0 ] );
+
+					if ( 'default' === value && allowDefault ) {
+						const removeDefaultTextDomain = ( fixer ) => {
+							return fixer.removeRange( [
+								previousArg.range[ 1 ],
+								range[ 1 ],
+							] );
+						};
 
 						context.report( {
 							node,
-							messageId: 'invalidValue',
-							data: {
-								textDomain: value,
-							},
-							fix:
-								allowedTextDomains.length === 1
-									? ( fixer ) => {
-											return fixer.replaceTextRange(
-												// account for quotes.
-												[
-													range[ 0 ] + 1,
-													range[ 1 ] - 1,
-												],
-												allowedTextDomains[ 0 ]
-											);
-									  }
-									: null,
+							messageId: 'unnecessaryDefault',
+							fix: removeDefaultTextDomain,
 						} );
-						break;
-					default:
-						break;
+						return;
+					}
+
+					const replaceTextDomain = ( fixer ) => {
+						return fixer.replaceTextRange(
+							// account for quotes.
+							[ range[ 0 ] + 1, range[ 1 ] - 1 ],
+							allowedTextDomains[ 0 ]
+						);
+					};
+
+					context.report( {
+						node,
+						messageId: 'invalidValue',
+						data: {
+							textDomain: value,
+						},
+						fix: canFixTextDomain ? replaceTextDomain : null,
+					} );
 				}
 			},
 		};
diff --git a/packages/eslint-plugin/rules/i18n-translator-comments.js b/packages/eslint-plugin/rules/i18n-translator-comments.js
index 271466943cd612..13646cbdb1cb1a 100644
--- a/packages/eslint-plugin/rules/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/i18n-translator-comments.js
@@ -5,14 +5,14 @@ const {
 	TRANSLATION_FUNCTIONS,
 	REGEXP_PLACEHOLDER,
 	getTranslateStrings,
-} = require( '../util' );
+} = require( '../utils' );
 
 module.exports = {
 	meta: {
 		type: 'problem',
 		messages: {
 			missing:
-				'Translation function is missing preceding translator comment',
+				'Translation function with placeholders is missing preceding translator comment',
 		},
 	},
 	create( context ) {
@@ -36,34 +36,35 @@ module.exports = {
 					return;
 				}
 
-				let hasPlaceholders = false;
-
-				for ( const candidate of candidates ) {
-					if ( candidate.match( REGEXP_PLACEHOLDER ) ) {
-						hasPlaceholders = true;
-						break;
-					}
-				}
+				const hasPlaceholders = candidates.some( ( candidate ) =>
+					candidate.match( REGEXP_PLACEHOLDER )
+				);
 
 				if ( ! hasPlaceholders ) {
 					return;
 				}
 
-				const comments = [];
-
-				comments.push( ...context.getCommentsBefore( node ) );
+				const comments = context.getCommentsBefore( node ).slice();
 
 				let parentNode = parent;
 
+				/**
+				 * Loop through all parent nodes and get their preceding comments as well.
+				 *
+				 * This way we can gather comments that are not directly preceding the translation
+				 * function call, but are just on the line above it. This case is commonly supported
+				 * by string extraction tools like WP-CLI's i18n command.
+				 */
 				while (
 					parentNode &&
+					parentNode.type !== 'Program' &&
 					Math.abs( parentNode.loc.start.line - currentLine ) <= 1
 				) {
 					comments.push( ...context.getCommentsBefore( parentNode ) );
 					parentNode = parentNode.parent;
 				}
 
-				for ( const comment of comments.filter( Boolean ) ) {
+				for ( const comment of comments ) {
 					const {
 						value: commentText,
 						loc: {
@@ -71,11 +72,22 @@ module.exports = {
 						},
 					} = comment;
 
+					/*
+					Skip cases like this:
+
+					// translators: %s: Preference
+					console.log(
+						sprintf(
+							__( 'Preference: %s' ),
+							preference
+						)
+					);
+					 */
 					if ( Math.abs( commentLine - currentLine ) > 1 ) {
 						continue;
 					}
 
-					if ( commentText.trim().match( /translators: /i ) ) {
+					if ( /translators:\s*\S+/i.test( commentText ) ) {
 						return;
 					}
 				}
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index c95be7fedfa162..6049c41a574d04 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -1,7 +1,7 @@
 /**
  * Internal dependencies
  */
-const { REGEXP_PLACEHOLDER, getTranslateStrings } = require( '../util' );
+const { REGEXP_PLACEHOLDER, getTranslateStrings } = require( '../utils' );
 
 module.exports = {
 	meta: {
diff --git a/packages/eslint-plugin/util/index.js b/packages/eslint-plugin/utils/index.js
similarity index 92%
rename from packages/eslint-plugin/util/index.js
rename to packages/eslint-plugin/utils/index.js
index ee74b4b27087c3..4deeb07faf58cd 100644
--- a/packages/eslint-plugin/util/index.js
+++ b/packages/eslint-plugin/utils/index.js
@@ -16,7 +16,7 @@ const REGEXP_PLACEHOLDER = /%[^%]/g;
  * @param {string}        functionName Function name.
  * @param {espree.Node[]} args         Espree argument Node objects.
  *
- * @return {?Array<string>} All possible translate function string results.
+ * @return {Array<string>|void} All possible translate function string results.
  */
 function getTranslateStrings( functionName, args ) {
 	switch ( functionName ) {

From 3af1fb5a56bded2048ea2e38879d31c35549862d Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 11 Mar 2020 14:01:07 +0100
Subject: [PATCH 08/41] Rename rule names

---
 packages/i18n/src/test/index.js | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/packages/i18n/src/test/index.js b/packages/i18n/src/test/index.js
index c8ae40cb23f072..946b6b7c2241e8 100644
--- a/packages/i18n/src/test/index.js
+++ b/packages/i18n/src/test/index.js
@@ -1,5 +1,5 @@
-/* eslint-disable @wordpress/valid-text-domain */
-/* eslint-disable @wordpress/no-missing-translator-comments */
+/* eslint-disable @wordpress/i18n-text-domain */
+/* eslint-disable @wordpress/i18n-translator-comments */
 
 // Mock memoization as identity function. Inline since Jest errors on out-of-
 // scope references in a mock callback.
@@ -189,5 +189,5 @@ function setDefaultLocalData() {
 	setLocaleData( localeData, 'test_domain' );
 }
 
-/* eslint-enable @wordpress/no-missing-translator-comments */
-/* eslint-enable @wordpress/valid-text-domain */
+/* eslint-enable @wordpress/i18n-translator-comments */
+/* eslint-enable @wordpress/i18n-text-domain */

From 82c956d431530b51f768da1ab807d3f7b3430f2c Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 11 Mar 2020 14:08:58 +0100
Subject: [PATCH 09/41] Simplify getting previousArg

---
 .../eslint-plugin/rules/__tests__/i18n-text-domain.js     | 6 ++++++
 packages/eslint-plugin/rules/i18n-text-domain.js          | 8 +++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
index eeec92a0c15e6a..362f47b1c6b574 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
@@ -120,6 +120,12 @@ ruleTester.run( 'i18n-text-domain', rule, {
 			options: [ { allowDefault: true } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
+		{
+			code: `__('default', 'default')`,
+			output: `__('default')`,
+			options: [ { allowDefault: true } ],
+			errors: [ { messageId: 'unnecessaryDefault' } ],
+		},
 		{
 			code: `_x('Hello World', 'context', 'default')`,
 			output: `_x('Hello World', 'context')`,
diff --git a/packages/eslint-plugin/rules/i18n-text-domain.js b/packages/eslint-plugin/rules/i18n-text-domain.js
index a9862ad89836d1..de2c1340ddd643 100644
--- a/packages/eslint-plugin/rules/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/i18n-text-domain.js
@@ -90,7 +90,7 @@ module.exports = {
 					return;
 				}
 
-				const { type, value, range, parent } = textDomain;
+				const { type, value, range } = textDomain;
 
 				if ( type !== 'Literal' ) {
 					context.report( {
@@ -101,10 +101,8 @@ module.exports = {
 				}
 
 				if ( ! allowedTextDomains.includes( value ) ) {
-					// avoids reverse() modifying the AST.
-					const previousArg = [ ...parent.arguments ]
-						.reverse()
-						.find( ( arg ) => arg.range[ 1 ] < range[ 0 ] );
+					const previousArgIndex = args.indexOf( textDomain ) - 1;
+					const previousArg = args[ previousArgIndex ];
 
 					if ( 'default' === value && allowDefault ) {
 						const removeDefaultTextDomain = ( fixer ) => {

From d611c85d950f5ab4685ec84a66fc09221a96ec93 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 11 Mar 2020 18:10:33 +0100
Subject: [PATCH 10/41] Extract and document utils

---
 packages/eslint-plugin/utils/constants.js     | 19 +++++++++
 .../utils/get-translate-strings.js            | 34 +++++++++++++++
 packages/eslint-plugin/utils/index.js         | 42 +------------------
 3 files changed, 55 insertions(+), 40 deletions(-)
 create mode 100644 packages/eslint-plugin/utils/constants.js
 create mode 100644 packages/eslint-plugin/utils/get-translate-strings.js

diff --git a/packages/eslint-plugin/utils/constants.js b/packages/eslint-plugin/utils/constants.js
new file mode 100644
index 00000000000000..5d3607929d7e40
--- /dev/null
+++ b/packages/eslint-plugin/utils/constants.js
@@ -0,0 +1,19 @@
+/**
+ * List of translation functions exposed by the `@wordpress/i18n` package.
+ *
+ * @type {string[]} Translation functions.
+ */
+const TRANSLATION_FUNCTIONS = [ '__', '_x', '_n', '_nx' ];
+
+/**
+ * Regular expression matching the presence of a printf format string
+ * placeholder. This naive pattern which does not validate the format.
+ *
+ * @type {RegExp}
+ */
+const REGEXP_PLACEHOLDER = /%[^%]/g;
+
+module.exports = {
+	TRANSLATION_FUNCTIONS,
+	REGEXP_PLACEHOLDER,
+};
diff --git a/packages/eslint-plugin/utils/get-translate-strings.js b/packages/eslint-plugin/utils/get-translate-strings.js
new file mode 100644
index 00000000000000..aa8ea980af6019
--- /dev/null
+++ b/packages/eslint-plugin/utils/get-translate-strings.js
@@ -0,0 +1,34 @@
+/**
+ * Given a function name and array of argument Node values, returns all
+ * possible string results from the corresponding translate function, or
+ * undefined if the function is not a translate function.
+ *
+ * @param {string}        functionName Function name.
+ * @param {espree.Node[]} args         Espree argument Node objects.
+ *
+ * @return {Array<string>|void} All possible translate function string results.
+ */
+function getTranslateStrings( functionName, args ) {
+	switch ( functionName ) {
+		case '__':
+		case '_x':
+			args = args.slice( 0, 1 );
+			break;
+
+		case '_n':
+		case '_nx':
+			args = args.slice( 0, 2 );
+			break;
+
+		default:
+			return;
+	}
+
+	return args
+		.filter( ( arg ) => arg.type === 'Literal' )
+		.map( ( arg ) => arg.value );
+}
+
+module.exports = {
+	getTranslateStrings,
+};
diff --git a/packages/eslint-plugin/utils/index.js b/packages/eslint-plugin/utils/index.js
index 4deeb07faf58cd..759d4c6d5c436e 100644
--- a/packages/eslint-plugin/utils/index.js
+++ b/packages/eslint-plugin/utils/index.js
@@ -1,43 +1,5 @@
-const TRANSLATION_FUNCTIONS = [ '__', '_x', '_n', '_nx' ];
-
-/**
- * Regular expression matching the presence of a printf format string
- * placeholder. This naive pattern which does not validate the format.
- *
- * @type {RegExp}
- */
-const REGEXP_PLACEHOLDER = /%[^%]/g;
-
-/**
- * Given a function name and array of argument Node values, returns all
- * possible string results from the corresponding translate function, or
- * undefined if the function is not a translate function.
- *
- * @param {string}        functionName Function name.
- * @param {espree.Node[]} args         Espree argument Node objects.
- *
- * @return {Array<string>|void} All possible translate function string results.
- */
-function getTranslateStrings( functionName, args ) {
-	switch ( functionName ) {
-		case '__':
-		case '_x':
-			args = args.slice( 0, 1 );
-			break;
-
-		case '_n':
-		case '_nx':
-			args = args.slice( 0, 2 );
-			break;
-
-		default:
-			return;
-	}
-
-	return args
-		.filter( ( arg ) => arg.type === 'Literal' )
-		.map( ( arg ) => arg.value );
-}
+const { TRANSLATION_FUNCTIONS, REGEXP_PLACEHOLDER } = require( './constants' );
+const { getTranslateStrings } = require( './get-translate-strings' );
 
 module.exports = {
 	TRANSLATION_FUNCTIONS,

From 8194ffa09a5b1922c218c53f57e30b2dce1fa7d9 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 11 Mar 2020 18:39:43 +0100
Subject: [PATCH 11/41] Combine comments

---
 packages/i18n/src/test/index.js | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/packages/i18n/src/test/index.js b/packages/i18n/src/test/index.js
index 946b6b7c2241e8..1b604d8753cca6 100644
--- a/packages/i18n/src/test/index.js
+++ b/packages/i18n/src/test/index.js
@@ -1,5 +1,4 @@
-/* eslint-disable @wordpress/i18n-text-domain */
-/* eslint-disable @wordpress/i18n-translator-comments */
+/* eslint-disable @wordpress/i18n-text-domain, @wordpress/i18n-translator-comments */
 
 // Mock memoization as identity function. Inline since Jest errors on out-of-
 // scope references in a mock callback.
@@ -189,5 +188,4 @@ function setDefaultLocalData() {
 	setLocaleData( localeData, 'test_domain' );
 }
 
-/* eslint-enable @wordpress/i18n-translator-comments */
-/* eslint-enable @wordpress/i18n-text-domain */
+/* eslint-enable @wordpress/i18n-text-domain, @wordpress/i18n-translator-comments */

From 6125f28f9b96c49ae0e374d0b743300cd16ef7e2 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 11 Mar 2020 20:23:26 +0100
Subject: [PATCH 12/41] Derive allowDefault from allowedTextDomains

---
 .eslintrc.js                                  |  2 +-
 .../docs/rules/i18n-text-domain.md            | 13 +++---
 .../rules/__tests__/i18n-text-domain.js       | 18 ++++----
 .../eslint-plugin/rules/i18n-text-domain.js   | 44 +++++++++----------
 4 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index 6c508ea7c7b6ea..c6b5b8431291f3 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -45,7 +45,7 @@ module.exports = {
 		'@wordpress/i18n-text-domain': [
 			'error',
 			{
-				allowDefault: true,
+				allowedTextDomains: [ 'default' ],
 			},
 		],
 		'@wordpress/i18n-translator-comments': 'error',
diff --git a/packages/eslint-plugin/docs/rules/i18n-text-domain.md b/packages/eslint-plugin/docs/rules/i18n-text-domain.md
index 551d6a794df63b..d25d2b98ca4609 100644
--- a/packages/eslint-plugin/docs/rules/i18n-text-domain.md
+++ b/packages/eslint-plugin/docs/rules/i18n-text-domain.md
@@ -7,21 +7,20 @@
 Examples of **incorrect** code for this rule:
 
 ```js
-__( 'Hello World' ); // unless allowDefault is set.
-__( 'Hello World', 'default' ); // with allowDefault set.
+__( 'Hello World' ); // unless allowedTextDomains contains 'default'
+__( 'Hello World', 'default' ); // with allowedTextDomains = [ 'default' ]
 __( 'Hello World', foo );
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__( 'Hello World' ); // with allowDefault set.
-__( 'Hello World', 'foo-bar' ); // with allowedTextDomains set
+__( 'Hello World' ); // with allowedTextDomains = [ 'default' ]
+__( 'Hello World', 'foo-bar' ); // with allowedTextDomains = [ 'foo-bar' ]
 ```
 
 ## Options
 
-This rule accepts two options:
+This rule accepts a single options argument:
 
-- Set the `allowDefault` boolean to allow omitting the text domain and using its default value (`'default'`).
-- Set `allowedTextDomains` to specify the list of allowed text domains, e.g. `'foo-bar'`.
+- Set `allowedTextDomains` to specify the list of allowed text domains, e.g. `[ 'foo', 'bar' ]`. The default is `[ 'default' ]`.
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
index 362f47b1c6b574..8ce65d5c5c2ba2 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
@@ -18,19 +18,19 @@ ruleTester.run( 'i18n-text-domain', rule, {
 	valid: [
 		{
 			code: `__('Hello World')`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
 			code: `_x('Hello World', 'context')`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
 			code: `var number = ''; _n('Singular', 'Plural', number)`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
 			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
 			code: `__('Hello World', 'foo')`,
@@ -117,31 +117,31 @@ ruleTester.run( 'i18n-text-domain', rule, {
 		{
 			code: `__('Hello World', 'default')`,
 			output: `__('Hello World')`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `__('default', 'default')`,
 			output: `__('default')`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `_x('Hello World', 'context', 'default')`,
 			output: `_x('Hello World', 'context')`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `var number = ''; _n('Singular', 'Plural', number, 'default')`,
 			output: `var number = ''; _n('Singular', 'Plural', number)`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'default')`,
 			output: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
-			options: [ { allowDefault: true } ],
+			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 	],
diff --git a/packages/eslint-plugin/rules/i18n-text-domain.js b/packages/eslint-plugin/rules/i18n-text-domain.js
index de2c1340ddd643..2506cea8de79be 100644
--- a/packages/eslint-plugin/rules/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/i18n-text-domain.js
@@ -32,10 +32,6 @@ module.exports = {
 			{
 				type: 'object',
 				properties: {
-					allowDefault: {
-						type: 'boolean',
-						default: false,
-					},
 					allowedTextDomains: {
 						type: 'array',
 						items: {
@@ -59,8 +55,9 @@ module.exports = {
 	},
 	create( context ) {
 		const options = context.options[ 0 ] || {};
-		const { allowDefault, allowedTextDomains = [] } = options;
+		const { allowedTextDomains = [ 'default' ] } = options;
 		const canFixTextDomain = allowedTextDomains.length === 1;
+		const allowDefault = allowedTextDomains.includes( 'default' );
 
 		return {
 			CallExpression( node ) {
@@ -73,8 +70,8 @@ module.exports = {
 
 				if ( textDomain === undefined ) {
 					if ( ! allowDefault ) {
-						const lastArg = args[ args.length - 1 ];
 						const addMissingTextDomain = ( fixer ) => {
+							const lastArg = args[ args.length - 1 ];
 							return fixer.insertTextAfter(
 								lastArg,
 								`, '${ allowedTextDomains[ 0 ] }'`
@@ -100,26 +97,25 @@ module.exports = {
 					return;
 				}
 
-				if ( ! allowedTextDomains.includes( value ) ) {
-					const previousArgIndex = args.indexOf( textDomain ) - 1;
-					const previousArg = args[ previousArgIndex ];
-
-					if ( 'default' === value && allowDefault ) {
-						const removeDefaultTextDomain = ( fixer ) => {
-							return fixer.removeRange( [
-								previousArg.range[ 1 ],
-								range[ 1 ],
-							] );
-						};
+				if ( 'default' === value && allowDefault ) {
+					const removeDefaultTextDomain = ( fixer ) => {
+						const previousArgIndex = args.indexOf( textDomain ) - 1;
+						const previousArg = args[ previousArgIndex ];
+						return fixer.removeRange( [
+							previousArg.range[ 1 ],
+							range[ 1 ],
+						] );
+					};
 
-						context.report( {
-							node,
-							messageId: 'unnecessaryDefault',
-							fix: removeDefaultTextDomain,
-						} );
-						return;
-					}
+					context.report( {
+						node,
+						messageId: 'unnecessaryDefault',
+						fix: removeDefaultTextDomain,
+					} );
+					return;
+				}
 
+				if ( ! allowedTextDomains.includes( value ) ) {
 					const replaceTextDomain = ( fixer ) => {
 						return fixer.replaceTextRange(
 							// account for quotes.

From 02e30c4b3a6b9382f15f80db25657d3a0f73b707 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Thu, 19 Mar 2020 09:34:02 +0000
Subject: [PATCH 13/41] Break early for line number mismatches

Co-Authored-By: Andrew Duthie <andrew@andrewduthie.com>
---
 packages/eslint-plugin/rules/i18n-translator-comments.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/eslint-plugin/rules/i18n-translator-comments.js b/packages/eslint-plugin/rules/i18n-translator-comments.js
index 13646cbdb1cb1a..c9fe05d4d4b14b 100644
--- a/packages/eslint-plugin/rules/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/i18n-translator-comments.js
@@ -84,7 +84,7 @@ module.exports = {
 					);
 					 */
 					if ( Math.abs( commentLine - currentLine ) > 1 ) {
-						continue;
+						break;
 					}
 
 					if ( /translators:\s*\S+/i.test( commentText ) ) {

From 11569f3a41768b91e7624067f3bf51ac42947599 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Thu, 19 Mar 2020 14:48:46 +0100
Subject: [PATCH 14/41] Support `i18n.*` usage in new rules

---
 .../eslint-plugin/rules/__tests__/i18n-text-domain.js  | 10 ++++++++++
 .../rules/__tests__/i18n-translator-comments.js        | 10 ++++++++++
 packages/eslint-plugin/rules/i18n-text-domain.js       | 10 ++++++++--
 .../eslint-plugin/rules/i18n-translator-comments.js    | 10 ++++++++--
 4 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
index 8ce65d5c5c2ba2..15f9bb90b755e7 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
@@ -48,6 +48,10 @@ ruleTester.run( 'i18n-text-domain', rule, {
 			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 		},
+		{
+			code: `i18n.__('Hello World')`,
+			options: [ { allowedTextDomains: [ 'default' ] } ],
+		},
 	],
 	invalid: [
 		{
@@ -144,5 +148,11 @@ ruleTester.run( 'i18n-text-domain', rule, {
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
+		{
+			code: `i18n.__('Hello World')`,
+			output: `i18n.__('Hello World', 'foo')`,
+			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			errors: [ { messageId: 'missing' } ],
+		},
 	],
 } );
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js b/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js
index 10ef70023862fe..df59a9bbe07083 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-translator-comments.js
@@ -29,6 +29,11 @@ sprintf(
 	address
 );`,
 		},
+		{
+			code: `
+// translators: %s: Color
+i18n.sprintf( i18n.__( 'Color: %s' ), color );`,
+		},
 	],
 	invalid: [
 		{
@@ -70,5 +75,10 @@ console.log(
 );`,
 			errors: [ { messageId: 'missing' } ],
 		},
+		{
+			code: `
+i18n.sprintf( i18n.__( 'Color: %s' ), color );`,
+			errors: [ { messageId: 'missing' } ],
+		},
 	],
 } );
diff --git a/packages/eslint-plugin/rules/i18n-text-domain.js b/packages/eslint-plugin/rules/i18n-text-domain.js
index 2506cea8de79be..202d36df3b4923 100644
--- a/packages/eslint-plugin/rules/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/i18n-text-domain.js
@@ -62,11 +62,17 @@ module.exports = {
 		return {
 			CallExpression( node ) {
 				const { callee, arguments: args } = node;
-				if ( ! TRANSLATION_FUNCTIONS.includes( callee.name ) ) {
+
+				const functionName =
+					callee.property && callee.property.name
+						? callee.property.name
+						: callee.name;
+
+				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
 					return;
 				}
 
-				const textDomain = getTextDomain( callee.name, args );
+				const textDomain = getTextDomain( functionName, args );
 
 				if ( textDomain === undefined ) {
 					if ( ! allowDefault ) {
diff --git a/packages/eslint-plugin/rules/i18n-translator-comments.js b/packages/eslint-plugin/rules/i18n-translator-comments.js
index c9fe05d4d4b14b..8d85e2d3bc3e00 100644
--- a/packages/eslint-plugin/rules/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/i18n-translator-comments.js
@@ -26,11 +26,17 @@ module.exports = {
 					parent,
 					arguments: args,
 				} = node;
-				if ( ! TRANSLATION_FUNCTIONS.includes( callee.name ) ) {
+
+				const functionName =
+					callee.property && callee.property.name
+						? callee.property.name
+						: callee.name;
+
+				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
 					return;
 				}
 
-				const candidates = getTranslateStrings( callee.name, args );
+				const candidates = getTranslateStrings( functionName, args );
 
 				if ( ! candidates ) {
 					return;

From 9e535ce46680a2d435b460e2fb1e56c59e3bddbf Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Tue, 24 Mar 2020 17:38:08 +0100
Subject: [PATCH 15/41] Add new i18n-no-variables rule

---
 .eslintrc.js                                  | 18 ----
 packages/eslint-plugin/CHANGELOG.md           |  1 +
 packages/eslint-plugin/configs/custom.js      |  1 +
 .../docs/rules/i18n-no-variables.md           | 20 ++++
 .../rules/__tests__/i18n-no-variables.js      | 92 +++++++++++++++++++
 .../eslint-plugin/rules/i18n-no-variables.js  | 71 ++++++++++++++
 6 files changed, 185 insertions(+), 18 deletions(-)
 create mode 100644 packages/eslint-plugin/docs/rules/i18n-no-variables.md
 create mode 100644 packages/eslint-plugin/rules/__tests__/i18n-no-variables.js
 create mode 100644 packages/eslint-plugin/rules/i18n-no-variables.js

diff --git a/.eslintrc.js b/.eslintrc.js
index c6b5b8431291f3..3e91908d4385aa 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -74,24 +74,6 @@ module.exports = {
 				message:
 					'Deprecated functions must be removed before releasing this version.',
 			},
-			{
-				selector:
-					'CallExpression[callee.name=/^(__|_n|_nx|_x)$/]:not([arguments.0.type=/^Literal|BinaryExpression$/])',
-				message:
-					'Translate function arguments must be string literals.',
-			},
-			{
-				selector:
-					'CallExpression[callee.name=/^(_n|_nx|_x)$/]:not([arguments.1.type=/^Literal|BinaryExpression$/])',
-				message:
-					'Translate function arguments must be string literals.',
-			},
-			{
-				selector:
-					'CallExpression[callee.name=_nx]:not([arguments.3.type=/^Literal|BinaryExpression$/])',
-				message:
-					'Translate function arguments must be string literals.',
-			},
 			{
 				selector:
 					'CallExpression[callee.name=/^(__|_x|_n|_nx)$/] Literal[value=/\\.{3}/]',
diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index c993fda5eea06c..72ff90abe27076 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -5,6 +5,7 @@
 - New Rule: [`@wordpress/i18n-text-domain`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-text-domain.md)
 - New Rule: [`@wordpress/i18n-translator-comments`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-translator-comments.md)
 - The `prefer-const` rule included in the `recommended` and `esnext` rulesets has been relaxed to allow a `let` assignment if any of a [destructuring assignment](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) are reassigned.
+- New Rule: [`@wordpress/i18n-no-variables`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-variables.md)
 
 ## 4.0.0 (2020-02-10)
 
diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index 08cb90b05a6f6c..520ab88190ab98 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -7,6 +7,7 @@ module.exports = {
 		'@wordpress/no-unguarded-get-range-at': 'error',
 		'@wordpress/i18n-translator-comments': 'error',
 		'@wordpress/i18n-text-domain': 'error',
+		'@wordpress/i18n-no-variables': 'error',
 		'no-restricted-syntax': [
 			'error',
 			{
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-variables.md b/packages/eslint-plugin/docs/rules/i18n-no-variables.md
new file mode 100644
index 00000000000000..5f8a0f64460e37
--- /dev/null
+++ b/packages/eslint-plugin/docs/rules/i18n-no-variables.md
@@ -0,0 +1,20 @@
+# Enforce string literals as translation function arguments (i18n-no-variables)
+
+[Translation functions](https://github.com/WordPress/gutenberg/blob/master/packages/i18n/README.md#api) must be called with valid string literals as arguments.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```js
+__('Hello World');
+_x('Hello World', 'context', 'foo');
+
+```
+
+Examples of **correct** code for this rule:
+
+```js
+__(`Hello ${foo}`);
+_x('Hello World', bar);
+```
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-no-variables.js b/packages/eslint-plugin/rules/__tests__/i18n-no-variables.js
new file mode 100644
index 00000000000000..d0656d2f0ab551
--- /dev/null
+++ b/packages/eslint-plugin/rules/__tests__/i18n-no-variables.js
@@ -0,0 +1,92 @@
+/**
+ * External dependencies
+ */
+import { RuleTester } from 'eslint';
+
+/**
+ * Internal dependencies
+ */
+import rule from '../i18n-no-variables';
+
+const ruleTester = new RuleTester( {
+	parserOptions: {
+		ecmaVersion: 6,
+	},
+} );
+
+ruleTester.run( 'i18n-no-variables', rule, {
+	valid: [
+		{
+			code: `__('Hello World')`,
+		},
+		{
+			code: `__('Hello' + 'World')`,
+		},
+		{
+			code: `_x('Hello World', 'context')`,
+		},
+		{
+			code: `var number = ''; _n('Singular', 'Plural', number)`,
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
+		},
+		{
+			code: `__('Hello World', 'foo')`,
+		},
+		{
+			code: `_x('Hello World', 'context', 'foo')`,
+		},
+		{
+			code: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
+		},
+		{
+			code: `i18n.__('Hello World')`,
+		},
+	],
+	invalid: [
+		{
+			code: `__(foo)`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: '__(`Hello ${foo}`)',
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `_x(foo, 'context')`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `_x('Hello World', bar)`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `var number = ''; _n(foo,'Plural', number)`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `var number = ''; _n('Singular', bar, number)`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `var number = ''; _nx(foo, 'Plural', number, 'context')`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `var number = ''; _nx('Singular', bar, number, 'context')`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `var number = ''; _nx('Singular', 'Plural', number, baz)`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+		{
+			code: `i18n.__(foo)`,
+			errors: [ { messageId: 'invalidArgument' } ],
+		},
+	],
+} );
diff --git a/packages/eslint-plugin/rules/i18n-no-variables.js b/packages/eslint-plugin/rules/i18n-no-variables.js
new file mode 100644
index 00000000000000..f2951a3321e68f
--- /dev/null
+++ b/packages/eslint-plugin/rules/i18n-no-variables.js
@@ -0,0 +1,71 @@
+/**
+ * Internal dependencies
+ */
+const { TRANSLATION_FUNCTIONS } = require( '../utils' );
+
+module.exports = {
+	meta: {
+		type: 'problem',
+		schema: [],
+		messages: {
+			invalidArgument:
+				'Translate function arguments must be string literals.',
+		},
+		fixable: 'code',
+	},
+	create( context ) {
+		function isAcceptableLiteralNode( node ) {
+			if ( 'BinaryExpression' === node.type ) {
+				return (
+					'+' === node.operator &&
+					isAcceptableLiteralNode( node.left ) &&
+					isAcceptableLiteralNode( node.right )
+				);
+			}
+
+			if ( 'TemplateLiteral' === node.type ) {
+				// Backticks are fine, but if there's any interpolation in it,
+				// that's a problem
+				return node.expressions.length === 0;
+			}
+
+			return 'Literal' === node.type;
+		}
+
+		return {
+			CallExpression( node ) {
+				const { callee, arguments: args } = node;
+
+				const functionName =
+					callee.property && callee.property.name
+						? callee.property.name
+						: callee.name;
+
+				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+					return;
+				}
+
+				const functionArgs = [ args[ 0 ] ];
+
+				if ( [ '_n', '_nx', '_x' ].includes( functionName ) ) {
+					functionArgs.push( args[ 1 ] );
+				}
+
+				if ( [ '_nx' ].includes( functionName ) ) {
+					functionArgs.push( args[ 3 ] );
+				}
+
+				for ( const arg of functionArgs ) {
+					if ( isAcceptableLiteralNode( arg ) ) {
+						continue;
+					}
+
+					context.report( {
+						node,
+						messageId: 'invalidArgument',
+					} );
+				}
+			},
+		};
+	},
+};

From 811c9514fd9618e693a348a9a176435a67866bb5 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 25 Mar 2020 09:26:53 +0100
Subject: [PATCH 16/41] Add new i18n-ellipsis rule

---
 .eslintrc.js                                  |  5 -
 packages/eslint-plugin/CHANGELOG.md           |  1 +
 packages/eslint-plugin/configs/custom.js      |  1 +
 .../eslint-plugin/docs/rules/i18n-ellipsis.md | 18 ++++
 .../rules/__tests__/i18n-ellipsis.js          | 65 ++++++++++++
 packages/eslint-plugin/rules/i18n-ellipsis.js | 98 +++++++++++++++++++
 .../eslint-plugin/rules/i18n-no-variables.js  | 47 +++++----
 .../eslint-plugin/rules/i18n-text-domain.js   | 10 +-
 .../rules/i18n-translator-comments.js         |  6 +-
 .../utils/get-text-content-from-node.js       | 38 +++++++
 .../utils/get-translate-function-name.js      | 17 ++++
 packages/eslint-plugin/utils/index.js         |  4 +
 12 files changed, 272 insertions(+), 38 deletions(-)
 create mode 100644 packages/eslint-plugin/docs/rules/i18n-ellipsis.md
 create mode 100644 packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js
 create mode 100644 packages/eslint-plugin/rules/i18n-ellipsis.js
 create mode 100644 packages/eslint-plugin/utils/get-text-content-from-node.js
 create mode 100644 packages/eslint-plugin/utils/get-translate-function-name.js

diff --git a/.eslintrc.js b/.eslintrc.js
index 3e91908d4385aa..dd8456a8570c88 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -74,11 +74,6 @@ module.exports = {
 				message:
 					'Deprecated functions must be removed before releasing this version.',
 			},
-			{
-				selector:
-					'CallExpression[callee.name=/^(__|_x|_n|_nx)$/] Literal[value=/\\.{3}/]',
-				message: 'Use ellipsis character (…) in place of three dots',
-			},
 			{
 				selector:
 					'ImportDeclaration[source.value="redux"] Identifier.imported[name="combineReducers"]',
diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 72ff90abe27076..650743e8f1ff0d 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -6,6 +6,7 @@
 - New Rule: [`@wordpress/i18n-translator-comments`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-translator-comments.md)
 - The `prefer-const` rule included in the `recommended` and `esnext` rulesets has been relaxed to allow a `let` assignment if any of a [destructuring assignment](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) are reassigned.
 - New Rule: [`@wordpress/i18n-no-variables`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-variables.md)
+- New Rule: [`@wordpress/i18n-ellipsis`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-ellipsis.md)
 
 ## 4.0.0 (2020-02-10)
 
diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index 520ab88190ab98..e1d465ad121030 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -8,6 +8,7 @@ module.exports = {
 		'@wordpress/i18n-translator-comments': 'error',
 		'@wordpress/i18n-text-domain': 'error',
 		'@wordpress/i18n-no-variables': 'error',
+		'@wordpress/i18n-ellipsis': 'error',
 		'no-restricted-syntax': [
 			'error',
 			{
diff --git a/packages/eslint-plugin/docs/rules/i18n-ellipsis.md b/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
new file mode 100644
index 00000000000000..5c6e7d20c74f6a
--- /dev/null
+++ b/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
@@ -0,0 +1,18 @@
+# Enforce using ellipsis (…) instead of three dots (i18n-ellipsis)
+
+Translatable strings should use ellipsis (…) instead of three dots.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```js
+__('Continue...');
+
+```
+
+Examples of **correct** code for this rule:
+
+```js
+__('Continue…');
+```
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js b/packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js
new file mode 100644
index 00000000000000..d16f4f2c89a055
--- /dev/null
+++ b/packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js
@@ -0,0 +1,65 @@
+/**
+ * External dependencies
+ */
+import { RuleTester } from 'eslint';
+
+/**
+ * Internal dependencies
+ */
+import rule from '../i18n-ellipsis';
+
+const ruleTester = new RuleTester( {
+	parserOptions: {
+		ecmaVersion: 6,
+	},
+} );
+
+ruleTester.run( 'i18n-ellipsis', rule, {
+	valid: [
+		{
+			code: `__('Hello World…')`,
+		},
+		{
+			code: `__('Hello' + 'World…')`,
+		},
+		{
+			code: `_x('Hello World…', 'context')`,
+		},
+		{
+			code: `_n('Singular…', 'Plural…', number)`,
+		},
+		{
+			code: `i18n.__('Hello World…')`,
+		},
+	],
+	invalid: [
+		{
+			code: `__('Hello World...')`,
+			output: `__('Hello World…')`,
+			errors: [ { messageId: 'foundThreeDots' } ],
+		},
+		{
+			code: `__('Hello' + 'World...')`,
+			output: `__('Hello' + 'World…')`,
+			errors: [ { messageId: 'foundThreeDots' } ],
+		},
+		{
+			code: `_x('Hello World...', 'context')`,
+			output: `_x('Hello World…', 'context')`,
+			errors: [ { messageId: 'foundThreeDots' } ],
+		},
+		{
+			code: `_n('Singular...', 'Plural...', number)`,
+			output: `_n('Singular…', 'Plural…', number)`,
+			errors: [
+				{ messageId: 'foundThreeDots' },
+				{ messageId: 'foundThreeDots' },
+			],
+		},
+		{
+			code: `i18n.__('Hello World...')`,
+			output: `i18n.__('Hello World…')`,
+			errors: [ { messageId: 'foundThreeDots' } ],
+		},
+	],
+} );
diff --git a/packages/eslint-plugin/rules/i18n-ellipsis.js b/packages/eslint-plugin/rules/i18n-ellipsis.js
new file mode 100644
index 00000000000000..28c9e93186382b
--- /dev/null
+++ b/packages/eslint-plugin/rules/i18n-ellipsis.js
@@ -0,0 +1,98 @@
+/**
+ * Internal dependencies
+ */
+const {
+	TRANSLATION_FUNCTIONS,
+	getTextContentFromNode,
+	getTranslateFunctionName,
+} = require( '../utils' );
+
+const THREE_DOTS = '...';
+const ELLIPSIS = '…';
+
+function replaceThreeDotsWithEllipsis( string ) {
+	return string.replace( /\.\.\./g, ELLIPSIS );
+}
+
+// see eslint-plugin-wpcalypso.
+function makeFixerFunction( arg ) {
+	return ( fixer ) => {
+		switch ( arg.type ) {
+			case 'TemplateLiteral':
+				return arg.quasis.reduce( ( fixes, quasi ) => {
+					if (
+						'TemplateElement' === quasi.type &&
+						quasi.value.raw.includes( THREE_DOTS )
+					) {
+						fixes.push(
+							fixer.replaceTextRange(
+								[ quasi.start, quasi.end ],
+								replaceThreeDotsWithEllipsis( quasi.value.raw )
+							)
+						);
+					}
+					return fixes;
+				}, [] );
+
+			case 'Literal':
+				return [
+					fixer.replaceText(
+						arg,
+						replaceThreeDotsWithEllipsis( arg.raw )
+					),
+				];
+
+			case 'BinaryExpression':
+				return [
+					...makeFixerFunction( arg.left )( fixer ),
+					...makeFixerFunction( arg.right )( fixer ),
+				];
+		}
+	};
+}
+
+module.exports = {
+	meta: {
+		type: 'problem',
+		schema: [],
+		messages: {
+			foundThreeDots: 'Use ellipsis character (…) in place of three dots',
+		},
+		fixable: 'code',
+	},
+	create( context ) {
+		return {
+			CallExpression( node ) {
+				const { callee, arguments: args } = node;
+
+				const functionName = getTranslateFunctionName( callee );
+
+				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+					return;
+				}
+
+				const functionArgs = [ args[ 0 ] ];
+
+				if ( [ '_n', '_nx' ].includes( functionName ) ) {
+					functionArgs.push( args[ 1 ] );
+				}
+
+				for ( const arg of functionArgs ) {
+					const argumentString = getTextContentFromNode( arg );
+					if (
+						! argumentString ||
+						! argumentString.includes( THREE_DOTS )
+					) {
+						continue;
+					}
+
+					context.report( {
+						node,
+						messageId: 'foundThreeDots',
+						fix: makeFixerFunction( arg ),
+					} );
+				}
+			},
+		};
+	},
+};
diff --git a/packages/eslint-plugin/rules/i18n-no-variables.js b/packages/eslint-plugin/rules/i18n-no-variables.js
index f2951a3321e68f..b5fd69ab0fc4eb 100644
--- a/packages/eslint-plugin/rules/i18n-no-variables.js
+++ b/packages/eslint-plugin/rules/i18n-no-variables.js
@@ -1,7 +1,28 @@
 /**
  * Internal dependencies
  */
-const { TRANSLATION_FUNCTIONS } = require( '../utils' );
+const {
+	TRANSLATION_FUNCTIONS,
+	getTranslateFunctionName,
+} = require( '../utils' );
+
+function isAcceptableLiteralNode( node ) {
+	if ( 'BinaryExpression' === node.type ) {
+		return (
+			'+' === node.operator &&
+			isAcceptableLiteralNode( node.left ) &&
+			isAcceptableLiteralNode( node.right )
+		);
+	}
+
+	if ( 'TemplateLiteral' === node.type ) {
+		// Backticks are fine, but if there's any interpolation in it,
+		// that's a problem
+		return node.expressions.length === 0;
+	}
+
+	return 'Literal' === node.type;
+}
 
 module.exports = {
 	meta: {
@@ -11,35 +32,13 @@ module.exports = {
 			invalidArgument:
 				'Translate function arguments must be string literals.',
 		},
-		fixable: 'code',
 	},
 	create( context ) {
-		function isAcceptableLiteralNode( node ) {
-			if ( 'BinaryExpression' === node.type ) {
-				return (
-					'+' === node.operator &&
-					isAcceptableLiteralNode( node.left ) &&
-					isAcceptableLiteralNode( node.right )
-				);
-			}
-
-			if ( 'TemplateLiteral' === node.type ) {
-				// Backticks are fine, but if there's any interpolation in it,
-				// that's a problem
-				return node.expressions.length === 0;
-			}
-
-			return 'Literal' === node.type;
-		}
-
 		return {
 			CallExpression( node ) {
 				const { callee, arguments: args } = node;
 
-				const functionName =
-					callee.property && callee.property.name
-						? callee.property.name
-						: callee.name;
+				const functionName = getTranslateFunctionName( callee );
 
 				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
 					return;
diff --git a/packages/eslint-plugin/rules/i18n-text-domain.js b/packages/eslint-plugin/rules/i18n-text-domain.js
index 202d36df3b4923..76300f6cc97bc8 100644
--- a/packages/eslint-plugin/rules/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/i18n-text-domain.js
@@ -1,7 +1,10 @@
 /**
  * Internal dependencies
  */
-const { TRANSLATION_FUNCTIONS } = require( '../utils' );
+const {
+	TRANSLATION_FUNCTIONS,
+	getTranslateFunctionName,
+} = require( '../utils' );
 
 /**
  * Returns the text domain passed to the given translation function.
@@ -63,10 +66,7 @@ module.exports = {
 			CallExpression( node ) {
 				const { callee, arguments: args } = node;
 
-				const functionName =
-					callee.property && callee.property.name
-						? callee.property.name
-						: callee.name;
+				const functionName = getTranslateFunctionName( callee );
 
 				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
 					return;
diff --git a/packages/eslint-plugin/rules/i18n-translator-comments.js b/packages/eslint-plugin/rules/i18n-translator-comments.js
index 8d85e2d3bc3e00..2cc1be195e0a03 100644
--- a/packages/eslint-plugin/rules/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/i18n-translator-comments.js
@@ -5,6 +5,7 @@ const {
 	TRANSLATION_FUNCTIONS,
 	REGEXP_PLACEHOLDER,
 	getTranslateStrings,
+	getTranslateFunctionName,
 } = require( '../utils' );
 
 module.exports = {
@@ -27,10 +28,7 @@ module.exports = {
 					arguments: args,
 				} = node;
 
-				const functionName =
-					callee.property && callee.property.name
-						? callee.property.name
-						: callee.name;
+				const functionName = getTranslateFunctionName( callee );
 
 				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
 					return;
diff --git a/packages/eslint-plugin/utils/get-text-content-from-node.js b/packages/eslint-plugin/utils/get-text-content-from-node.js
new file mode 100644
index 00000000000000..9ca8e7811b1523
--- /dev/null
+++ b/packages/eslint-plugin/utils/get-text-content-from-node.js
@@ -0,0 +1,38 @@
+/**
+ * Returns the actual text content from an argument passed to a translation function.
+ *
+ * @see eslint-plugin-wpcalypso
+ *
+ * @param  {Object} node    A Literal, TemplateLiteral or BinaryExpression (+) node
+ * @return {string|boolean} The concatenated string or false.
+ */
+function getTextContentFromNode( node ) {
+	if ( 'Literal' === node.type ) {
+		return node.value;
+	}
+
+	if ( 'BinaryExpression' === node.type && '+' === node.operator ) {
+		const left = getTextContentFromNode( node.left );
+		const right = getTextContentFromNode( node.right );
+
+		if ( left === false || right === false ) {
+			return false;
+		}
+
+		return left + right;
+	}
+
+	if ( node.type === 'TemplateLiteral' ) {
+		return node.quasis
+			.map( function( quasis ) {
+				return quasis.value.raw;
+			} )
+			.join( '' );
+	}
+
+	return false;
+}
+
+module.exports = {
+	getTextContentFromNode,
+};
diff --git a/packages/eslint-plugin/utils/get-translate-function-name.js b/packages/eslint-plugin/utils/get-translate-function-name.js
new file mode 100644
index 00000000000000..f62143572c62e6
--- /dev/null
+++ b/packages/eslint-plugin/utils/get-translate-function-name.js
@@ -0,0 +1,17 @@
+/**
+ * Get the actual translation function name from a CallExpression callee.
+ *
+ * Returns the "__" part from __ or i18n.__.
+ *
+ * @param {Object} callee
+ * @return {string} Function name.
+ */
+function getTranslateFunctionName( callee ) {
+	return callee.property && callee.property.name
+		? callee.property.name
+		: callee.name;
+}
+
+module.exports = {
+	getTranslateFunctionName,
+};
diff --git a/packages/eslint-plugin/utils/index.js b/packages/eslint-plugin/utils/index.js
index 759d4c6d5c436e..2fce7bc5a9b878 100644
--- a/packages/eslint-plugin/utils/index.js
+++ b/packages/eslint-plugin/utils/index.js
@@ -1,8 +1,12 @@
 const { TRANSLATION_FUNCTIONS, REGEXP_PLACEHOLDER } = require( './constants' );
 const { getTranslateStrings } = require( './get-translate-strings' );
+const { getTextContentFromNode } = require( './get-text-content-from-node' );
+const { getTranslateFunctionName } = require( './get-translate-function-name' );
 
 module.exports = {
 	TRANSLATION_FUNCTIONS,
 	REGEXP_PLACEHOLDER,
 	getTranslateStrings,
+	getTextContentFromNode,
+	getTranslateFunctionName,
 };

From 210759d49a711c9ba76558e52b3bc5c89f113c41 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 25 Mar 2020 13:02:39 +0100
Subject: [PATCH 17/41] Add new i18n-no-placeholders-only rule

---
 packages/eslint-plugin/CHANGELOG.md           |  3 +-
 packages/eslint-plugin/configs/custom.js      |  1 +
 .../docs/rules/i18n-no-placeholders-only.md   | 17 ++++++
 .../__tests__/i18n-no-placeholders-only.js    | 48 +++++++++++++++
 .../rules/i18n-no-placeholders-only.js        | 60 +++++++++++++++++++
 5 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
 create mode 100644 packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
 create mode 100644 packages/eslint-plugin/rules/i18n-no-placeholders-only.js

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 650743e8f1ff0d..37d43e7936616b 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -2,10 +2,11 @@
 
 ### New Features
 
+- The `prefer-const` rule included in the `recommended` and `esnext` rulesets has been relaxed to allow a `let` assignment if any of a [destructuring assignment](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) are reassigned.
 - New Rule: [`@wordpress/i18n-text-domain`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-text-domain.md)
 - New Rule: [`@wordpress/i18n-translator-comments`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-translator-comments.md)
-- The `prefer-const` rule included in the `recommended` and `esnext` rulesets has been relaxed to allow a `let` assignment if any of a [destructuring assignment](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) are reassigned.
 - New Rule: [`@wordpress/i18n-no-variables`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-variables.md)
+- New Rule: [`@wordpress/i18n-no-placeholders-only`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md)
 - New Rule: [`@wordpress/i18n-ellipsis`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-ellipsis.md)
 
 ## 4.0.0 (2020-02-10)
diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index e1d465ad121030..6ce3e43c8a77a1 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -7,6 +7,7 @@ module.exports = {
 		'@wordpress/no-unguarded-get-range-at': 'error',
 		'@wordpress/i18n-translator-comments': 'error',
 		'@wordpress/i18n-text-domain': 'error',
+		'@wordpress/i18n-no-placeholders-only': 'error',
 		'@wordpress/i18n-no-variables': 'error',
 		'@wordpress/i18n-ellipsis': 'error',
 		'no-restricted-syntax': [
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md b/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
new file mode 100644
index 00000000000000..d09f2caa3f061c
--- /dev/null
+++ b/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
@@ -0,0 +1,17 @@
+# Prevent using only placeholders in translatable strings (i18n-no-placeholders-only)
+
+Translatable strings that consist of nothing but a placeholder are rather pointless and not really translatable.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```js
+__('%s');
+```
+
+Examples of **correct** code for this rule:
+
+```js
+__('Hello %s');
+```
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js b/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
new file mode 100644
index 00000000000000..acc4e918b4e3be
--- /dev/null
+++ b/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
@@ -0,0 +1,48 @@
+/**
+ * External dependencies
+ */
+import { RuleTester } from 'eslint';
+
+/**
+ * Internal dependencies
+ */
+import rule from '../i18n-no-placeholders-only';
+
+const ruleTester = new RuleTester( {
+	parserOptions: {
+		ecmaVersion: 6,
+	},
+} );
+
+ruleTester.run( 'i18n-no-placeholders-only', rule, {
+	valid: [
+		{
+			code: `__('Hello %s')`,
+		},
+		{
+			code: `__('%d%%')`,
+		},
+	],
+	invalid: [
+		{
+			code: `__('%s')`,
+			errors: [ { messageId: 'noPlaceholdersOnly' } ],
+		},
+		{
+			code: `__('%s%s')`,
+			errors: [ { messageId: 'noPlaceholdersOnly' } ],
+		},
+		// @todo: Update placeholder regex, see https://github.com/WordPress/gutenberg/pull/20574.
+		/*{
+			code: `_x('%1$s')`,
+			errors: [ { messageId: 'noPlaceholdersOnly' } ],
+		},*/
+		{
+			code: `_n('%s', '%s', number)`,
+			errors: [
+				{ messageId: 'noPlaceholdersOnly' },
+				{ messageId: 'noPlaceholdersOnly' },
+			],
+		},
+	],
+} );
diff --git a/packages/eslint-plugin/rules/i18n-no-placeholders-only.js b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
new file mode 100644
index 00000000000000..8292f5e8c70285
--- /dev/null
+++ b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
@@ -0,0 +1,60 @@
+/**
+ * Internal dependencies
+ */
+const {
+	TRANSLATION_FUNCTIONS,
+	REGEXP_PLACEHOLDER,
+	getTextContentFromNode,
+	getTranslateFunctionName,
+} = require( '../utils' );
+
+module.exports = {
+	meta: {
+		type: 'problem',
+		schema: [],
+		messages: {
+			noPlaceholdersOnly:
+				'Translatable strings should not contain nothing but placeholders',
+		},
+	},
+	create( context ) {
+		return {
+			CallExpression( node ) {
+				const { callee, arguments: args } = node;
+
+				const functionName = getTranslateFunctionName( callee );
+
+				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+					return;
+				}
+
+				const functionArgs = [ args[ 0 ] ];
+
+				if ( [ '_n', '_nx' ].includes( functionName ) ) {
+					functionArgs.push( args[ 1 ] );
+				}
+
+				for ( const arg of functionArgs ) {
+					const argumentString = getTextContentFromNode( arg );
+					if ( ! argumentString ) {
+						continue;
+					}
+
+					const modifiedString = argumentString.replace(
+						REGEXP_PLACEHOLDER,
+						''
+					);
+
+					if ( modifiedString.length > 0 ) {
+						continue;
+					}
+
+					context.report( {
+						node,
+						messageId: 'noPlaceholdersOnly',
+					} );
+				}
+			},
+		};
+	},
+};

From 4511fd21ae5c9cc8d017d75e43506a95b9c3381a Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Wed, 25 Mar 2020 13:28:53 +0100
Subject: [PATCH 18/41] Add new i18n-no-collapsible-whitespace rule

---
 packages/eslint-plugin/CHANGELOG.md           |  1 +
 packages/eslint-plugin/configs/custom.js      |  1 +
 .../rules/i18n-no-collapsible-whitespace.md   | 17 +++++
 .../i18n-no-collapsible-whitespace.js         | 62 ++++++++++++++++
 .../rules/i18n-no-collapsible-whitespace.js   | 74 +++++++++++++++++++
 5 files changed, 155 insertions(+)
 create mode 100644 packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
 create mode 100644 packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js
 create mode 100644 packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 37d43e7936616b..842f2865bcc6cf 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -7,6 +7,7 @@
 - New Rule: [`@wordpress/i18n-translator-comments`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-translator-comments.md)
 - New Rule: [`@wordpress/i18n-no-variables`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-variables.md)
 - New Rule: [`@wordpress/i18n-no-placeholders-only`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md)
+- New Rule: [`@wordpress/i18n-no-collapsible-whitespace`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md)
 - New Rule: [`@wordpress/i18n-ellipsis`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-ellipsis.md)
 
 ## 4.0.0 (2020-02-10)
diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index 6ce3e43c8a77a1..04340b13339b75 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -7,6 +7,7 @@ module.exports = {
 		'@wordpress/no-unguarded-get-range-at': 'error',
 		'@wordpress/i18n-translator-comments': 'error',
 		'@wordpress/i18n-text-domain': 'error',
+		'@wordpress/i18n-no-collapsible-whitespace': 'error',
 		'@wordpress/i18n-no-placeholders-only': 'error',
 		'@wordpress/i18n-no-variables': 'error',
 		'@wordpress/i18n-ellipsis': 'error',
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md b/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
new file mode 100644
index 00000000000000..044da4d72f56cc
--- /dev/null
+++ b/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
@@ -0,0 +1,17 @@
+# Disallow collapsible whitespace in translatable strings. (i18n-no-collapsible-whitespace)
+
+Translatable strings that consist of nothing but a placeholder are rather pointless and not really translatable.
+
+## Rule details
+
+Examples of **incorrect** code for this rule:
+
+```js
+__('%s');
+```
+
+Examples of **correct** code for this rule:
+
+```js
+__('Hello %s');
+```
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js b/packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js
new file mode 100644
index 00000000000000..5746eb48584947
--- /dev/null
+++ b/packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js
@@ -0,0 +1,62 @@
+/**
+ * External dependencies
+ */
+import { RuleTester } from 'eslint';
+
+/**
+ * Internal dependencies
+ */
+import rule from '../i18n-no-collapsible-whitespace';
+
+const ruleTester = new RuleTester( {
+	parserOptions: {
+		ecmaVersion: 6,
+	},
+} );
+
+ruleTester.run( 'i18n-no-collapsible-whitespace', rule, {
+	valid: [
+		{
+			code: `__('Hello World…')`,
+		},
+		{
+			code:
+				'__( `A long string ` +\n `spread over ` +\n  `multiple lines.` );',
+		},
+	],
+	invalid: [
+		{
+			code: '__( "My double-quoted string\\nwith a newline" );',
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+		{
+			code: "__( 'My single quoted string\\nwith a newline' );",
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+		{
+			code: '__( `My template literal\non two lines` );',
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+		{
+			code: "__( '	My tab-indented string.' );",
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+		{
+			code: "__( '\tMy string with a tab escape sequence.' );",
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+		{
+			code: "__( '\u0009My string with a unicode tab.' );",
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+		{
+			code: '__( `A string with \r a carriage return.` );',
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+		{
+			code:
+				"__( 'A string with consecutive spaces.  These two are after a full stop.' );",
+			errors: [ { messageId: 'noCollapsibleWhitespace' } ],
+		},
+	],
+} );
diff --git a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
new file mode 100644
index 00000000000000..bb71ed11aff274
--- /dev/null
+++ b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
@@ -0,0 +1,74 @@
+/**
+ * Internal dependencies
+ */
+const {
+	TRANSLATION_FUNCTIONS,
+	getTextContentFromNode,
+	getTranslateFunctionName,
+} = require( '../utils' );
+
+module.exports = {
+	meta: {
+		type: 'problem',
+		schema: [],
+		messages: {
+			noCollapsibleWhitespace:
+				'Translations should not contain collapsible whitespace{{problem}}',
+		},
+	},
+	create( context ) {
+		return {
+			CallExpression( node ) {
+				const { callee, arguments: args } = node;
+
+				const functionName = getTranslateFunctionName( callee );
+
+				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+					return;
+				}
+
+				const functionArgs = [ args[ 0 ] ];
+
+				if ( [ '_n', '_nx' ].includes( functionName ) ) {
+					functionArgs.push( args[ 1 ] );
+				}
+
+				const problemsByCharCode = {
+					9: '\\t',
+					10: '\\n',
+					13: '\\r',
+					32: 'consecutive spaces',
+				};
+
+				for ( const arg of functionArgs ) {
+					const argumentString = getTextContentFromNode( arg );
+					if ( ! argumentString ) {
+						continue;
+					}
+
+					const collapsibleWhitespace = argumentString.match(
+						/(\n|\t|\r|(?: {2}))/
+					);
+
+					if ( ! collapsibleWhitespace ) {
+						continue;
+					}
+
+					const problem =
+						problemsByCharCode[
+							collapsibleWhitespace[ 0 ].charCodeAt( 0 )
+						];
+					const problemString = problem ? ` (${ problem })` : '';
+
+					context.report( {
+						node,
+						messageId: 'noCollapsibleWhitespace',
+						data: {
+							problem: problemString,
+						},
+					} );
+				}
+			},
+		};
+	},
+};

From 0d8b751f3c2938488fe5967a995cb2e584da757a Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Mon, 2 Mar 2020 10:33:18 +0100
Subject: [PATCH 19/41] Use messageId in valid-sprintf rule

---
 .../rules/__tests__/valid-sprintf.js          | 53 +++----------------
 packages/eslint-plugin/rules/valid-sprintf.js | 41 ++++++++------
 2 files changed, 34 insertions(+), 60 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
index 119653220b3c0a..7cc9f06a81af72 100644
--- a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
@@ -41,72 +41,35 @@ ruleTester.run( 'valid-sprintf', rule, {
 	invalid: [
 		{
 			code: `sprintf()`,
-			errors: [
-				{ message: 'sprintf must be called with a format string' },
-			],
+			errors: [ { messageId: 'noFormatString' } ],
 		},
 		{
 			code: `sprintf( '%s' )`,
-			errors: [
-				{
-					message:
-						'sprintf must be called with placeholder value argument(s)',
-				},
-			],
+			errors: [ { messageId: 'noPlaceholderArgs' } ],
 		},
 		{
 			code: `sprintf( 1, 'substitute' )`,
-			errors: [
-				{
-					message:
-						'sprintf must be called with a valid format string',
-				},
-			],
+			errors: [ { messageId: 'invalidFormatString' } ],
 		},
 		{
 			code: `sprintf( [], 'substitute' )`,
-			errors: [
-				{
-					message:
-						'sprintf must be called with a valid format string',
-				},
-			],
+			errors: [ { messageId: 'invalidFormatString' } ],
 		},
 		{
 			code: `sprintf( '%%', 'substitute' )`,
-			errors: [
-				{
-					message:
-						'sprintf format string must contain at least one placeholder',
-				},
-			],
+			errors: [ { messageId: 'noPlaceholders' } ],
 		},
 		{
 			code: `sprintf( __( '%%' ), 'substitute' )`,
-			errors: [
-				{
-					message:
-						'sprintf format string must contain at least one placeholder',
-				},
-			],
+			errors: [ { messageId: 'noPlaceholders' } ],
 		},
 		{
 			code: `sprintf( _n( '%s', '' ), 'substitute' )`,
-			errors: [
-				{
-					message:
-						'sprintf format string options must have the same number of placeholders',
-				},
-			],
+			errors: [ { messageId: 'placeholderMismatch' } ],
 		},
 		{
 			code: `sprintf( _n( '%s', '%s %s' ), 'substitute' )`,
-			errors: [
-				{
-					message:
-						'sprintf format string options must have the same number of placeholders',
-				},
-			],
+			errors: [ { messageId: 'placeholderMismatch' } ],
 		},
 	],
 } );
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index 6049c41a574d04..0c7d2bddcef4ad 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -7,6 +7,17 @@ module.exports = {
 	meta: {
 		type: 'problem',
 		schema: [],
+		messages: {
+			noFormatString: 'sprintf must be called with a format string',
+			invalidFormatString:
+				'sprintf must be called with a valid format string',
+			noPlaceholderArgs:
+				'sprintf must be called with placeholder value argument(s)',
+			noPlaceholders:
+				'sprintf format string must contain at least one placeholder',
+			placeholderMismatch:
+				'sprintf format string options must have the same number of placeholders',
+		},
 	},
 	create( context ) {
 		return {
@@ -17,18 +28,18 @@ module.exports = {
 				}
 
 				if ( ! args.length ) {
-					context.report(
+					context.report( {
 						node,
-						'sprintf must be called with a format string'
-					);
+						messageId: 'noFormatString',
+					} );
 					return;
 				}
 
 				if ( args.length < 2 ) {
-					context.report(
+					context.report( {
 						node,
-						'sprintf must be called with placeholder value argument(s)'
-					);
+						messageId: 'noPlaceholderArgs',
+					} );
 					return;
 				}
 
@@ -70,10 +81,10 @@ module.exports = {
 				}
 
 				if ( ! candidates.length ) {
-					context.report(
+					context.report( {
 						node,
-						'sprintf must be called with a valid format string'
-					);
+						messageId: 'invalidFormatString',
+					} );
 					return;
 				}
 
@@ -88,18 +99,18 @@ module.exports = {
 						numPlaceholders !== undefined &&
 						( ! match || numPlaceholders !== match.length )
 					) {
-						context.report(
+						context.report( {
 							node,
-							'sprintf format string options must have the same number of placeholders'
-						);
+							messageId: 'placeholderMismatch',
+						} );
 						return;
 					}
 
 					if ( ! match ) {
-						context.report(
+						context.report( {
 							node,
-							'sprintf format string must contain at least one placeholder'
-						);
+							messageId: 'noPlaceholders',
+						} );
 						return;
 					}
 

From 40fd705cb04bebcc563907ea742f334d7da7affb Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Thu, 19 Mar 2020 13:11:45 +0100
Subject: [PATCH 20/41] Catch mix of ordered and non-ordered placeholders in
 valid-sprintf rule

---
 packages/eslint-plugin/CHANGELOG.md           |  4 ++
 .../rules/__tests__/valid-sprintf.js          | 27 +++++++++++++
 .../rules/i18n-translator-comments.js         |  4 +-
 packages/eslint-plugin/rules/valid-sprintf.js | 38 ++++++++++++++++---
 packages/eslint-plugin/utils/constants.js     | 20 ++++++++--
 packages/eslint-plugin/utils/index.js         |  9 ++++-
 6 files changed, 89 insertions(+), 13 deletions(-)

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index c993fda5eea06c..93aace18536243 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -6,6 +6,10 @@
 - New Rule: [`@wordpress/i18n-translator-comments`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-translator-comments.md)
 - The `prefer-const` rule included in the `recommended` and `esnext` rulesets has been relaxed to allow a `let` assignment if any of a [destructuring assignment](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) are reassigned.
 
+### Enhancements
+
+- The `@wordpress/valid-sprintf` rule now recognizes mix of ordered and non-ordered placeholders.
+
 ## 4.0.0 (2020-02-10)
 
 ### Breaking Changes
diff --git a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
index 7cc9f06a81af72..0e105b0697e4ec 100644
--- a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
@@ -37,6 +37,19 @@ ruleTester.run( 'valid-sprintf', rule, {
 		{
 			code: `var value = ''; sprintf( value, 'substitute' )`,
 		},
+		{
+			code: `
+sprintf(
+	/* translators: 1: number of blocks. 2: average rating. */
+	_n(
+		'This author has %1$d block, with an average rating of %2$d.',
+		'This author has %1$d blocks, with an average rating of %2$d.',
+		authorBlockCount
+	),
+	authorBlockCount,
+	authorBlockRating
+);`,
+		},
 	],
 	invalid: [
 		{
@@ -71,5 +84,19 @@ ruleTester.run( 'valid-sprintf', rule, {
 			code: `sprintf( _n( '%s', '%s %s' ), 'substitute' )`,
 			errors: [ { messageId: 'placeholderMismatch' } ],
 		},
+		{
+			code: `
+sprintf(
+	/* translators: 1: number of blocks. 2: average rating. */
+	_n(
+		'This author has %d block, with an average rating of %d.',
+		'This author has %d blocks, with an average rating of %d.',
+		authorBlockCount
+	),
+	authorBlockCount,
+	authorBlockRating
+);`,
+			errors: [ { messageId: 'noNumberedPlaceholders' } ],
+		},
 	],
 } );
diff --git a/packages/eslint-plugin/rules/i18n-translator-comments.js b/packages/eslint-plugin/rules/i18n-translator-comments.js
index 8d85e2d3bc3e00..77cec31caaf0e1 100644
--- a/packages/eslint-plugin/rules/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/i18n-translator-comments.js
@@ -3,7 +3,7 @@
  */
 const {
 	TRANSLATION_FUNCTIONS,
-	REGEXP_PLACEHOLDER,
+	SPRINTF_PLACEHOLDER_REGEX,
 	getTranslateStrings,
 } = require( '../utils' );
 
@@ -43,7 +43,7 @@ module.exports = {
 				}
 
 				const hasPlaceholders = candidates.some( ( candidate ) =>
-					candidate.match( REGEXP_PLACEHOLDER )
+					candidate.match( SPRINTF_PLACEHOLDER_REGEX )
 				);
 
 				if ( ! hasPlaceholders ) {
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index 0c7d2bddcef4ad..1becf83b638e7a 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -1,7 +1,11 @@
 /**
  * Internal dependencies
  */
-const { REGEXP_PLACEHOLDER, getTranslateStrings } = require( '../utils' );
+const {
+	SPRINTF_PLACEHOLDER_REGEX,
+	UNORDERED_SPRINTF_PLACEHOLDER_REGEX,
+	getTranslateStrings,
+} = require( '../utils' );
 
 module.exports = {
 	meta: {
@@ -17,6 +21,8 @@ module.exports = {
 				'sprintf format string must contain at least one placeholder',
 			placeholderMismatch:
 				'sprintf format string options must have the same number of placeholders',
+			noNumberedPlaceholders:
+				'Multiple sprintf placeholders should be ordered. Mix of ordered and non-ordered placeholders found.',
 		},
 	},
 	create( context ) {
@@ -89,15 +95,21 @@ module.exports = {
 				}
 
 				let numPlaceholders;
-				for ( let i = 0; i < candidates.length; i++ ) {
-					const match = candidates[ i ].match( REGEXP_PLACEHOLDER );
+				for ( const candidate of candidates ) {
+					const allMatches = candidate.match(
+						SPRINTF_PLACEHOLDER_REGEX
+					);
+					const unorderedMatches = candidate.match(
+						UNORDERED_SPRINTF_PLACEHOLDER_REGEX
+					);
 
 					// Prioritize placeholder number consistency over matching
 					// placeholder, since it's a more common error to omit a
 					// placeholder from the singular form of pluralization.
 					if (
 						numPlaceholders !== undefined &&
-						( ! match || numPlaceholders !== match.length )
+						( ! allMatches ||
+							numPlaceholders !== allMatches.length )
 					) {
 						context.report( {
 							node,
@@ -106,7 +118,21 @@ module.exports = {
 						return;
 					}
 
-					if ( ! match ) {
+					if (
+						unorderedMatches &&
+						allMatches &&
+						unorderedMatches.length > 0 &&
+						allMatches.length > 1 &&
+						unorderedMatches.length !== allMatches.length
+					) {
+						context.report( {
+							node,
+							messageId: 'noNumberedPlaceholders',
+						} );
+						return;
+					}
+
+					if ( ! allMatches ) {
 						context.report( {
 							node,
 							messageId: 'noPlaceholders',
@@ -118,7 +144,7 @@ module.exports = {
 						// Track the number of placeholders discovered in the
 						// string to verify that all other candidate options
 						// have the same number.
-						numPlaceholders = match.length;
+						numPlaceholders = allMatches.length;
 					}
 				}
 			},
diff --git a/packages/eslint-plugin/utils/constants.js b/packages/eslint-plugin/utils/constants.js
index 5d3607929d7e40..ce56a438c74c0c 100644
--- a/packages/eslint-plugin/utils/constants.js
+++ b/packages/eslint-plugin/utils/constants.js
@@ -7,13 +7,27 @@ const TRANSLATION_FUNCTIONS = [ '__', '_x', '_n', '_nx' ];
 
 /**
  * Regular expression matching the presence of a printf format string
- * placeholder. This naive pattern which does not validate the format.
+ * placeholder.
+ *
+ * Originally copied from http://php.net/manual/en/function.sprintf.php#93552.
+ *
+ * @see https://github.com/WordPress/WordPress-Coding-Standards/blob/2f927b0ba2bfcbffaa8f3251c086e109302d6622/WordPress/Sniffs/WP/I18nSniff.php#L37-L60
+ *
+ * @type {RegExp}
+ */
+const REGEXP_SPRINTF_PLACEHOLDER = /(?:(?<!%)(%(?:[0-9]+\$)?[+-]?(?:(?:0|\'.)?-?[0-9]*(?:\.(?:[ 0]|\'.)?[0-9]+)?|(?:[ ])?-?[0-9]+(?:\.(?:[ 0]|\'.)?[0-9]+)?)[bcdeEfFgGosuxX]))/g;
+
+/**
+ * "Unordered" means there's no position specifier: '%s', not '%2$s'.
+ *
+ * @see https://github.com/WordPress/WordPress-Coding-Standards/blob/2f927b0ba2bfcbffaa8f3251c086e109302d6622/WordPress/Sniffs/WP/I18nSniff.php#L62-L81
  *
  * @type {RegExp}
  */
-const REGEXP_PLACEHOLDER = /%[^%]/g;
+const REGEXP_SPRINTF_PLACEHOLDER_UNORDERED = /(?:(?<!%)%[+-]?(?:(?:0|'.)?-?[0-9]*(?:\.(?:[ 0]|'.)?[0-9]+)?|(?:[ ])?-?[0-9]+(?:\.(?:[ 0]|'.)?[0-9]+)?)[bcdeEfFgGosuxX])/;
 
 module.exports = {
 	TRANSLATION_FUNCTIONS,
-	REGEXP_PLACEHOLDER,
+	REGEXP_SPRINTF_PLACEHOLDER,
+	REGEXP_SPRINTF_PLACEHOLDER_UNORDERED,
 };
diff --git a/packages/eslint-plugin/utils/index.js b/packages/eslint-plugin/utils/index.js
index 759d4c6d5c436e..aa70b2ab54c12c 100644
--- a/packages/eslint-plugin/utils/index.js
+++ b/packages/eslint-plugin/utils/index.js
@@ -1,8 +1,13 @@
-const { TRANSLATION_FUNCTIONS, REGEXP_PLACEHOLDER } = require( './constants' );
+const {
+	TRANSLATION_FUNCTIONS,
+	REGEXP_SPRINTF_PLACEHOLDER,
+	REGEXP_SPRINTF_PLACEHOLDER_UNORDERED,
+} = require( './constants' );
 const { getTranslateStrings } = require( './get-translate-strings' );
 
 module.exports = {
 	TRANSLATION_FUNCTIONS,
-	REGEXP_PLACEHOLDER,
+	REGEXP_SPRINTF_PLACEHOLDER,
+	REGEXP_SPRINTF_PLACEHOLDER_UNORDERED,
 	getTranslateStrings,
 };

From 70401a46697d55588c2dcb1fb5c6363313f59725 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Mon, 2 Mar 2020 11:24:56 +0100
Subject: [PATCH 21/41] Fix code base after change

---
 packages/blocks/src/api/utils.js | 12 ++++++------
 packages/scripts/utils/env.js    |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/packages/blocks/src/api/utils.js b/packages/blocks/src/api/utils.js
index 11b333d8bdef23..dbf1dc79049b9d 100644
--- a/packages/blocks/src/api/utils.js
+++ b/packages/blocks/src/api/utils.js
@@ -184,7 +184,7 @@ export function getAccessibleBlockLabel(
 	if ( hasPosition && direction === 'vertical' ) {
 		if ( hasLabel ) {
 			return sprintf(
-				/* translators: accessibility text. %1: The block title, %2: The block row number, %3: The block label.. */
+				/* translators: accessibility text. 1: The block title. 2: The block row number. 3: The block label.. */
 				__( '%1$s Block. Row %2$d. %3$s' ),
 				title,
 				position,
@@ -193,15 +193,15 @@ export function getAccessibleBlockLabel(
 		}
 
 		return sprintf(
-			/* translators: accessibility text. %s: The block title, %d The block row number. */
-			__( '%s Block. Row %d' ),
+			/* translators: accessibility text. 1: The block title. 2: The block row number. */
+			__( '%1$s Block. Row %2$d' ),
 			title,
 			position
 		);
 	} else if ( hasPosition && direction === 'horizontal' ) {
 		if ( hasLabel ) {
 			return sprintf(
-				/* translators: accessibility text. %1: The block title, %2: The block column number, %3: The block label.. */
+				/* translators: accessibility text. 1: The block title. 2: The block column number. 3: The block label.. */
 				__( '%1$s Block. Column %2$d. %3$s' ),
 				title,
 				position,
@@ -210,8 +210,8 @@ export function getAccessibleBlockLabel(
 		}
 
 		return sprintf(
-			/* translators: accessibility text. %s: The block title, %d The block column number. */
-			__( '%s Block. Column %d' ),
+			/* translators: accessibility text. 1: The block title. 2: The block column number. */
+			__( '%1$s Block. Column %2$d' ),
 			title,
 			position
 		);
diff --git a/packages/scripts/utils/env.js b/packages/scripts/utils/env.js
index 372ef3479192dc..284131217d602b 100644
--- a/packages/scripts/utils/env.js
+++ b/packages/scripts/utils/env.js
@@ -176,7 +176,7 @@ function buildWordPress( newInstall, fastInstall ) {
 
 		if ( env.npm_package_wp_env_welcome_build_command ) {
 			const nextStep = sprintf(
-				'\nRun %s to build the latest version of %s, then open %s to get started!\n',
+				'\nRun %1$s to build the latest version of %2$s, then open %3$s to get started!\n',
 				chalk.blue( env.npm_package_wp_env_welcome_build_command ),
 				chalk.green( env.npm_package_wp_env_plugin_name ),
 				chalk.blue( currentUrl )
@@ -191,7 +191,7 @@ function buildWordPress( newInstall, fastInstall ) {
 		);
 
 		const access = sprintf(
-			'Default username: %s, password: %s\n',
+			'Default username: %1$s, password: %2$s\n',
 			chalk.blue( 'admin' ),
 			chalk.blue( 'password' )
 		);

From b0fba98c1ea72bbea9655efa2a65739d0895958e Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Thu, 19 Mar 2020 13:13:13 +0100
Subject: [PATCH 22/41] Mark as breaking change in the readme

---
 packages/eslint-plugin/CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 93aace18536243..1ec508f2019668 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -6,7 +6,7 @@
 - New Rule: [`@wordpress/i18n-translator-comments`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-translator-comments.md)
 - The `prefer-const` rule included in the `recommended` and `esnext` rulesets has been relaxed to allow a `let` assignment if any of a [destructuring assignment](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment) are reassigned.
 
-### Enhancements
+### Breaking Changes
 
 - The `@wordpress/valid-sprintf` rule now recognizes mix of ordered and non-ordered placeholders.
 

From 85d6fc27faefad7b74dc5945df33f8f70ea4f4cc Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Thu, 19 Mar 2020 15:10:37 +0100
Subject: [PATCH 23/41] Support `i18n.*` usage in `valid-sprintf` rule

---
 packages/eslint-plugin/CHANGELOG.md              |  4 ++++
 .../rules/__tests__/valid-sprintf.js             | 14 ++++++++++++++
 packages/eslint-plugin/rules/valid-sprintf.js    | 16 ++++++++++++++--
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 1ec508f2019668..a821d129ed57ce 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -10,6 +10,10 @@
 
 - The `@wordpress/valid-sprintf` rule now recognizes mix of ordered and non-ordered placeholders.
 
+### Bug Fix
+
+- The `@wordpress/valid-sprintf` rule now detects usage of `sprintf` via `i18n.sprintf` (e.g. when using `import * as i18n from '@wordpress/i18n'`).
+
 ## 4.0.0 (2020-02-10)
 
 ### Breaking Changes
diff --git a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
index 0e105b0697e4ec..fb5a3f665001c9 100644
--- a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
@@ -50,6 +50,12 @@ sprintf(
 	authorBlockRating
 );`,
 		},
+		{
+			code: `i18n.sprintf( '%s', 'substitute' )`,
+		},
+		{
+			code: `i18n.sprintf( i18n.__( '%s' ), 'substitute' )`,
+		},
 	],
 	invalid: [
 		{
@@ -98,5 +104,13 @@ sprintf(
 );`,
 			errors: [ { messageId: 'noNumberedPlaceholders' } ],
 		},
+		{
+			code: `i18n.sprintf()`,
+			errors: [ { messageId: 'noFormatString' } ],
+		},
+		{
+			code: `i18n.sprintf( i18n.__( '%%' ), 'substitute' )`,
+			errors: [ { messageId: 'noPlaceholders' } ],
+		},
 	],
 } );
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index 1becf83b638e7a..b14b850976c622 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -29,7 +29,13 @@ module.exports = {
 		return {
 			CallExpression( node ) {
 				const { callee, arguments: args } = node;
-				if ( callee.name !== 'sprintf' ) {
+
+				const functionName =
+					callee.property && callee.property.name
+						? callee.property.name
+						: callee.name;
+
+				if ( functionName !== 'sprintf' ) {
 					return;
 				}
 
@@ -60,10 +66,16 @@ module.exports = {
 						break;
 
 					case 'CallExpression':
+						const argFunctionName =
+							args[ 0 ].callee.property &&
+							args[ 0 ].callee.property.name
+								? args[ 0 ].callee.property.name
+								: args[ 0 ].callee.name;
+
 						// All possible options (arguments) from a translate
 						// function must be valid.
 						candidates = getTranslateStrings(
-							args[ 0 ].callee.name,
+							argFunctionName,
 							args[ 0 ].arguments
 						);
 

From bee529ea2809c48e55b1eeb2085c0acb000e4b0c Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Thu, 26 Mar 2020 13:30:11 +0100
Subject: [PATCH 24/41] Disable i18n-no-collapsible-whitespace rule for now

---
 .eslintrc.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index dd8456a8570c88..8427c936cf4b02 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -48,7 +48,7 @@ module.exports = {
 				allowedTextDomains: [ 'default' ],
 			},
 		],
-		'@wordpress/i18n-translator-comments': 'error',
+		'@wordpress/i18n-no-collapsible-whitespace': 'off',
 		'no-restricted-syntax': [
 			'error',
 			// NOTE: We can't include the forward slash in our regex or

From d388f13008dda35ed98552f454b063fe5aceb659 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 10:25:47 +0100
Subject: [PATCH 25/41] Remove unneded capture group

---
 packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
index bb71ed11aff274..3a0baae60c92a8 100644
--- a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
+++ b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
@@ -47,7 +47,7 @@ module.exports = {
 					}
 
 					const collapsibleWhitespace = argumentString.match(
-						/(\n|\t|\r|(?: {2}))/
+						/(\n|\t|\r| {2})/
 					);
 
 					if ( ! collapsibleWhitespace ) {

From 67e38e540db2d6b4c09b672c845a681071d2671e Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 10:27:38 +0100
Subject: [PATCH 26/41] Use Set for list of translation functions

---
 packages/eslint-plugin/rules/i18n-ellipsis.js                 | 2 +-
 .../eslint-plugin/rules/i18n-no-collapsible-whitespace.js     | 2 +-
 packages/eslint-plugin/rules/i18n-no-placeholders-only.js     | 2 +-
 packages/eslint-plugin/rules/i18n-no-variables.js             | 2 +-
 packages/eslint-plugin/rules/i18n-text-domain.js              | 2 +-
 packages/eslint-plugin/rules/i18n-translator-comments.js      | 2 +-
 packages/eslint-plugin/utils/constants.js                     | 4 ++--
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/packages/eslint-plugin/rules/i18n-ellipsis.js b/packages/eslint-plugin/rules/i18n-ellipsis.js
index 28c9e93186382b..fc4cd9b2e59d6c 100644
--- a/packages/eslint-plugin/rules/i18n-ellipsis.js
+++ b/packages/eslint-plugin/rules/i18n-ellipsis.js
@@ -67,7 +67,7 @@ module.exports = {
 
 				const functionName = getTranslateFunctionName( callee );
 
-				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+				if ( ! TRANSLATION_FUNCTIONS.has( functionName ) ) {
 					return;
 				}
 
diff --git a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
index 3a0baae60c92a8..f0a75a66805848 100644
--- a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
+++ b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
@@ -23,7 +23,7 @@ module.exports = {
 
 				const functionName = getTranslateFunctionName( callee );
 
-				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+				if ( ! TRANSLATION_FUNCTIONS.has( functionName ) ) {
 					return;
 				}
 
diff --git a/packages/eslint-plugin/rules/i18n-no-placeholders-only.js b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
index 8292f5e8c70285..8920c41eacfcc3 100644
--- a/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
+++ b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
@@ -24,7 +24,7 @@ module.exports = {
 
 				const functionName = getTranslateFunctionName( callee );
 
-				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+				if ( ! TRANSLATION_FUNCTIONS.has( functionName ) ) {
 					return;
 				}
 
diff --git a/packages/eslint-plugin/rules/i18n-no-variables.js b/packages/eslint-plugin/rules/i18n-no-variables.js
index b5fd69ab0fc4eb..34860f773ffec2 100644
--- a/packages/eslint-plugin/rules/i18n-no-variables.js
+++ b/packages/eslint-plugin/rules/i18n-no-variables.js
@@ -40,7 +40,7 @@ module.exports = {
 
 				const functionName = getTranslateFunctionName( callee );
 
-				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+				if ( ! TRANSLATION_FUNCTIONS.has( functionName ) ) {
 					return;
 				}
 
diff --git a/packages/eslint-plugin/rules/i18n-text-domain.js b/packages/eslint-plugin/rules/i18n-text-domain.js
index 76300f6cc97bc8..e6afe1810e9119 100644
--- a/packages/eslint-plugin/rules/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/i18n-text-domain.js
@@ -68,7 +68,7 @@ module.exports = {
 
 				const functionName = getTranslateFunctionName( callee );
 
-				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+				if ( ! TRANSLATION_FUNCTIONS.has( functionName ) ) {
 					return;
 				}
 
diff --git a/packages/eslint-plugin/rules/i18n-translator-comments.js b/packages/eslint-plugin/rules/i18n-translator-comments.js
index 2cc1be195e0a03..ac3b00720d4f4b 100644
--- a/packages/eslint-plugin/rules/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/i18n-translator-comments.js
@@ -30,7 +30,7 @@ module.exports = {
 
 				const functionName = getTranslateFunctionName( callee );
 
-				if ( ! TRANSLATION_FUNCTIONS.includes( functionName ) ) {
+				if ( ! TRANSLATION_FUNCTIONS.has( functionName ) ) {
 					return;
 				}
 
diff --git a/packages/eslint-plugin/utils/constants.js b/packages/eslint-plugin/utils/constants.js
index 5d3607929d7e40..5ef2980ed62553 100644
--- a/packages/eslint-plugin/utils/constants.js
+++ b/packages/eslint-plugin/utils/constants.js
@@ -1,9 +1,9 @@
 /**
  * List of translation functions exposed by the `@wordpress/i18n` package.
  *
- * @type {string[]} Translation functions.
+ * @type {Set<string>} Translation functions.
  */
-const TRANSLATION_FUNCTIONS = [ '__', '_x', '_n', '_nx' ];
+const TRANSLATION_FUNCTIONS = new Set( [ '__', '_x', '_n', '_nx' ] );
 
 /**
  * Regular expression matching the presence of a printf format string

From 545790bcc09c53d883f13c2d32826f570a6f4003 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 10:28:53 +0100
Subject: [PATCH 27/41] Move const to top scope

---
 .../rules/i18n-no-collapsible-whitespace.js      | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
index f0a75a66805848..cd38f8920f9d4a 100644
--- a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
+++ b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
@@ -7,6 +7,13 @@ const {
 	getTranslateFunctionName,
 } = require( '../utils' );
 
+const PROBLEMS_BY_CHAR_CODE = {
+	9: '\\t',
+	10: '\\n',
+	13: '\\r',
+	32: 'consecutive spaces',
+};
+
 module.exports = {
 	meta: {
 		type: 'problem',
@@ -33,13 +40,6 @@ module.exports = {
 					functionArgs.push( args[ 1 ] );
 				}
 
-				const problemsByCharCode = {
-					9: '\\t',
-					10: '\\n',
-					13: '\\r',
-					32: 'consecutive spaces',
-				};
-
 				for ( const arg of functionArgs ) {
 					const argumentString = getTextContentFromNode( arg );
 					if ( ! argumentString ) {
@@ -55,7 +55,7 @@ module.exports = {
 					}
 
 					const problem =
-						problemsByCharCode[
+						PROBLEMS_BY_CHAR_CODE[
 							collapsibleWhitespace[ 0 ].charCodeAt( 0 )
 						];
 					const problemString = problem ? ` (${ problem })` : '';

From 05a8920c446c97e7527bfa483592a61825f41435 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 10:36:25 +0100
Subject: [PATCH 28/41] Coding standards in code examples

---
 packages/eslint-plugin/docs/rules/i18n-ellipsis.md        | 4 ++--
 .../docs/rules/i18n-no-collapsible-whitespace.md          | 4 ++--
 .../eslint-plugin/docs/rules/i18n-no-placeholders-only.md | 4 ++--
 packages/eslint-plugin/docs/rules/i18n-no-variables.md    | 8 ++++----
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/packages/eslint-plugin/docs/rules/i18n-ellipsis.md b/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
index 5c6e7d20c74f6a..752b57ea52ac9e 100644
--- a/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
+++ b/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
@@ -7,12 +7,12 @@ Translatable strings should use ellipsis (…) instead of three dots.
 Examples of **incorrect** code for this rule:
 
 ```js
-__('Continue...');
+__( 'Continue...' );
 
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__('Continue…');
+__( 'Continue…' );
 ```
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md b/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
index 044da4d72f56cc..e60ae22c73bf62 100644
--- a/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
+++ b/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
@@ -7,11 +7,11 @@ Translatable strings that consist of nothing but a placeholder are rather pointl
 Examples of **incorrect** code for this rule:
 
 ```js
-__('%s');
+__( '%s' );
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__('Hello %s');
+__( 'Hello %s' );
 ```
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md b/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
index d09f2caa3f061c..a1dcd915df259c 100644
--- a/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
+++ b/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
@@ -7,11 +7,11 @@ Translatable strings that consist of nothing but a placeholder are rather pointl
 Examples of **incorrect** code for this rule:
 
 ```js
-__('%s');
+__( '%s' );
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__('Hello %s');
+__( 'Hello %s' );
 ```
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-variables.md b/packages/eslint-plugin/docs/rules/i18n-no-variables.md
index 5f8a0f64460e37..e40aff4076eb29 100644
--- a/packages/eslint-plugin/docs/rules/i18n-no-variables.md
+++ b/packages/eslint-plugin/docs/rules/i18n-no-variables.md
@@ -7,14 +7,14 @@
 Examples of **incorrect** code for this rule:
 
 ```js
-__('Hello World');
-_x('Hello World', 'context', 'foo');
+__( 'Hello World' );
+_x( 'Hello World', 'context', 'foo' );
 
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__(`Hello ${foo}`);
-_x('Hello World', bar);
+__( `Hello ${foo}` );
+_x( 'Hello World', bar );
 ```

From f845baea12d9ecb5c8e99e60e252a315986c6fde Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 11:14:48 +0100
Subject: [PATCH 29/41] Refactor utils to make code more DRY

---
 packages/eslint-plugin/rules/i18n-ellipsis.js | 12 +++---
 .../rules/i18n-no-collapsible-whitespace.js   | 12 +++---
 .../rules/i18n-no-placeholders-only.js        | 12 +++---
 .../eslint-plugin/rules/i18n-no-variables.js  | 16 +++-----
 .../rules/i18n-translator-comments.js         | 12 ++++--
 packages/eslint-plugin/rules/valid-sprintf.js | 15 ++++---
 .../utils/get-translate-function-args.js      | 40 +++++++++++++++++++
 .../utils/get-translate-strings.js            | 34 ----------------
 packages/eslint-plugin/utils/index.js         |  4 +-
 9 files changed, 85 insertions(+), 72 deletions(-)
 create mode 100644 packages/eslint-plugin/utils/get-translate-function-args.js
 delete mode 100644 packages/eslint-plugin/utils/get-translate-strings.js

diff --git a/packages/eslint-plugin/rules/i18n-ellipsis.js b/packages/eslint-plugin/rules/i18n-ellipsis.js
index fc4cd9b2e59d6c..8e1b5eabf9b5e5 100644
--- a/packages/eslint-plugin/rules/i18n-ellipsis.js
+++ b/packages/eslint-plugin/rules/i18n-ellipsis.js
@@ -5,6 +5,7 @@ const {
 	TRANSLATION_FUNCTIONS,
 	getTextContentFromNode,
 	getTranslateFunctionName,
+	getTranslateFunctionArgs,
 } = require( '../utils' );
 
 const THREE_DOTS = '...';
@@ -71,13 +72,12 @@ module.exports = {
 					return;
 				}
 
-				const functionArgs = [ args[ 0 ] ];
+				const candidates = getTranslateFunctionArgs(
+					functionName,
+					args
+				);
 
-				if ( [ '_n', '_nx' ].includes( functionName ) ) {
-					functionArgs.push( args[ 1 ] );
-				}
-
-				for ( const arg of functionArgs ) {
+				for ( const arg of candidates ) {
 					const argumentString = getTextContentFromNode( arg );
 					if (
 						! argumentString ||
diff --git a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
index cd38f8920f9d4a..1d34472bc7b4a7 100644
--- a/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
+++ b/packages/eslint-plugin/rules/i18n-no-collapsible-whitespace.js
@@ -5,6 +5,7 @@ const {
 	TRANSLATION_FUNCTIONS,
 	getTextContentFromNode,
 	getTranslateFunctionName,
+	getTranslateFunctionArgs,
 } = require( '../utils' );
 
 const PROBLEMS_BY_CHAR_CODE = {
@@ -34,13 +35,12 @@ module.exports = {
 					return;
 				}
 
-				const functionArgs = [ args[ 0 ] ];
+				const candidates = getTranslateFunctionArgs(
+					functionName,
+					args
+				);
 
-				if ( [ '_n', '_nx' ].includes( functionName ) ) {
-					functionArgs.push( args[ 1 ] );
-				}
-
-				for ( const arg of functionArgs ) {
+				for ( const arg of candidates ) {
 					const argumentString = getTextContentFromNode( arg );
 					if ( ! argumentString ) {
 						continue;
diff --git a/packages/eslint-plugin/rules/i18n-no-placeholders-only.js b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
index 8920c41eacfcc3..796e42c0b0dc16 100644
--- a/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
+++ b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
@@ -6,6 +6,7 @@ const {
 	REGEXP_PLACEHOLDER,
 	getTextContentFromNode,
 	getTranslateFunctionName,
+	getTranslateFunctionArgs,
 } = require( '../utils' );
 
 module.exports = {
@@ -28,13 +29,12 @@ module.exports = {
 					return;
 				}
 
-				const functionArgs = [ args[ 0 ] ];
+				const candidates = getTranslateFunctionArgs(
+					functionName,
+					args
+				);
 
-				if ( [ '_n', '_nx' ].includes( functionName ) ) {
-					functionArgs.push( args[ 1 ] );
-				}
-
-				for ( const arg of functionArgs ) {
+				for ( const arg of candidates ) {
 					const argumentString = getTextContentFromNode( arg );
 					if ( ! argumentString ) {
 						continue;
diff --git a/packages/eslint-plugin/rules/i18n-no-variables.js b/packages/eslint-plugin/rules/i18n-no-variables.js
index 34860f773ffec2..c942ba440a7676 100644
--- a/packages/eslint-plugin/rules/i18n-no-variables.js
+++ b/packages/eslint-plugin/rules/i18n-no-variables.js
@@ -4,6 +4,7 @@
 const {
 	TRANSLATION_FUNCTIONS,
 	getTranslateFunctionName,
+	getTranslateFunctionArgs,
 } = require( '../utils' );
 
 function isAcceptableLiteralNode( node ) {
@@ -44,17 +45,12 @@ module.exports = {
 					return;
 				}
 
-				const functionArgs = [ args[ 0 ] ];
+				const candidates = getTranslateFunctionArgs(
+					functionName,
+					args
+				);
 
-				if ( [ '_n', '_nx', '_x' ].includes( functionName ) ) {
-					functionArgs.push( args[ 1 ] );
-				}
-
-				if ( [ '_nx' ].includes( functionName ) ) {
-					functionArgs.push( args[ 3 ] );
-				}
-
-				for ( const arg of functionArgs ) {
+				for ( const arg of candidates ) {
 					if ( isAcceptableLiteralNode( arg ) ) {
 						continue;
 					}
diff --git a/packages/eslint-plugin/rules/i18n-translator-comments.js b/packages/eslint-plugin/rules/i18n-translator-comments.js
index ac3b00720d4f4b..f6c4d2a814f1bd 100644
--- a/packages/eslint-plugin/rules/i18n-translator-comments.js
+++ b/packages/eslint-plugin/rules/i18n-translator-comments.js
@@ -4,8 +4,9 @@
 const {
 	TRANSLATION_FUNCTIONS,
 	REGEXP_PLACEHOLDER,
-	getTranslateStrings,
 	getTranslateFunctionName,
+	getTranslateFunctionArgs,
+	getTextContentFromNode,
 } = require( '../utils' );
 
 module.exports = {
@@ -34,9 +35,14 @@ module.exports = {
 					return;
 				}
 
-				const candidates = getTranslateStrings( functionName, args );
+				const candidates = getTranslateFunctionArgs(
+					functionName,
+					args
+				)
+					.map( getTextContentFromNode )
+					.filter( Boolean );
 
-				if ( ! candidates ) {
+				if ( candidates.length === 0 ) {
 					return;
 				}
 
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index 6049c41a574d04..b37cfa65f0a340 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -1,7 +1,11 @@
 /**
  * Internal dependencies
  */
-const { REGEXP_PLACEHOLDER, getTranslateStrings } = require( '../utils' );
+const {
+	REGEXP_PLACEHOLDER,
+	getTranslateFunctionArgs,
+	getTextContentFromNode,
+} = require( '../utils' );
 
 module.exports = {
 	meta: {
@@ -45,15 +49,16 @@ module.exports = {
 					case 'CallExpression':
 						// All possible options (arguments) from a translate
 						// function must be valid.
-						candidates = getTranslateStrings(
+						candidates = getTranslateFunctionArgs(
 							args[ 0 ].callee.name,
-							args[ 0 ].arguments
-						);
+							args[ 0 ].arguments,
+							false
+						).map( getTextContentFromNode );
 
 						// An unknown function call may produce a valid string
 						// value. Ideally its result is verified, but this is
 						// not straight-forward to implement. Thus, bail.
-						if ( candidates === undefined ) {
+						if ( candidates.filter( Boolean ).length === 0 ) {
 							return;
 						}
 
diff --git a/packages/eslint-plugin/utils/get-translate-function-args.js b/packages/eslint-plugin/utils/get-translate-function-args.js
new file mode 100644
index 00000000000000..2ae5a611bb1b80
--- /dev/null
+++ b/packages/eslint-plugin/utils/get-translate-function-args.js
@@ -0,0 +1,40 @@
+/**
+ * Given a function name and array of argument Node values,
+ * returns all arguments except for text domain and number arguments.
+ *
+ * @param {string}        functionName   Function name.
+ * @param {espree.Node[]} args           Espree argument Node objects.
+ * @param {boolean}       includeContext Whether to include the context argument or not.
+ *
+ * @return {espree.Node[]} Translate function arguments.
+ */
+function getTranslateFunctionArgs( functionName, args, includeContext = true ) {
+	switch ( functionName ) {
+		case '__':
+			// __( text, domain ) -> [ text ].
+			return args.slice( 0, 1 );
+
+		case '_x':
+			// _x( text, context, domain ) -> [ text, context ].
+			return includeContext ? args.slice( 0, 2 ) : args.slice( 0, 1 );
+
+		case '_n':
+			// _n( single, plural, number, domain ) -> [ single, plural ].
+			return args.slice( 0, 2 );
+
+		case '_nx':
+			// _nx( single, plural, number, context, domain ) -> [ single, plural, context ].
+			const result = args.slice( 0, 2 );
+			if ( includeContext ) {
+				result.push( args[ 3 ] );
+			}
+			return result;
+
+		default:
+			return [];
+	}
+}
+
+module.exports = {
+	getTranslateFunctionArgs,
+};
diff --git a/packages/eslint-plugin/utils/get-translate-strings.js b/packages/eslint-plugin/utils/get-translate-strings.js
deleted file mode 100644
index aa8ea980af6019..00000000000000
--- a/packages/eslint-plugin/utils/get-translate-strings.js
+++ /dev/null
@@ -1,34 +0,0 @@
-/**
- * Given a function name and array of argument Node values, returns all
- * possible string results from the corresponding translate function, or
- * undefined if the function is not a translate function.
- *
- * @param {string}        functionName Function name.
- * @param {espree.Node[]} args         Espree argument Node objects.
- *
- * @return {Array<string>|void} All possible translate function string results.
- */
-function getTranslateStrings( functionName, args ) {
-	switch ( functionName ) {
-		case '__':
-		case '_x':
-			args = args.slice( 0, 1 );
-			break;
-
-		case '_n':
-		case '_nx':
-			args = args.slice( 0, 2 );
-			break;
-
-		default:
-			return;
-	}
-
-	return args
-		.filter( ( arg ) => arg.type === 'Literal' )
-		.map( ( arg ) => arg.value );
-}
-
-module.exports = {
-	getTranslateStrings,
-};
diff --git a/packages/eslint-plugin/utils/index.js b/packages/eslint-plugin/utils/index.js
index 2fce7bc5a9b878..4cb2f792f648de 100644
--- a/packages/eslint-plugin/utils/index.js
+++ b/packages/eslint-plugin/utils/index.js
@@ -1,12 +1,12 @@
 const { TRANSLATION_FUNCTIONS, REGEXP_PLACEHOLDER } = require( './constants' );
-const { getTranslateStrings } = require( './get-translate-strings' );
+const { getTranslateFunctionArgs } = require( './get-translate-function-args' );
 const { getTextContentFromNode } = require( './get-text-content-from-node' );
 const { getTranslateFunctionName } = require( './get-translate-function-name' );
 
 module.exports = {
 	TRANSLATION_FUNCTIONS,
 	REGEXP_PLACEHOLDER,
-	getTranslateStrings,
+	getTranslateFunctionArgs,
 	getTextContentFromNode,
 	getTranslateFunctionName,
 };

From 63c86060a1ffc479c38b4bda6d9cfb672c5077ab Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 11:17:42 +0100
Subject: [PATCH 30/41] Coding standards in test code

---
 .../rules/__tests__/i18n-ellipsis.js          | 30 +++----
 .../i18n-no-collapsible-whitespace.js         |  2 +-
 .../__tests__/i18n-no-placeholders-only.js    | 12 +--
 .../rules/__tests__/i18n-no-variables.js      | 32 ++++----
 .../rules/__tests__/i18n-text-domain.js       | 82 +++++++++----------
 5 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js b/packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js
index d16f4f2c89a055..2bc487e92d42bd 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-ellipsis.js
@@ -17,48 +17,48 @@ const ruleTester = new RuleTester( {
 ruleTester.run( 'i18n-ellipsis', rule, {
 	valid: [
 		{
-			code: `__('Hello World…')`,
+			code: `__( 'Hello World…' )`,
 		},
 		{
-			code: `__('Hello' + 'World…')`,
+			code: `__( 'Hello' + 'World…' )`,
 		},
 		{
-			code: `_x('Hello World…', 'context')`,
+			code: `_x( 'Hello World…', 'context' )`,
 		},
 		{
-			code: `_n('Singular…', 'Plural…', number)`,
+			code: `_n( 'Singular…', 'Plural…', number)`,
 		},
 		{
-			code: `i18n.__('Hello World…')`,
+			code: `i18n.__( 'Hello World…' )`,
 		},
 	],
 	invalid: [
 		{
-			code: `__('Hello World...')`,
-			output: `__('Hello World…')`,
+			code: `__( 'Hello World...' )`,
+			output: `__( 'Hello World…' )`,
 			errors: [ { messageId: 'foundThreeDots' } ],
 		},
 		{
-			code: `__('Hello' + 'World...')`,
-			output: `__('Hello' + 'World…')`,
+			code: `__( 'Hello' + 'World...' )`,
+			output: `__( 'Hello' + 'World…' )`,
 			errors: [ { messageId: 'foundThreeDots' } ],
 		},
 		{
-			code: `_x('Hello World...', 'context')`,
-			output: `_x('Hello World…', 'context')`,
+			code: `_x( 'Hello World...', 'context' )`,
+			output: `_x( 'Hello World…', 'context' )`,
 			errors: [ { messageId: 'foundThreeDots' } ],
 		},
 		{
-			code: `_n('Singular...', 'Plural...', number)`,
-			output: `_n('Singular…', 'Plural…', number)`,
+			code: `_n( 'Singular...', 'Plural...', number)`,
+			output: `_n( 'Singular…', 'Plural…', number)`,
 			errors: [
 				{ messageId: 'foundThreeDots' },
 				{ messageId: 'foundThreeDots' },
 			],
 		},
 		{
-			code: `i18n.__('Hello World...')`,
-			output: `i18n.__('Hello World…')`,
+			code: `i18n.__( 'Hello World...' )`,
+			output: `i18n.__( 'Hello World…' )`,
 			errors: [ { messageId: 'foundThreeDots' } ],
 		},
 	],
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js b/packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js
index 5746eb48584947..35e4e47301d7aa 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-no-collapsible-whitespace.js
@@ -17,7 +17,7 @@ const ruleTester = new RuleTester( {
 ruleTester.run( 'i18n-no-collapsible-whitespace', rule, {
 	valid: [
 		{
-			code: `__('Hello World…')`,
+			code: `__( 'Hello World…' )`,
 		},
 		{
 			code:
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js b/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
index acc4e918b4e3be..747e380e67f7ee 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
@@ -17,28 +17,28 @@ const ruleTester = new RuleTester( {
 ruleTester.run( 'i18n-no-placeholders-only', rule, {
 	valid: [
 		{
-			code: `__('Hello %s')`,
+			code: `__( 'Hello %s' )`,
 		},
 		{
-			code: `__('%d%%')`,
+			code: `__( '%d%%' )`,
 		},
 	],
 	invalid: [
 		{
-			code: `__('%s')`,
+			code: `__( '%s' )`,
 			errors: [ { messageId: 'noPlaceholdersOnly' } ],
 		},
 		{
-			code: `__('%s%s')`,
+			code: `__( '%s%s' )`,
 			errors: [ { messageId: 'noPlaceholdersOnly' } ],
 		},
 		// @todo: Update placeholder regex, see https://github.com/WordPress/gutenberg/pull/20574.
 		/*{
-			code: `_x('%1$s')`,
+			code: `_x( '%1$s' )`,
 			errors: [ { messageId: 'noPlaceholdersOnly' } ],
 		},*/
 		{
-			code: `_n('%s', '%s', number)`,
+			code: `_n( '%s', '%s', number)`,
 			errors: [
 				{ messageId: 'noPlaceholdersOnly' },
 				{ messageId: 'noPlaceholdersOnly' },
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-no-variables.js b/packages/eslint-plugin/rules/__tests__/i18n-no-variables.js
index d0656d2f0ab551..1ee322944ef9fd 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-no-variables.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-no-variables.js
@@ -17,34 +17,34 @@ const ruleTester = new RuleTester( {
 ruleTester.run( 'i18n-no-variables', rule, {
 	valid: [
 		{
-			code: `__('Hello World')`,
+			code: `__( 'Hello World' )`,
 		},
 		{
-			code: `__('Hello' + 'World')`,
+			code: `__( 'Hello' + 'World' )`,
 		},
 		{
-			code: `_x('Hello World', 'context')`,
+			code: `_x( 'Hello World', 'context' )`,
 		},
 		{
-			code: `var number = ''; _n('Singular', 'Plural', number)`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number)`,
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context' )`,
 		},
 		{
-			code: `__('Hello World', 'foo')`,
+			code: `__( 'Hello World', 'foo' )`,
 		},
 		{
-			code: `_x('Hello World', 'context', 'foo')`,
+			code: `_x( 'Hello World', 'context', 'foo' )`,
 		},
 		{
-			code: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'foo' )`,
 		},
 		{
-			code: `i18n.__('Hello World')`,
+			code: `i18n.__( 'Hello World' )`,
 		},
 	],
 	invalid: [
@@ -57,11 +57,11 @@ ruleTester.run( 'i18n-no-variables', rule, {
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
-			code: `_x(foo, 'context')`,
+			code: `_x(foo, 'context' )`,
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
-			code: `_x('Hello World', bar)`,
+			code: `_x( 'Hello World', bar)`,
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
@@ -69,19 +69,19 @@ ruleTester.run( 'i18n-no-variables', rule, {
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
-			code: `var number = ''; _n('Singular', bar, number)`,
+			code: `var number = ''; _n( 'Singular', bar, number)`,
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
-			code: `var number = ''; _nx(foo, 'Plural', number, 'context')`,
+			code: `var number = ''; _nx(foo, 'Plural', number, 'context' )`,
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
-			code: `var number = ''; _nx('Singular', bar, number, 'context')`,
+			code: `var number = ''; _nx( 'Singular', bar, number, 'context' )`,
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, baz)`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, baz)`,
 			errors: [ { messageId: 'invalidArgument' } ],
 		},
 		{
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
index 15f9bb90b755e7..992b893c36a929 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
@@ -17,140 +17,140 @@ const ruleTester = new RuleTester( {
 ruleTester.run( 'i18n-text-domain', rule, {
 	valid: [
 		{
-			code: `__('Hello World')`,
+			code: `__( 'Hello World' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
-			code: `_x('Hello World', 'context')`,
+			code: `_x( 'Hello World', 'context' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
-			code: `var number = ''; _n('Singular', 'Plural', number)`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number)`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
-			code: `__('Hello World', 'foo')`,
+			code: `__( 'Hello World', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 		},
 		{
-			code: `_x('Hello World', 'context', 'foo')`,
+			code: `_x( 'Hello World', 'context', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 		},
 		{
-			code: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 		},
 		{
-			code: `i18n.__('Hello World')`,
+			code: `i18n.__( 'Hello World' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 	],
 	invalid: [
 		{
-			code: `__('Hello World')`,
-			output: `__('Hello World', 'foo')`,
+			code: `__( 'Hello World' )`,
+			output: `__( 'Hello World', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
-			code: `_x('Hello World', 'context')`,
-			output: `_x('Hello World', 'context', 'foo')`,
+			code: `_x( 'Hello World', 'context' )`,
+			output: `_x( 'Hello World', 'context', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
-			code: `var number = ''; _n('Singular', 'Plural', number)`,
-			output: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number)`,
+			output: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
-			output: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context' )`,
+			output: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
-			code: `__('Hello World', 'bar')`,
-			output: `__('Hello World', 'foo')`,
+			code: `__( 'Hello World', 'bar' )`,
+			output: `__( 'Hello World', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
-			code: `_x('Hello World', 'context', 'bar')`,
-			output: `_x('Hello World', 'context', 'foo')`,
+			code: `_x( 'Hello World', 'context', 'bar' )`,
+			output: `_x( 'Hello World', 'context', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
-			code: `var number = ''; _n('Singular', 'Plural', number, 'bar')`,
-			output: `var number = ''; _n('Singular', 'Plural', number, 'foo')`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number, 'bar' )`,
+			output: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'bar')`,
-			output: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'foo')`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'bar' )`,
+			output: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
-			code: `var value = ''; __('Hello World', value)`,
+			code: `var value = ''; __( 'Hello World', value)`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
-			code: `var value = ''; _x('Hello World', 'context', value)`,
+			code: `var value = ''; _x( 'Hello World', 'context', value)`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
-			code: `var value = ''; var number = ''; _n('Singular', 'Plural', number, value)`,
+			code: `var value = ''; var number = ''; _n( 'Singular', 'Plural', number, value)`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
-			code: `var value = ''; var number = ''; _nx('Singular', 'Plural', number, 'context', value)`,
+			code: `var value = ''; var number = ''; _nx( 'Singular', 'Plural', number, 'context', value)`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
-			code: `__('Hello World', 'default')`,
-			output: `__('Hello World')`,
+			code: `__( 'Hello World', 'default' )`,
+			output: `__( 'Hello World' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
-			code: `__('default', 'default')`,
-			output: `__('default')`,
+			code: `__( 'default', 'default' )`,
+			output: `__( 'default' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
-			code: `_x('Hello World', 'context', 'default')`,
-			output: `_x('Hello World', 'context')`,
+			code: `_x( 'Hello World', 'context', 'default' )`,
+			output: `_x( 'Hello World', 'context' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
-			code: `var number = ''; _n('Singular', 'Plural', number, 'default')`,
-			output: `var number = ''; _n('Singular', 'Plural', number)`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number, 'default' )`,
+			output: `var number = ''; _n( 'Singular', 'Plural', number)`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
-			code: `var number = ''; _nx('Singular', 'Plural', number, 'context', 'default')`,
-			output: `var number = ''; _nx('Singular', 'Plural', number, 'context')`,
+			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'default' )`,
+			output: `var number = ''; _nx( 'Singular', 'Plural', number, 'context' )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
-			code: `i18n.__('Hello World')`,
-			output: `i18n.__('Hello World', 'foo')`,
+			code: `i18n.__( 'Hello World' )`,
+			output: `i18n.__( 'Hello World', 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'missing' } ],
 		},

From 63fd381fb68a2ac4e7ba676cc5a8d6a2cf2804ae Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 11:19:28 +0100
Subject: [PATCH 31/41] Remove now unneeded no-restricted-syntax config

---
 packages/eslint-plugin/configs/custom.js | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index 04340b13339b75..d5726805695c95 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -11,27 +11,6 @@ module.exports = {
 		'@wordpress/i18n-no-placeholders-only': 'error',
 		'@wordpress/i18n-no-variables': 'error',
 		'@wordpress/i18n-ellipsis': 'error',
-		'no-restricted-syntax': [
-			'error',
-			{
-				selector:
-					'CallExpression[callee.name=/^(__|_n|_nx|_x)$/]:not([arguments.0.type=/^Literal|BinaryExpression$/])',
-				message:
-					'Translate function arguments must be string literals.',
-			},
-			{
-				selector:
-					'CallExpression[callee.name=/^(_n|_nx|_x)$/]:not([arguments.1.type=/^Literal|BinaryExpression$/])',
-				message:
-					'Translate function arguments must be string literals.',
-			},
-			{
-				selector:
-					'CallExpression[callee.name=_nx]:not([arguments.3.type=/^Literal|BinaryExpression$/])',
-				message:
-					'Translate function arguments must be string literals.',
-			},
-		],
 	},
 	overrides: [
 		{

From e6fe285161d28611dca449c57165eae1ebbe7a28 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 11:21:24 +0100
Subject: [PATCH 32/41] Add i18n rules to new i18n config

---
 packages/eslint-plugin/README.md                     |  1 +
 packages/eslint-plugin/configs/custom.js             |  7 -------
 packages/eslint-plugin/configs/i18n.js               | 12 ++++++++++++
 .../configs/recommended-with-formatting.js           |  1 +
 4 files changed, 14 insertions(+), 7 deletions(-)
 create mode 100644 packages/eslint-plugin/configs/i18n.js

diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md
index 31979f983b52f9..e6bf980d326a3f 100644
--- a/packages/eslint-plugin/README.md
+++ b/packages/eslint-plugin/README.md
@@ -36,6 +36,7 @@ Alternatively, you can opt-in to only the more granular rulesets offered by the
 - `jsdoc`
 - `jsx-a11y`
 - `react`
+- `i18n`
 - `test-e2e`
 - `test-unit`
 
diff --git a/packages/eslint-plugin/configs/custom.js b/packages/eslint-plugin/configs/custom.js
index d5726805695c95..276d1f15a2584a 100644
--- a/packages/eslint-plugin/configs/custom.js
+++ b/packages/eslint-plugin/configs/custom.js
@@ -2,15 +2,8 @@ module.exports = {
 	plugins: [ '@wordpress' ],
 	rules: {
 		'@wordpress/no-unused-vars-before-return': 'error',
-		'@wordpress/valid-sprintf': 'error',
 		'@wordpress/no-base-control-with-label-without-id': 'error',
 		'@wordpress/no-unguarded-get-range-at': 'error',
-		'@wordpress/i18n-translator-comments': 'error',
-		'@wordpress/i18n-text-domain': 'error',
-		'@wordpress/i18n-no-collapsible-whitespace': 'error',
-		'@wordpress/i18n-no-placeholders-only': 'error',
-		'@wordpress/i18n-no-variables': 'error',
-		'@wordpress/i18n-ellipsis': 'error',
 	},
 	overrides: [
 		{
diff --git a/packages/eslint-plugin/configs/i18n.js b/packages/eslint-plugin/configs/i18n.js
new file mode 100644
index 00000000000000..c3271214e3ef5c
--- /dev/null
+++ b/packages/eslint-plugin/configs/i18n.js
@@ -0,0 +1,12 @@
+module.exports = {
+	plugins: [ '@wordpress' ],
+	rules: {
+		'@wordpress/valid-sprintf': 'error',
+		'@wordpress/i18n-translator-comments': 'error',
+		'@wordpress/i18n-text-domain': 'error',
+		'@wordpress/i18n-no-collapsible-whitespace': 'error',
+		'@wordpress/i18n-no-placeholders-only': 'error',
+		'@wordpress/i18n-no-variables': 'error',
+		'@wordpress/i18n-ellipsis': 'error',
+	},
+};
diff --git a/packages/eslint-plugin/configs/recommended-with-formatting.js b/packages/eslint-plugin/configs/recommended-with-formatting.js
index 61e08359128075..d6e51149a928e9 100644
--- a/packages/eslint-plugin/configs/recommended-with-formatting.js
+++ b/packages/eslint-plugin/configs/recommended-with-formatting.js
@@ -5,6 +5,7 @@ module.exports = {
 		require.resolve( './custom.js' ),
 		require.resolve( './react.js' ),
 		require.resolve( './esnext.js' ),
+		require.resolve( './i18n.js' ),
 	],
 	env: {
 		node: true,

From 5ceb0c7b664bc4d0c6a18b4bcadc9f8570bb58dc Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 11:23:47 +0100
Subject: [PATCH 33/41] Mark new ruleset as breaking change

---
 packages/eslint-plugin/CHANGELOG.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/packages/eslint-plugin/CHANGELOG.md b/packages/eslint-plugin/CHANGELOG.md
index 842f2865bcc6cf..df3ba08a957755 100644
--- a/packages/eslint-plugin/CHANGELOG.md
+++ b/packages/eslint-plugin/CHANGELOG.md
@@ -10,6 +10,11 @@
 - New Rule: [`@wordpress/i18n-no-collapsible-whitespace`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md)
 - New Rule: [`@wordpress/i18n-ellipsis`](https://github.com/WordPress/gutenberg/blob/master/packages/eslint-plugin/docs/rules/i18n-ellipsis.md)
 
+### Breaking Changes
+
+- There is a new `i18n` ruleset that includes all i18n-related rules and is included in the `recommended` ruleset.
+- The `valid-sprintf` rule has been moved from the `custom` ruleset to the `i18n` ruleset.
+
 ## 4.0.0 (2020-02-10)
 
 ### Breaking Changes

From a2e16d5607fb73269b09664c2b87f0a85169f28c Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 27 Mar 2020 11:42:23 +0100
Subject: [PATCH 34/41] Update docs

---
 .../eslint-plugin/docs/rules/i18n-ellipsis.md     |  4 ++--
 .../docs/rules/i18n-no-collapsible-whitespace.md  | 15 ++++++++++++---
 .../docs/rules/i18n-no-placeholders-only.md       |  2 +-
 .../eslint-plugin/docs/rules/i18n-no-variables.md | 14 +++++++++-----
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/packages/eslint-plugin/docs/rules/i18n-ellipsis.md b/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
index 752b57ea52ac9e..f08a43bc56af4a 100644
--- a/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
+++ b/packages/eslint-plugin/docs/rules/i18n-ellipsis.md
@@ -1,6 +1,6 @@
-# Enforce using ellipsis (…) instead of three dots (i18n-ellipsis)
+# Disallow using three dots in translatable strings (i18n-ellipsis)
 
-Translatable strings should use ellipsis (…) instead of three dots.
+Three dots for indicating an ellipsis should be replaced with the UTF-8 character … (Horizontal Ellipsis, U+2026) as it has a more semantic meaning.
 
 ## Rule details
 
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md b/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
index e60ae22c73bf62..d9564698535a96 100644
--- a/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
+++ b/packages/eslint-plugin/docs/rules/i18n-no-collapsible-whitespace.md
@@ -1,17 +1,26 @@
 # Disallow collapsible whitespace in translatable strings. (i18n-no-collapsible-whitespace)
 
-Translatable strings that consist of nothing but a placeholder are rather pointless and not really translatable.
+Using complex whitespace in translatable strings and relying on HTML to collapse it can make translation more difficult and lead to unnecessary retranslation.
+
+Whitespace can be appropriate in longer translatable content, for example a whole blog post. These cases are unlikely to occur in the code scanned by eslint but if they do, [disable the rule with inline comments](http://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments. ( e.g. `// eslint-disable-line i18n-no-collapsible-whitespace` ).
 
 ## Rule details
 
 Examples of **incorrect** code for this rule:
 
 ```js
-__( '%s' );
+__( "A string\non two lines" );
+__( 'A string\non two lines' );
+__( `A string
+on two lines` );
+__( `A	string	with	tabs` );
+__( "Multiple spaces.  Even after a full stop.  (We're going there)" );
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__( 'Hello %s' );
+__( `A long string ` +
+	`spread over ` +
+	`multiple lines.` );
 ```
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md b/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
index a1dcd915df259c..286690a79e70c0 100644
--- a/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
+++ b/packages/eslint-plugin/docs/rules/i18n-no-placeholders-only.md
@@ -1,6 +1,6 @@
 # Prevent using only placeholders in translatable strings (i18n-no-placeholders-only)
 
-Translatable strings that consist of nothing but a placeholder are rather pointless and not really translatable.
+Translatable strings that consist of nothing but a placeholder cannot be translated.
 
 ## Rule details
 
diff --git a/packages/eslint-plugin/docs/rules/i18n-no-variables.md b/packages/eslint-plugin/docs/rules/i18n-no-variables.md
index e40aff4076eb29..3478f02acf755d 100644
--- a/packages/eslint-plugin/docs/rules/i18n-no-variables.md
+++ b/packages/eslint-plugin/docs/rules/i18n-no-variables.md
@@ -2,19 +2,23 @@
 
 [Translation functions](https://github.com/WordPress/gutenberg/blob/master/packages/i18n/README.md#api) must be called with valid string literals as arguments.
 
+They cannot be variables or functions due to the way these strings are extracted through static analysis of the code. The exception to this rule is string concatenation within the argument itself.
+
+This limitation applies to both singular and plural strings, as well as the `context` argument if present.
+
 ## Rule details
 
 Examples of **incorrect** code for this rule:
 
 ```js
-__( 'Hello World' );
-_x( 'Hello World', 'context', 'foo' );
-
+__( `Hello ${foo}` );
+__( foo );
+_x( 'Hello World', bar );
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__( `Hello ${foo}` );
-_x( 'Hello World', bar );
+__( 'Hello World' );
+_x( 'Hello' + ' World', 'context', 'foo' );
 ```

From 5bcac71605a30a170e33a724d38bba789fcab0f7 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Mon, 30 Mar 2020 11:53:03 +0200
Subject: [PATCH 35/41] Fix tests

---
 .../rules/__tests__/i18n-text-domain.js            | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
index 992b893c36a929..a5fa82411ecc5e 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
@@ -25,7 +25,7 @@ ruleTester.run( 'i18n-text-domain', rule, {
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
-			code: `var number = ''; _n( 'Singular', 'Plural', number)`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 		},
 		{
@@ -67,7 +67,7 @@ ruleTester.run( 'i18n-text-domain', rule, {
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
-			code: `var number = ''; _n( 'Singular', 'Plural', number)`,
+			code: `var number = ''; _n( 'Singular', 'Plural', number )`,
 			output: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
 			options: [ { allowedTextDomains: [ 'foo' ] } ],
 			errors: [ { messageId: 'missing' } ],
@@ -103,19 +103,19 @@ ruleTester.run( 'i18n-text-domain', rule, {
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
-			code: `var value = ''; __( 'Hello World', value)`,
+			code: `var value = ''; __( 'Hello World', value )`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
-			code: `var value = ''; _x( 'Hello World', 'context', value)`,
+			code: `var value = ''; _x( 'Hello World', 'context', value )`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
-			code: `var value = ''; var number = ''; _n( 'Singular', 'Plural', number, value)`,
+			code: `var value = ''; var number = ''; _n( 'Singular', 'Plural', number, value )`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
-			code: `var value = ''; var number = ''; _nx( 'Singular', 'Plural', number, 'context', value)`,
+			code: `var value = ''; var number = ''; _nx( 'Singular', 'Plural', number, 'context', value )`,
 			errors: [ { messageId: 'invalidType' } ],
 		},
 		{
@@ -138,7 +138,7 @@ ruleTester.run( 'i18n-text-domain', rule, {
 		},
 		{
 			code: `var number = ''; _n( 'Singular', 'Plural', number, 'default' )`,
-			output: `var number = ''; _n( 'Singular', 'Plural', number)`,
+			output: `var number = ''; _n( 'Singular', 'Plural', number )`,
 			options: [ { allowedTextDomains: [ 'default' ] } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},

From e7187a0071013a35b2afaa9ecae72a9579eb9625 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Tue, 31 Mar 2020 18:19:22 +0200
Subject: [PATCH 36/41] Rename argument to allowedTextDomain and allow strings
 and arrays

---
 .eslintrc.js                                  |  2 +-
 .../docs/rules/i18n-text-domain.md            | 10 ++--
 .../rules/__tests__/i18n-text-domain.js       | 58 +++++++++++--------
 .../eslint-plugin/rules/i18n-text-domain.js   | 27 ++++++---
 4 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/.eslintrc.js b/.eslintrc.js
index 8427c936cf4b02..d5440467750e79 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -45,7 +45,7 @@ module.exports = {
 		'@wordpress/i18n-text-domain': [
 			'error',
 			{
-				allowedTextDomains: [ 'default' ],
+				allowedTextDomain: 'default',
 			},
 		],
 		'@wordpress/i18n-no-collapsible-whitespace': 'off',
diff --git a/packages/eslint-plugin/docs/rules/i18n-text-domain.md b/packages/eslint-plugin/docs/rules/i18n-text-domain.md
index d25d2b98ca4609..7a637fc43aaeb1 100644
--- a/packages/eslint-plugin/docs/rules/i18n-text-domain.md
+++ b/packages/eslint-plugin/docs/rules/i18n-text-domain.md
@@ -7,20 +7,20 @@
 Examples of **incorrect** code for this rule:
 
 ```js
-__( 'Hello World' ); // unless allowedTextDomains contains 'default'
-__( 'Hello World', 'default' ); // with allowedTextDomains = [ 'default' ]
+__( 'Hello World' ); // unless allowedTextDomain contains 'default'
+__( 'Hello World', 'default' ); // with allowedTextDomain = [ 'default' ]
 __( 'Hello World', foo );
 ```
 
 Examples of **correct** code for this rule:
 
 ```js
-__( 'Hello World' ); // with allowedTextDomains = [ 'default' ]
-__( 'Hello World', 'foo-bar' ); // with allowedTextDomains = [ 'foo-bar' ]
+__( 'Hello World' ); // with allowedTextDomain = [ 'default' ]
+__( 'Hello World', 'foo-bar' ); // with allowedTextDomain = [ 'foo-bar' ]
 ```
 
 ## Options
 
 This rule accepts a single options argument:
 
-- Set `allowedTextDomains` to specify the list of allowed text domains, e.g. `[ 'foo', 'bar' ]`. The default is `[ 'default' ]`.
+- Set `allowedTextDomain` to specify the list of allowed text domains, e.g. `[ 'foo', 'bar' ]`. The default is `[ 'default' ]`.
diff --git a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
index a5fa82411ecc5e..1212c90d998ed7 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-text-domain.js
@@ -18,88 +18,88 @@ ruleTester.run( 'i18n-text-domain', rule, {
 	valid: [
 		{
 			code: `__( 'Hello World' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 		},
 		{
 			code: `_x( 'Hello World', 'context' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 		},
 		{
 			code: `var number = ''; _n( 'Singular', 'Plural', number )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 		},
 		{
 			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 		},
 		{
 			code: `__( 'Hello World', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 		},
 		{
 			code: `_x( 'Hello World', 'context', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 		},
 		{
 			code: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 		},
 		{
 			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 		},
 		{
 			code: `i18n.__( 'Hello World' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 		},
 	],
 	invalid: [
 		{
 			code: `__( 'Hello World' )`,
 			output: `__( 'Hello World', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `_x( 'Hello World', 'context' )`,
 			output: `_x( 'Hello World', 'context', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `var number = ''; _n( 'Singular', 'Plural', number )`,
 			output: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context' )`,
 			output: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 		{
 			code: `__( 'Hello World', 'bar' )`,
 			output: `__( 'Hello World', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
 			code: `_x( 'Hello World', 'context', 'bar' )`,
 			output: `_x( 'Hello World', 'context', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
 			code: `var number = ''; _n( 'Singular', 'Plural', number, 'bar' )`,
 			output: `var number = ''; _n( 'Singular', 'Plural', number, 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
 			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'bar' )`,
 			output: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
 			errors: [ { messageId: 'invalidValue' } ],
 		},
 		{
@@ -121,37 +121,49 @@ ruleTester.run( 'i18n-text-domain', rule, {
 		{
 			code: `__( 'Hello World', 'default' )`,
 			output: `__( 'Hello World' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `__( 'default', 'default' )`,
 			output: `__( 'default' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `_x( 'Hello World', 'context', 'default' )`,
 			output: `_x( 'Hello World', 'context' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `var number = ''; _n( 'Singular', 'Plural', number, 'default' )`,
 			output: `var number = ''; _n( 'Singular', 'Plural', number )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `var number = ''; _nx( 'Singular', 'Plural', number, 'context', 'default' )`,
 			output: `var number = ''; _nx( 'Singular', 'Plural', number, 'context' )`,
-			options: [ { allowedTextDomains: [ 'default' ] } ],
+			options: [ { allowedTextDomain: 'default' } ],
 			errors: [ { messageId: 'unnecessaryDefault' } ],
 		},
 		{
 			code: `i18n.__( 'Hello World' )`,
 			output: `i18n.__( 'Hello World', 'foo' )`,
-			options: [ { allowedTextDomains: [ 'foo' ] } ],
+			options: [ { allowedTextDomain: 'foo' } ],
+			errors: [ { messageId: 'missing' } ],
+		},
+		{
+			code: `__( 'Hello World' )`,
+			output: `__( 'Hello World', 'foo' )`,
+			options: [ { allowedTextDomain: [ 'foo' ] } ],
+			errors: [ { messageId: 'missing' } ],
+		},
+		{
+			code: `__( 'Hello World' )`,
+			output: `__( 'Hello World' )`,
+			options: [ { allowedTextDomain: [ 'foo', 'bar' ] } ],
 			errors: [ { messageId: 'missing' } ],
 		},
 	],
diff --git a/packages/eslint-plugin/rules/i18n-text-domain.js b/packages/eslint-plugin/rules/i18n-text-domain.js
index e6afe1810e9119..c93efd2d784764 100644
--- a/packages/eslint-plugin/rules/i18n-text-domain.js
+++ b/packages/eslint-plugin/rules/i18n-text-domain.js
@@ -35,12 +35,22 @@ module.exports = {
 			{
 				type: 'object',
 				properties: {
-					allowedTextDomains: {
-						type: 'array',
-						items: {
-							type: 'string',
-						},
-						uniqueItems: true,
+					// Supports a single string as the majority use case,
+					// but also an array of text domains.
+					allowedTextDomain: {
+						anyOf: [
+							{
+								type: 'array',
+								items: {
+									type: 'string',
+								},
+								uniqueItems: true,
+							},
+							{
+								type: 'string',
+								default: 'default',
+							},
+						],
 					},
 				},
 				additionalProperties: false,
@@ -58,7 +68,10 @@ module.exports = {
 	},
 	create( context ) {
 		const options = context.options[ 0 ] || {};
-		const { allowedTextDomains = [ 'default' ] } = options;
+		const { allowedTextDomain = 'default' } = options;
+		const allowedTextDomains = Array.isArray( allowedTextDomain )
+			? allowedTextDomain
+			: new Array( allowedTextDomain );
 		const canFixTextDomain = allowedTextDomains.length === 1;
 		const allowDefault = allowedTextDomains.includes( 'default' );
 

From e1c6ee1835484f54b28a953be777afb6be319cec Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Thu, 2 Apr 2020 13:30:51 +0200
Subject: [PATCH 37/41] Fix case when using object spread

---
 packages/eslint-plugin/rules/__tests__/valid-sprintf.js | 3 +++
 packages/eslint-plugin/rules/valid-sprintf.js           | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
index fb5a3f665001c9..52211fad4c29c5 100644
--- a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
@@ -56,6 +56,9 @@ sprintf(
 		{
 			code: `i18n.sprintf( i18n.__( '%s' ), 'substitute' )`,
 		},
+		{
+			code: `sprintf( ...args )`,
+		},
 	],
 	invalid: [
 		{
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index ee9b8abf487c23..720cbf905982d1 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -49,6 +49,10 @@ module.exports = {
 				}
 
 				if ( args.length < 2 ) {
+					if ( args[ 0 ].type === 'SpreadElement' ) {
+						return;
+					}
+
 					context.report( {
 						node,
 						messageId: 'noPlaceholderArgs',

From 96bbd6fc04646923905eba9b04b8cd0d7b28d92a Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 3 Apr 2020 11:29:25 +0200
Subject: [PATCH 38/41] Apply changes from code review

---
 packages/eslint-plugin/rules/valid-sprintf.js | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index 720cbf905982d1..0dbbc2628e1ad1 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -4,6 +4,7 @@
 const {
 	REGEXP_SPRINTF_PLACEHOLDER,
 	REGEXP_SPRINTF_PLACEHOLDER_UNORDERED,
+	getTranslateFunctionName,
 	getTranslateFunctionArgs,
 	getTextContentFromNode,
 } = require( '../utils' );
@@ -71,11 +72,9 @@ module.exports = {
 						break;
 
 					case 'CallExpression':
-						const argFunctionName =
-							args[ 0 ].callee.property &&
-							args[ 0 ].callee.property.name
-								? args[ 0 ].callee.property.name
-								: args[ 0 ].callee.name;
+						const argFunctionName = getTranslateFunctionName(
+							args[ 0 ].callee
+						);
 
 						// All possible options (arguments) from a translate
 						// function must be valid.
@@ -117,9 +116,6 @@ module.exports = {
 					const allMatches = candidate.match(
 						REGEXP_SPRINTF_PLACEHOLDER
 					);
-					const unorderedMatches = candidate.match(
-						REGEXP_SPRINTF_PLACEHOLDER_UNORDERED
-					);
 
 					// Prioritize placeholder number consistency over matching
 					// placeholder, since it's a more common error to omit a
@@ -136,6 +132,10 @@ module.exports = {
 						return;
 					}
 
+					const unorderedMatches = candidate.match(
+						REGEXP_SPRINTF_PLACEHOLDER_UNORDERED
+					);
+
 					if (
 						unorderedMatches &&
 						allMatches &&

From 1c14199de21fb03bde6d0031ac9f3f0526ece038 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 3 Apr 2020 11:32:03 +0200
Subject: [PATCH 39/41] Rename messageId to make it more clear

---
 packages/eslint-plugin/rules/__tests__/valid-sprintf.js | 2 +-
 packages/eslint-plugin/rules/valid-sprintf.js           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
index 52211fad4c29c5..949af13b2eb020 100644
--- a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
@@ -105,7 +105,7 @@ sprintf(
 	authorBlockCount,
 	authorBlockRating
 );`,
-			errors: [ { messageId: 'noNumberedPlaceholders' } ],
+			errors: [ { messageId: 'noOrderedPlaceholders' } ],
 		},
 		{
 			code: `i18n.sprintf()`,
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index 0dbbc2628e1ad1..f3ab557a8112c6 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -23,7 +23,7 @@ module.exports = {
 				'sprintf format string must contain at least one placeholder',
 			placeholderMismatch:
 				'sprintf format string options must have the same number of placeholders',
-			noNumberedPlaceholders:
+			noOrderedPlaceholders:
 				'Multiple sprintf placeholders should be ordered. Mix of ordered and non-ordered placeholders found.',
 		},
 	},
@@ -145,7 +145,7 @@ module.exports = {
 					) {
 						context.report( {
 							node,
-							messageId: 'noNumberedPlaceholders',
+							messageId: 'noOrderedPlaceholders',
 						} );
 						return;
 					}

From e6fc69cab2daa243983dbc5a610123cec202fed0 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 3 Apr 2020 12:07:52 +0200
Subject: [PATCH 40/41] Update regex to properly allow named arguments which
 are supported

---
 .../rules/__tests__/valid-sprintf.js          | 12 ++++++
 packages/eslint-plugin/rules/valid-sprintf.js |  6 ++-
 packages/eslint-plugin/utils/constants.js     | 37 ++++++++++++++++---
 3 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
index 949af13b2eb020..9b2b7de255d47b 100644
--- a/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/__tests__/valid-sprintf.js
@@ -19,6 +19,9 @@ ruleTester.run( 'valid-sprintf', rule, {
 		{
 			code: `sprintf( '%s', 'substitute' )`,
 		},
+		{
+			code: `sprintf( '%1$d%%', 500 )`,
+		},
 		{
 			code: `sprintf( __( '%s' ), 'substitute' )`,
 		},
@@ -59,6 +62,15 @@ sprintf(
 		{
 			code: `sprintf( ...args )`,
 		},
+		{
+			code: `sprintf( '%1$s %2$s', 'foo', 'bar' )`,
+		},
+		{
+			code: `sprintf( '%(greeting)s', 'Hello' )`,
+		},
+		{
+			code: `sprintf( '%(greeting)s %(toWhom)s', 'Hello', 'World' )`,
+		},
 	],
 	invalid: [
 		{
diff --git a/packages/eslint-plugin/rules/valid-sprintf.js b/packages/eslint-plugin/rules/valid-sprintf.js
index f3ab557a8112c6..593ea11b564a02 100644
--- a/packages/eslint-plugin/rules/valid-sprintf.js
+++ b/packages/eslint-plugin/rules/valid-sprintf.js
@@ -150,7 +150,11 @@ module.exports = {
 						return;
 					}
 
-					if ( ! allMatches ) {
+					// Catch cases where a string only contains %% (escaped percentage sign).
+					if (
+						! allMatches ||
+						( allMatches.length === 1 && allMatches[ 0 ] === '%%' )
+					) {
 						context.report( {
 							node,
 							messageId: 'noPlaceholders',
diff --git a/packages/eslint-plugin/utils/constants.js b/packages/eslint-plugin/utils/constants.js
index f7b6f03910dcb8..66ba5b52b63a5b 100644
--- a/packages/eslint-plugin/utils/constants.js
+++ b/packages/eslint-plugin/utils/constants.js
@@ -6,16 +6,43 @@
 const TRANSLATION_FUNCTIONS = new Set( [ '__', '_x', '_n', '_nx' ] );
 
 /**
- * Regular expression matching the presence of a printf format string
- * placeholder.
+ * Regular expression matching format placeholder syntax.
  *
- * Originally copied from http://php.net/manual/en/function.sprintf.php#93552.
+ * The pattern for matching named arguments is a naive and incomplete matcher
+ * against valid JavaScript identifier names.
  *
- * @see https://github.com/WordPress/WordPress-Coding-Standards/blob/2f927b0ba2bfcbffaa8f3251c086e109302d6622/WordPress/Sniffs/WP/I18nSniff.php#L37-L60
+ * via Mathias Bynens:
+ *
+ * >An identifier must start with $, _, or any character in the Unicode
+ * >categories “Uppercase letter (Lu)”, “Lowercase letter (Ll)”, “Titlecase
+ * >letter (Lt)”, “Modifier letter (Lm)”, “Other letter (Lo)”, or “Letter
+ * >number (Nl)”.
+ * >
+ * >The rest of the string can contain the same characters, plus any U+200C zero
+ * >width non-joiner characters, U+200D zero width joiner characters, and
+ * >characters in the Unicode categories “Non-spacing mark (Mn)”, “Spacing
+ * >combining mark (Mc)”, “Decimal digit number (Nd)”, or “Connector
+ * >punctuation (Pc)”.
+ *
+ * If browser support is constrained to those supporting ES2015, this could be
+ * made more accurate using the `u` flag:
+ *
+ * ```
+ * /^[$_\p{L}\p{Nl}][$_\p{L}\p{Nl}\u200C\u200D\p{Mn}\p{Mc}\p{Nd}\p{Pc}]*$/u;
+ * ```
+ *
+ * @see http://www.pixelbeat.org/programming/gcc/format_specs.html
+ * @see https://mathiasbynens.be/notes/javascript-identifiers#valid-identifier-names
  *
  * @type {RegExp}
  */
-const REGEXP_SPRINTF_PLACEHOLDER = /(?:(?<!%)(%(?:[0-9]+\$)?[+-]?(?:(?:0|\'.)?-?[0-9]*(?:\.(?:[ 0]|\'.)?[0-9]+)?|(?:[ ])?-?[0-9]+(?:\.(?:[ 0]|\'.)?[0-9]+)?)[bcdeEfFgGosuxX]))/g;
+const REGEXP_SPRINTF_PLACEHOLDER = /%(((\d+)\$)|(\(([$_a-zA-Z][$_a-zA-Z0-9]*)\)))?[ +0#-]*\d*(\.(\d+|\*))?(ll|[lhqL])?([cduxXefgsp%])/g;
+//                                    ▲         ▲                    ▲       ▲  ▲            ▲           ▲ type
+//                                    │         │                    │       │  │            └ Length (unsupported)
+//                                    │         │                    │       │  └ Precision / max width
+//                                    │         │                    │       └ Min width (unsupported)
+//                                    │         │                    └ Flags (unsupported)
+//                                    └ Index   └ Name (for named arguments)
 
 /**
  * "Unordered" means there's no position specifier: '%s', not '%2$s'.

From f0beb82bad56dd32ba4a22d5fc2a23486473d2a3 Mon Sep 17 00:00:00 2001
From: Pascal Birchler <pascalb@google.com>
Date: Fri, 3 Apr 2020 13:02:59 +0200
Subject: [PATCH 41/41] Update i18n-no-placeholders-only rule

---
 .../rules/__tests__/i18n-no-placeholders-only.js          | 8 +++++---
 packages/eslint-plugin/rules/i18n-no-placeholders-only.js | 7 +++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js b/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
index 747e380e67f7ee..7743798805267c 100644
--- a/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
+++ b/packages/eslint-plugin/rules/__tests__/i18n-no-placeholders-only.js
@@ -19,6 +19,9 @@ ruleTester.run( 'i18n-no-placeholders-only', rule, {
 		{
 			code: `__( 'Hello %s' )`,
 		},
+		{
+			code: `i18n.__( 'Hello %s' )`,
+		},
 		{
 			code: `__( '%d%%' )`,
 		},
@@ -32,11 +35,10 @@ ruleTester.run( 'i18n-no-placeholders-only', rule, {
 			code: `__( '%s%s' )`,
 			errors: [ { messageId: 'noPlaceholdersOnly' } ],
 		},
-		// @todo: Update placeholder regex, see https://github.com/WordPress/gutenberg/pull/20574.
-		/*{
+		{
 			code: `_x( '%1$s' )`,
 			errors: [ { messageId: 'noPlaceholdersOnly' } ],
-		},*/
+		},
 		{
 			code: `_n( '%s', '%s', number)`,
 			errors: [
diff --git a/packages/eslint-plugin/rules/i18n-no-placeholders-only.js b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
index 67b7645a54f4ed..3cc2b3dc34fb0f 100644
--- a/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
+++ b/packages/eslint-plugin/rules/i18n-no-placeholders-only.js
@@ -40,10 +40,9 @@ module.exports = {
 						continue;
 					}
 
-					const modifiedString = argumentString.replace(
-						REGEXP_SPRINTF_PLACEHOLDER,
-						''
-					);
+					const modifiedString = argumentString
+						.replace( /%%/g, 'VALID_ESCAPED_PERCENTAGE_SIGN' )
+						.replace( REGEXP_SPRINTF_PLACEHOLDER, '' );
 
 					if ( modifiedString.length > 0 ) {
 						continue;