Add decorator support to require-computed-property-dependencies rule

docs/rules/require-computed-property-dependencies.md

@@ -53,9 +53,3 @@ This rule takes an optional object containing:
 ## References
 
 * [Guide](https://guides.emberjs.com/release/object-model/computed-properties/) for computed properties
-
-## Help Wanted
-
-| Issue | Link |
-| :-- | :-- |
-| :x: Missing native JavaScript class support | [#560](https://github.com/ember-cli/eslint-plugin-ember/issues/560) |

lib/rules/require-computed-property-dependencies.js

@@ -124,12 +124,10 @@ function parseComputedDependencies(args) {
   const keys = [];
   const dynamicKeys = [];
 
-  for (let i = 0; i < args.length - 1; i++) {
-    const arg = args[i];
-
+  for (const arg of args) {
     if (types.isStringLiteral(arg) || isTwoPartStringLiteral(arg)) {
       keys.push(arg);
-    } else {
+    } else if (!computedPropertyUtils.isComputedPropertyBodyArg(arg)) {
       dynamicKeys.push(arg);
     }
   }
@@ -224,7 +222,7 @@ function findInjectedServiceNames(node) {
   new Traverser().traverse(node, {
     enter(child) {
       if (
-        types.isProperty(child) &&
+        (types.isProperty(child) || types.isClassProperty(child)) &&
         emberUtils.isInjectedServiceProp(child) &&
         types.isIdentifier(child.key)
       ) {
@@ -398,7 +396,7 @@ module.exports = {
       },
 
       CallExpression(node) {
-        if (isEmberComputed(node.callee) && node.arguments.length >= 1) {
+        if (isEmberComputed(node.callee)) {
           const declaredDependencies = parseComputedDependencies(node.arguments);
 
           if (!allowDynamicKeys) {
@@ -410,7 +408,6 @@ module.exports = {
             });
           }
 
-          const lastArg = node.arguments[node.arguments.length - 1];
           const computedPropertyFunctionBody = computedPropertyUtils.getComputedPropertyFunctionBody(
             node
           );
@@ -468,16 +465,47 @@ module.exports = {
                   ...missingDependenciesAsArgumentsForStringKeys,
                 ].join(', ');
 
-                if (node.arguments.length > 1) {
-                  const firstDependency = node.arguments[0];
-                  const lastDependency = node.arguments[node.arguments.length - 2];
-
-                  return fixer.replaceTextRange(
-                    [firstDependency.range[0], lastDependency.range[1]],
+                if (node.arguments.length > 0) {
+                  const lastArg = node.arguments[node.arguments.length - 1];
+                  if (computedPropertyUtils.isComputedPropertyBodyArg(lastArg)) {
+                    if (node.arguments.length > 1) {
+                      const firstDependency = node.arguments[0];
+                      const lastDependency = node.arguments[node.arguments.length - 2];
+
+                      // Replace the dependent keys before the function body argument.
+                      // Before: computed('first', function() {})
+                      // After: computed('first', 'last', function() {})
+                      return fixer.replaceTextRange(
+                        [firstDependency.range[0], lastDependency.range[1]],
+                        missingDependenciesAsArguments
+                      );
+                    } else {
+                      // Add dependent keys before the function body argument.
+                      // Before: computed(function() {})
+                      // After: computed('key', function() {})
+                      return fixer.insertTextBefore(lastArg, `${missingDependenciesAsArguments}, `);
+                    }
+                  } else {
+                    // All arguments are dependent keys, so replace them all.
+                    // Before: @computed('first')
+                    // After: @computed('first', 'last')
+                    const firstDependency = node.arguments[0];
+                    const lastDependency = lastArg;
+                    return fixer.replaceTextRange(
+                      [firstDependency.range[0], lastDependency.range[1]],
+                      missingDependenciesAsArguments
+                    );
+                  }
+                } else {
+                  // Insert dependencies inside empty parenthesis.
+                  // Before: @computed()
+                  // After: @computed('first')
+                  const nodeText = sourceCode.getText(node);
+                  const positionAfterParenthesis = node.range[0] + nodeText.indexOf('(') + 1;
+                  return fixer.insertTextAfterRange(
+                    [node.range[0], positionAfterParenthesis],
                     missingDependenciesAsArguments
                   );
-                } else {
-                  return fixer.insertTextBefore(lastArg, `${missingDependenciesAsArguments}, `);
                 }
               },
             });

lib/utils/computed-properties.js

@@ -2,14 +2,36 @@ const types = require('./types');
 const assert = require('assert');
 
 module.exports = {
+  isComputedPropertyBodyArg,
   getComputedPropertyFunctionBody,
 };
 
+/**
+ * Checks whether a computed property argument is the type of node that could contain the
+ * function body of the computed property (which would be passed as the last arg).
+ *
+ * Handles:
+ * * computed('prop1', 'prop2', function() { ... })
+ * * computed('prop1', 'prop2', () => { ... })
+ * * computed('prop1', 'prop2', { get() { ... } })
+ *
+ * @param {ASTNode} node - computed property argument to check
+ * @returns {boolean} whether the node could be the function body argument of a computed property
+ */
+function isComputedPropertyBodyArg(node) {
+  return (
+    types.isFunctionExpression(node) ||
+    types.isArrowFunctionExpression(node) ||
+    types.isObjectExpression(node)
+  );
+}
+
 /**
  * Gets the function body of the computed property.
  *
  * Handles:
  * * computed('prop1', 'prop2', function() { ... })
+ * * computed('prop1', 'prop2', () => { ... })
  * * computed('prop1', 'prop2', { get() { ... } })
  *
  * @param {ASTNode} node - computed property CallExpression node
@@ -22,15 +44,26 @@ function getComputedPropertyFunctionBody(node) {
 
   let computedPropertyFunctionBody = undefined;
   if (types.isArrowFunctionExpression(lastArg) || types.isFunctionExpression(lastArg)) {
+    // Example: computed('prop1', 'prop2', function() { ... })
+    // Example: computed('prop1', 'prop2', () => { ... })
     computedPropertyFunctionBody = lastArg.body;
   } else if (types.isObjectExpression(lastArg)) {
+    // Example: computed('prop1', 'prop2', { get() { ... } })
     const getFunction = lastArg.properties.find(
       (property) =>
         property.method && types.isIdentifier(property.key) && property.key.name === 'get'
     );
     if (getFunction) {
       computedPropertyFunctionBody = getFunction.value.body;
     }
+  } else if (
+    types.isDecorator(node.parent) &&
+    types.isMethodDefinition(node.parent.parent) &&
+    node.parent.parent.kind === 'get'
+  ) {
+    // Example: @computed('first', 'last') get fullName() {}
+    computedPropertyFunctionBody = node.parent.parent.value.body;
   }
+
   return computedPropertyFunctionBody;
 }

tests/lib/rules/require-computed-property-dependencies.js

@@ -150,12 +150,26 @@ ruleTester.run('require-computed-property-dependencies', rule, {
     },
     // Decorator:
     {
-      // TODO: this should be an invalid test case.
-      // Still missing native class and decorator support: https://github.com/ember-cli/eslint-plugin-ember/issues/560
       code: `
         class Test {
-          @computed()
-          get someProp() { return this.undeclared; }
+          @computed('first', 'last')
+          get fullName() { return this.first + ' ' + this.last; }
+        }
+      `,
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
+    // Decorator:
+    {
+      code: `
+        class Test {
+          @service i18n; // Service names not required as dependent keys by default.
+          @computed('first', 'last')
+          get fullName() { return this.i18n.t(this.first + ' ' + this.last); }
         }
       `,
       parser: require.resolve('babel-eslint'),
@@ -846,5 +860,86 @@ ruleTester.run('require-computed-property-dependencies', rule, {
         ecmaFeatures: { legacyDecorators: true },
       },
     },
+    // Decorator with no args:
+    {
+      code: `
+        class Test {
+          @computed()
+          get someProp() { return this.undeclared; }
+        }
+      `,
+      output: `
+        class Test {
+          @computed('undeclared')
+          get someProp() { return this.undeclared; }
+        }
+      `,
+      errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: undeclared',
+          type: 'CallExpression',
+        },
+      ],
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
+    // Decorator with arg:
+    {
+      code: `
+        class Test {
+          @computed('first')
+          get fullName() { return this.first + ' ' + this.last; }
+        }
+      `,
+      output: `
+        class Test {
+          @computed('first', 'last')
+          get fullName() { return this.first + ' ' + this.last; }
+        }
+      `,
+      errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: last',
+          type: 'CallExpression',
+        },
+      ],
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
+    // Decorator with two arg:
+    {
+      code: `
+        class Test {
+          @computed('first', 'last')
+          get fullName() { return this.first + ' ' + this.last + ' ' + this.undeclared; }
+        }
+      `,
+      output: `
+        class Test {
+          @computed('first', 'last', 'undeclared')
+          get fullName() { return this.first + ' ' + this.last + ' ' + this.undeclared; }
+        }
+      `,
+      errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: undeclared',
+          type: 'CallExpression',
+        },
+      ],
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
   ],
 });