Skip to content

Commit

Permalink
Added ESlint plugin for checking translation issues
Browse files Browse the repository at this point in the history
  • Loading branch information
lslezak committed Oct 13, 2023
1 parent 99ab3e4 commit 2db6d79
Show file tree
Hide file tree
Showing 10 changed files with 242 additions and 1 deletion.
7 changes: 7 additions & 0 deletions doc/i18n.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- [Formatting Texts](#formatting-texts)
- [TRANSLATORS Comments](#translators-comments)
- [Missing Translations](#missing-translations)
- [ESLint Plugin](#eslint-plugin)
- [Testing Language](#testing-language)
- [Building POT File](#building-pot-file)
- [Cockpit Details](#cockpit-details)
Expand Down Expand Up @@ -405,6 +406,12 @@ even work, no crash. But there are still translation problems with them.
In both cases the strings will not be extracted to the POT file.
### ESLint Plugin
The [eslint-plugin-agama-i18n](../web/eslint-plugin-agama-i18n/) subdirectory
contains an ESLint plugin which ensures that a string literal is passed
to the translation function. See more details there.
### Testing Language
The Agama web interface supports special testing `xx` language. That language
Expand Down
10 changes: 9 additions & 1 deletion web/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@
},
"sourceType": "module"
},
"plugins": ["flowtype", "i18next", "react", "react-hooks", "@typescript-eslint"],
"plugins": ["agama-i18n", "flowtype", "i18next", "react", "react-hooks", "@typescript-eslint"],
"rules": {
"agama-i18n/string-literals": "error",
"i18next/no-literal-string": "error",
"indent": ["error", 2,
{
Expand Down Expand Up @@ -66,6 +67,13 @@
"rules": {
"i18next/no-literal-string": "off"
}
},
{
// do not check translation arguments in the test, it checks some internals by passing variables
"files": ["i18n.test.js"],
"rules": {
"agama-i18n/string-literals": "off"
}
}
],
"globals": {
Expand Down
31 changes: 31 additions & 0 deletions web/eslint-plugin-agama-i18n/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# The ESLint Plugin

This directory contains an ESLint plugin which checks that only string literals
are passed to the translation functions.

It is bundled here because it is closely tied to the Agama project and probably
does not make sense for other projects.

## Disabling the Check

In some rare cases using a variable instead of a string literal is correct. In
that case disable the check locally:

```js
const SIZES = [ N_("small"), N_("medium"), N_("large") ];

// returns one of the sizes above
const sz = getSize();

// eslint-disable-next-line agama-i18n/string-literals
return <span>{_(sz)}</span>;
```

## Links

- https://eslint.org/docs/latest/extend/custom-rule-tutorial - tutorial for
writing an ESLint plugin
- https://eslint.org/docs/latest/extend/custom-rules - documentation for
writing an ESLint plugin
- https://astexplorer.net - online tool for browsing a parsed AST tree,
useful for inspecting the properties of parsed source code
29 changes: 29 additions & 0 deletions web/eslint-plugin-agama-i18n/eslint-plugin-agama-i18n.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Copyright (c) [2023] SUSE LLC
*
* All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License as published
* by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, contact SUSE LLC.
*
* To contact SUSE LLC about this file by physical or electronic mail, you may
* find current contact information at www.suse.com.
*/

const stringLiteralsRule = require("./string-literals");

module.exports = {
rules: {
// name of the rule
"string-literals": stringLiteralsRule
}
};
5 changes: 5 additions & 0 deletions web/eslint-plugin-agama-i18n/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "eslint-plugin-agama-i18n",
"version": "0.0.1",
"main": "eslint-plugin-agama-i18n.js"
}
84 changes: 84 additions & 0 deletions web/eslint-plugin-agama-i18n/string-literals.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright (c) [2023] SUSE LLC
*
* All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License as published
* by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, contact SUSE LLC.
*
* To contact SUSE LLC about this file by physical or electronic mail, you may
* find current contact information at www.suse.com.
*/

// names of all translation functions
const translations = ["_", "n_", "N_", "Nn_"];
// names of the plural translation functions
const plurals = ["n_", "Nn_"];

const errorMsgLiteral = "Use a string literal argument in the translation functions";
const errorMsgMissing = "Missing argument";

/**
* Check whether the AST tree node is a string literal
* @param {Object} node the node
* @returns {boolean} true if the node is a string literal
*/
function isStringLiteral(node) {
if (!node) return false;

return node.type === "Literal" && (typeof node.value === "string");
}

/**
* Check whether the ATS node is a string literal
* @param {Object} node the node to check
* @param {Object} parentNode parent node for reporting error if `node` is undefined
* @param {Object} context the context for reporting an error
*/
function checkNode(node, parentNode, context) {
if (node) {
// string literal?
if (!isStringLiteral(node)) {
context.report(node, errorMsgLiteral);
}
} else {
// missing argument
context.report(parentNode, errorMsgMissing);
}
}

// define the eslint rule
module.exports = {
meta: {
type: "problem",
docs: {
description: "Check that the arguments if the translation functions are string literals.",
},
},
create: function (context) {
return {
// callback for handling function calls
CallExpression(node) {
// not a translation function, skip it
if (!translations.includes(node.callee.name)) return;

// check the first argument
checkNode(node.arguments[0], node, context);

// check also the second argument for the plural forms
if (plurals.includes(node.callee.name)) {
checkNode(node.arguments[1], node, context);
}
}
};
}
};
62 changes: 62 additions & 0 deletions web/eslint-plugin-agama-i18n/string-literals.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright (c) [2023] SUSE LLC
*
* All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License as published
* by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, contact SUSE LLC.
*
* To contact SUSE LLC about this file by physical or electronic mail, you may
* find current contact information at www.suse.com.
*/

const { RuleTester } = require("eslint");
const stringLiteralsRule = require("./string-literals");

const ruleTester = new RuleTester({
parserOptions: { ecmaVersion: 2015 }
});

ruleTester.run(
"string-literals",
stringLiteralsRule,
{
// valid code examples, these should pass
valid: [
{ code: "_(\"foo\")" },
{ code: "_('foo')" },
{ code: "n_(\"one\", \"many\", count)" },
{ code: "n_('one', 'many', count)" },
],
// invalid examples, these should fail
invalid: [
// string literal errors
{ code: "_(null)", errors: 1 },
{ code: "_(undefined)", errors: 1 },
{ code: "_(42)", errors: 1 },
{ code: "_(foo)", errors: 1 },
{ code: "_(foo())", errors: 1 },
{ code: "_(`foo`)", errors: 1 },
{ code: "_(\"foo\" + \"bar\")", errors: 1 },
{ code: "_('foo' + 'bar')", errors: 1 },
// missing argument errors
{ code: "_()", errors: 1 },
{ code: "n_('foo')", errors: 1 },
{ code: "n_(\"foo\")", errors: 1 },
// string literal + missing argument errors
{ code: "n_(foo)", errors: 2 },
// string literal error twice
{ code: "n_(foo, bar)", errors: 2 },
{ code: "Nn_(foo, bar)", errors: 2 },
],
}
);
12 changes: 12 additions & 0 deletions web/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"eslint-config-standard": "^17.0.0",
"eslint-config-standard-jsx": "^11.0.0",
"eslint-config-standard-react": "^13.0.0",
"eslint-plugin-agama-i18n": "file:eslint-plugin-agama-i18n",
"eslint-plugin-flowtype": "^8.0.3",
"eslint-plugin-i18next": "^6.0.3",
"eslint-plugin-import": "^2.22.1",
Expand Down
2 changes: 2 additions & 0 deletions web/src/components/storage/VolumeForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const SizeUnitFormSelect = ({ units, ...formSelectProps }) => {
return (
<FormSelect { ...formSelectProps }>
{/* the unit values are marked for translation in the utils.js file */}
{/* eslint-disable-next-line agama-i18n/string-literals */}
{ units.map(unit => <FormSelectOption key={unit} value={unit} label={_(unit)} />) }
</FormSelect>
);
Expand Down Expand Up @@ -295,6 +296,7 @@ const SizeOptions = ({ errors, formData, volume, onChange }) => {
<Radio
id={value}
key={`size-${value}`}
// eslint-disable-next-line agama-i18n/string-literals
label={_(SIZE_OPTION_LABELS[value] || value)}
value={value}
name="size-option"
Expand Down

0 comments on commit 2db6d79

Please sign in to comment.