Merge pull request #472 from bmish/require-computed-property-dependen…

…cies-nested-braces Improve handling of nested keys inside braces for `require-computed-property-dependencies` rule
ember-cli · Aug 14, 2019 · 2170ff2 · 2170ff2
2 parents 5844540 + 2502840
commit 2170ff2
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 19 deletions.
diff --git a/lib/rules/require-computed-property-dependencies.js b/lib/rules/require-computed-property-dependencies.js
@@ -347,7 +347,7 @@ module.exports = {
                 ', '
               )}`,
               fix(fixer) {
-                const missingDependenciesAsArguments = dependencyKeys(
+                const missingDependenciesAsArguments = collapseKeys(
                   removeRedundantKeys([...undeclaredKeys, ...expandedDeclaredKeys])
                 );
 
@@ -375,11 +375,16 @@ module.exports = {
 };
 
 /**
- * ["foo.bar", "foo.baz", "quux.[]"] => "'foo.{bar,baz}', 'quux.[]'"
+ * Collapse dependency keys with braces if possible.
+ *
+ * Example:
+ * Input: ["foo.bar", "foo.baz", "quux.[]"]
+ * Output: "'foo.{bar,baz}', 'quux.[]'"
+ *
  * @param {Array<string>} keys
  * @returns string
  */
-function dependencyKeys(keys) {
+function collapseKeys(keys) {
   const uniqueKeys = Array.from(new Set(keys));
 
   function isBare(key) {
@@ -424,26 +429,42 @@ function expandKeys(keys) {
 }
 
 /**
+ * Expand any brace usage in a dependency key.
+ *
+ * Example:
+ * Input: "foo.{bar,baz}"
+ * Output: ["foo.bar", "foo.baz"]
+ *
  * @param {string} key
  * @returns {Array<string>}
  */
 function expandKey(key) {
-  return key
-    .split('.') // ["foo", "{bar,baz}"]
-    .map(part => part.replace(/\{|\}/g, '').split(',')) // [["foo"], ["baz", "baz"]]
-    .reduce(
-      (acc, nextParts) =>
-        // iteration 1 (["foo"]): do nothing (duplicate 0 times), resulting in acc === [["foo"]]
-        // iteration 2 (["bar", "baz"]): duplicate acc once, resulting in `[["foo"], ["foo"]]
-        duplicateArrays(acc, nextParts.length - 1).map((base, index) =>
-          // evenly distribute the parts across the repeated base keys.
-          // nextParts[0 % 2] => "bar"
-          // nextParts[1 % 2] => "baz"
-          base.concat(nextParts[index % nextParts.length])
-        ),
-      [[]]
-    ) // [["foo", "bar"], ["foo", "baz"]]
-    .map(expanded => expanded.join('.')); // ["foo.bar", "foo.baz"]
+  if (key.includes('{')) {
+    // key = "foo.{bar,baz}"
+    const keyParts = key.split('{'); // ["foo", "{bar,baz}"]
+    const keyBeforeCurly = keyParts[0].substring(0, keyParts[0].length - 1); // "foo"
+    const keyAfterCurly = keyParts[1]; // "{bar,baz}"
+    const keyAfterCurlySplitByCommas = keyAfterCurly.replace(/\{|\}/g, '').split(','); // ["bar", "baz"]
+    const keyRecombined = [[keyBeforeCurly], keyAfterCurlySplitByCommas]; // [["foo"], ["baz", "baz"]]
+    return keyRecombined
+      .reduce(
+        (acc, nextParts) =>
+          // iteration 1 (["foo"]): do nothing (duplicate 0 times), resulting in acc === [["foo"]]
+          // iteration 2 (["bar", "baz"]): duplicate acc once, resulting in `[["foo"], ["foo"]]
+          duplicateArrays(acc, nextParts.length - 1).map((base, index) =>
+            // evenly distribute the parts across the repeated base keys.
+            // nextParts[0 % 2] => "bar"
+            // nextParts[1 % 2] => "baz"
+            base.concat(nextParts[index % nextParts.length])
+          ),
+        [[]]
+      ) // [["foo", "bar"], ["foo", "baz"]]
+      .map(expanded => expanded.join('.')); // ["foo.bar", "foo.baz"]
+  } else {
+    // No braces.
+    // Example: "hello.world"
+    return key;
+  }
 }
 
 /**

diff --git a/tests/lib/rules/require-computed-property-dependencies.js b/tests/lib/rules/require-computed-property-dependencies.js
@@ -77,6 +77,18 @@ ruleTester.run('require-computed-property-dependencies', rule, {
         return this.get('foo').mapBy('bar') + this.get('foo').mapBy('bar');
       });
     `,
+    // Array inside braces:
+    `
+      Ember.computed('article.{comments.[],title}', function() {
+        return this.article.title + someFunction(this.article.comments.mapBy(function(comment) { return comment.author; }));
+      });
+    `,
+    // Nesting inside braces:
+    `
+      Ember.computed('article.{comments.innerProperty,title}', function() {
+        return this.article.title + someFunction(this.article.comments.innerProperty);
+      });
+    `,
     // Computed macro that should be ignored:
     `
       Ember.computed.someMacro(function() {
@@ -524,6 +536,26 @@ ruleTester.run('require-computed-property-dependencies', rule, {
         },
       ],
     },
+    // Gracefully handles nesting/array inside braces:
+    {
+      code: `
+        Ember.computed('article.{comments.[],title,first.second}', function() {
+          return this.article.title + this.missingProp + someFunction(this.article.comments) + this.article.first.second;
+        });
+      `,
+      // TODO: an improvement would be to use braces here: 'article.{comments.[],first.second,title}'
+      output: `
+        Ember.computed('article.comments.[]', 'article.first.second', 'article.title', 'missingProp', function() {
+          return this.article.title + this.missingProp + someFunction(this.article.comments) + this.article.first.second;
+        });
+      `,
+      errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: missingProp',
+          type: 'CallExpression',
+        },
+      ],
+    },
     {
       // don't expand @each.{foo,bar}
       code: `