ESLint Plugin: Add I18N ESLint rules

[swissspidy]

Is your feature request related to a problem? Please describe.

Similar to the i18n sniffs in WPCS there could be some custom ESLint rules to catch some common i18n mistakes.

Describe the solution you'd like

Off the top of my head, there could be rules like follows:

Done

• Missing text domain
• Mismatched text domain (would require setting the expected text domain in the ESLint config)
• Unnecessary text domain (e.g. when passing default instead of just omitting it)
• Missing translator comments when using sprintf with placeholders
• Not using numbered placeholders when using %s and sprintf -> tracked separately in sprintf ESLint rule does not check for numbered placeholders #20554.

Likely to-do

• Using variables instead of string literals (see https://github.com/Automattic/wp-calypso/blob/master/packages/eslint-plugin-wpcalypso/lib/rules/i18n-no-variables.js,

  gutenberg/.eslintrc.js

  Lines 70 to 87 in 622362a

  	{
  	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.',
  	},

  )
• Using three dots instead of ellipsis (see https://github.com/Automattic/wp-calypso/blob/8fec020a3cd6baee6a84bf2b200ed11170593ad7/packages/eslint-plugin-wpcalypso/lib/rules/i18n-ellipsis.js,

  gutenberg/.eslintrc.js

  Lines 88 to 92 in 622362a

  	{
  	selector:
  	'CallExpression[callee.name=/^(__|_x|_n|_nx)$/] Literal[value=/\\.{3}/]',
  	message: 'Use ellipsis character (…) in place of three dots',
  	},

  )
• Using collapsible whitespace in strings (see https://github.com/Automattic/wp-calypso/blob/master/packages/eslint-plugin-wpcalypso/lib/rules/i18n-no-collapsible-whitespace.js)
• Using strings which include only placeholders (see https://github.com/Automattic/wp-calypso/blob/master/packages/eslint-plugin-wpcalypso/lib/rules/i18n-no-placeholders-only.js)
• String containing thing which should probably be %s placeholders instead (e.g. URLs)

On the fence

• Non-translatable text
  Needs some way to detect whether text is passed as a prop or child that should probably be translatable (e.g. aria-label prop). Helps catch places where __() is forgotten.
• String concatenation within __() (e.g. __( 'Hello' + '' + 'World' )
• Suggest using _x() instead of __() under certain conditions (needs list of ambiguous terms)
• When using markup in strings but not __experimentalCreateInterpolateElement
• String concatenation with markup instead of using __experimentalCreateInterpolateElement
• Using translation functions as defaultProps (when using prop-types)

Actual implementation should be split up into multiple PRs.

Describe alternatives you've considered

Find other solutions out there that likely won't work with @wordpress/i18n or build something custom.

[swissspidy]

I have a working version of text domain validation ready, will push it soon.

Passing a variable to __() et al instead of a string literal

This is currently enforced using no-restricted-syntax:

gutenberg/packages/eslint-plugin/configs/custom.js

Lines 8 to 28 in 4ea0668

	'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.',
	},
	],

Should be fairly easy to port this to its own proper rule.

[swissspidy]

Needs some way to detect whether text is passed as a prop or child that should probably be translatable

Some cases, off the top of my head:

• aria-*,title, alt attributes
• string between tags, e.g. <div>Hello World</div> or <div>{ 'Hello World' }</div>
• additional attributes could be added via whitelist

Might even be fixable by wrapping the found strings in __()

[gziolo]

It looks like a lot of tasks listed are done. Excellent work! There is only one left on the list that was initially marked as “likely to do”. @swissspidy, what would be your current recommendation on criteria to consider this issue finished?

[swissspidy]

I was just thinking the pther day that a rule for catching trailing whitespace would be nice. See https://core.trac.wordpress.org/ticket/55669

This mistake wasn‘t caught in the Gutenberg code base.

[gziolo]

I was just thinking the pther day that a rule for catching trailing whitespace would be nice. See https://core.trac.wordpress.org/ticket/55669

This mistake wasn‘t caught in the Gutenberg code base.

I agree that it would be great to cover as much as possible from the cases that slipped through WP 6.0 Beta phase from the Gutenberg repository ❤️

[swissspidy]

Looks like #38225 introduced such a rule (i18n-no-flanking-whitespace)

Edit: this begs the question why it didn't catch dd7db32

[swissspidy]

Looks like the rule was only enabled for React Native code and not the whole code base 🙃

@jhnstn Do you recall why it wasn't considered to apply the i18n-no-flanking-whitespace rule for the rest of the code base? Seems like a missed opportunity.

[gziolo]

@jhnstn Do you recall why it wasn't considered to apply the i18n-no-flanking-whitespace rule for the rest of the code base? Seems like a missed opportunity.

It would be great to find out and potentially enable for the rest of Gutenberg 👍🏻