i18n: Update lint rule suggested import location to `@grafana/i18n` (#105091)

pull/105618/head
Tom Ratcliffe 1 month ago committed by GitHub
parent b3a73a5282
commit c2ebb9cbbf
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 22
      eslint.config.js
  2. 13
      packages/grafana-eslint-rules/rules/no-untranslated-strings.cjs
  3. 158
      packages/grafana-eslint-rules/rules/translation-utils.cjs
  4. 354
      packages/grafana-eslint-rules/tests/no-untranslated-strings.test.js

@ -119,21 +119,23 @@ module.exports = [
'no-restricted-imports': [
'error',
{
paths: [
patterns: [
{
name: 'react-redux',
importNames: ['useDispatch', 'useSelector'],
message: 'Please import from app/types instead.',
group: ['react-i18next', 'i18next'],
importNames: ['t'],
message: 'Please import useTranslate from @grafana/i18n and use the t function instead',
},
{
name: 'react-i18next',
importNames: ['Trans', 't'],
message: 'Please import from app/core/internationalization instead',
group: ['react-i18next'],
importNames: ['Trans'],
message: 'Please import from @grafana/i18n instead',
},
],
paths: [
{
name: 'i18next',
importNames: ['t'],
message: 'Please import from app/core/internationalization instead',
name: 'react-redux',
importNames: ['useDispatch', 'useSelector'],
message: 'Please import from app/types instead.',
},
],
},

@ -82,17 +82,16 @@ const noUntranslatedStrings = createRule({
if (expression.type === AST_NODE_TYPES.ConditionalExpression) {
const alternateIsString = isExpressionUntranslated(expression.alternate);
const consequentIsString = isExpressionUntranslated(expression.consequent);
const untranslatedExpressions = [
alternateIsString ? expression.alternate : undefined,
consequentIsString ? expression.consequent : undefined,
].filter((node) => !!node);
if (alternateIsString || consequentIsString) {
if (untranslatedExpressions.length) {
const messageId =
parentType === AST_NODE_TYPES.JSXAttribute ? 'noUntranslatedStringsProp' : 'noUntranslatedStrings';
const nodesToReport = [
alternateIsString ? expression.alternate : undefined,
consequentIsString ? expression.consequent : undefined,
].filter((node) => !!node);
nodesToReport.forEach((nodeToReport) => {
untranslatedExpressions.forEach((nodeToReport) => {
context.report({
node: nodeToReport,
messageId,

@ -22,6 +22,22 @@ const elementIsTrans = (node) => {
);
};
/**
* @param {Node} node
* @param {RuleContextWithOptions} context
*/
const getParentMethod = (node, context) => {
const ancestors = context.sourceCode.getAncestors(node);
return ancestors.find((anc) => {
return (
anc.type === AST_NODE_TYPES.ArrowFunctionExpression ||
anc.type === AST_NODE_TYPES.FunctionDeclaration ||
anc.type === AST_NODE_TYPES.FunctionExpression ||
anc.type === AST_NODE_TYPES.ClassDeclaration
);
});
};
/**
* @param {Node} node
*/
@ -72,17 +88,9 @@ function canBeFixed(node, context) {
// We can only fix JSX attribute strings that are within a function,
// otherwise the `t` function call will be made too early
if (node.type === AST_NODE_TYPES.JSXAttribute) {
const ancestors = context.sourceCode.getAncestors(node);
const isInFunction = ancestors.some((anc) => {
return [
AST_NODE_TYPES.ArrowFunctionExpression,
AST_NODE_TYPES.FunctionDeclaration,
AST_NODE_TYPES.ClassDeclaration,
].includes(anc.type);
});
if (!isInFunction) {
const parentMethod = getParentMethod(node, context);
if (!parentMethod) {
return false;
}
if (node.value?.type === AST_NODE_TYPES.JSXExpressionContainer) {
@ -121,7 +129,7 @@ function canBeFixed(node, context) {
* @returns {string|null} The translation prefix or null
*/
function getTranslationPrefix(context) {
const filename = context.getFilename();
const filename = context.filename;
const match = filename.match(/public\/app\/features\/([^/]+)/);
if (match) {
return match[1];
@ -209,24 +217,69 @@ function getComponentNames(node, context) {
return names;
}
/**
* Checks if a method has a variable declaration of `t`
* that came from a `useTranslate` call
* @param {Node} method The node
* @param {RuleContextWithOptions} context
*/
function methodHasUseTranslate(method, context) {
const tDeclaration = method ? context.sourceCode.getScope(method).variables.find((v) => v.name === 't') : null;
return (
tDeclaration &&
tDeclaration.defs.find((definition) => {
const isVariableDeclaration = definition.node.type === AST_NODE_TYPES.VariableDeclarator;
const declarationInit = isVariableDeclaration ? definition.node.init : null;
return (
isVariableDeclaration &&
declarationInit &&
declarationInit.type === AST_NODE_TYPES.CallExpression &&
declarationInit.callee.type === AST_NODE_TYPES.Identifier &&
declarationInit.callee.name === 'useTranslate'
);
})
);
}
/**
* Gets the import fixer for a node
* @param {JSXElement|JSXFragment|JSXAttribute} node
* @param {RuleFixer} fixer The fixer
* @param {string} importName The import name
* @param {'Trans'|'t'|'useTranslate'} importName The member to import from either `@grafana/i18n` or `@grafana/i18n/internal`
* @param {RuleContextWithOptions} context
* @returns {import('@typescript-eslint/utils/ts-eslint').RuleFix|undefined} The fix
*/
function getImportsFixer(node, fixer, importName, context) {
const body = context.sourceCode.ast.body;
/** Map of where we expect to import each translation util from */
const importPackage = {
Trans: '@grafana/i18n',
useTranslate: '@grafana/i18n',
t: '@grafana/i18n/internal',
};
const parentMethod = getParentMethod(node, context);
if (importName === 't') {
// If we're trying to import `t`,
// and there's already a `t` variable declaration in the parent method that came from `useTranslate`,
// do nothing
const declarationFromUseTranslate = parentMethod ? methodHasUseTranslate(parentMethod, context) : false;
if (declarationFromUseTranslate) {
return;
}
}
const expectedImport = importPackage[importName];
const existingAppCoreI18n = body.find(
(node) => node.type === AST_NODE_TYPES.ImportDeclaration && node.source.value === 'app/core/internationalization'
(node) => node.type === AST_NODE_TYPES.ImportDeclaration && node.source.value === importPackage[importName]
);
// If there's no existing import at all, add it
if (!existingAppCoreI18n) {
return fixer.insertTextBefore(body[0], `import { ${importName} } from 'app/core/internationalization';\n`);
return fixer.insertTextBefore(body[0], `import { ${importName} } from '${expectedImport}';\n`);
}
// To keep the typechecker happy - we have to explicitly check the type
@ -276,6 +329,72 @@ const getTransFixers = (node, context) => (fixer) => {
return fixes;
};
/**
* @param {string} str
*/
const firstCharIsUpper = (str) => {
return str.charAt(0) === str.charAt(0).toUpperCase();
};
/**
* @param {JSXAttribute} node
* @param {RuleFixer} fixer
* @param {RuleContextWithOptions} context
* @returns {import('@typescript-eslint/utils/ts-eslint').RuleFix|undefined} The fix
*/
const getUseTranslateFixer = (node, fixer, context) => {
const parentMethod = getParentMethod(node, context);
const functionIsNotUpperCase =
parentMethod &&
parentMethod.type === AST_NODE_TYPES.FunctionDeclaration &&
(!parentMethod.id || !firstCharIsUpper(parentMethod.id.name));
const variableDeclaratorIsNotUpperCase =
parentMethod &&
parentMethod.parent.type === AST_NODE_TYPES.VariableDeclarator &&
parentMethod.parent.id.type === AST_NODE_TYPES.Identifier &&
!firstCharIsUpper(parentMethod.parent.id.name);
// If the node is not within a function, or the parent method does not start with an uppercase letter,
// then we can't reliably add `useTranslate`, as this may not be a React component
if (
!parentMethod ||
functionIsNotUpperCase ||
variableDeclaratorIsNotUpperCase ||
parentMethod.body.type !== AST_NODE_TYPES.BlockStatement
) {
return;
}
const returnStatement = parentMethod.body.body.find((node) => node.type === AST_NODE_TYPES.ReturnStatement);
if (!returnStatement) {
return;
}
const returnStatementIsJsx =
returnStatement.argument &&
(returnStatement.argument.type === AST_NODE_TYPES.JSXElement ||
returnStatement.argument.type === AST_NODE_TYPES.JSXFragment);
if (!returnStatementIsJsx) {
return;
}
const useTranslateExists = methodHasUseTranslate(parentMethod, context);
if (useTranslateExists) {
return;
}
// If we've got all this way, then:
// - There is a parent method
// - It returns JSX
// - The method name starts with a capital letter
// - There is not already a call to `useTranslate` in the parent method
// In that scenario, we assume that we can fix and add a usage of the hook to the start of the body of the method
return fixer.insertTextBefore(parentMethod.body.body[0], 'const { t } = useTranslate();\n');
};
/**
* @param {JSXAttribute} node
* @param {RuleContextWithOptions} context
@ -291,10 +410,19 @@ const getTFixers = (node, context) => (fixer) => {
fixer.replaceText(node, `${node.name.name}={t("${i18nKey}", ${wrappingQuotes}${value}${wrappingQuotes})}`)
);
const importsFixer = getImportsFixer(node, fixer, 't', context);
// Check if we need to add `useTranslate` to the node
const useTranslateFixer = getUseTranslateFixer(node, fixer, context);
if (useTranslateFixer) {
fixes.push(useTranslateFixer);
}
// Check if we need to add `t` or `useTranslate` to the imports
const importToAdd = useTranslateFixer ? 'useTranslate' : 't';
const importsFixer = getImportsFixer(node, fixer, importToAdd, context);
if (importsFixer) {
fixes.push(importsFixer);
}
return fixes;
};

@ -16,6 +16,13 @@ RuleTester.setDefaultConfig({
const filename = 'public/app/features/some-feature/SomeFile.tsx';
const packageName = '@grafana/i18n';
const TRANS_IMPORT = `import { Trans } from '${packageName}';`;
const T_IMPORT = `import { t } from '${packageName}/internal';`;
const USE_TRANSLATE_IMPORT = `import { useTranslate } from '${packageName}';`;
const TRANS_AND_USE_TRANSLATE_IMPORT = `import { Trans, useTranslate } from '${packageName}';`;
const ruleTester = new RuleTester();
ruleTester.run('eslint no-untranslated-strings', noUntranslatedStrings, {
@ -86,7 +93,11 @@ ruleTester.run('eslint no-untranslated-strings', noUntranslatedStrings, {
},
{
name: 'Ternary with falsy strings',
code: `<div icon={isAThing ? foo : ''} />`,
code: `<div title={isAThing ? foo : ''} />`,
},
{
name: 'Ternary with no strings',
code: `<div title={isAThing ? 1 : 2} />`,
},
],
invalid: [
@ -108,7 +119,7 @@ const Foo = () => <div>Untranslated text</div>`,
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => <div><Trans i18nKey="some-feature.foo.untranslated-text">Untranslated text</Trans></div>`,
},
],
@ -118,7 +129,8 @@ const Foo = () => <div><Trans i18nKey="some-feature.foo.untranslated-text">Untra
{
name: 'Text inside JSXElement, not in a function',
code: `const thing = <div>foo</div>`,
code: `
const thing = <div>foo</div>`,
filename,
errors: [
{
@ -126,7 +138,8 @@ const Foo = () => <div><Trans i18nKey="some-feature.foo.untranslated-text">Untra
suggestions: [
{
messageId: 'wrapWithTrans',
output: `import { Trans } from 'app/core/internationalization';
output: `
${TRANS_IMPORT}
const thing = <div><Trans i18nKey="some-feature.thing.foo">foo</Trans></div>`,
},
],
@ -146,7 +159,7 @@ const Foo = () => <div>This is a longer string that we will translate</div>`,
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => <div><Trans i18nKey="some-feature.foo.longer-string-translate">This is a longer string that we will translate</Trans></div>`,
},
],
@ -166,7 +179,7 @@ const Foo = () => <div>lots of sho rt word s to be filt ered</div>`,
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => <div><Trans i18nKey="some-feature.foo.lots-of-sho-rt-word-s">lots of sho rt word s to be filt ered</Trans></div>`,
},
],
@ -186,7 +199,7 @@ const foo = <>hello</>`,
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const foo = <><Trans i18nKey="some-feature.foo.hello">hello</Trans></>`,
},
],
@ -206,7 +219,7 @@ const Foo = () => <div><TestingComponent someProp={<>Test</>} /></div>`,
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => <div><TestingComponent someProp={<><Trans i18nKey="some-feature.foo.test">Test</Trans></>} /></div>`,
},
],
@ -214,10 +227,181 @@ const Foo = () => <div><TestingComponent someProp={<><Trans i18nKey="some-featur
],
},
{
name: 'Fixes basic prop case and adds useTranslate',
code: `
const Foo = () => {
const fooBar = 'a';
return (
<div title="foo" />
)
}`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
const fooBar = 'a';
return (
<div title={t("some-feature.foo.title-foo", "foo")} />
)
}`,
},
],
},
],
},
{
name: 'Fixes using t when not inside something that looks like a React component',
code: `
function foo() {
return (
<div title="foo" />
)
}`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${T_IMPORT}
function foo() {
return (
<div title={t("some-feature.foo.title-foo", "foo")} />
)
}`,
},
],
},
],
},
{
name: 'Fixes using t when not inside something that looks like a React component - anonymous function',
code: `
const foo = function() {
return <div title="foo" />;
}`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${T_IMPORT}
const foo = function() {
return <div title={t("some-feature.foo.title-foo", "foo")} />;
}`,
},
],
},
],
},
{
name: 'Fixes when Trans import already exists',
code: `
${TRANS_IMPORT}
const Foo = () => {
return (
<div title="foo" />
)
}`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${TRANS_AND_USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
return (
<div title={t("some-feature.foo.title-foo", "foo")} />
)
}`,
},
],
},
],
},
{
name: 'Fixes when looks in an upper cased function but does not return JSX',
code: `
const Foo = () => {
return {
foo: <div title="foo" />
}
}`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${T_IMPORT}
const Foo = () => {
return {
foo: <div title={t("some-feature.foo.title-foo", "foo")} />
}
}`,
},
],
},
],
},
{
name: 'Fixes correctly when useTranslate already exists',
code: `
${USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
return (
<div title="foo" />
)
}`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
return (
<div title={t("some-feature.foo.title-foo", "foo")} />
)
}`,
},
],
},
],
},
{
name: 'Fixes and uses ID from attribute if exists',
code: `
import { t } from 'app/core/internationalization';
${T_IMPORT}
const Foo = () => <div id="someid" title="foo"/>`,
filename,
errors: [
@ -227,7 +411,7 @@ const Foo = () => <div id="someid" title="foo"/>`,
{
messageId: 'wrapWithT',
output: `
import { t } from 'app/core/internationalization';
${T_IMPORT}
const Foo = () => <div id="someid" title={t("some-feature.foo.someid-title-foo", "foo")}/>`,
},
],
@ -238,7 +422,7 @@ const Foo = () => <div id="someid" title={t("some-feature.foo.someid-title-foo",
{
name: 'Fixes correctly when Trans import already exists',
code: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => <div>Untranslated text</div>`,
filename,
errors: [
@ -248,7 +432,7 @@ const Foo = () => <div>Untranslated text</div>`,
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => <div><Trans i18nKey="some-feature.foo.untranslated-text">Untranslated text</Trans></div>`,
},
],
@ -259,7 +443,7 @@ const Foo = () => <div><Trans i18nKey="some-feature.foo.untranslated-text">Untra
{
name: 'Fixes correctly when t() import already exists',
code: `
import { t } from 'app/core/internationalization';
${T_IMPORT}
const Foo = () => <div title="foo" />`,
filename,
errors: [
@ -269,7 +453,7 @@ const Foo = () => <div title="foo" />`,
{
messageId: 'wrapWithT',
output: `
import { t } from 'app/core/internationalization';
${T_IMPORT}
const Foo = () => <div title={t("some-feature.foo.title-foo", "foo")} />`,
},
],
@ -277,10 +461,71 @@ const Foo = () => <div title={t("some-feature.foo.title-foo", "foo")} />`,
],
},
{
name: 'Fixes correctly when useTranslate import already exists',
code: `
${USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
return (<>
<div title={t("some-feature.foo.title-foo", "foo")} />
<div title={"bar"} />
</>)
}
`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
return (<>
<div title={t("some-feature.foo.title-foo", "foo")} />
<div title={t("some-feature.foo.title-bar", "bar")} />
</>)
}
`,
},
],
},
],
},
{
name: 'Fixes correctly when no return statement',
code: `
const Foo = () => {
const foo = <div title="foo" />
}
`,
filename,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${T_IMPORT}
const Foo = () => {
const foo = <div title={t(\"some-feature.foo.foo.title-foo\", \"foo\")} />
}
`,
},
],
},
],
},
{
name: 'Fixes correctly when import exists but needs to add t()',
code: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => <div title="foo" />`,
filename,
errors: [
@ -290,7 +535,8 @@ const Foo = () => <div title="foo" />`,
{
messageId: 'wrapWithT',
output: `
import { Trans, t } from 'app/core/internationalization';
${T_IMPORT}
${TRANS_IMPORT}
const Foo = () => <div title={t("some-feature.foo.title-foo", "foo")} />`,
},
],
@ -314,7 +560,7 @@ class Foo extends React.Component {
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
class Foo extends React.Component {
render() {
return <div><Trans i18nKey="some-feature.foo.untranslated-text">untranslated text</Trans></div>;
@ -338,7 +584,7 @@ const Foo = () => <div title="foo" />`,
{
messageId: 'wrapWithT',
output: `
import { t } from 'app/core/internationalization';
${T_IMPORT}
const Foo = () => <div title={t("some-feature.foo.title-foo", "foo")} />`,
},
],
@ -358,7 +604,7 @@ const Foo = () => <div title={"foo"} />`,
{
messageId: 'wrapWithT',
output: `
import { t } from 'app/core/internationalization';
${T_IMPORT}
const Foo = () => <div title={t("some-feature.foo.title-foo", "foo")} />`,
},
],
@ -378,7 +624,7 @@ const Foo = () => <div title='"foo"' />`,
{
messageId: 'wrapWithT',
output: `
import { t } from 'app/core/internationalization';
${T_IMPORT}
const Foo = () => <div title={t("some-feature.foo.title-foo", '"foo"')} />`,
},
],
@ -389,7 +635,7 @@ const Foo = () => <div title={t("some-feature.foo.title-foo", '"foo"')} />`,
{
name: 'Fixes case with nested functions/components',
code: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => {
const getSomething = () => {
return <div>foo</div>;
@ -406,7 +652,7 @@ const Foo = () => {
{
messageId: 'wrapWithTrans',
output: `
import { Trans } from 'app/core/internationalization';
${TRANS_IMPORT}
const Foo = () => {
const getSomething = () => {
return <div><Trans i18nKey="some-feature.foo.get-something.foo">foo</Trans></div>;
@ -421,6 +667,62 @@ const Foo = () => {
],
},
/**
* AUTO FIXES
*/
{
name: 'Auto fixes when options are configured',
code: `const Foo = () => <div>test</div>`,
filename,
options: [{ forceFix: ['public/app/features/some-feature'] }],
output: `${TRANS_IMPORT}
const Foo = () => <div><Trans i18nKey="some-feature.foo.test">test</Trans></div>`,
errors: [
{
messageId: 'noUntranslatedStrings',
suggestions: [
{
messageId: 'wrapWithTrans',
output: `${TRANS_IMPORT}
const Foo = () => <div><Trans i18nKey="some-feature.foo.test">test</Trans></div>`,
},
],
},
],
},
{
name: 'Auto fixes when options are configured - prop',
code: `
const Foo = () => {
return <div title="foo" />
}`,
filename,
options: [{ forceFix: ['public/app/features/some-feature'] }],
output: `
${USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
return <div title={t("some-feature.foo.title-foo", "foo")} />
}`,
errors: [
{
messageId: 'noUntranslatedStringsProp',
suggestions: [
{
messageId: 'wrapWithT',
output: `
${USE_TRANSLATE_IMPORT}
const Foo = () => {
const { t } = useTranslate();
return <div title={t("some-feature.foo.title-foo", "foo")} />
}`,
},
],
},
],
},
/**
* UNFIXABLE CASES
*/
@ -505,11 +807,17 @@ const Foo = () => {
},
{
name: 'Invalid when ternary with string literals',
name: 'Invalid when ternary with string literals - both',
code: `const Foo = () => <div>{isAThing ? 'Foo' : 'Bar'}</div>`,
filename,
errors: [{ messageId: 'noUntranslatedStrings' }, { messageId: 'noUntranslatedStrings' }],
},
{
name: 'Invalid when ternary with string literals - alternate',
code: `const Foo = () => <div>{isAThing ? 'Foo' : 1}</div>`,
filename,
errors: [{ messageId: 'noUntranslatedStrings' }],
},
{
name: 'Invalid when ternary with string literals - prop',
code: `const Foo = () => <div title={isAThing ? 'Foo' : 'Bar'} />`,

Loading…
Cancel
Save