Repository: ember-intl/ember-intl (ts)

Issue:
Ability to silence "ICU argument missing" warnings for missing/non-translated keys
<!--
  Hi there! Thank you for discovering and submitting an issue. 

  Before you submit this; let's make sure of a few things. 
  Please make sure the following boxes are ticked if they are correct.
  If not, please try and fulfill these first.
-->

- [x] I am on the [latest](https://github.com/ember-intl/ember-intl/releases/latest) ember-intl version
- [x] I have searched the [issues](https://github.com/ember-intl/ember-intl/issues) of this repo and believe that this is not a duplicate

<!-- 
  Once those are done, if you're able to fill in the following list with your information,
  it'd be very helpful to whoever handles the issue.
-->

### Environment

- Ember Version: 3.9.0
- Ember CLI Version: 3.9.0
- Ember Intl Version: 4.2.4
- Browser(s): N/A
- Node Version: 13.11.0

### Steps to Reproduce

1. Using `requiresTranslation` in `ember-intl.js` config to silence missing translations for the non-default locale works fine
	```js
		module.exports = function() {
  			return {
    			disablePolyfill: true,
    			publicOnly: true,
    			requiresTranslation(key, locale) {
      				return locale === 'en-us';
    			}
  			};
		};
	```

2. If the `en-us` translation uses arguments in a translation key and the other locales are not translated then "ICU argument missing warnings" appear for the key
3. Building the ember application spams the build log with over a thousand lines of ICU warnings (I have ~55 translation files with most in the early stages/less than 10%)
4. Valid errors in `en-us` or locale translation mistakes are buried

I've read through the docs but haven't stumbled upon anything that would achieve this.

en-us.yaml
```yml
timeUnits:
  years: "{count, plural, =1 {year} other {years}}"
string: "Example"
```

es-es.yaml
```yml
string: "Ejemplo"
```

build output
```
[ember-intl] "timeUnits.years" ICU argument missing: "es-es": "count"
```

![image](https://user-images.githubusercontent.com/3440094/78396791-e1897a80-75e7-11ea-81ed-0375e8aad7da.png)
Missing ICU argument warnings still appear for keys not present in 5.0.6
<!--
  Hi there! Thank you for discovering and submitting an issue. 

  Before you submit this; let's make sure of a few things. 
  Please make sure the following boxes are ticked if they are correct.
  If not, please try and fulfill these first.
-->

- [x] I am on the [latest](https://github.com/ember-intl/ember-intl/releases/latest) ember-intl version
- [x] I have searched the [issues](https://github.com/ember-intl/ember-intl/issues) of this repo and believe that this is not a duplicate

<!-- 
  Once those are done, if you're able to fill in the following list with your information,
  it'd be very helpful to whoever handles the issue.
-->

5.0.0 added https://github.com/ember-intl/ember-intl/pull/1224 which was intended to suppress missing ICU argument warnings for keys not present in a given localisation file (while still warning if the key is missing for required translations). However it appears to still be warning for the same keys as in 4.2.4 before the PR was merged.

### Environment

- Ember Version: 3.9.1
- Ember CLI Version: 3.9.0
- Ember Intl Version: 5.0.6
- Browser(s): N/A
- Node Version: 14.1.0

### Steps to Reproduce

1. Config:

```js
module.exports = function() {
  return {
    publicOnly: true,
    requiresTranslation(key, locale) {
      return locale === 'en-us';
    }
  };
};
```

2. `en-us.yaml`:

```yaml
hello: "World"
timeUnits:
  days: "{count, plural, =1 {day} other {days}}"
```

3. `af-za.yaml`:

```yaml
hello: "World"
```

4. Build log:

```
[ember-intl] "timeUnits.days" ICU argument missing: "af-za": "count"
```

Pull request description:
fix: Do not report missing ICU arguments for locales that have not implemented translation.
fixes #1218, #1283

Also reverts a behavior that shouldn't have been added where if you used `fallbackLocale` it opted you out of ICU argument mismatch checking.

Patch:
diff --git a/blueprints/ember-intl/files/__config__/ember-intl.js b/blueprints/ember-intl/files/__config__/ember-intl.js
index e7a00316..120cf447 100644
--- a/blueprints/ember-intl/files/__config__/ember-intl.js
+++ b/blueprints/ember-intl/files/__config__/ember-intl.js
@@ -6,7 +6,9 @@ module.exports = function (/* environment */) {
      * Merges the fallback locale's translations into all other locales as a
      * build-time fallback strategy.
      *
-     * NOTE: a side effect of this option could result in missing translation warnings to be masked.
+     * This will **not** prevent missing translation warnings or errors from occurring.
+     * It's meant as safety net when warning are enabled.
+     * When enabled along with `errorOnMissingTranslations` any fallback attempts will result in an error.
      *
      * @property fallbackLocale
      * @type {String?}
@@ -38,6 +40,15 @@ module.exports = function (/* environment */) {
      */
     publicOnly: false,
 
+    /**
+     * Add the subdirectories of the translations as a namespace for all keys.
+     *
+     * @property wrapTranslationsWithNamespace
+     * @type {Boolean}
+     * @default "false"
+     */
+    wrapTranslationsWithNamespace: false,
+
     /**
      * Cause a build error if ICU argument mismatches are detected.
      *
@@ -63,19 +74,10 @@ module.exports = function (/* environment */) {
      *
      * @property stripEmptyTranslations
      * @type {Boolean}
-     * @default false
+     * @default "false"
      */
     stripEmptyTranslations: false,
 
-    /**
-     * Add the subdirectories of the translations as a namespace for all keys.
-     *
-     * @property wrapTranslationsWithNamespace
-     * @type {Boolean}
-     * @default false
-     */
-    wrapTranslationsWithNamespace: false,
-
     /**
      * Filter missing translations to ignore expected missing translations.
      *
@@ -83,7 +85,7 @@ module.exports = function (/* environment */) {
      *
      * @property requiresTranslation
      * @type {Function}
-     * @default "function(key,locale){return true}"
+     * @default "function(key,locale) { return true }"
      */
     requiresTranslation(/* key, locale */) {
       return true;
diff --git a/config/ember-intl.js b/config/ember-intl.js
index fc37295e..f30c7d2a 100644
--- a/config/ember-intl.js
+++ b/config/ember-intl.js
@@ -6,8 +6,6 @@ module.exports = function (/* environment */) {
      * Merges the fallback locale's translations into all other locales as a
      * build-time fallback strategy.
      *
-     * NOTE: a side effect of this option could result in missing translation warnings to be masked.
-     *
      * @property fallbackLocale
      * @type {String?}
      * @default "null"
@@ -38,6 +36,15 @@ module.exports = function (/* environment */) {
      */
     publicOnly: false,
 
+    /**
+     * Removes empty translations from the build output.
+     *
+     * @property stripEmptyTranslations
+     * @type {Boolean}
+     * @default "false"
+     */
+    stripEmptyTranslations: false,
+
     /**
      * Cause a build error if ICU argument mismatches are detected.
      *
@@ -58,15 +65,6 @@ module.exports = function (/* environment */) {
      */
     errorOnMissingTranslations: false,
 
-    /**
-     * removes empty translations from the build output.
-     *
-     * @property stripEmptyTranslations
-     * @type {Boolean}
-     * @default false
-     */
-    stripEmptyTranslations: false,
-
     /**
      * Filter missing translations to ignore expected missing translations.
      *
diff --git a/lib/broccoli/translation-reducer/index.js b/lib/broccoli/translation-reducer/index.js
index 3920d320..17672c71 100644
--- a/lib/broccoli/translation-reducer/index.js
+++ b/lib/broccoli/translation-reducer/index.js
@@ -61,12 +61,12 @@ class TranslationReducer extends CachingWriter {
       ...options,
     };
 
+    this.options.fallbackLocale = normalizeLocale(this.options.fallbackLocale);
+
     this.linter = new Linter({
-      fallbackLocale: this.options.fallbackLocale,
       requiresTranslation: this.options.requiresTranslation,
     });
 
-    this.options.fallbackLocale = normalizeLocale(this.options.fallbackLocale);
     this.supportsUnicode = hasUnicode();
   }
 
@@ -113,6 +113,7 @@ class TranslationReducer extends CachingWriter {
 
       let translation = readAsObject(filepath);
 
+      // TODO: make the default in 6.0.0
       if (this.options.stripEmptyTranslations === true) {
         translation = stripNestedNulls(translation);
       }
@@ -159,7 +160,7 @@ class TranslationReducer extends CachingWriter {
           .map(([locale, missingICUArgs]) => `"${locale}": ${missingICUArgs.map((arg) => `"${arg}"`).join(', ')}`)
           .join(', ');
 
-        return `"${key}" ICU argument missing: ${missingString}`;
+        return `"${key}" ICU argument mismatch: ${missingString}`;
       });
 
       if (
@@ -202,11 +203,12 @@ class TranslationReducer extends CachingWriter {
 
   build() {
     const translations = this.mergeTranslations(this.listFiles());
-    const fallbacks = translations[this.options.fallbackLocale];
-    const filepath = path.join(this.outputPath, this.options.outputPath);
     const lintResults = this.linter.lint(translations);
-
     this.handleLintResult(lintResults);
+
+    const filepath = path.join(this.outputPath, this.options.outputPath);
+    const fallbacks = translations[this.options.fallbackLocale];
+
     mkdirp.sync(filepath);
 
     for (const locale in translations) {
diff --git a/lib/broccoli/translation-reducer/linter/find-missing-icu-arguments.js b/lib/broccoli/translation-reducer/linter/find-missing-icu-arguments.js
index e1370fbb..2ce949aa 100644
--- a/lib/broccoli/translation-reducer/linter/find-missing-icu-arguments.js
+++ b/lib/broccoli/translation-reducer/linter/find-missing-icu-arguments.js
@@ -1,5 +1,6 @@
 function findMissingIcuArguments(key, allIcuArguments, locales, icuArguments) {
   const keyIcuArguments = Array.from(allIcuArguments[key]);
+
   return locales
     .map((locale) => [
       locale,
diff --git a/lib/broccoli/translation-reducer/linter/find-missing-translations.js b/lib/broccoli/translation-reducer/linter/find-missing-translations.js
index f5b9162c..7f25abb7 100644
--- a/lib/broccoli/translation-reducer/linter/find-missing-translations.js
+++ b/lib/broccoli/translation-reducer/linter/find-missing-translations.js
@@ -1,7 +1,11 @@
-function findMissingTranslations(key, localeKeys, requiresTranslation) {
+function findLocalesMissingTranslation(key, localeKeys) {
   return localeKeys
-    .filter(([locale, translationKeys]) => !translationKeys.includes(key) && requiresTranslation(key, locale))
-    .map(([locale]) => locale);
+    .filter(([, translationKeys]) => {
+      return !translationKeys.includes(key);
+    })
+    .map(([locale]) => {
+      return locale;
+    });
 }
 
-module.exports = findMissingTranslations;
+module.exports = findLocalesMissingTranslation;
diff --git a/lib/broccoli/translation-reducer/linter/index.js b/lib/broccoli/translation-reducer/linter/index.js
index ca5f0d3e..a12b6d41 100644
--- a/lib/broccoli/translation-reducer/linter/index.js
+++ b/lib/broccoli/translation-reducer/linter/index.js
@@ -1,5 +1,5 @@
 const extractICUArguments = require('./extract-icu-arguments');
-const findMissingTranslations = require('./find-missing-translations');
+const findLocalesMissingTranslation = require('./find-missing-translations');
 const findMissingICUArguments = require('./find-missing-icu-arguments');
 const forEachMessage = require('../utils/for-each-message');
 
@@ -44,25 +44,31 @@ module.exports = class Linter {
 
     // create flattened list of all unique translation keys
     const allTranslationKeys = new Set(Array.prototype.concat(...localeKeys.map(([, keys]) => keys)));
-    const { requiresTranslation, fallbackLocale } = this.options;
+    const { requiresTranslation } = this.options;
 
     allTranslationKeys.forEach((key) => {
-      const notInLocales = findMissingTranslations(key, localeKeys, requiresTranslation);
+      const translationMissingFromLocales = findLocalesMissingTranslation(key, localeKeys);
 
-      if (notInLocales.length) {
-        result.missingTranslations.push([key, notInLocales]);
-      }
-
-      let missingICUArgs = findMissingICUArguments(key, allIcuArguments, locales, icuArguments);
+      if (translationMissingFromLocales.length) {
+        const alertableMissingTranslations = translationMissingFromLocales.filter((locale) =>
+          requiresTranslation(key, locale)
+        );
 
-      if (fallbackLocale) {
-        missingICUArgs = missingICUArgs.filter(([locale]) => fallbackLocale === locale);
+        if (alertableMissingTranslations.length) {
+          result.missingTranslations.push([key, alertableMissingTranslations]);
+        }
       }
 
-      if (notInLocales.length) {
-        // keep only the ICU mismatches where the key isn't missing (key itself will still be reported as missing)
-        missingICUArgs = missingICUArgs.filter(([locale]) => !notInLocales.includes(locale));
-      }
+      const localesToScanMissingICUArguments = locales.filter((locale) => {
+        return !translationMissingFromLocales.includes(locale);
+      });
+
+      let missingICUArgs = findMissingICUArguments(
+        key,
+        allIcuArguments,
+        localesToScanMissingICUArguments,
+        icuArguments
+      );
 
       if (missingICUArgs.length) {
         result.icuMismatch.push([key, missingICUArgs]);


Test patch:
diff --git a/tests-node/unit/broccoli/translation-reducer/find-missing-translations-test.js b/tests-node/unit/broccoli/translation-reducer/find-missing-translations-test.js
index 698596d8..aaeffcff 100644
--- a/tests-node/unit/broccoli/translation-reducer/find-missing-translations-test.js
+++ b/tests-node/unit/broccoli/translation-reducer/find-missing-translations-test.js
@@ -1,10 +1,8 @@
 const expect = require('chai').expect;
 
-const findMissingTranslations = require('../../../../lib/broccoli/translation-reducer/linter/find-missing-translations');
+const findLocalesMissingTranslation = require('../../../../lib/broccoli/translation-reducer/linter/find-missing-translations');
 
-const isTrue = () => true;
-
-describe('findMissingTranslations', function () {
+describe('findLocalesMissingTranslation', function () {
   it('finds nothing if nothing is missing', function () {
     const key = 'foo';
     const localeKeys = [
@@ -12,7 +10,7 @@ describe('findMissingTranslations', function () {
       ['en', ['foo']],
     ];
 
-    expect(findMissingTranslations(key, localeKeys, isTrue)).to.deep.equal([]);
+    expect(findLocalesMissingTranslation(key, localeKeys)).to.deep.equal([]);
   });
 
   it('finds missing translations', function () {
@@ -22,17 +20,6 @@ describe('findMissingTranslations', function () {
       ['en', ['bar']],
     ];
 
-    expect(findMissingTranslations(key, localeKeys, isTrue)).to.deep.equal(['en']);
-  });
-
-  it('ignores missing translations if not required', function () {
-    const key = 'foo';
-    const localeKeys = [
-      ['de', ['foo']],
-      ['en', ['bar']],
-    ];
-    const isAllowed = (key, locale) => locale !== 'en';
-
-    expect(findMissingTranslations(key, localeKeys, isAllowed)).to.deep.equal([]);
+    expect(findLocalesMissingTranslation(key, localeKeys)).to.deep.equal(['en']);
   });
 });
diff --git a/tests-node/unit/broccoli/translation-reducer/index-test.js b/tests-node/unit/broccoli/translation-reducer/index-test.js
index 182aabc0..a226f3fb 100644
--- a/tests-node/unit/broccoli/translation-reducer/index-test.js
+++ b/tests-node/unit/broccoli/translation-reducer/index-test.js
@@ -73,7 +73,11 @@ describe('translation-reducer', function () {
         'en-us.json': `{ "greet": "hello" }`,
       });
 
-      return expect(build(createBuilder(subject))).to.eventually.rejectedWith(/"greet" was not found in "de-de"/);
+      return expect(
+        build(createBuilder(subject), () => {
+          throw new Error('Should have have been reached.');
+        })
+      ).to.eventually.rejectedWith(/"greet" was not found in "de-de"/);
     });
 
     it('should not not overwrite translations', function () {
@@ -230,9 +234,9 @@ describe('translation-reducer', function () {
       subject._log = (msg) => logs.push(msg);
 
       expect(() => subject.handleLintResult(subject.linter.lint(this.icuFixture))).throws(`ICU arguments mismatch:
-- "foo" ICU argument missing: "de": "whos", "en": "who"
-- "missingArg" ICU argument missing: "en": "numPhotos"
-- "deep.nested.ok" ICU argument missing: "de": "code"`);
+- "foo" ICU argument mismatch: "de": "whos", "en": "who"
+- "missingArg" ICU argument mismatch: "en": "numPhotos"
+- "deep.nested.ok" ICU argument mismatch: "de": "code"`);
     });
 
     describe('mergeTranslations', function () {
diff --git a/tests-node/unit/broccoli/translation-reducer/lint-test.js b/tests-node/unit/broccoli/translation-reducer/lint-test.js
index 70265d93..b253bcd5 100644
--- a/tests-node/unit/broccoli/translation-reducer/lint-test.js
+++ b/tests-node/unit/broccoli/translation-reducer/lint-test.js
@@ -130,21 +130,22 @@ describe('linting', function () {
     );
   });
 
-  it('returns list of icu argument mismatch without those that are required', function () {
-    this.linter = new Linter({
-      fallbackLocale: 'en',
-      requiresTranslation() {
+  it('ignores icu mismatch reporting for translations that do not exist', function () {
+    const linter = new Linter({
+      requiresTranslation(/* key, locale */) {
         return true;
       },
     });
 
-    delete this.icuFixture['de'].greeting;
-
-    const { icuMismatch } = this.linter.lint(this.icuFixture);
+    const results = linter.lint({
+      en: { one: '{one}' }, // not reported, no match
+      es: { missing: '{foo}' }, // not reported, no match
+    });
 
-    expect(icuMismatch).to.deep.equal([
-      ['missingArg', [['en', ['numPhotos']]]],
-      ['deep.nested.missing', [['en', ['code']]]],
+    expect(results.icuMismatch).to.deep.equal([]);
+    expect(results.missingTranslations).to.deep.equal([
+      ['one', ['es']],
+      ['missing', ['en']],
     ]);
   });
 });
diff --git a/tests/dummy/config/ember-intl.js b/tests/dummy/config/ember-intl.js
index 37c1559a..5df58b70 100644
--- a/tests/dummy/config/ember-intl.js
+++ b/tests/dummy/config/ember-intl.js
@@ -6,13 +6,11 @@ module.exports = function (/* environment */) {
      * Merges the fallback locale's translations into all other locales as a
      * build-time fallback strategy.
      *
-     * NOTE: a side effect of this option could result in missing translation warnings to be masked.
-     *
      * @property fallbackLocale
      * @type {String?}
      * @default "null"
      */
-    fallbackLocale: 'en-us',
+    fallbackLocale: null,
 
     /**
      * Path where translations are kept.  This is relative to the project root.
@@ -25,6 +23,24 @@ module.exports = function (/* environment */) {
      */
     inputPath: './tests/dummy/translations',
 
+    /**
+     * Removes empty translations from the build output.
+     *
+     * @property stripEmptyTranslations
+     * @type {Boolean}
+     * @default false
+     */
+    stripEmptyTranslations: false,
+
+    /**
+     * Add the subdirectories of the translations as a namespace for all keys.
+     *
+     * @property wrapTranslationsWithNamespace
+     * @type {Boolean}
+     * @default false
+     */
+    wrapTranslationsWithNamespace: true,
+
     /**
      * Prevents the translations from being bundled with the application code.
      * This enables asynchronously loading the translations for the active locale
@@ -58,24 +74,6 @@ module.exports = function (/* environment */) {
      */
     errorOnMissingTranslations: false,
 
-    /**
-     * removes empty translations from the build output.
-     *
-     * @property stripEmptyTranslations
-     * @type {Boolean}
-     * @default false
-     */
-    stripEmptyTranslations: false,
-
-    /**
-     * Add the subdirectories of the translations as a namespace for all keys.
-     *
-     * @property wrapTranslationsWithNamespace
-     * @type {Boolean}
-     * @default false
-     */
-    wrapTranslationsWithNamespace: true,
-
     /**
      * Filter missing translations to ignore expected missing translations.
      *