Chore: Add new no-translation-top-level eslint rule (#101550)

.betterer.eslint.config.js

@@ -113,6 +113,7 @@ module.exports = [
     ignores: ['public/app/plugins/**', '**/*.story.tsx', '**/*.{test,spec}.{ts,tsx}', '**/__mocks__/', 'public/test'],
     rules: {
       '@grafana/no-untranslated-strings': 'error',
+      '@grafana/no-translation-top-level': 'error',
     },
   },
 ];

.betterer.results

@@ -549,6 +549,11 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"]
     ],
+    "packages/grafana-ui/src/components/DataSourceSettings/DataSourceHttpSettings.tsx:5381": [
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "0"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "1"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "2"]
+    ],
     "packages/grafana-ui/src/components/DataSourceSettings/types.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],
       [0, 0, 0, "Unexpected any. Specify a different type.", "1"]
@@ -3044,7 +3049,8 @@ exports[`better eslint`] = {
       [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"]
     ],
     "public/app/features/connections/tabs/ConnectData/Search/Search.tsx:5381": [
-      [0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "0"]
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "0"],
+      [0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "1"]
     ],
     "public/app/features/connections/tabs/ConnectData/Search/index.tsx:5381": [
       [0, 0, 0, "Do not use export all (\`export * from ...\`)", "0"]
@@ -4568,9 +4574,18 @@ exports[`better eslint`] = {
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "2"],
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "3"]
     ],
+    "public/app/features/explore/RichHistory/RichHistorySettingsTab.tsx:5381": [
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "0"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "1"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "2"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "3"]
+    ],
     "public/app/features/explore/RichHistory/RichHistoryStarredTab.tsx:5381": [
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"]
     ],
+    "public/app/features/explore/ShortLinkButtonMenu.tsx:5381": [
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "0"]
+    ],
     "public/app/features/explore/SupplementaryResultError.tsx:5381": [
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"]
     ],
