diff --git a/eslint.config.js b/eslint.config.js index dc732177d64..666e275dae1 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -119,22 +119,24 @@ module.exports = [ 'no-restricted-imports': [ 'error', { + patterns: [ + { + group: ['react-i18next', 'i18next'], + importNames: ['t'], + message: 'Please import useTranslate from @grafana/i18n and use the t function instead', + }, + { + group: ['react-i18next'], + importNames: ['Trans'], + message: 'Please import from @grafana/i18n instead', + }, + ], paths: [ { name: 'react-redux', importNames: ['useDispatch', 'useSelector'], message: 'Please import from app/types instead.', }, - { - name: 'react-i18next', - importNames: ['Trans', 't'], - message: 'Please import from app/core/internationalization instead', - }, - { - name: 'i18next', - importNames: ['t'], - message: 'Please import from app/core/internationalization instead', - }, ], }, ], diff --git a/packages/grafana-eslint-rules/rules/no-untranslated-strings.cjs b/packages/grafana-eslint-rules/rules/no-untranslated-strings.cjs index b82adf2b412..18d44a34daf 100644 --- a/packages/grafana-eslint-rules/rules/no-untranslated-strings.cjs +++ b/packages/grafana-eslint-rules/rules/no-untranslated-strings.cjs @@ -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, diff --git a/packages/grafana-eslint-rules/rules/translation-utils.cjs b/packages/grafana-eslint-rules/rules/translation-utils.cjs index a5e3aad8c63..3fe5540286d 100644 --- a/packages/grafana-eslint-rules/rules/translation-utils.cjs +++ b/packages/grafana-eslint-rules/rules/translation-utils.cjs @@ -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; }; diff --git a/packages/grafana-eslint-rules/tests/no-untranslated-strings.test.js b/packages/grafana-eslint-rules/tests/no-untranslated-strings.test.js index 9beb56cf544..b57f97ddfc3 100644 --- a/packages/grafana-eslint-rules/tests/no-untranslated-strings.test.js +++ b/packages/grafana-eslint-rules/tests/no-untranslated-strings.test.js @@ -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: `
`, + code: `
`, + }, + { + name: 'Ternary with no strings', + code: `
`, }, ], invalid: [ @@ -108,7 +119,7 @@ const Foo = () =>
Untranslated text
`, { messageId: 'wrapWithTrans', output: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () =>
Untranslated text
`, }, ], @@ -118,7 +129,8 @@ const Foo = () =>
Untra { name: 'Text inside JSXElement, not in a function', - code: `const thing =
foo
`, + code: ` +const thing =
foo
`, filename, errors: [ { @@ -126,7 +138,8 @@ const Foo = () =>
Untra suggestions: [ { messageId: 'wrapWithTrans', - output: `import { Trans } from 'app/core/internationalization'; + output: ` +${TRANS_IMPORT} const thing =
foo
`, }, ], @@ -146,7 +159,7 @@ const Foo = () =>
This is a longer string that we will translate
`, { messageId: 'wrapWithTrans', output: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () =>
This is a longer string that we will translate
`, }, ], @@ -166,7 +179,7 @@ const Foo = () =>
lots of sho rt word s to be filt ered
`, { messageId: 'wrapWithTrans', output: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () =>
lots of sho rt word s to be filt ered
`, }, ], @@ -186,7 +199,7 @@ const foo = <>hello`, { messageId: 'wrapWithTrans', output: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const foo = <>hello`, }, ], @@ -206,7 +219,7 @@ const Foo = () =>
Test} />
`, { messageId: 'wrapWithTrans', output: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () =>
Test} />
`, }, ], @@ -214,10 +227,181 @@ const Foo = () =>
+ ) +}`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); +const fooBar = 'a'; + return ( +
+ ) +}`, + }, + ], + }, + ], + }, + + { + name: 'Fixes using t when not inside something that looks like a React component', + code: ` +function foo() { + return ( +
+ ) +}`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${T_IMPORT} +function foo() { + return ( +
+ ) +}`, + }, + ], + }, + ], + }, + + { + name: 'Fixes using t when not inside something that looks like a React component - anonymous function', + code: ` +const foo = function() { + return
; +}`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${T_IMPORT} +const foo = function() { + return
; +}`, + }, + ], + }, + ], + }, + + { + name: 'Fixes when Trans import already exists', + code: ` +${TRANS_IMPORT} +const Foo = () => { + return ( +
+ ) +}`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${TRANS_AND_USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); +return ( +
+ ) +}`, + }, + ], + }, + ], + }, + + { + name: 'Fixes when looks in an upper cased function but does not return JSX', + code: ` +const Foo = () => { + return { + foo:
+ } +}`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${T_IMPORT} +const Foo = () => { + return { + foo:
+ } +}`, + }, + ], + }, + ], + }, + { + name: 'Fixes correctly when useTranslate already exists', + code: ` +${USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); + return ( +
+ ) +}`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); + return ( +
+ ) +}`, + }, + ], + }, + ], + }, + { name: 'Fixes and uses ID from attribute if exists', code: ` -import { t } from 'app/core/internationalization'; +${T_IMPORT} const Foo = () =>
`, filename, errors: [ @@ -227,7 +411,7 @@ const Foo = () =>
`, { messageId: 'wrapWithT', output: ` -import { t } from 'app/core/internationalization'; +${T_IMPORT} const Foo = () =>
`, }, ], @@ -238,7 +422,7 @@ const Foo = () =>
Untranslated text
`, filename, errors: [ @@ -248,7 +432,7 @@ const Foo = () =>
Untranslated text
`, { messageId: 'wrapWithTrans', output: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () =>
Untranslated text
`, }, ], @@ -259,7 +443,7 @@ const Foo = () =>
Untra { name: 'Fixes correctly when t() import already exists', code: ` -import { t } from 'app/core/internationalization'; +${T_IMPORT} const Foo = () =>
`, filename, errors: [ @@ -269,7 +453,7 @@ const Foo = () =>
`, { messageId: 'wrapWithT', output: ` -import { t } from 'app/core/internationalization'; +${T_IMPORT} const Foo = () =>
`, }, ], @@ -277,10 +461,71 @@ const Foo = () =>
`, ], }, + { + name: 'Fixes correctly when useTranslate import already exists', + code: ` +${USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); + return (<> +
+
+ ) +} +`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); + return (<> +
+
+ ) +} +`, + }, + ], + }, + ], + }, + + { + name: 'Fixes correctly when no return statement', + code: ` +const Foo = () => { + const foo =
+} +`, + filename, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${T_IMPORT} +const Foo = () => { + const foo =
+} +`, + }, + ], + }, + ], + }, + { name: 'Fixes correctly when import exists but needs to add t()', code: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () =>
`, filename, errors: [ @@ -290,7 +535,8 @@ const Foo = () =>
`, { messageId: 'wrapWithT', output: ` -import { Trans, t } from 'app/core/internationalization'; +${T_IMPORT} +${TRANS_IMPORT} const 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
untranslated text
; @@ -338,7 +584,7 @@ const Foo = () =>
`, { messageId: 'wrapWithT', output: ` -import { t } from 'app/core/internationalization'; +${T_IMPORT} const Foo = () =>
`, }, ], @@ -358,7 +604,7 @@ const Foo = () =>
`, { messageId: 'wrapWithT', output: ` -import { t } from 'app/core/internationalization'; +${T_IMPORT} const Foo = () =>
`, }, ], @@ -378,7 +624,7 @@ const Foo = () =>
`, { messageId: 'wrapWithT', output: ` -import { t } from 'app/core/internationalization'; +${T_IMPORT} const Foo = () =>
`, }, ], @@ -389,7 +635,7 @@ const Foo = () =>
`, { name: 'Fixes case with nested functions/components', code: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () => { const getSomething = () => { return
foo
; @@ -406,7 +652,7 @@ const Foo = () => { { messageId: 'wrapWithTrans', output: ` -import { Trans } from 'app/core/internationalization'; +${TRANS_IMPORT} const Foo = () => { const getSomething = () => { return
foo
; @@ -421,6 +667,62 @@ const Foo = () => { ], }, + /** + * AUTO FIXES + */ + { + name: 'Auto fixes when options are configured', + code: `const Foo = () =>
test
`, + filename, + options: [{ forceFix: ['public/app/features/some-feature'] }], + output: `${TRANS_IMPORT} +const Foo = () =>
test
`, + errors: [ + { + messageId: 'noUntranslatedStrings', + suggestions: [ + { + messageId: 'wrapWithTrans', + output: `${TRANS_IMPORT} +const Foo = () =>
test
`, + }, + ], + }, + ], + }, + + { + name: 'Auto fixes when options are configured - prop', + code: ` +const Foo = () => { + return
+}`, + filename, + options: [{ forceFix: ['public/app/features/some-feature'] }], + output: ` +${USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); +return
+}`, + errors: [ + { + messageId: 'noUntranslatedStringsProp', + suggestions: [ + { + messageId: 'wrapWithT', + output: ` +${USE_TRANSLATE_IMPORT} +const Foo = () => { + const { t } = useTranslate(); +return
+}`, + }, + ], + }, + ], + }, + /** * 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 = () =>
{isAThing ? 'Foo' : 'Bar'}
`, filename, errors: [{ messageId: 'noUntranslatedStrings' }, { messageId: 'noUntranslatedStrings' }], }, + { + name: 'Invalid when ternary with string literals - alternate', + code: `const Foo = () =>
{isAThing ? 'Foo' : 1}
`, + filename, + errors: [{ messageId: 'noUntranslatedStrings' }], + }, { name: 'Invalid when ternary with string literals - prop', code: `const Foo = () =>
`,