From b0bc24f01b16df61aa7b8b3781fb69f997286f64 Mon Sep 17 00:00:00 2001
From: dakkar <dakkar@thenautilus.net>
Date: Wed, 16 Oct 2024 15:57:15 +0100
Subject: [PATCH] lint Vue templates as well

the argument detection doesn't work inside templates when invoked via
the `<I18n>` component, because it's too complicated for me now
---
 eslint/locale.js      | 241 ++++++++++++++++++++++++------------------
 eslint/locale.test.js |  47 +++++---
 2 files changed, 168 insertions(+), 120 deletions(-)

diff --git a/eslint/locale.js b/eslint/locale.js
index a686a4b616..7e5f3419fc 100644
--- a/eslint/locale.js
+++ b/eslint/locale.js
@@ -50,6 +50,15 @@ function findCallExpression(node) {
 	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';
@@ -82,117 +91,141 @@ function setDifference(a,b) {
 
 /* 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];
-	return {
-		// for all object member access that have an identifier 'i18n'...
-		'MemberExpression:has(> Identifier[name=i18n])': (node) => {
-			// 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;
-
-			// some more checks on how the translation is called
-			if (method == 'ts') {
-				if (translation.match(/\{/)) {
-					context.report({
-						node,
-						message: `translation for ${pathStr} is parametric, but called via 'ts'`,
-					});
-					return;
-				}
-
-				if (findCallExpression(node)) {
-					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;
-				}
-
-				const callExpression = findCallExpression(node);
-				if (!callExpression) {
-					context.report({
-						node,
-						message: `translation for ${pathStr} is parametric, but not called as a function`,
-					});
-					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(' ')}`,
-					});
-				}
-			}
+	// 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 = {
diff --git a/eslint/locale.test.js b/eslint/locale.test.js
index 626fe1587c..9c7ed0f45d 100644
--- a/eslint/locale.test.js
+++ b/eslint/locale.test.js
@@ -3,31 +3,46 @@ const localeRule = require("./locale");
 
 const locale = { foo: { bar: 'ok', baz: 'good {x}' }, top: '123' };
 
-const ruleTester = new RuleTester();
+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: [
-      {code: 'i18n.ts.foo.bar', options: [locale] },
+      testCase('i18n.ts.foo.bar'),
       // we don't detect the problem here, but should still accept it
-      {code: 'i18n.ts.foo["something"]', options: [locale] },
-      {code: 'i18n.ts.top', options: [locale] },
-      {code: 'i18n.tsx.foo.baz({x:1})', options: [locale] },
-      {code: 'whatever.i18n.ts.blah.blah', options: [locale] },
-      {code: 'whatever.i18n.tsx.does.not.matter', options: [locale] },
-      {code: 'whatever(i18n.ts.foo.bar)', options: [locale] },
+      testCase('i18n.ts.foo["something"]'),
+      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>'),
     ],
     invalid: [
-      {code: 'i18n.ts.not', options: [locale], errors: 1 },
-      {code: 'i18n.tsx.deep.not', options: [locale], errors: 1 },
-      {code: 'i18n.tsx.deep.not({x:12})', options: [locale], errors: 1 },
-      {code: 'i18n.tsx.top({x:1})', options: [locale], errors: 1 },
-      {code: 'i18n.ts.foo.baz', options: [locale], errors: 1 },
-      {code: 'i18n.tsx.foo.baz', options: [locale], errors: 1 },
-      {code: 'i18n.tsx.foo.baz({y:2})', options: [locale], errors: 2 },
+      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),
     ],
   },
 );
-