@@ -4929,8 +4944,14 @@ exports[`better eslint`] = {
       [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "4"]
     ],
     "public/app/features/inspector/InspectJSONTab.tsx:5381": [
-      [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"],
-      [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "1"]
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "0"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "1"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "2"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "3"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "4"],
+      [0, 0, 0, "Do not use the t() function outside of a component or function", "5"],
+      [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "6"],
+      [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "7"]
     ],
     "public/app/features/inspector/InspectStatsTab.tsx:5381": [
       [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"]

packages/grafana-eslint-rules/README.md

@@ -156,7 +156,18 @@ const SearchTitle = ({ term }) => (
 
 #### How to translate props or attributes
 
-Right now, we only check if a string is wrapped up by the `Trans` tag. We currently do not apply this rule to props, attributes or similar, but we also ask for them to be translated with the `t()` function.
+This rule checks if a string is wrapped up by the `Trans` tag, or if certain props contain untranslated strings.
+We ask for such props to be translated with the `t()` function.
+
+The below props are checked for untranslated strings:
+
+- `label`
+- `description`
+- `placeholder`
+- `aria-label`
+- `title`
+- `text`
+- `tooltip`
 
 ```tsx
 // Bad ❌
@@ -168,3 +179,50 @@ return <input type="value" placeholder={placeholder} />;
 ```
 
 Check more info about how translations work in Grafana in [Internationalization.md](https://github.com/grafana/grafana/blob/main/contribute/internationalization.md)
+
+### `no-translation-top-level`
+
+Ensure that `t()` translation method is not used at the top level of a file, outside of a component of method.
+This is to prevent calling the translation method before it's been instantiated.
+
+This does not cause an error if a file is lazily loaded, but refactors can cause errors, and it can cause problems in tests.
+Fix the
+
+```tsx
+// Bad ❌
+const someTranslatedText = t('some.key', 'Some text');
+const SomeComponent = () => {
+  return <div title={someTranslatedText} />;
+};
+
+// Good ✅
+const SomeComponent = () => {
+  const someTranslatedText = t('some.key', 'Some text');
+  return <div title={someTranslatedText} />;
+};
+
+// Bad ❌
+const someConfigThatHasToBeShared = [{ foo: t('some.key', 'Some text') }];
+const SomeComponent = () => {
+  return (
+    <div>
+      {someConfigThatHasToBeShared.map((cfg) => {
+        return <div>{cfg.foo}</div>;
+      })}
+    </div>
+  );
+};
+
+// Good ✅
+const someConfigThatHasToBeShared = () => [{ foo: t('some.key', 'Some text') }];
+const SomeComponent = () => {
+  const configs = someConfigThatHasToBeShared();
+  return (
+    <div>
+      {configs.map((cfg) => {
+        return <div>{cfg.foo}</div>;
+      })}
+    </div>
+  );
+};
+```

packages/grafana-eslint-rules/index.cjs

@@ -2,6 +2,7 @@ const noAriaLabelSelectors = require('./rules/no-aria-label-e2e-selectors.cjs');
 const noBorderRadiusLiteral = require('./rules/no-border-radius-literal.cjs');
 const noUnreducedMotion = require('./rules/no-unreduced-motion.cjs');
 const noUntranslatedStrings = require('./rules/no-untranslated-strings.cjs');
+const noTranslationTopLevel = require('./rules/no-translation-top-level.cjs');
 const themeTokenUsage = require('./rules/theme-token-usage.cjs');
 
 module.exports = {
@@ -11,5 +12,6 @@ module.exports = {
     'no-border-radius-literal': noBorderRadiusLiteral,
     'theme-token-usage': themeTokenUsage,
     'no-untranslated-strings': noUntranslatedStrings,
+    'no-translation-top-level': noTranslationTopLevel,
   },
 };

packages/grafana-eslint-rules/rules/no-translation-top-level.cjs

@@ -0,0 +1,59 @@
+// @ts-check
+const { ESLintUtils, AST_NODE_TYPES } = require('@typescript-eslint/utils');
+
+/**
+ * @typedef {import('@typescript-eslint/utils').TSESTree.Node} Node
+ * @typedef {import('@typescript-eslint/utils').TSESLint.RuleContext<string, unknown[]>} RuleContext
+ */
+
+const createRule = ESLintUtils.RuleCreator(
+  (name) => `https://github.com/grafana/grafana/blob/main/packages/grafana-eslint-rules/README.md#${name}`
+);
+
+/**
+ * @param {RuleContext} context
+ * @param {Node} node
+ * @returns {boolean}
+ */
+const isInFunction = (context, node) => {
+  const ancestors = context.sourceCode.getAncestors(node);
+  return ancestors.some((anc) => {
+    return [
+      AST_NODE_TYPES.ArrowFunctionExpression,
+      AST_NODE_TYPES.FunctionDeclaration,
+      AST_NODE_TYPES.FunctionExpression,
+      AST_NODE_TYPES.ClassDeclaration,
+    ].includes(anc.type);
+  });
+};
+
+const noTranslationTopLevel = createRule({
+  create(context) {
+    return {
+      CallExpression(node) {
+        if (node.callee.type === AST_NODE_TYPES.Identifier && node.callee.name === 't') {
+          if (!isInFunction(context, node)) {
+            context.report({
+              node,
+              messageId: 'noMethodOutsideComponent',
+            });
+          }
+        }
+      },
+    };
+  },
+  name: 'no-translation-top-level',
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'Do not use translation functions outside of components',
+    },
+    messages: {
+      noMethodOutsideComponent: 'Do not use the t() function outside of a component or function',
+    },
+    schema: [],
+  },
+  defaultOptions: [],
+});
+
+module.exports = noTranslationTopLevel;

packages/grafana-eslint-rules/tests/no-translation-top-level.test.js

@@ -0,0 +1,58 @@
+import { RuleTester } from 'eslint';
+
+import noTranslationTopLevel from '../rules/no-translation-top-level.cjs';
+
+RuleTester.setDefaultConfig({
+  languageOptions: {
+    ecmaVersion: 2018,
+    sourceType: 'module',
+    parserOptions: {
+      ecmaFeatures: {
+        jsx: true,
+      },
+    },
+  },
+});
+
+const expectedErrorMessage = 'Do not use the t() function outside of a component or function';
+
+const ruleTester = new RuleTester();
+
+ruleTester.run('eslint no-translation-top-level', noTranslationTopLevel, {
+  valid: [
+    {
+      code: `
+function Component() {
+  return <div>{t('some.key', 'Some text')}</div>;
+}
+      `,
+    },
+    {
+      code: `const foo = () => t('some.key', 'Some text');`,
+    },
+    {
+      code: `const foo = ttt('some.key', 'Some text');`,
+    },
+    {
+      code: `class Component {
+render() {
+  return t('some.key', 'Some text');
+}
+}`,
+    },
+  ],
+  invalid: [
+    {
+      code: `const thing = t('some.key', 'Some text');`,
+      errors: [{ message: expectedErrorMessage }],
+    },
+    {
+      code: `const things = [t('some.key', 'Some text')];`,
+      errors: [{ message: expectedErrorMessage }],
+    },
+    {
+      code: `const objectThings = [{foo: t('some.key', 'Some text')}];`,
+      errors: [{ message: expectedErrorMessage }],
+    },
+  ],
+});