chore: add eslint custom plugin to prevent translation variables (#19828)

superset-frontend/.eslintrc.js

@@ -67,7 +67,13 @@ module.exports = {
       version: 'detect',
     },
   },
-  plugins: ['prettier', 'react', 'file-progress', 'theme-colors'],
+  plugins: [
+    'prettier',
+    'react',
+    'file-progress',
+    'theme-colors',
+    'translation-vars',
+  ],
   overrides: [
     {
       files: ['*.ts', '*.tsx'],
@@ -198,12 +204,14 @@ module.exports = {
       ],
       rules: {
         'theme-colors/no-literal-colors': 0,
+        'translation-vars/no-template-vars': 0,
         'no-restricted-imports': 0,
       },
     },
   ],
   rules: {
     'theme-colors/no-literal-colors': 1,
+    'translation-vars/no-template-vars': ['error', true],
     camelcase: [
       'error',
       {

superset-frontend/jest.config.js

@@ -19,7 +19,7 @@
 
 module.exports = {
   testRegex:
-    '\\/superset-frontend\\/(spec|src|plugins|packages)\\/.*(_spec|\\.test)\\.[jt]sx?$',
+    '\\/superset-frontend\\/(spec|src|plugins|packages|tools)\\/.*(_spec|\\.test)\\.[jt]sx?$',
   moduleNameMapper: {
     '\\.(css|less|geojson)$': '<rootDir>/spec/__mocks__/mockExportObject.js',
     '\\.(gif|ttf|eot|png|jpg)$': '<rootDir>/spec/__mocks__/mockExportString.js',

superset-frontend/package-lock.json

@@ -234,6 +234,7 @@
         "eslint-plugin-react-hooks": "^4.2.0",
         "eslint-plugin-testing-library": "^3.10.1",
         "eslint-plugin-theme-colors": "file:tools/eslint-plugin-theme-colors",
+        "eslint-plugin-translation-vars": "file:tools/eslint-plugin-translation-vars",
         "exports-loader": "^0.7.0",
         "fetch-mock": "^7.7.3",
         "fork-ts-checker-webpack-plugin": "^6.3.3",
@@ -33942,6 +33943,10 @@
       "resolved": "tools/eslint-plugin-theme-colors",
       "link": true
     },
+    "node_modules/eslint-plugin-translation-vars": {
+      "resolved": "tools/eslint-plugin-translation-vars",
+      "link": true
+    },
     "node_modules/eslint-scope": {
       "version": "5.1.1",
       "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-5.1.1.tgz",
@@ -60691,7 +60696,23 @@
     "tools/eslint-plugin-theme-colors": {
       "version": "1.0.0",
       "dev": true,
-      "license": "Apache-2.0"
+      "license": "Apache-2.0",
+      "engines": {
+        "node": "^16.9.1",
+        "npm": "^7.5.4"
+      }
+    },
+    "tools/eslint-plugin-translation-vars": {
+      "version": "1.0.0",
+      "dev": true,
+      "license": "Apache-2.0",
+      "engines": {
+        "node": "^16.9.1",
+        "npm": "^7.5.4"
+      },
+      "peerDependencies": {
+        "eslint": ">=0.8.0"
+      }
     }
   },
   "dependencies": {
@@ -87649,6 +87670,10 @@
     "eslint-plugin-theme-colors": {
       "version": "file:tools/eslint-plugin-theme-colors"
     },
+    "eslint-plugin-translation-vars": {
+      "version": "file:tools/eslint-plugin-translation-vars",
+      "requires": {}
+    },
     "eslint-scope": {
       "version": "5.1.1",
       "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-5.1.1.tgz",

superset-frontend/package.json

@@ -294,6 +294,7 @@
     "eslint-plugin-react-hooks": "^4.2.0",
     "eslint-plugin-testing-library": "^3.10.1",
     "eslint-plugin-theme-colors": "file:tools/eslint-plugin-theme-colors",
+    "eslint-plugin-translation-vars": "file:tools/eslint-plugin-translation-vars",
     "exports-loader": "^0.7.0",
     "fetch-mock": "^7.7.3",
     "fork-ts-checker-webpack-plugin": "^6.3.3",

superset-frontend/src/SqlLab/utils/newQueryTabName.ts

@@ -40,7 +40,7 @@ export const newQueryTabName = (
       // When there are query tabs open, and at least one is called "Untitled Query #"
       // Where # is a valid number
       const largestNumber: number = Math.max(...untitledQueryNumbers);
-      return t(`${untitledQuery}%s`, largestNumber + 1);
+      return t('%s%s', untitledQuery, largestNumber + 1);
     }
     return resultTitle;
   }

superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx

@@ -280,7 +280,8 @@ function AnnotationList({
       {annotationCurrentlyDeleting && (
         <DeleteModal
           description={t(
-            `Are you sure you want to delete ${annotationCurrentlyDeleting?.short_descr}?`,
+            'Are you sure you want to delete %s?',
+            annotationCurrentlyDeleting?.short_descr,
           )}
           onConfirm={() => {
             if (annotationCurrentlyDeleting) {

superset-frontend/tools/eslint-plugin-translation-vars/index.js

@@ -0,0 +1,56 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @fileoverview Rule to warn about translation template variables
+ * @author Apache
+ */
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+/** @type {import('eslint').Rule.RuleModule} */
+module.exports = {
+  rules: {
+    'no-template-vars': {
+      create(context) {
+        function handler(node) {
+          if (node.arguments.length) {
+            const firstArgs = node.arguments[0];
+            if (
+              firstArgs.type === 'TemplateLiteral' &&
+              firstArgs.expressions.length
+            ) {
+              context.report({
+                node,
+                message:
+                  "Don't use variables in translation string templates. Flask-babel is a static translation translation service, so it can’t handle strings that include variables",
+              });
+            }
+          }
+        }
+        return {
+          "CallExpression[callee.name='t']": handler,
+          "CallExpression[callee.name='tn']": handler,
+        };
+      },
+    },
+  },
+};

superset-frontend/tools/eslint-plugin-translation-vars/no-template-vars.test.js

@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @fileoverview Rule to warn about translation template variables
+ * @author Apache
+ */
+/* eslint-disable no-template-curly-in-string */
+const { RuleTester } = require('eslint');
+const plugin = require('.');
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+const rule = plugin.rules['no-template-vars'];
+
+const errors = [
+  {
+    type: 'CallExpression',
+  },
+];
+
+ruleTester.run('no-template-vars', rule, {
+  valid: [
+    't(`foo`)',
+    'tn(`foo`)',
+    't(`foo %s bar`)',
+    'tn(`foo %s bar`)',
+    't(`foo %s bar %s`)',
+    'tn(`foo %s bar %s`)',
+  ],
+  invalid: [
+    {
+      code: 't(`foo${bar}`)',
+      errors,
+    },
+    {
+      code: 't(`foo${bar} ${baz}`)',
+      errors,
+    },
+    {
+      code: 'tn(`foo${bar}`)',
+      errors,
+    },
+    {
+      code: 'tn(`foo${bar} ${baz}`)',
+      errors,
+    },
+  ],
+});

superset-frontend/tools/eslint-plugin-translation-vars/package.json

@@ -0,0 +1,20 @@
+{
+  "name": "eslint-plugin-translation-vars",
+  "version": "1.0.0",
+  "description": "Warns about translation variables",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "keywords": [],
+  "license": "Apache-2.0",
+  "author": "Apache",
+  "dependencies": {},
+  "peerDependencies": {
+    "eslint": ">=0.8.0"
+  },
+  "engines": {
+    "node": "^16.9.1",
+    "npm": "^7.5.4"
+  }
+}