From a3260706f4afc02448717f6d50e8f5aceef3f351 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 22 Mar 2022 14:06:23 +0100 Subject: [PATCH] Add support for concat expressions in HBS files --- README.md | 24 ++++ __snapshots__/test.js.snap | 17 +++ .../app/templates/application.hbs | 10 ++ .../concat-expression/translations/en.yaml | 23 ++++ .../no-issues/app/templates/application.hbs | 1 + index.js | 105 +++++++++++------- test.js | 3 + 7 files changed, 145 insertions(+), 38 deletions(-) create mode 100644 fixtures/concat-expression/app/templates/application.hbs create mode 100644 fixtures/concat-expression/translations/en.yaml diff --git a/README.md b/README.md index 0483e9e0..ed2f240d 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,30 @@ To prevent that from happening you can configure a `whitelist`, which accepts an array of regular expressions that will be checked when looking for unused translations. +### `analyzeConcatExpression` + +If your template contains translations like this: +```hbs +{{t (concat "actions." (if @isEditing "save" "publish"))}} +``` +then ember-intl-analyzer does not detect that `actions.save` and `actions.publish` +are in fact used translations, so they can be incorrectly flagged as missing or +unused. As the `concat` helper can make it harder to read, it's encouraged to +rewrite it to for example: +```hbs +{{if @isEditing (t "actions.save") (t "actions.publish")}} +``` + +However, if your application relies heavily on this `concat` helper, then rewriting +may not be the best option for you. In that case, you can opt-in to analyze `concat` +expressions too by setting the `analyzeConcatExpression` flag in the configuration file: + +```js +export default { + analyzeConcatExpression: true, +}; +``` + ### `externalPaths` If your application uses translations provided by (external) addons, then those diff --git a/__snapshots__/test.js.snap b/__snapshots__/test.js.snap index 638c96fd..40c1dad1 100644 --- a/__snapshots__/test.js.snap +++ b/__snapshots__/test.js.snap @@ -1,5 +1,22 @@ // 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! + + ⚠️ Found 2 missing translations! + + - prefix.simple-but-missing (used in app/templates/application.hbs) + - prefix.key-that-should-exist-but-is-missing.value (used in app/templates/application.hbs) +" +`; + +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..626baf5e --- /dev/null +++ b/fixtures/concat-expression/app/templates/application.hbs @@ -0,0 +1,10 @@ +{{t (concat "prefix" "." "simple")}} +{{t (concat "prefix" "." "simple-but-missing")}} +{{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")}} +{{t (concat "prefix." (if true this.dynamicKey "key-that-should-exist-but-is-missing") ".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/fixtures/no-issues/app/templates/application.hbs b/fixtures/no-issues/app/templates/application.hbs index 8588d949..850dcc06 100644 --- a/fixtures/no-issues/app/templates/application.hbs +++ b/fixtures/no-issues/app/templates/application.hbs @@ -1 +1,2 @@ {{t "hbs-translation"}} +{{t (concat "concat-" "expression.is-skipped")}} diff --git a/index.js b/index.js index 1b4fa6dc..492c06d3 100755 --- a/index.js +++ b/index.js @@ -26,6 +26,8 @@ async function run(rootDir, options = {}) { const step = num => chalk.dim(`[${num}/${NUM_STEPS}]`); let config = options.config || readConfig(rootDir); + let analyzeConcatExpression = options.analyzeConcatExpression || config.analyzeConcatExpression; + let analyzeOptions = { analyzeConcatExpression }; log(`${step(1)} 🔍 Finding JS and HBS files...`); let appFiles = await findAppFiles(rootDir); @@ -33,7 +35,7 @@ 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 = await analyzeFiles(rootDir, files, analyzeOptions); log(`${step(3)} ⚙️ Checking for unused translations...`); @@ -161,11 +163,11 @@ function joinPaths(inputPathOrPaths, outputPaths) { } } -async function analyzeFiles(cwd, files) { +async function analyzeFiles(cwd, files, options) { let allTranslationKeys = new Map(); for (let file of files) { - let translationKeys = await analyzeFile(cwd, file); + let translationKeys = await analyzeFile(cwd, file, options); for (let key of translationKeys) { if (allTranslationKeys.has(key)) { @@ -179,17 +181,17 @@ async function analyzeFiles(cwd, files) { return allTranslationKeys; } -async function analyzeFile(cwd, file) { +async function analyzeFile(cwd, file, options) { let content = fs.readFileSync(`${cwd}/${file}`, 'utf8'); let extension = path.extname(file).toLowerCase(); if (extension === '.js') { - return analyzeJsFile(content); + return analyzeJsFile(content, options); } else if (extension === '.hbs') { - return analyzeHbsFile(content); + return analyzeHbsFile(content, options); } else if (extension === '.emblem') { let hbs = Emblem.compile(content, { quiet: true }); - return analyzeHbsFile(hbs); + return analyzeHbsFile(hbs, options); } else { throw new Error(`Unknown extension: ${extension} (${file})`); } @@ -232,50 +234,77 @@ async function analyzeJsFile(content) { return translationKeys; } -async function analyzeHbsFile(content) { +async function analyzeHbsFile(content, { analyzeConcatExpression = false }) { let translationKeys = new Set(); // 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 ( + analyzeConcatExpression && + 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); }, }); diff --git a/test.js b/test.js index 9a586ae7..a053f77d 100644 --- a/test.js +++ b/test.js @@ -12,8 +12,10 @@ describe('Test Fixtures', () => { 'unused-translations', 'in-repo-translations', 'external-addon-translations', + 'concat-expression', ]; let fixturesWithFix = ['remove-unused-translations', 'remove-unused-translations-nested']; + let fixturesWithConcat = ['concat-expression']; let fixturesWithConfig = { 'external-addon-translations': { externalPaths: ['@*/*', 'external-addon'], @@ -41,6 +43,7 @@ describe('Test Fixtures', () => { color: false, writeToFile, config: fixturesWithConfig[fixture], + analyzeConcatExpression: fixturesWithConcat.includes(fixture), }); let expectedReturnValue = fixturesWithErrors.includes(fixture) ? 1 : 0;