From ec41a75cdb54192f155352bb25c70be6307a3c9f Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 22 Mar 2022 14:06:23 +0100 Subject: [PATCH 1/2] Add support for concat expressions in HBS files --- __snapshots__/test.js.snap | 14 ++++ .../app/templates/application.hbs | 8 ++ .../concat-expression/translations/en.yaml | 23 +++++ index.js | 83 ++++++++++++------- 4 files changed, 98 insertions(+), 30 deletions(-) create mode 100644 fixtures/concat-expression/app/templates/application.hbs create mode 100644 fixtures/concat-expression/translations/en.yaml diff --git a/__snapshots__/test.js.snap b/__snapshots__/test.js.snap index 638c96fd..cfa8f7e8 100644 --- a/__snapshots__/test.js.snap +++ b/__snapshots__/test.js.snap @@ -1,5 +1,19 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Test Fixtures concat-expression 1`] = ` +"[1/4] 🔍 Finding JS and HBS files... +[2/4] 🔍 Searching for translations keys in JS and HBS files... +[3/4] ⚙️ Checking for unused translations... +[4/4] ⚙️ Checking for missing translations... + + 👏 No unused translations were found! + + 👏 No missing translations were found! +" +`; + +exports[`Test Fixtures concat-expression 2`] = `Map {}`; + exports[`Test Fixtures decorators 1`] = ` "[1/4] 🔍 Finding JS and HBS files... [2/4] 🔍 Searching for translations keys in JS and HBS files... diff --git a/fixtures/concat-expression/app/templates/application.hbs b/fixtures/concat-expression/app/templates/application.hbs new file mode 100644 index 00000000..6278ff0b --- /dev/null +++ b/fixtures/concat-expression/app/templates/application.hbs @@ -0,0 +1,8 @@ +{{t (concat "prefix" "." "simple")}} +{{t (concat "prefix." (if true "with-if-first" "with-if-second"))}} +{{t (concat "prefix.some-action" (if this.isCompact "-short"))}} +{{t (concat "prefix." (if this.isEditing "edit" "new") ".label")}} +{{t (if true "foo" (concat "prefix." (if this.isEditing "edit" "new") ".nested"))}} +{{t (concat "prefix." this.dynamicKey ".not-missing")}} +{{t (concat "prefix." (if true "a1" (if false "b1" (concat "c" (if false "1" "2")))) ".value")}} +{{t (concat "prefix." (if true this.dynamicKey "key-that-should-exist") ".value")}} diff --git a/fixtures/concat-expression/translations/en.yaml b/fixtures/concat-expression/translations/en.yaml new file mode 100644 index 00000000..4283cb98 --- /dev/null +++ b/fixtures/concat-expression/translations/en.yaml @@ -0,0 +1,23 @@ +prefix: + simple: Simple concatenation + with-if-first: First condition + with-if-second: Second condition + some-action: Action that can be long + some-action-short: Action + edit: + label: Label on edit branch + nested: Nested concat on edit branch + new: + label: Label on new branch + nested: Nested concat on new branch + a1: + value: Value + b1: + value: Value + c1: + value: Value + c2: + value: Value + key-that-should-exist: + value: value +foo: Foo diff --git a/index.js b/index.js index 1b4fa6dc..863b912f 100755 --- a/index.js +++ b/index.js @@ -238,44 +238,67 @@ async function analyzeHbsFile(content) { // parse the HBS file let ast = Glimmer.preprocess(content); + function findKeysInIfExpression(node) { + let keysInFirstParam = findKeysInNode(node.params[1]); + let keysInSecondParam = node.params.length > 2 ? findKeysInNode(node.params[2]) : ['']; + + return [...keysInFirstParam, ...keysInSecondParam]; + } + + function findKeysInConcatExpression(node) { + let potentialKeys = ['']; + + for (let param of node.params) { + let keysInParam = findKeysInNode(param); + + if (keysInParam.length === 0) return []; + + potentialKeys = potentialKeys.reduce((newPotentialKeys, potentialKey) => { + for (let key of keysInParam) { + newPotentialKeys.push(potentialKey + key); + } + + return newPotentialKeys; + }, []); + } + + return potentialKeys; + } + + function findKeysInNode(node) { + if (!node) return []; + + if (node.type === 'StringLiteral') { + return [node.value]; + } else if (node.type === 'SubExpression' && node.path.original === 'if') { + return findKeysInIfExpression(node); + } else if (node.type === 'SubExpression' && node.path.original === 'concat') { + return findKeysInConcatExpression(node); + } + + return []; + } + + function processNode(node) { + if (node.path.type !== 'PathExpression') return; + if (node.path.original !== 't') return; + if (node.params.length === 0) return; + + for (let key of findKeysInNode(node.params[0])) { + translationKeys.add(key); + } + } + // find translation keys in the syntax tree Glimmer.traverse(ast, { // handle {{t "foo"}} case MustacheStatement(node) { - if (node.path.type !== 'PathExpression') return; - if (node.path.original !== 't') return; - if (node.params.length === 0) return; - - let firstParam = node.params[0]; - if (firstParam.type === 'StringLiteral') { - translationKeys.add(firstParam.value); - } else if (firstParam.type === 'SubExpression' && firstParam.path.original === 'if') { - if (firstParam.params[1].type === 'StringLiteral') { - translationKeys.add(firstParam.params[1].value); - } - if (firstParam.params[2].type === 'StringLiteral') { - translationKeys.add(firstParam.params[2].value); - } - } + processNode(node); }, // handle {{some-component foo=(t "bar")}} case SubExpression(node) { - if (node.path.type !== 'PathExpression') return; - if (node.path.original !== 't') return; - if (node.params.length === 0) return; - - let firstParam = node.params[0]; - if (firstParam.type === 'StringLiteral') { - translationKeys.add(firstParam.value); - } else if (firstParam.type === 'SubExpression' && firstParam.path.original === 'if') { - if (firstParam.params[1].type === 'StringLiteral') { - translationKeys.add(firstParam.params[1].value); - } - if (firstParam.params[2].type === 'StringLiteral') { - translationKeys.add(firstParam.params[2].value); - } - } + processNode(node); }, }); From ac0e717ef694a3170a93e8b2c8c8ce748c4670fa Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 26 May 2022 17:03:50 +0200 Subject: [PATCH 2/2] Add option to log dynamic translation lookups --- __snapshots__/test.js.snap | 5 ++ bin/cli.js | 5 +- index.js | 109 +++++++++++++++++++++++++++++++++---- test.js | 2 + 4 files changed, 109 insertions(+), 12 deletions(-) diff --git a/__snapshots__/test.js.snap b/__snapshots__/test.js.snap index cfa8f7e8..1667498e 100644 --- a/__snapshots__/test.js.snap +++ b/__snapshots__/test.js.snap @@ -3,6 +3,11 @@ exports[`Test Fixtures concat-expression 1`] = ` "[1/4] 🔍 Finding JS and HBS files... [2/4] 🔍 Searching for translations keys in JS and HBS files... + + ⭐ Found 2 dynamic translations. This might cause this tool to report more unused translations than there actually are! + + - prefix.{{this.dynamicKey}}.not-missing (used in app/templates/application.hbs) + - prefix.{{this.dynamicKey}}.value (used in app/templates/application.hbs) [3/4] ⚙️ Checking for unused translations... [4/4] ⚙️ Checking for missing translations... diff --git a/bin/cli.js b/bin/cli.js index 553acee2..9752162e 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -8,7 +8,10 @@ const { run } = require('../index'); let rootDir = pkgDir.sync(); -run(rootDir, { fix: process.argv.includes('--fix') }) +run(rootDir, { + fix: process.argv.includes('--fix'), + logDynamic: process.argv.includes('--log-dynamic'), +}) .then(exitCode => { process.exitCode = exitCode; }) diff --git a/index.js b/index.js index 863b912f..a5c4d590 100755 --- a/index.js +++ b/index.js @@ -15,6 +15,7 @@ async function run(rootDir, options = {}) { let log = options.log || console.log; let writeToFile = options.writeToFile || fs.writeFileSync; let shouldFix = options.fix || false; + let logDynamic = options.logDynamic || false; let chalkOptions = {}; if (options.color === false) { @@ -33,7 +34,26 @@ async function run(rootDir, options = {}) { let files = [...appFiles, ...inRepoFiles]; log(`${step(2)} 🔍 Searching for translations keys in JS and HBS files...`); - let usedTranslationKeys = await analyzeFiles(rootDir, files); + let [usedTranslationKeys, usedDynamicTranslations] = await analyzeFiles(rootDir, files); + + if (logDynamic) { + if (usedDynamicTranslations.size === 0) { + log(); + log(' ⭐ No dynamic translations were found.'); + log(); + } else { + log(); + log( + ` ⭐ Found ${chalk.bold.yellow( + usedDynamicTranslations.size + )} dynamic translations. This might cause this tool to report more unused translations than there actually are!` + ); + log(); + for (let [key, files] of usedDynamicTranslations) { + log(` - ${key} ${chalk.dim(`(used in ${generateFileList(files)})`)}`); + } + } + } log(`${step(3)} ⚙️ Checking for unused translations...`); @@ -163,9 +183,10 @@ function joinPaths(inputPathOrPaths, outputPaths) { async function analyzeFiles(cwd, files) { let allTranslationKeys = new Map(); + let allDynamicTranslations = new Map(); for (let file of files) { - let translationKeys = await analyzeFile(cwd, file); + let [translationKeys, dynamicTranslations] = await analyzeFile(cwd, file); for (let key of translationKeys) { if (allTranslationKeys.has(key)) { @@ -174,9 +195,17 @@ async function analyzeFiles(cwd, files) { allTranslationKeys.set(key, new Set([file])); } } + + for (let dynamicTranslation of dynamicTranslations) { + if (allDynamicTranslations.has(dynamicTranslation)) { + allDynamicTranslations.get(dynamicTranslation).add(file); + } else { + allDynamicTranslations.set(dynamicTranslation, new Set([file])); + } + } } - return allTranslationKeys; + return [allTranslationKeys, allDynamicTranslations]; } async function analyzeFile(cwd, file) { @@ -229,24 +258,78 @@ async function analyzeJsFile(content) { }, }); - return translationKeys; + return [translationKeys, new Set()]; } async function analyzeHbsFile(content) { let translationKeys = new Set(); + let dynamicTranslations = new Set(); // parse the HBS file let ast = Glimmer.preprocess(content); + class StringKey { + constructor(value) { + this.value = value; + } + + join(otherKey) { + if (otherKey instanceof StringKey) { + return new StringKey(this.value + otherKey.value); + } else { + return new CompositeKey(this, otherKey); + } + } + + toString() { + return this.value; + } + } + + class DynamicKey { + constructor(node) { + this.node = node; + } + + join(otherKey) { + return new CompositeKey(this, otherKey); + } + + toString() { + if (this.node.type === 'PathExpression') { + return `{{${this.node.original}}}`; + } else if (this.node.type === 'SubExpression') { + return `{{${this.node.path.original} helper}}`; + } + + return '{{dynamic key}}'; + } + } + + class CompositeKey { + constructor(...values) { + this.values = values; + } + + join(otherKey) { + return new CompositeKey(...this.values, otherKey); + } + + toString() { + return this.values.reduce((string, value) => string + value.toString(), ''); + } + } + function findKeysInIfExpression(node) { let keysInFirstParam = findKeysInNode(node.params[1]); - let keysInSecondParam = node.params.length > 2 ? findKeysInNode(node.params[2]) : ['']; + let keysInSecondParam = + node.params.length > 2 ? findKeysInNode(node.params[2]) : [new StringKey('')]; return [...keysInFirstParam, ...keysInSecondParam]; } function findKeysInConcatExpression(node) { - let potentialKeys = ['']; + let potentialKeys = [new StringKey('')]; for (let param of node.params) { let keysInParam = findKeysInNode(param); @@ -255,7 +338,7 @@ async function analyzeHbsFile(content) { potentialKeys = potentialKeys.reduce((newPotentialKeys, potentialKey) => { for (let key of keysInParam) { - newPotentialKeys.push(potentialKey + key); + newPotentialKeys.push(potentialKey.join(key)); } return newPotentialKeys; @@ -269,14 +352,14 @@ async function analyzeHbsFile(content) { if (!node) return []; if (node.type === 'StringLiteral') { - return [node.value]; + return [new StringKey(node.value)]; } else if (node.type === 'SubExpression' && node.path.original === 'if') { return findKeysInIfExpression(node); } else if (node.type === 'SubExpression' && node.path.original === 'concat') { return findKeysInConcatExpression(node); } - return []; + return [new DynamicKey(node)]; } function processNode(node) { @@ -285,7 +368,11 @@ async function analyzeHbsFile(content) { if (node.params.length === 0) return; for (let key of findKeysInNode(node.params[0])) { - translationKeys.add(key); + if (key instanceof StringKey) { + translationKeys.add(key.value); + } else { + dynamicTranslations.add(key.toString()); + } } } @@ -302,7 +389,7 @@ async function analyzeHbsFile(content) { }, }); - return translationKeys; + return [translationKeys, dynamicTranslations]; } async function analyzeTranslationFiles(cwd, files) { diff --git a/test.js b/test.js index 9a586ae7..3df93637 100644 --- a/test.js +++ b/test.js @@ -14,6 +14,7 @@ describe('Test Fixtures', () => { 'external-addon-translations', ]; let fixturesWithFix = ['remove-unused-translations', 'remove-unused-translations-nested']; + let fixturesWithDynamicKeys = ['concat-expression']; let fixturesWithConfig = { 'external-addon-translations': { externalPaths: ['@*/*', 'external-addon'], @@ -41,6 +42,7 @@ describe('Test Fixtures', () => { color: false, writeToFile, config: fixturesWithConfig[fixture], + logDynamic: fixturesWithDynamicKeys.includes(fixture), }); let expectedReturnValue = fixturesWithErrors.includes(fixture) ? 1 : 0;