merge: teach eslint to check translations (!695)

View MR for information: https://activitypub.software/TransFem-org/Sharkey/-/merge_requests/695

Approved-by: Marie <github@yuugi.dev>
Approved-by: Hazelnoot <acomputerdog@gmail.com>

eslint/locale.js

@@ -0,0 +1,251 @@
+/*
+ * SPDX-FileCopyrightText: dakkar and other Sharkey contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+*/
+
+/* This is a ESLint rule to report use of the `i18n.ts` and `i18n.tsx`
+ * objects that reference translation items that don't actually exist
+ * in the lexicon (the `locale/` files)
+ */
+
+/* given a MemberExpression node, collects all the member names
+ *
+ * e.g. for a bit of code like `foo=one.two.three`, `collectMembers`
+ * called on the node for `three` would return `['one', 'two',
+ * 'three']`
+ */
+function collectMembers(node) {
+	if (!node) return [];
+	if (node.type !== 'MemberExpression') return [];
+	// this is something like `foo[bar]`
+	if (node.computed) return [];
+	return [ node.property.name, ...collectMembers(node.parent) ];
+}
+
+/* given an object and an array of names, recursively descends the
+ * object via those names
+ *
+ * e.g. `walkDown({one:{two:{three:15}}},['one','two','three'])` would
+ * return 15
+ */
+function walkDown(locale, path) {
+	if (!locale) return null;
+	if (!path || path.length === 0 || !path[0]) return locale;
+	return walkDown(locale[path[0]], path.slice(1));
+}
+
+/* given a MemberExpression node, returns its attached CallExpression
+ * node if present
+ *
+ * e.g. for a bit of code like `foo=one.two.three()`,
+ * `findCallExpression` called on the node for `three` would return
+ * the node for function call (which is the parent of the `one` and
+ * `two` nodes, and holds the nodes for the argument list)
+ *
+ * if the code had been `foo=one.two.three`, `findCallExpression`
+ * would have returned null, because there's no function call attached
+ * to the MemberExpressions
+ */
+function findCallExpression(node) {
+	if (!node.parent) return null;
+
+	// the second half of this guard protects from cases like
+	// `foo(one.two.three)` where the CallExpression is parent of the
+	// MemberExpressions, but via `arguments`, not `callee`
+	if (node.parent.type === 'CallExpression' && node.parent.callee === node) return node.parent;
+	if (node.parent.type === 'MemberExpression') return findCallExpression(node.parent);
+	return null;
+}
+
+// same, but for Vue expressions (`<I18n :src="i18n.ts.foo">`)
+function findVueExpression(node) {
+	if (!node.parent) return null;
+
+	if (node.parent.type.match(/^VExpr/) && node.parent.expression === node) return node.parent;
+	if (node.parent.type === 'MemberExpression') return findVueExpression(node.parent);
+	return null;
+}
+
+function areArgumentsOneObject(node) {
+	return node.arguments.length === 1 &&
+		node.arguments[0].type === 'ObjectExpression';
+}
+
+// only call if `areArgumentsOneObject(node)` is true
+function getArgumentObjectProperties(node) {
+	return new Set(node.arguments[0].properties.map(
+		p => {
+			if (p.key && p.key.type === 'Identifier') return p.key.name;
+			return null;
+		},
+	));
+}
+
+function getTranslationParameters(translation) {
+	return new Set(Array.from(translation.matchAll(/\{(\w+)\}/g)).map( m => m[1] ));
+}
+
+function setDifference(a,b) {
+	const result = [];
+	for (const element of a.values()) {
+		if (!b.has(element)) {
+			result.push(element);
+		}
+	}
+
+	return result;
+}
+
+/* the actual rule body
+ */
+function theRuleBody(context,node) {
+	// we get the locale/translations via the options; it's the data
+	// that goes into a specific language's JSON file, see
+	// `scripts/build-assets.mjs`
+	const locale = context.options[0];
+
+	// sometimes we get MemberExpression nodes that have a
+	// *descendent* with the right identifier: skip them, we'll get
+	// the right ones as well
+	if (node.object?.name !== 'i18n') {
+		return;
+	}
+
+	// `method` is going to be `'ts'` or `'tsx'`, `path` is going to
+	// be the various translation steps/names
+	const [ method, ...path ] = collectMembers(node);
+	const pathStr = `i18n.${method}.${path.join('.')}`;
+
+	// does that path point to a real translation?
+	const translation = walkDown(locale, path);
+	if (!translation) {
+		context.report({
+			node,
+			message: `translation missing for ${pathStr}`,
+		});
+		return;
+	}
+
+	// we hit something weird, assume the programmers know what
+	// they're doing (this is usually some complicated slicing of
+	// the translation structure)
+	if (typeof(translation) !== 'string') return;
+
+	const callExpression = findCallExpression(node);
+	const vueExpression = findVueExpression(node);
+
+	// some more checks on how the translation is called
+	if (method === 'ts') {
+		// the `<I18n> component gets parametric translations via
+		// `i18n.ts.*`, but we error out elsewhere
+		if (translation.match(/\{/) && !vueExpression) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} is parametric, but called via 'ts'`,
+			});
+			return;
+		}
+
+		if (callExpression) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} is not parametric, but is called as a function`,
+			});
+		}
+	}
+
+	if (method === 'tsx') {
+		if (!translation.match(/\{/)) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} is not parametric, but called via 'tsx'`,
+			});
+			return;
+		}
+
+		if (!callExpression && !vueExpression) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} is parametric, but not called as a function`,
+			});
+			return;
+		}
+
+		// we're not currently checking arguments when used via the
+		// `<I18n>` component, because it's too complicated (also, it
+		// would have to be done inside the `if (method === 'ts')`)
+		if (!callExpression) return;
+
+		if (!areArgumentsOneObject(callExpression)) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} should be called with a single object as argument`,
+			});
+			return;
+		}
+
+		const translationParameters = getTranslationParameters(translation);
+		const parameterCount = translationParameters.size;
+		const callArguments = getArgumentObjectProperties(callExpression);
+		const argumentCount = callArguments.size;
+
+		if (parameterCount !== argumentCount) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} has ${parameterCount} parameters, but is called with ${argumentCount} arguments`,
+			});
+		}
+
+		// node 20 doesn't have `Set.difference`...
+		const extraArguments = setDifference(callArguments, translationParameters);
+		const missingArguments = setDifference(translationParameters, callArguments);
+
+		if (extraArguments.length > 0) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} passes unused arguments ${extraArguments.join(' ')}`,
+			});
+		}
+
+		if (missingArguments.length > 0) {
+			context.report({
+				node,
+				message: `translation for ${pathStr} does not pass arguments ${missingArguments.join(' ')}`,
+			});
+		}
+	}
+}
+
+function theRule(context) {
+	// we get the locale/translations via the options; it's the data
+	// that goes into a specific language's JSON file, see
+	// `scripts/build-assets.mjs`
+	const locale = context.options[0];
+
+	// for all object member access that have an identifier 'i18n'...
+	return context.getSourceCode().parserServices.defineTemplateBodyVisitor(
+		{
+			// this is for <template> bits, needs work
+			'MemberExpression:has(Identifier[name=i18n])': (node) => theRuleBody(context, node),
+		},
+		{
+			// this is for normal code
+			'MemberExpression:has(Identifier[name=i18n])': (node) => theRuleBody(context, node),
+		},
+	);
+}
+
+module.exports = {
+	meta: {
+		type: 'problem',
+		docs: {
+			description: 'assert that all translations used are present in the locale files',
+		},
+		schema: [
+			// here we declare that we need the locale/translation as a
+			// generic object
+			{ type: 'object', additionalProperties: true },
+		],
+	},
+	create: theRule,
+};

eslint/locale.test.js

@@ -0,0 +1,54 @@
+/*
+ * SPDX-FileCopyrightText: dakkar and other Sharkey contributors
+ * SPDX-License-Identifier: AGPL-3.0-only
+*/
+
+const {RuleTester} = require("eslint");
+const localeRule = require("./locale");
+
+const locale = { foo: { bar: 'ok', baz: 'good {x}' }, top: '123' };
+
+const ruleTester = new RuleTester({
+  languageOptions: {
+    parser: require('vue-eslint-parser'),
+    ecmaVersion: 2015,
+  },
+});
+
+function testCase(code,errors) {
+  return { code, errors, options: [ locale ], filename: 'test.ts' };
+}
+function testCaseVue(code,errors) {
+  return { code, errors, options: [ locale ], filename: 'test.vue' };
+}
+
+ruleTester.run(
+  'sharkey-locale',
+  localeRule,
+  {
+    valid: [
+      testCase('i18n.ts.foo.bar'),
+      testCase('i18n.ts.top'),
+      testCase('i18n.tsx.foo.baz({x:1})'),
+      testCase('whatever.i18n.ts.blah.blah'),
+      testCase('whatever.i18n.tsx.does.not.matter'),
+      testCase('whatever(i18n.ts.foo.bar)'),
+      testCaseVue('<template><p>{{ i18n.ts.foo.bar }}</p></template>'),
+      testCaseVue('<template><I18n :src="i18n.ts.foo.baz"/></template>'),
+      // we don't detect the problem here, but should still accept it
+      testCase('i18n.ts.foo["something"]'),
+      testCase('i18n.ts.foo[something]'),
+    ],
+    invalid: [
+      testCase('i18n.ts.not', 1),
+      testCase('i18n.tsx.deep.not', 1),
+      testCase('i18n.tsx.deep.not({x:12})', 1),
+      testCase('i18n.tsx.top({x:1})', 1),
+      testCase('i18n.ts.foo.baz', 1),
+      testCase('i18n.tsx.foo.baz', 1),
+      testCase('i18n.tsx.foo.baz({y:2})', 2),
+      testCaseVue('<template><p>{{ i18n.ts.not }}</p></template>', 1),
+      testCaseVue('<template><I18n :src="i18n.ts.not"/></template>', 1),
+    ],
+  },
+);

packages/frontend-embed/eslint.config.js

@@ -4,6 +4,8 @@ import parser from 'vue-eslint-parser';
 import pluginVue from 'eslint-plugin-vue';
 import pluginMisskey from '@misskey-dev/eslint-plugin';
 import sharedConfig from '../shared/eslint.config.js';
+import localeRule from '../../eslint/locale.js';
+import { build as buildLocales } from '../../locales/index.js';
 
 export default [
 	...sharedConfig,
@@ -14,6 +16,7 @@ export default [
 	...pluginVue.configs['flat/recommended'],
 	{
 		files: ['{src,test,js,@types}/**/*.{ts,vue}'],
+		plugins: { sharkey: { rules: { locale: localeRule } } },
 		languageOptions: {
 			globals: {
 				...Object.fromEntries(Object.entries(globals.node).map(([key]) => [key, 'off'])),
@@ -44,6 +47,8 @@ export default [
 			},
 		},
 		rules: {
+			'sharkey/locale': ['error', buildLocales()['en-US']],
+
 			'@typescript-eslint/no-empty-interface': ['error', {
 				allowSingleExtends: true,
 			}],

packages/frontend/eslint.config.js

@@ -4,6 +4,8 @@ import parser from 'vue-eslint-parser';
 import pluginVue from 'eslint-plugin-vue';
 import pluginMisskey from '@misskey-dev/eslint-plugin';
 import sharedConfig from '../shared/eslint.config.js';
+import localeRule from '../../eslint/locale.js';
+import { build as buildLocales } from '../../locales/index.js';
 
 export default [
 	...sharedConfig,
@@ -14,6 +16,7 @@ export default [
 	...pluginVue.configs['flat/recommended'],
 	{
 		files: ['{src,test,js,@types}/**/*.{ts,vue}'],
+		plugins: { sharkey: { rules: { locale: localeRule } } },
 		languageOptions: {
 			globals: {
 				...Object.fromEntries(Object.entries(globals.node).map(([key]) => [key, 'off'])),
@@ -44,6 +47,8 @@ export default [
 			},
 		},
 		rules: {
+			'sharkey/locale': ['error', buildLocales()['en-US']],
+
 			'@typescript-eslint/no-empty-interface': ['error', {
 				allowSingleExtends: true,
 			}],

packages/frontend/src/pages/admin-user.vue

@@ -138,7 +138,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 			</div>
 
 			<div v-else-if="tab === 'announcements'" class="_gaps">
-				<MkButton primary rounded @click="createAnnouncement"><i class="ti ti-plus"></i> {{ i18n.ts.new }}</MkButton>
+				<MkButton primary rounded @click="createAnnouncement"><i class="ti ti-plus"></i> {{ i18n.ts._announcement.new }}</MkButton>
 
 				<MkPagination :pagination="announcementsPagination">
 					<template #default="{ items }">

packages/frontend/src/pages/admin/email-settings.vue

@@ -100,7 +100,7 @@ async function init() {
 
 async function testEmail() {
 	const { canceled, result: destination } = await os.inputText({
-		title: i18n.ts.destination,
+		title: i18n.ts.emailDestination,
 		type: 'email',
 		default: instance.maintainerEmail ?? '',
 		placeholder: 'test@example.com',

packages/frontend/src/pages/auth.vue

@@ -25,7 +25,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 				<h1>{{ i18n.ts._auth.denied }}</h1>
 			</div>
 			<div v-if="state == 'accepted' && session">
-				<h1>{{ session.app.isAuthorized ? i18n.ts['already-authorized'] : i18n.ts.allowed }}</h1>
+				<h1>{{ session.app.isAuthorized ? i18n.ts['already-authorized'] : i18n.ts._auth.allowed }}</h1>
 				<p v-if="session.app.callbackUrl">
 					{{ i18n.ts._auth.callback }}
 					<MkEllipsis/>

packages/frontend/src/pages/page.vue

@@ -266,7 +266,7 @@ function showMenu(ev: MouseEvent) {
 	if ($i && $i.id === page.value.userId) {
 		menuItems.push({
 			icon: 'ti ti-pencil',
-			text: i18n.ts.editThisPage,
+			text: i18n.ts._pages.editThisPage,
 			action: () => router.push(`/pages/edit/${page.value.id}`),
 		});
 

packages/frontend/src/pages/registry.keys.vue

@@ -22,7 +22,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 			<MkButton primary @click="createKey">{{ i18n.ts._registry.createKey }}</MkButton>
 
 			<FormSection v-if="keys">
-				<template #label>{{ i18n.ts.keys }}</template>
+				<template #label>{{ i18n.ts._registry.keys }}</template>
 				<div class="_gaps_s">
 					<FormLink v-for="key in keys" :to="`/registry/value/${props.domain}/${scope.join('/')}/${key[0]}`" class="_monospace">{{ key[0] }}<template #suffix>{{ key[1].toUpperCase() }}</template></FormLink>
 				</div>

packages/frontend/src/ui/_common_/upload.vue

@@ -11,7 +11,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 			<div class="top">
 				<p class="name"><MkLoading :em="true"/>{{ ctx.name }}</p>
 				<p class="status">
-					<span v-if="ctx.progressValue === undefined" class="initing">{{ i18n.ts.waiting }}<MkEllipsis/></span>
+					<span v-if="ctx.progressValue === undefined" class="initing">{{ i18n.ts.uploading }}</span>
 					<span v-if="ctx.progressValue !== undefined" class="kb">{{ String(Math.floor(ctx.progressValue / 1024)).replace(/(\d)(?=(\d\d\d)+(?!\d))/g, '$1,') }}<i>KB</i> / {{ String(Math.floor(ctx.progressMax / 1024)).replace(/(\d)(?=(\d\d\d)+(?!\d))/g, '$1,') }}<i>KB</i></span>
 					<span v-if="ctx.progressValue !== undefined" class="percentage">{{ Math.floor((ctx.progressValue / ctx.progressMax) * 100) }}</span>
 				</p>

packages/frontend/src/widgets/WidgetMemo.vue

@@ -9,7 +9,7 @@ SPDX-License-Identifier: AGPL-3.0-only
 	<template #header>{{ i18n.ts._widgets.memo }}</template>
 
 	<div :class="$style.root">
-		<textarea v-model="text" :style="`height: ${widgetProps.height}px;`" :class="$style.textarea" :placeholder="i18n.ts.placeholder" @input="onChange"></textarea>
+		<textarea v-model="text" :style="`height: ${widgetProps.height}px;`" :class="$style.textarea" @input="onChange"></textarea>
 		<button :class="$style.save" :disabled="!changed" class="_buttonPrimary" @click="saveMemo">{{ i18n.ts.save }}</button>
 	</div>
 </MkContainer>

packages/frontend/test/i18n.test.ts

@@ -7,6 +7,8 @@ import { describe, expect, it } from 'vitest';
 import { I18n } from '../../frontend-shared/js/i18n.js'; // @@で参照できなかったので
 import { ParameterizedString } from '../../../locales/index.js';
 
+/* eslint "sharkey/locale":"off" */
+
 // TODO: このテストはfrontend-sharedに移動する
 
 describe('i18n', () => {

sharkey-locales/en-US.yml

@@ -9,6 +9,12 @@ openRemoteProfile: "Open remote profile"
 trustedLinkUrlPatterns: "Link to external site warning exclusion list"
 trustedLinkUrlPatternsDescription: "Separate with spaces for an AND condition or with line breaks for an OR condition. Using surrounding keywords with slashes will turn them into a regular expression. If you write only the domain name, it will be a backward match."
 mutuals: "Mutuals"
+isLocked: "Private account"
+isAdmin: "Administrator"
+isBot: "Bot user"
+open: "Open"
+emailDestination: "Destination address"
+date: "Date"
 renote: "Boost"
 unrenote: "Remove boost"
 renoted: "Boosted."
@@ -149,6 +155,7 @@ showNonPublicNotes: "Show non-public"
 allowClickingNotifications: "Allow clicking on pop-up notifications"
 pinnedOnly: "Pinned"
 blockingYou: "Blocking you"
+warnExternalUrl: "Show warning when opening external URLs"
 _delivery:
   stop: "Suspend delivery"
   resume: "Resume delivery"
@@ -383,4 +390,10 @@ _externalNavigationWarning:
   title: "Navigate to an external site"
   description: "Leave {host} and go to an external site"
   trustThisDomain: "Trust this domain on this device in the future"
+
 remoteFollowersWarning: "Remote followers may have incomplete or outdated activity"
+
+_auth:
+  allowed: "Allowed"
+_announcement:
+  new: "New"