From 601f87c2ec192a2f6fcf49a09f96a74376e6b56c Mon Sep 17 00:00:00 2001
From: Pete Bacon Darwin <pete@bacondarwin.com>
Date: Wed, 2 Oct 2019 18:17:56 +0100
Subject: [PATCH] fix(ivy): i18n - throw an error if a translation contains an
 invalid placeholder (#32867)

Previously if a translation contains a placeholder that
does not exist in the message being translated, that
placeholder is evaluated as `undefined`.

Translations should never contain such placeholder names
so now `translate` will throw a helpful error in instead.

PR Close #32867
---
 packages/localize/src/utils/translations.ts       | 14 +++++++++++---
 packages/localize/test/utils/translations_spec.ts | 12 ++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/packages/localize/src/utils/translations.ts b/packages/localize/src/utils/translations.ts
index c27310114f..4dd5a2ba79 100644
--- a/packages/localize/src/utils/translations.ts
+++ b/packages/localize/src/utils/translations.ts
@@ -33,7 +33,9 @@ export type ParsedTranslations = Record<MessageId, ParsedTranslation>;
  * `substitutions`.
  * The translation may reorder (or remove) substitutions as appropriate.
  *
- * If no translation matches then an error is thrown.
+ * If there is no translation with a matching message id then an error is thrown.
+ * If a translation contains a placeholder that is not found in the message being translated then an
+ * error is thrown.
  */
 export function translate(
     translations: Record<string, ParsedTranslation>, messageParts: TemplateStringsArray,
@@ -42,8 +44,14 @@ export function translate(
   const translation = translations[message.messageId];
   if (translation !== undefined) {
     return [
-      translation.messageParts,
-      translation.placeholderNames.map(placeholder => message.substitutions[placeholder])
+      translation.messageParts, translation.placeholderNames.map(placeholder => {
+        if (message.substitutions.hasOwnProperty(placeholder)) {
+          return message.substitutions[placeholder];
+        } else {
+          throw new Error(
+              `No placeholder found with name ${placeholder} in message "${message.messageId}" ("${message.messageString}").`);
+        }
+      })
     ];
   } else {
     throw new Error(
diff --git a/packages/localize/test/utils/translations_spec.ts b/packages/localize/test/utils/translations_spec.ts
index bd5cdbac6b..a6ce5083af 100644
--- a/packages/localize/test/utils/translations_spec.ts
+++ b/packages/localize/test/utils/translations_spec.ts
@@ -80,6 +80,18 @@ describe('utils', () => {
   });
 
   describe('translate', () => {
+    it('should throw an error if there is no matching translation', () => {
+      expect(() => doTranslate({}, parts `abc`))
+          .toThrowError('No translation found for "2674653928643152084" ("abc").');
+    });
+
+    it('should throw an error if the translation contains placeholders that are not in the message',
+       () => {
+         expect(() => doTranslate({'abc': 'a{$PH}bc'}, parts `abc`))
+             .toThrowError(
+                 'No placeholder found with name PH in message "2674653928643152084" ("abc").');
+       });
+
     it('(with identity translations) should render template literals as-is', () => {
       const translations = {
         'abc': 'abc',