From 7d38349a5918ff993e6b62615dc078e742a202d2 Mon Sep 17 00:00:00 2001
From: chrisgarrity <chrisg@media.mit.edu>
Date: Thu, 10 Jan 2019 11:14:35 -0500
Subject: [PATCH] Fix placeholder checking
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The placeholder checking didn’t work if the translation’s grammar generated a different number of parsed elements.

E.g., in English:
something {placeholder} — two elements
translation
something {placeholder} something — three elements

I determined that placeholders are always array elements in the parsed array, so filter for Array elements and check that there are the same number of array elements.
---
 scripts/tx-util.js | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/scripts/tx-util.js b/scripts/tx-util.js
index 65ee9cab..b5d364bc 100644
--- a/scripts/tx-util.js
+++ b/scripts/tx-util.js
@@ -9,12 +9,30 @@ const flattenJson = (translations) => {
     return JSON.stringify(messages, null, 4);
 };
 
+// filter placeholders out of a message
+// parse('a message with a {value} and {count, plural, one {one} other {more}}.')
+// returns an array:
+// [ 'a message with a ',
+//   [ 'value' ],
+//   ' and ',
+//   [ 'count', 'plural', 0, { one: [Array], other: [Array] } ],
+//   '.'
+// ]
+// placeholders are always an array, so filter for array elements to find the placeholders
+const placeholders = message => (
+    // this will throw an error if the message is not valid ICU
+    parse(message).filter(item => Array.isArray(item))
+);
+
 const validMessage = (message, source) => {
-    // this will throw an error if the message is not valid icu
-    const t = parse(message);
-    const s = parse(source);
-    // the syntax tree for both messages should have the same number of elements
-    return t.length === s.length;
+    const transPlaceholders = placeholders(message);
+    const srcPlaceholders = placeholders(source);
+    // different number of placeholders
+    if (transPlaceholders.length !== srcPlaceholders.length) {
+        return false;
+    }
+    // TODO: Add checking to make sure placeholders in source have not been translated
+    return true;
 };
 
 const validateTranslations = (translation, source) => {