From dd1d7f09acd71042678e316263630179e5a65733 Mon Sep 17 00:00:00 2001 From: Stephen Liu <750188453@qq.com> Date: Wed, 27 Apr 2022 17:44:13 +0800 Subject: [PATCH] chore: add eslint custom plugin to prevent translation variables (#19828) --- superset-frontend/.eslintrc.js | 10 ++- superset-frontend/jest.config.js | 2 +- superset-frontend/package-lock.json | 27 +++++++- superset-frontend/package.json | 1 + .../src/SqlLab/utils/newQueryTabName.ts | 2 +- .../views/CRUD/annotation/AnnotationList.tsx | 3 +- .../eslint-plugin-translation-vars/index.js | 56 +++++++++++++++ .../no-template-vars.test.js | 68 +++++++++++++++++++ .../package.json | 20 ++++++ 9 files changed, 184 insertions(+), 5 deletions(-) create mode 100644 superset-frontend/tools/eslint-plugin-translation-vars/index.js create mode 100644 superset-frontend/tools/eslint-plugin-translation-vars/no-template-vars.test.js create mode 100644 superset-frontend/tools/eslint-plugin-translation-vars/package.json diff --git a/superset-frontend/.eslintrc.js b/superset-frontend/.eslintrc.js index d4751c18cea46..b77de41338555 100644 --- a/superset-frontend/.eslintrc.js +++ b/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', { diff --git a/superset-frontend/jest.config.js b/superset-frontend/jest.config.js index 8ef49454b022f..18d20a1f97043 100644 --- a/superset-frontend/jest.config.js +++ b/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)$': '/spec/__mocks__/mockExportObject.js', '\\.(gif|ttf|eot|png|jpg)$': '/spec/__mocks__/mockExportString.js', diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 7ac71b41cdba6..07f2ab7601ae2 100644 --- a/superset-frontend/package-lock.json +++ b/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", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 5cf75e7c44ff4..118377ee102fe 100644 --- a/superset-frontend/package.json +++ b/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", diff --git a/superset-frontend/src/SqlLab/utils/newQueryTabName.ts b/superset-frontend/src/SqlLab/utils/newQueryTabName.ts index a719a74af59af..3815226cd4ba5 100644 --- a/superset-frontend/src/SqlLab/utils/newQueryTabName.ts +++ b/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; } diff --git a/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx b/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx index 44f99039cf646..f74a430c8b9df 100644 --- a/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx +++ b/superset-frontend/src/views/CRUD/annotation/AnnotationList.tsx @@ -282,7 +282,8 @@ function AnnotationList({ {annotationCurrentlyDeleting && ( { if (annotationCurrentlyDeleting) { diff --git a/superset-frontend/tools/eslint-plugin-translation-vars/index.js b/superset-frontend/tools/eslint-plugin-translation-vars/index.js new file mode 100644 index 0000000000000..69493f3b9e39d --- /dev/null +++ b/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, + }; + }, + }, + }, +}; diff --git a/superset-frontend/tools/eslint-plugin-translation-vars/no-template-vars.test.js b/superset-frontend/tools/eslint-plugin-translation-vars/no-template-vars.test.js new file mode 100644 index 0000000000000..295a2f9fb8c50 --- /dev/null +++ b/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, + }, + ], +}); diff --git a/superset-frontend/tools/eslint-plugin-translation-vars/package.json b/superset-frontend/tools/eslint-plugin-translation-vars/package.json new file mode 100644 index 0000000000000..d4353a88df3d2 --- /dev/null +++ b/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" + } +}