diff --git a/packages/kbn-eslint-plugin-i18n/README.mdx b/packages/kbn-eslint-plugin-i18n/README.mdx index f72f01c06a63..100f83d167b6 100644 --- a/packages/kbn-eslint-plugin-i18n/README.mdx +++ b/packages/kbn-eslint-plugin-i18n/README.mdx @@ -9,13 +9,14 @@ tags: ['kibana', 'dev', 'contributor', 'operations', 'eslint', 'i18n'] `@kbn/eslint-plugin-i18n` is an ESLint plugin providing custom rules for validating JSXCode in the Kibana repo to make sure they are translated. Note: At the moment these rules only work for apps that are inside `/x-pack/plugins`. - If you want to enable this rule on code that is outside of this path, adjust `/helpers/get_i18n_identifier_from_file_path.ts`. ## `@kbn/i18n/strings_should_be_translated_with_i18n` This rule warns engineers to translate their strings by using i18n.translate from the '@kbn/i18n' package. It provides an autofix that takes into account the context of the translatable string in the JSX tree to generate a translation ID. +It kicks in on JSXText elements and specific JSXAttributes (`label` and `aria-label`) which expect a translated value. ## `@kbn/i18n/strings_should_be_translated_with_formatted_message` This rule warns engineers to translate their strings by using `` from the '@kbn/i18n-react' package. It provides an autofix that takes into account the context of the translatable string in the JSX tree and to generate a translation ID. +It kicks in on JSXText elements and specific JSXAttributes (`label` and `aria-label`) which expect a translated value. diff --git a/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.test.ts b/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.test.ts index d1157b7b16f1..6e01b89b2356 100644 --- a/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.test.ts +++ b/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.test.ts @@ -14,6 +14,10 @@ const testMap = [ ['x-pack/plugins/observability/foo/bar/baz/header_actions.tsx', 'xpack.observability'], ['x-pack/plugins/apm/public/components/app/correlations/correlations_table.tsx', 'xpack.apm'], ['x-pack/plugins/cases/public/components/foo.tsx', 'xpack.cases'], + [ + 'x-pack/plugins/synthetics/public/apps/synthetics/components/alerts/toggle_alert_flyout_button.tsx', + 'xpack.synthetics', + ], [ 'packages/kbn-alerts-ui-shared/src/alert_lifecycle_status_badge/index.tsx', 'app_not_found_in_i18nrc', diff --git a/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.ts b/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.ts index 288e2692bd76..d23a42f4ebcf 100644 --- a/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.ts +++ b/packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.ts @@ -24,6 +24,8 @@ export function getI18nIdentifierFromFilePath(fileName: string, cwd: string) { const i18nrc = JSON.parse(i18nrcFile); return i18nrc && i18nrc.paths - ? findKey(i18nrc.paths, (v) => v === path) ?? 'app_not_found_in_i18nrc' + ? findKey(i18nrc.paths, (v) => + Array.isArray(v) ? v.find((e) => e === path) : typeof v === 'string' && v === path + ) ?? 'app_not_found_in_i18nrc' : 'could_not_find_i18nrc'; } diff --git a/packages/kbn-eslint-plugin-i18n/helpers/get_intent_from_node.ts b/packages/kbn-eslint-plugin-i18n/helpers/get_intent_from_node.ts index 4cbd6bb9e330..687bfd31cfba 100644 --- a/packages/kbn-eslint-plugin-i18n/helpers/get_intent_from_node.ts +++ b/packages/kbn-eslint-plugin-i18n/helpers/get_intent_from_node.ts @@ -5,12 +5,12 @@ * in compliance with, at your election, the Elastic License 2.0 or the Server * Side Public License, v 1. */ -import { TSESTree } from '@typescript-eslint/typescript-estree'; +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/typescript-estree'; import { lowerCaseFirstLetter, upperCaseFirstLetter } from './utils'; -export function getIntentFromNode(originalNode: TSESTree.JSXText): string { - const value = lowerCaseFirstLetter( - originalNode.value +export function getIntentFromNode(value: string, parent: TSESTree.Node | undefined): string { + const processedValue = lowerCaseFirstLetter( + value .replace(/[?!@#$%^&*()_+\][{}|/<>,'"]/g, '') .trim() .split(' ') @@ -19,8 +19,6 @@ export function getIntentFromNode(originalNode: TSESTree.JSXText): string { .join('') ); - const { parent } = originalNode; - if ( parent && 'openingElement' in parent && @@ -30,11 +28,25 @@ export function getIntentFromNode(originalNode: TSESTree.JSXText): string { const parentTagName = String(parent.openingElement.name.name); if (parentTagName.includes('Eui')) { - return `${value}${parentTagName.replace('Eui', '')}Label`; + return `${processedValue}${parentTagName.replace('Eui', '')}Label`; } - return `${lowerCaseFirstLetter(parentTagName)}.${value}Label`; + return `${lowerCaseFirstLetter(parentTagName)}.${processedValue}Label`; } - return `${value}Label`; + if ( + parent && + 'parent' in parent && + parent.parent && + 'name' in parent.parent && + typeof parent.parent.name !== 'string' && + 'type' in parent.parent.name && + parent.parent.name.type === AST_NODE_TYPES.JSXIdentifier + ) { + const parentTagName = String(parent.parent.name.name); + + return `${lowerCaseFirstLetter(parentTagName)}.${processedValue}Label`; + } + + return `${processedValue}Label`; } diff --git a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.test.ts b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.test.ts index aa545c9bb0ee..10bdbda35189 100644 --- a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.test.ts +++ b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.test.ts @@ -95,6 +95,18 @@ function YetAnotherComponent() { ) }`, }, + { + filename: 'x-pack/plugins/observability/public/test_component.tsx', + code: ` +import React from 'react'; +import { FormattedMessage } from '@kbn/i18n-react'; + +function TestComponent() { + return ( + } /> + ) + }`, + }, ]; const invalid = [ @@ -160,6 +172,25 @@ function YetAnotherComponent() { ], output: valid[2].code, }, + { + filename: valid[3].filename, + code: ` +import React from 'react'; +import { FormattedMessage } from '@kbn/i18n-react'; + +function TestComponent() { + return ( + + ) + }`, + errors: [ + { + line: 7, + message: `Strings should be translated with . Use the autofix suggestion or add your own.`, + }, + ], + output: valid[3].code, + }, ]; for (const [name, tester] of [tsTester, babelTester]) { diff --git a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.ts b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.ts index 251fb3b3752f..67e2aaec256d 100644 --- a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.ts +++ b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_formatted_message.ts @@ -37,7 +37,7 @@ export const StringsShouldBeTranslatedWithFormattedMessage: Rule.RuleModule = { const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd); const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration; const functionName = getFunctionName(functionDeclaration); - const intent = getIntentFromNode(node); + const intent = getIntentFromNode(value, node.parent); const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel' @@ -72,6 +72,64 @@ export const StringsShouldBeTranslatedWithFormattedMessage: Rule.RuleModule = { }, }); }, + JSXAttribute: (node: TSESTree.JSXAttribute) => { + if (node.name.name !== 'aria-label' && node.name.name !== 'label') return; + + let val: string = ''; + + // label={'foo'} + if ( + node.value && + 'expression' in node.value && + 'value' in node.value.expression && + typeof node.value.expression.value === 'string' + ) { + val = node.value.expression.value; + } + + // label="foo" + if (node.value && 'value' in node.value && typeof node.value.value === 'string') { + val = node.value.value; + } + + if (!val) return; + + // Start building the translation ID suggestion + const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd); + const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration; + const functionName = getFunctionName(functionDeclaration); + const intent = getIntentFromNode(val, node); + + const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel' + + // Check if i18n has already been imported into the file. + const { + hasI18nImportLine, + i18nPackageImportLine: i18nImportLine, + rangeToAddI18nImportLine, + } = getI18nImportFixer({ + sourceCode, + mode: 'FormattedMessage', + }); + + // Show warning to developer and offer autofix suggestion + report({ + node: node as any, + message: + 'Strings should be translated with . Use the autofix suggestion or add your own.', + fix(fixer) { + return [ + fixer.replaceTextRange( + node.value!.range, + `{}` + ), + !hasI18nImportLine + ? fixer.insertTextAfterRange(rangeToAddI18nImportLine, `\n${i18nImportLine}`) + : null, + ].filter(isTruthy); + }, + }); + }, } as Rule.RuleListener; }, }; diff --git a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.test.ts b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.test.ts index 4c5916831b6c..dc938cd6effd 100644 --- a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.test.ts +++ b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.test.ts @@ -46,10 +46,10 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; function TestComponent() { - return ( -
{i18n.translate('app_not_found_in_i18nrc.testComponent.div.thisIsATestLabel', { defaultMessage: "This is a test"})}
- ) -}`, + return ( +
{i18n.translate('app_not_found_in_i18nrc.testComponent.div.thisIsATestLabel', { defaultMessage: 'This is a test'})}
+ ) + }`, }, { filename: 'x-pack/plugins/observability/public/another_component.tsx', @@ -58,30 +58,42 @@ import React from 'react'; import { i18n } from '@kbn/i18n'; function AnotherComponent() { - return ( - - - - {i18n.translate('app_not_found_in_i18nrc.anotherComponent.thisIsATestButtonLabel', { defaultMessage: "This is a test"})} - - - - ) -}`, + return ( + + + + {i18n.translate('app_not_found_in_i18nrc.anotherComponent.thisIsATestButtonLabel', { defaultMessage: 'This is a test'})} + + + + ) + }`, }, { filename: 'x-pack/plugins/observability/public/yet_another_component.tsx', code: ` + import React from 'react'; +import { i18n } from '@kbn/i18n'; + + function YetAnotherComponent() { + return ( +
+ {i18n.translate('app_not_found_in_i18nrc.yetAnotherComponent.selectMeSelectLabel', { defaultMessage: 'Select me'})} +
+ ) + }`, + }, + { + filename: 'x-pack/plugins/observability/public/test_component.tsx', + code: ` import React from 'react'; import { i18n } from '@kbn/i18n'; -function YetAnotherComponent() { - return ( -
- {i18n.translate('app_not_found_in_i18nrc.yetAnotherComponent.selectMeSelectLabel', { defaultMessage: "Select me"})} -
- ) -}`, +function TestComponent() { + return ( + + ) + }`, }, ]; @@ -92,10 +104,10 @@ const invalid = [ import React from 'react'; function TestComponent() { - return ( -
This is a test
- ) -}`, + return ( +
This is a test
+ ) + }`, errors: [ { line: 6, @@ -110,16 +122,16 @@ function TestComponent() { import React from 'react'; function AnotherComponent() { - return ( - - - - This is a test - - - - ) -}`, + return ( + + + + This is a test + + + + ) + }`, errors: [ { line: 9, @@ -131,15 +143,15 @@ function AnotherComponent() { { filename: valid[2].filename, code: ` -import React from 'react'; + import React from 'react'; -function YetAnotherComponent() { - return ( -
- Select me -
- ) -}`, + function YetAnotherComponent() { + return ( +
+ Select me +
+ ) + }`, errors: [ { line: 7, @@ -148,6 +160,25 @@ function YetAnotherComponent() { ], output: valid[2].code, }, + { + filename: valid[3].filename, + code: ` +import React from 'react'; +import { i18n } from '@kbn/i18n'; + +function TestComponent() { + return ( + + ) + }`, + errors: [ + { + line: 7, + message: `Strings should be translated with i18n. Use the autofix suggestion or add your own.`, + }, + ], + output: valid[3].code, + }, ]; for (const [name, tester] of [tsTester, babelTester]) { diff --git a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.ts b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.ts index a1e0da36921b..ba31f6109075 100644 --- a/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.ts +++ b/packages/kbn-eslint-plugin-i18n/rules/strings_should_be_translated_with_i18n.ts @@ -37,7 +37,65 @@ export const StringsShouldBeTranslatedWithI18n: Rule.RuleModule = { const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd); const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration; const functionName = getFunctionName(functionDeclaration); - const intent = getIntentFromNode(node); + const intent = getIntentFromNode(value, node.parent); + + const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel' + + // Check if i18n has already been imported into the file + const { + hasI18nImportLine, + i18nPackageImportLine: i18nImportLine, + rangeToAddI18nImportLine, + } = getI18nImportFixer({ + sourceCode, + mode: 'i18n.translate', + }); + + // Show warning to developer and offer autofix suggestion + report({ + node: node as any, + message: + 'Strings should be translated with i18n. Use the autofix suggestion or add your own.', + fix(fixer) { + return [ + fixer.replaceText( + node, + `${whiteSpaces}{i18n.translate('${translationIdSuggestion}', { defaultMessage: '${value}'})}` + ), + !hasI18nImportLine + ? fixer.insertTextAfterRange(rangeToAddI18nImportLine, `\n${i18nImportLine}`) + : null, + ].filter(isTruthy); + }, + }); + }, + JSXAttribute: (node: TSESTree.JSXAttribute) => { + if (node.name.name !== 'aria-label' && node.name.name !== 'label') return; + + let val: string = ''; + + // label={'foo'} + if ( + node.value && + 'expression' in node.value && + 'value' in node.value.expression && + typeof node.value.expression.value === 'string' + ) { + val = node.value.expression.value; + } + + // label="foo" + if (node.value && 'value' in node.value && typeof node.value.value === 'string') { + val = node.value.value; + } + + if (!val) return; + + // Start building the translation ID suggestion + const i18nAppId = getI18nIdentifierFromFilePath(filename, cwd); + const functionDeclaration = getScope().block as TSESTree.FunctionDeclaration; + const functionName = getFunctionName(functionDeclaration); + const intent = getIntentFromNode(val, node); const translationIdSuggestion = `${i18nAppId}.${functionName}.${intent}`; // 'xpack.observability.overview.logs.loadMoreLabel' @@ -58,9 +116,9 @@ export const StringsShouldBeTranslatedWithI18n: Rule.RuleModule = { 'Strings should be translated with i18n. Use the autofix suggestion or add your own.', fix(fixer) { return [ - fixer.replaceText( - node, - `${whiteSpaces}{i18n.translate('${translationIdSuggestion}', { defaultMessage: "${value}"})}` + fixer.replaceTextRange( + node.value!.range, + `{i18n.translate('${translationIdSuggestion}', { defaultMessage: '${val}'})}` ), !hasI18nImportLine ? fixer.insertTextAfterRange(rangeToAddI18nImportLine, `\n${i18nImportLine}`)