Merge pull request #483 from robinborst95/feature/concat-expression

Add support for concat expressions in HBS files

Author: Mikek2252

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

__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...

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")}}

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

fixtures/no-issues/app/templates/application.hbs

@@ -1 +1,2 @@
 {{t "hbs-translation"}}
+{{t (concat "concat-" "expression.is-skipped")}}

index.js

@@ -26,14 +26,16 @@ 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);
   let inRepoFiles = await findInRepoFiles(rootDir);
   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);
     },
   });
 

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;