Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for concat expressions in HBS files #483

Merged
merged 1 commit into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions __snapshots__/test.js.snap
Original file line number Diff line number Diff line change
@@ -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...
Expand Down
10 changes: 10 additions & 0 deletions fixtures/concat-expression/app/templates/application.hbs
Original file line number Diff line number Diff line change
@@ -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")}}
23 changes: 23 additions & 0 deletions fixtures/concat-expression/translations/en.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions fixtures/no-issues/app/templates/application.hbs
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
{{t "hbs-translation"}}
{{t (concat "concat-" "expression.is-skipped")}}
105 changes: 67 additions & 38 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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...`);

Expand Down Expand Up @@ -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)) {
Expand All @@ -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})`);
}
Expand Down Expand Up @@ -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);
},
});

Expand Down
3 changes: 3 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -41,6 +43,7 @@ describe('Test Fixtures', () => {
color: false,
writeToFile,
config: fixturesWithConfig[fixture],
analyzeConcatExpression: fixturesWithConcat.includes(fixture),
});

let expectedReturnValue = fixturesWithErrors.includes(fixture) ? 1 : 0;
Expand Down