Repository: i18next/i18next-parser
Pull request #202 (2020-01-29 16:23:16)

Check for conflicting keys with different types
I got a crash when my html files contained keys like one and one.two both mapped to a string. This fix overwrites any object that might be contained in the existing map, i.e. replacing the subtree of one with the new string or the string of one with the new subtree.


Patch:
diff --git a/bin/cli.js b/bin/cli.js
index 9362d1e..f7a747d 100755
--- a/bin/cli.js
+++ b/bin/cli.js
@@ -64,7 +64,12 @@ else if (config.input) {
   }
 
   globs = config.input.map(function (s) {
-    return path.resolve(path.dirname(path.resolve(program.config)), s)
+    var negate = ''
+    if (s.startsWith('!')) {
+      negate = '!'
+      s = s.substr(1)
+    }
+    return negate + path.resolve(path.dirname(path.resolve(program.config)), s)
   })
 }
 
diff --git a/src/helpers.js b/src/helpers.js
index 8784836..6aea43a 100644
--- a/src/helpers.js
+++ b/src/helpers.js
@@ -2,9 +2,14 @@
  * Take an entry for the Parser and turn it into a hash,
  * turning the key path 'foo.bar' into an hash {foo: {bar: ""}}
  * The generated hash can be merged with an optional `target`.
- * @returns An `{ target, duplicate }` object. `target` is the hash that
- * was passed as an argument or a new hash if none was passed. `duplicate`
- * indicates whether the entry already existed in the `target` hash.
+ * @returns An `{ target, duplicate, conflict }` object. `target` is the hash
+ * that was passed as an argument or a new hash if none was passed. `duplicate`
+ * indicates whether the entry already existed in the `target` hash. `conflict`
+ * is `"key"` if a parent of the key was already mapped to a string (e.g. when
+ * merging entry {one: {two: "bla"}} with target {one: "bla"}) or the key was
+ * already mapped to a map (e.g. when merging entry {one: "bla"} with target
+ * {one: {two: "bla"}}), `"value"` if the same key already exists swith a
+ * different value, or `false`.
  */
 function dotPathToHash(entry, target = {}, options = {}) {
   let path = entry.key
@@ -24,10 +29,14 @@ function dotPathToHash(entry, target = {}, options = {}) {
 
   const segments = path.split(separator)
   let inner = target
+  let conflict = false
   for (let i = 0; i < segments.length - 1; i += 1) {
     const segment = segments[i]
     if (segment) {
-      if (inner[segment] === undefined) {
+      if (typeof inner[segment] === 'string') {
+        conflict = 'key'
+      }
+      if (inner[segment] === undefined || conflict) {
         inner[segment] = {}
       }
       inner = inner[segment]
@@ -36,8 +45,10 @@ function dotPathToHash(entry, target = {}, options = {}) {
 
   const lastSegment = segments[segments.length - 1];
   const oldValue = inner[lastSegment];
-  const duplicate = oldValue !== undefined
-  const conflict = oldValue !== undefined && oldValue !== newValue
+  if (oldValue !== undefined && oldValue !== newValue) {
+    conflict = typeof oldValue !== typeof newValue ? 'key' : 'value';
+  }
+  const duplicate = oldValue !== undefined || conflict !== false
   inner[lastSegment] = newValue
 
   return { target, duplicate, conflict }
diff --git a/src/transform.js b/src/transform.js
index 2310b58..f2c7f43 100644
--- a/src/transform.js
+++ b/src/transform.js
@@ -9,10 +9,6 @@ import YAML from 'yamljs'
 import BaseLexer from './lexers/base-lexer'
 import i18next from 'i18next'
 
-function warn(...args) {
-  console.warn('\x1b[33m%s\x1b[0m', ...args)
-}
-
 function getPluralSuffix(numberOfPluralForms, nthForm) {
   if (numberOfPluralForms.length > 2) {
     return nthForm // key_0, key_1, etc.
@@ -56,8 +52,8 @@ export default class i18nTransform extends Transform {
     this.entries = []
 
     this.parser = new Parser(this.options)
-    this.parser.on('error', error => this.emit('error', error))
-    this.parser.on('warning', warning => this.emit('warning', warning))
+    this.parser.on('error', error => this.error(error))
+    this.parser.on('warning', warning => this.warn(warning))
 
     this.localeRegex = /\$LOCALE/g
     this.namespaceRegex = /\$NAMESPACE/g
@@ -65,6 +61,20 @@ export default class i18nTransform extends Transform {
     i18next.init();
   }
 
+  error(error) {
+    this.emit('error', error)
+    if (this.options.verbose) {
+      console.error('\x1b[31m%s\x1b[0m', error)
+    }
+  }
+
+  warn(warning) {
+    this.emit('warning', warning)
+    if (this.options.verbose) {
+      console.warn('\x1b[33m%s\x1b[0m', warning)
+    }
+  }
+
   _transform(file, encoding, done) {
     let content
     if (file.isBuffer()) {
@@ -136,12 +146,13 @@ export default class i18nTransform extends Transform {
 
         if (duplicate) {
           uniqueCount -= 1
-          if (conflict) {
+          if (conflict === 'key') {
+            const warning = `Found translation key already mapped to a map or parent of new key already mapped to a string: ${entry.key}`
+            this.warn(warning)
+          }
+          else if (conflict === 'value') {
             const warning = `Found same keys with different values: ${entry.key}`
-            this.emit('warning', warning)
-            if (this.options.verbose) {
-              warn(warning)
-            }
+            this.warn(warning)
           }
         } else {
           countWithPlurals += 1


Test patch:
diff --git a/test/helpers/dotPathToHash.test.js b/test/helpers/dotPathToHash.test.js
index 3fb35ca..f636d07 100644
--- a/test/helpers/dotPathToHash.test.js
+++ b/test/helpers/dotPathToHash.test.js
@@ -95,7 +95,29 @@ describe('dotPathToHash helper function', () => {
     )
     assert.deepEqual(target, { one: { two: { three: 'new' } } })
     assert.equal(duplicate, true)
-    assert.equal(conflict, true)
+    assert.equal(conflict, 'value')
+    done()
+  })
+
+  it('overwrites keys already mapped to a string with an object value', (done) => {
+    const { target, duplicate, conflict } = dotPathToHash(
+      { key: 'one', defaultValue: 'bla' },
+      { one: { two: { three: 'bla' } } },
+    )
+    assert.deepEqual(target, { one: 'bla' })
+    assert.equal(duplicate, true)
+    assert.equal(conflict, 'key')
+    done()
+  })
+
+  it('overwrites keys already mapped to an object with a string value', (done) => {
+    const { target, duplicate, conflict } = dotPathToHash(
+      { key: 'one.two.three', defaultValue: 'bla' },
+      { one: 'bla' },
+    )
+    assert.deepEqual(target, { one: { two: { three: 'bla' } } })
+    assert.equal(duplicate, true)
+    assert.equal(conflict, 'key')
     done()
   })
 })