Merge pull request #13231 from nickiaconis/cp-commas

Correctly handle commas in CP property keys
emberjs · Feb 2, 2017 · fc5be0b · fc5be0b
2 parents f38b4b8 + 14edb5c
commit fc5be0b
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 45 deletions.
diff --git a/packages/ember-metal/lib/expand_properties.js b/packages/ember-metal/lib/expand_properties.js
@@ -5,8 +5,7 @@ import { assert } from './debug';
 @submodule ember-metal
 */
 
-const SPLIT_REGEX = /\{|\}/;
-const END_WITH_EACH_REGEX = /\.@each$/;
+var END_WITH_EACH_REGEX = /\.@each$/;
 
 /**
   Expands `pattern`, invoking `callback` for each expansion.
@@ -41,53 +40,55 @@ export default function expandProperties(pattern, callback) {
     'Brace expanded properties cannot contain spaces, e.g. "user.{firstName, lastName}" should be "user.{firstName,lastName}"',
     pattern.indexOf(' ') === -1
   );
-  assert(
-    `Brace expanded properties have to be balanced and cannot be nested, pattern: ${pattern}`,
-    ((str) => {
-      let inBrace = 0;
-      let char;
-      for (let i = 0; i < str.length; i++) {
-        char = str.charAt(i);
 
-        if (char === '{') {
-          inBrace++;
-        } else if (char === '}') {
-          inBrace--;
+  let unbalancedNestedError = `Brace expanded properties have to be balanced and cannot be nested, pattern: ${pattern}`;
+  let properties = [pattern];
+
+  // Iterating backward over the pattern makes dealing with indices easier.
+  let bookmark;
+  let inside = false;
+  for (let i = pattern.length; i > 0; --i) {
+    let current = pattern[i - 1];
+
+    switch (current) {
+      // Closing curly brace will be the first character of the brace expansion we encounter.
+      // Bookmark its index so long as we're not already inside a brace expansion.
+      case '}':
+        if (!inside) {
+          bookmark = i - 1;
+          inside = true;
+        } else {
+          assert(unbalancedNestedError, false);
         }
-
-        if (inBrace > 1 || inBrace < 0) {
-          return false;
+        break;
+      // Opening curly brace will be the last character of the brace expansion we encounter.
+      // Apply the brace expansion so long as we've already seen a closing curly brace.
+      case '{':
+        if (inside) {
+          let expansion = pattern.slice(i, bookmark).split(',');
+          // Iterating backward allows us to push new properties w/out affecting our "cursor".
+          for (let j = properties.length; j > 0; --j) {
+            // Extract the unexpanded property from the array.
+            let property = properties.splice(j - 1, 1)[0];
+            // Iterate over the expansion, pushing the newly formed properties onto the array.
+            for (let k = 0; k < expansion.length; ++k) {
+              properties.push(property.slice(0, i - 1) +
+                              expansion[k] +
+                              property.slice(bookmark + 1));
+            }
+          }
+          inside = false;
+        } else {
+          assert(unbalancedNestedError, false);
         }
-      }
-
-      return true;
-    })(pattern));
-
-  let parts = pattern.split(SPLIT_REGEX);
-  let properties = [parts];
-
-  for (let i = 0; i < parts.length; i++) {
-    let part = parts[i];
-    if (part.indexOf(',') >= 0) {
-      properties = duplicateAndReplace(properties, part.split(','), i);
+        break;
     }
   }
+  if (inside) {
+    assert(unbalancedNestedError, false);
+  }
 
   for (let i = 0; i < properties.length; i++) {
-    callback(properties[i].join('').replace(END_WITH_EACH_REGEX, '.[]'));
+    callback(properties[i].replace(END_WITH_EACH_REGEX, '.[]'));
   }
 }
-
-function duplicateAndReplace(properties, currentParts, index) {
-  let all = [];
-
-  properties.forEach(property => {
-    currentParts.forEach(part => {
-      let current = property.slice(0);
-      current[index] = part;
-      all.push(current);
-    });
-  });
-
-  return all;
-}
diff --git a/packages/ember-metal/tests/expand_properties_test.js b/packages/ember-metal/tests/expand_properties_test.js
@@ -89,6 +89,14 @@ QUnit.test('Nested brace expansions are not allowed', function() {
   }, /Brace expanded properties have to be balanced and cannot be nested/);
 });
 
+QUnit.test('A property with no braces does not expand', function() {
+  expect(1);
+
+  expandProperties('a,b,c.d.e,f', addProperty);
+
+  deepEqual(foundProperties, ['a,b,c.d.e,f']);
+});
+
 QUnit.test('A pattern must be a string', function() {
   expect(1);
 
@@ -100,7 +108,7 @@ QUnit.test('A pattern must be a string', function() {
 QUnit.test('A pattern must not contain a space', function() {
   expect(1);
 
-  expectAssertion(() => {
-    expandProperties('a, b', addProperty);
+  expectAssertion(function() {
+    expandProperties('{a, b}', addProperty);
   }, /Brace expanded properties cannot contain spaces, e.g. "user.{firstName, lastName}" should be "user.{firstName,lastName}"/);
 });