diff options
| author | dakkar <dakkar@thenautilus.net> | 2024-10-16 15:57:15 +0100 |
|---|---|---|
| committer | dakkar <dakkar@thenautilus.net> | 2024-10-22 12:02:24 +0100 |
| commit | b0bc24f01b16df61aa7b8b3781fb69f997286f64 (patch) | |
| tree | c86059bdad664b5d70949a141411416f18178ba5 | |
| parent | ignore weirder cases (diff) | |
| download | sharkey-b0bc24f01b16df61aa7b8b3781fb69f997286f64.tar.gz sharkey-b0bc24f01b16df61aa7b8b3781fb69f997286f64.tar.bz2 sharkey-b0bc24f01b16df61aa7b8b3781fb69f997286f64.zip | |
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
| -rw-r--r-- | eslint/locale.js | 219 | ||||
| -rw-r--r-- | eslint/locale.test.js | 47 |
2 files changed, 157 insertions, 109 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 theRule(context) { +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]; - 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('.')}`; + // 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; - } + // 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`, + }); + } + } - // 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; + if (method === 'tsx') { + if (!translation.match(/\{/)) { + context.report({ + node, + message: `translation for ${pathStr} is not parametric, but called via 'tsx'`, + }); + 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 (!callExpression && !vueExpression) { + context.report({ + node, + message: `translation for ${pathStr} is parametric, but not called as a function`, + }); + return; + } - if (findCallExpression(node)) { - context.report({ - node, - message: `translation for ${pathStr} is not parametric, but is called as a function`, - }); - } - } + // 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 (method == 'tsx') { - if (!translation.match(/\{/)) { - context.report({ - node, - message: `translation for ${pathStr} is not parametric, but called via 'tsx'`, - }); - return; - } + if (!areArgumentsOneObject(callExpression)) { + context.report({ + node, + message: `translation for ${pathStr} should be called with a single object as argument`, + }); + return; + } - const callExpression = findCallExpression(node); - if (!callExpression) { - context.report({ - node, - message: `translation for ${pathStr} is parametric, but not called as a function`, - }); - return; - } + const translationParameters = getTranslationParameters(translation); + const parameterCount = translationParameters.size; + const callArguments = getArgumentObjectProperties(callExpression); + const argumentCount = callArguments.size; - if (!areArgumentsOneObject(callExpression)) { - context.report({ - node, - message: `translation for ${pathStr} should be called with a single object as argument`, - }); - return; - } + if (parameterCount !== argumentCount) { + context.report({ + node, + message: `translation for ${pathStr} has ${parameterCount} parameters, but is called with ${argumentCount} arguments`, + }); + } - const translationParameters = getTranslationParameters(translation); - const parameterCount = translationParameters.size; - const callArguments = getArgumentObjectProperties(callExpression); - const argumentCount = callArguments.size; + // node 20 doesn't have `Set.difference`... + const extraArguments = setDifference(callArguments, translationParameters); + const missingArguments = setDifference(translationParameters, callArguments); - if (parameterCount !== argumentCount) { - context.report({ - node, - message: `translation for ${pathStr} has ${parameterCount} parameters, but is called with ${argumentCount} arguments`, - }); - } + if (extraArguments.length > 0) { + context.report({ + node, + message: `translation for ${pathStr} passes unused arguments ${extraArguments.join(' ')}`, + }); + } - // node 20 doesn't have `Set.difference`... - const extraArguments = setDifference(callArguments, translationParameters); - const missingArguments = setDifference(translationParameters, callArguments); + if (missingArguments.length > 0) { + context.report({ + node, + message: `translation for ${pathStr} does not pass arguments ${missingArguments.join(' ')}`, + }); + } + } +} - if (extraArguments.length > 0) { - context.report({ - node, - message: `translation for ${pathStr} passes unused arguments ${extraArguments.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]; - 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), ], }, ); - |