fix: add allowDynamicKeys option (default true) to `require-compute…

…d-property-dependencies` rule Also fixes handling of dynamic keys for the rule's autofixer.
ember-cli · Aug 16, 2019 · 2733279 · 2733279
1 parent 2170ff2
commit 2733279
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 11 deletions.
diff --git a/docs/rules/require-computed-property-dependencies.md b/docs/rules/require-computed-property-dependencies.md
@@ -40,6 +40,12 @@ export default EmberObject.extend({
 });
 ```
 
+## Configuration
+
+This rule takes an optional object containing:
+
+* `boolean` -- `allowDynamicKeys` -- whether the rule should allow or disallow dynamic (variable / non-string) dependency keys in computed properties (default `true`)
+
 ## References
 
 * [Guide](https://guides.emberjs.com/release/object-model/computed-properties/) for computed properties
diff --git a/lib/rules/require-computed-property-dependencies.js b/lib/rules/require-computed-property-dependencies.js
@@ -290,6 +290,17 @@ module.exports = {
     },
 
     fixable: 'code',
+
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          allowDynamicKeys: {
+            type: 'boolean',
+          },
+        },
+      },
+    ],
   },
 
   ERROR_MESSAGE_NON_STRING_VALUE,
@@ -300,12 +311,14 @@ module.exports = {
         if (isEmberComputed(node.callee) && node.arguments.length >= 1) {
           const declaredDependencies = parseComputedDependencies(node.arguments);
 
-          declaredDependencies.dynamicKeys.forEach(key => {
-            context.report({
-              node: key,
-              message: ERROR_MESSAGE_NON_STRING_VALUE,
+          if (context.options[0] && !context.options[0].allowDynamicKeys) {
+            declaredDependencies.dynamicKeys.forEach(key => {
+              context.report({
+                node: key,
+                message: ERROR_MESSAGE_NON_STRING_VALUE,
+              });
             });
-          });
+          }
 
           const computedPropertyFunction = node.arguments[node.arguments.length - 1];
 
@@ -347,10 +360,20 @@ module.exports = {
                 ', '
               )}`,
               fix(fixer) {
-                const missingDependenciesAsArguments = collapseKeys(
+                const sourceCode = context.getSourceCode();
+
+                const missingDependenciesAsArgumentsForDynamicKeys = declaredDependencies.dynamicKeys.map(
+                  dynamicKey => sourceCode.getText(dynamicKey)
+                );
+                const missingDependenciesAsArgumentsForStringKeys = collapseKeys(
                   removeRedundantKeys([...undeclaredKeys, ...expandedDeclaredKeys])
                 );
 
+                const missingDependenciesAsArguments = [
+                  ...missingDependenciesAsArgumentsForDynamicKeys,
+                  ...missingDependenciesAsArgumentsForStringKeys,
+                ].join(', ');
+
                 if (node.arguments.length > 1) {
                   const firstDependency = node.arguments[0];
                   const lastDependency = node.arguments[node.arguments.length - 2];
@@ -379,7 +402,7 @@ module.exports = {
  *
  * Example:
  * Input: ["foo.bar", "foo.baz", "quux.[]"]
- * Output: "'foo.{bar,baz}', 'quux.[]'"
+ * Output: ["foo.{bar,baz}", "quux.[]"]
  *
  * @param {Array<string>} keys
  * @returns string
@@ -412,10 +435,7 @@ function collapseKeys(keys) {
     return `${parent}.${children[0]}`;
   });
 
-  return [...bareKeys, ...joined]
-    .sort()
-    .map(key => `'${key}'`)
-    .join(', ');
+  return [...bareKeys, ...joined].sort().map(key => `'${key}'`);
 }
 
 /**

diff --git a/tests/lib/rules/require-computed-property-dependencies.js b/tests/lib/rules/require-computed-property-dependencies.js
@@ -98,27 +98,80 @@ ruleTester.run('require-computed-property-dependencies', rule, {
     `
       Ember.computed.someMacro('test')
     `,
+    // Dynamic key:
+    `
+      Ember.computed(dynamic, function() {});
+    `,
+    // Dynamic key:
+    {
+      code: `
+        Ember.computed(...PROPERTIES, function() {});
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    },
     // Incorrect usage that should be ignored:
     `
       Ember.computed(123)
     `,
   ],
   invalid: [
+    // Dynamic key:
     {
       code: 'Ember.computed(dynamic, function() {});',
       output: null,
+      options: [{ allowDynamicKeys: false }],
       errors: [
         {
           message: ERROR_MESSAGE_NON_STRING_VALUE,
           type: 'Identifier',
         },
       ],
     },
+    // Dynamic keys:
     {
       code: 'Ember.computed(...PROPERTIES, function() {});',
       output: null,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      options: [{ allowDynamicKeys: false }],
+      errors: [
+        {
+          message: ERROR_MESSAGE_NON_STRING_VALUE,
+          type: 'SpreadElement',
+        },
+      ],
+    },
+    // Dynamic key with missing dependency:
+    {
+      code: 'Ember.computed(dynamic, function() { return this.undeclared; });',
+      output: "Ember.computed(dynamic, 'undeclared', function() { return this.undeclared; });",
+      options: [{ allowDynamicKeys: false }],
+      errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: undeclared',
+          type: 'CallExpression',
+        },
+        {
+          message: ERROR_MESSAGE_NON_STRING_VALUE,
+          type: 'Identifier',
+        },
+      ],
+    },
+    // Multiple dynamic (identifier and spread) keys with missing dependency:
+    {
+      code: 'Ember.computed(dynamic, ...moreDynamic, function() { return this.undeclared; });',
+      output:
+        "Ember.computed(dynamic, ...moreDynamic, 'undeclared', function() { return this.undeclared; });",
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      options: [{ allowDynamicKeys: false }],
       errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: undeclared',
+          type: 'CallExpression',
+        },
+        {
+          message: ERROR_MESSAGE_NON_STRING_VALUE,
+          type: 'Identifier',
+        },
         {
           message: ERROR_MESSAGE_NON_STRING_VALUE,
           type: 'SpreadElement',