Improvements for eslint-i18n-package (elastic#171588)

Co-authored-by: Kibana Machine <[email protected]>

.eslintrc.js

@@ -941,7 +941,7 @@ module.exports = {
       ],
       rules: {
         '@kbn/i18n/strings_should_be_translated_with_i18n': 'warn',
-        '@kbn/i18n/strings_should_be_translated_with_formatted_message': 'warn',
+        '@kbn/i18n/i18n_translate_should_start_with_the_right_id': 'warn',
       },
     },
     {

.github/CODEOWNERS

@@ -368,7 +368,7 @@ src/plugins/es_ui_shared @elastic/platform-deployment-management
 packages/kbn-eslint-config @elastic/kibana-operations
 packages/kbn-eslint-plugin-disable @elastic/kibana-operations
 packages/kbn-eslint-plugin-eslint @elastic/kibana-operations
-packages/kbn-eslint-plugin-i18n @elastic/obs-knowledge-team
+packages/kbn-eslint-plugin-i18n @elastic/obs-knowledge-team @elastic/kibana-operations
 packages/kbn-eslint-plugin-imports @elastic/kibana-operations
 packages/kbn-eslint-plugin-telemetry @elastic/obs-knowledge-team
 x-pack/test/encrypted_saved_objects_api_integration/plugins/api_consumer_plugin @elastic/kibana-security

packages/kbn-eslint-plugin-i18n/README.mdx

@@ -6,22 +6,88 @@ description: Custom ESLint rules to support translations in the Kibana repositor
 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.
+# Summary
 
-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/eslint-plugin-i18n` is an ESLint plugin providing custom ESLint rules to help validating code in the Kibana repo in the area of translations.
+
+The aim of this package is to help engineers type less and have a nicer experience.
+
+If a rule does not behave as you expect or you have an idea of how these rules can be improved, please reach out to the Observability Knowledge Team or the Kibana Operations team.
+
+# Rules
 
 ## `@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.
+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.
+
+This rule kicks in on:
+
+- JSXText elements;
+- specific JSXAttributes (`label` and `aria-label`) which expect a translated value.
+
+### Example
+
+This code:
+
+```
+// Filename: /x-pack/plugins/observability/public/my_component.tsx
+
+import React from 'react';
+import { EuiText } from '@elastic/eui';
+
+function MyComponent() {
+    return (
+        <EuiText>You know, for search</EuiText>
+    )
+}
+```
+
+will be autofixed with:
+
+```
+import React from 'react';
+import { i18n } from '@kbn/i18n';
+import { EuiText } from '@elastic/eui';
+
+function MyComponent() {
+    return (
+        <EuiText>
+            {i18n.translate('xpack.observability.myComponent.textLabel', { defaultMessage: 'You know, for search' } )}
+        </EuiText>
+    )
+}
+```
+
+If `i18n` has not been imported yet, the autofix will automatically add the import statement as well.
+
+### Exemptions and exceptions
+
+A JSXText element or JSXAttribute `label` or `aria-label` of which the value is:
+
+- wrapped in a `EuiCode` or `EuiBetaBadge` component,
+- made up of non alpha characters such as `!@#$%^&*(){}` or numbers,
+- wrapped in three backticks,
+
+are exempt from this rule.
+
+If this rule kicks in on a string value that you don't like, you can escape it by wrapping the string inside a JSXExpression: `{'my escaped value'}`.
+
+---
 
 ## `@kbn/i18n/strings_should_be_translated_with_formatted_message`
 
-This rule warns engineers to translate their strings by using `<FormattedMessage>` 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.
+This rule warns engineers to translate their strings by using `<FormattedMessage>` 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.
+
+This rule kicks in on:
 
-## Exemptions and exceptions
+- JSXText elements;
+- specific JSXAttributes (`label` and `aria-label`) which expect a translated value.
+
+### Exemptions and exceptions
 
 A JSXText element or JSXAttribute `label` or `aria-label` of which the value is:
 
@@ -32,3 +98,52 @@ A JSXText element or JSXAttribute `label` or `aria-label` of which the value is:
 are exempt from this rule.
 
 If this rule kicks in on a string value that you don't like, you can escape it by wrapping the string inside a JSXExpression: `{'my escaped value'}`.
+
+---
+
+## `@kbn/i18n/i18n_translate_should_start_with_the_right_id`
+
+This rule checks every instance of `i18n.translate()` if the first parameter passed:
+
+1. has a string value,
+2. if the parameter starts with the correct i18n app identifier for the file.
+
+It checks the repo for the `i18nrc.json` and `/x-pack/i18nrc.json` files and determines what the right i18n identifier should be.
+
+If the parameter is missing or does not start with the right i18n identifier, it can autofix the parameter.
+
+This rule is useful when defining translated values in plain functions (non-JSX), but it works in JSX as well.
+
+### Example
+
+This code:
+
+```
+// Filename: /x-pack/plugins/observability/public/my_function.ts
+
+function myFunction() {
+    const translations = [
+        {
+            id: 'copy';
+            label: i18n.translate()
+        }
+    ]
+}
+```
+
+will be autofixed with:
+
+```
+import { i18n } from '@kbn/i18n';
+
+function myFunction() {
+    const translations = [
+        {
+            id: 'copy';
+            label: i18n.translate('xpack.observability.myFunction.', { defaultMessage: '' })
+        }
+    ]
+}
+```
+
+If `i18n` has not been imported yet, the autofix will automatically add the import statement as well.

packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.test.ts

@@ -11,17 +11,15 @@ import { getI18nIdentifierFromFilePath } from './get_i18n_identifier_from_file_p
 const SYSTEMPATH = 'systemPath';
 
 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/observability/public/header_actions.tsx', 'xpack.observability'],
+  ['x-pack/plugins/apm/common/components/app/correlations/correlations_table.tsx', 'xpack.apm'],
+  ['x-pack/plugins/cases/server/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',
-  ],
+  ['src/plugins/vis_types/gauge/public/editor/collections.ts', 'visTypeGauge'],
+  ['packages/kbn-alerts-ui-shared/src/alert_lifecycle_status_badge/index.tsx', 'alertsUIShared'],
 ];
 
 describe('Get i18n Identifier for file', () => {

packages/kbn-eslint-plugin-i18n/helpers/get_i18n_identifier_from_file_path.ts

@@ -14,18 +14,38 @@ export function getI18nIdentifierFromFilePath(fileName: string, cwd: string) {
   const { dir } = parse(fileName);
   const relativePathToFile = dir.replace(cwd, '');
 
-  const relativePathArray = relativePathToFile.split('/');
+  // We need to match the path of the file that is being worked in with the path
+  // that is noted in the values inside the i18nrc.json object.
+  // These values differ depending on which i18nrc.json object you look at (there are multiple)
+  // so we need to account for both notations.
+  const relativePathArray = relativePathToFile.includes('src')
+    ? relativePathToFile.split('/').slice(1)
+    : relativePathToFile.split('/').slice(2);
 
-  const path = `${relativePathArray[2]}/${relativePathArray[3]}`;
+  const pluginNameIndex = relativePathArray.findIndex(
+    (el) => el === 'public' || el === 'server' || el === 'common'
+  );
+
+  const path = relativePathArray.slice(0, pluginNameIndex).join('/');
 
   const xpackRC = resolve(join(__dirname, '../../../'), 'x-pack/.i18nrc.json');
+  const rootRC = resolve(join(__dirname, '../../../'), '.i18nrc.json');
+
+  const xpackI18nrcFile = fs.readFileSync(xpackRC, 'utf8');
+  const xpackI18nrc = JSON.parse(xpackI18nrcFile);
+
+  const rootI18nrcFile = fs.readFileSync(rootRC, 'utf8');
+  const rootI18nrc = JSON.parse(rootI18nrcFile);
+
+  const allPaths = { ...xpackI18nrc.paths, ...rootI18nrc.paths };
 
-  const i18nrcFile = fs.readFileSync(xpackRC, 'utf8');
-  const i18nrc = JSON.parse(i18nrcFile);
+  if (Object.keys(allPaths).length === 0) return 'could_not_find_i18nrc';
 
-  return i18nrc && i18nrc.paths
-    ? 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';
+  return (
+    findKey(allPaths, (value) =>
+      Array.isArray(value)
+        ? value.find((el) => el === path)
+        : typeof value === 'string' && value === path
+    ) ?? 'app_not_found_in_i18nrc'
+  );
 }

packages/kbn-eslint-plugin-i18n/helpers/get_i18n_import_fixer.ts

@@ -10,10 +10,10 @@ import { SourceCode } from 'eslint';
 
 export function getI18nImportFixer({
   sourceCode,
-  mode,
+  translationFunction,
 }: {
   sourceCode: SourceCode;
-  mode: 'i18n.translate' | 'FormattedMessage';
+  translationFunction: 'i18n.translate' | 'FormattedMessage';
 }) {
   let existingI18nImportLineIndex = -1;
   let i18nImportLineToBeAdded = '';
@@ -27,7 +27,7 @@ export function getI18nImportFixer({
    *
    * */
 
-  if (mode === 'i18n.translate') {
+  if (translationFunction === 'i18n.translate') {
     existingI18nImportLineIndex = sourceCode.lines.findIndex((l) => l.includes("from '@kbn/i18n'"));
 
     const i18nImportLineInSource = sourceCode.lines[existingI18nImportLineIndex];
@@ -46,7 +46,7 @@ export function getI18nImportFixer({
     }
   }
 
-  if (mode === 'FormattedMessage') {
+  if (translationFunction === 'FormattedMessage') {
     existingI18nImportLineIndex = sourceCode.lines.findIndex((l) =>
       l.includes("from '@kbn/i18n-react'")
     );
@@ -83,21 +83,27 @@ export function getI18nImportFixer({
     return {
       i18nImportLine: i18nImportLineToBeAdded,
       rangeToAddI18nImportLine: [start, end] as [number, number],
-      mode: 'replace',
+      replaceMode: 'replace',
     };
   }
 
   // If the file doesn't have an import line for the translation package yet, we need to add it.
   // Pretty safe bet to add it underneath the import line for React.
-  const lineIndex = sourceCode.lines.findIndex((l) => l.includes("from 'react'"));
+  let lineIndex = sourceCode.lines.findIndex((l) => l.includes("from 'react'") || l.includes('*/'));
+
+  if (lineIndex === -1) {
+    lineIndex = 0;
+  }
+
   const targetLine = sourceCode.lines[lineIndex];
 
+  // `getIndexFromLoc` is 0-based, so we need to add 1 to the line index.
   const start = sourceCode.getIndexFromLoc({ line: lineIndex + 1, column: 0 });
   const end = start + targetLine.length;
 
   return {
     i18nImportLine: i18nImportLineToBeAdded,
     rangeToAddI18nImportLine: [start, end] as [number, number],
-    mode: 'insert',
+    replaceMode: 'insert',
   };
 }

packages/kbn-eslint-plugin-i18n/index.ts

@@ -8,6 +8,7 @@
 
 import { StringsShouldBeTranslatedWithI18n } from './rules/strings_should_be_translated_with_i18n';
 import { StringsShouldBeTranslatedWithFormattedMessage } from './rules/strings_should_be_translated_with_formatted_message';
+import { I18nTranslateShouldStartWithTheRightId } from './rules/i18n_translate_should_start_with_the_right_id';
 
 /**
  * Custom ESLint rules, add `'@kbn/eslint-plugin-i18n'` to your eslint config to use them
@@ -17,4 +18,5 @@ export const rules = {
   strings_should_be_translated_with_i18n: StringsShouldBeTranslatedWithI18n,
   strings_should_be_translated_with_formatted_message:
     StringsShouldBeTranslatedWithFormattedMessage,
+  i18n_translate_should_start_with_the_right_id: I18nTranslateShouldStartWithTheRightId,
 };

packages/kbn-eslint-plugin-i18n/kibana.jsonc

@@ -1,6 +1,6 @@
 {
   "type": "shared-common",
   "id": "@kbn/eslint-plugin-i18n",
-  "owner": "@elastic/obs-knowledge-team",
+  "owner": ["@elastic/obs-knowledge-team", "@elastic/kibana-operations"],
   "devOnly": true
 }