Repository: ember-cli/eslint-plugin-ember
Pull request #775 (2020-04-14 03:00:03)

Handle explicit getter functions in `require-computed-property-dependencies` rule
Catches and autofixes missing dependent keys when using a `get()` function inside a computed property.

```js
// missing dependent key: lastName
computed('firstName', {
  get() {
    return this.firstName + ' ' + this.lastName;
  },
  set(key, value) {}
})
```


Patch:
diff --git a/lib/rules/require-computed-property-dependencies.js b/lib/rules/require-computed-property-dependencies.js
index 84e59a82..58a639b6 100644
--- a/lib/rules/require-computed-property-dependencies.js
+++ b/lib/rules/require-computed-property-dependencies.js
@@ -14,6 +14,7 @@ function getTraverser() {
 
 const Traverser = getTraverser();
 const emberUtils = require('../utils/ember');
+const computedPropertyUtils = require('../utils/computed-properties');
 const types = require('../utils/types');
 const propertyGetterUtils = require('../utils/property-getter');
 const assert = require('assert');
@@ -409,13 +410,16 @@ module.exports = {
             });
           }
 
-          const computedPropertyFunction = node.arguments[node.arguments.length - 1];
+          const lastArg = node.arguments[node.arguments.length - 1];
+          const computedPropertyFunctionBody = computedPropertyUtils.getComputedPropertyFunctionBody(
+            node
+          );
 
           const usedKeys1 = flatMap(
-            findEmberGetCalls(computedPropertyFunction.body),
+            findEmberGetCalls(computedPropertyFunctionBody),
             extractEmberGetDependencies
           );
-          const usedKeys2 = flatMap(findThisGetCalls(computedPropertyFunction.body), (node) => {
+          const usedKeys2 = flatMap(findThisGetCalls(computedPropertyFunctionBody), (node) => {
             return extractThisGetDependencies(node, context);
           });
           const usedKeys = [...usedKeys1, ...usedKeys2];
@@ -473,10 +477,7 @@ module.exports = {
                     missingDependenciesAsArguments
                   );
                 } else {
-                  return fixer.insertTextBefore(
-                    computedPropertyFunction,
-                    `${missingDependenciesAsArguments}, `
-                  );
+                  return fixer.insertTextBefore(lastArg, `${missingDependenciesAsArguments}, `);
                 }
               },
             });
diff --git a/lib/utils/computed-properties.js b/lib/utils/computed-properties.js
new file mode 100644
index 00000000..ea2f4b0e
--- /dev/null
+++ b/lib/utils/computed-properties.js
@@ -0,0 +1,36 @@
+const types = require('./types');
+const assert = require('assert');
+
+module.exports = {
+  getComputedPropertyFunctionBody,
+};
+
+/**
+ * Gets the function body of the computed property.
+ *
+ * Handles:
+ * * computed('prop1', 'prop2', function() { ... })
+ * * computed('prop1', 'prop2', { get() { ... } })
+ *
+ * @param {ASTNode} node - computed property CallExpression node
+ * @returns {ASTNode} function body of computed property
+ */
+function getComputedPropertyFunctionBody(node) {
+  assert(types.isCallExpression(node), 'Should only call this function on a CallExpression');
+
+  const lastArg = node.arguments[node.arguments.length - 1];
+
+  let computedPropertyFunctionBody = undefined;
+  if (types.isArrowFunctionExpression(lastArg) || types.isFunctionExpression(lastArg)) {
+    computedPropertyFunctionBody = lastArg.body;
+  } else if (types.isObjectExpression(lastArg)) {
+    const getFunction = lastArg.properties.find(
+      (property) =>
+        property.method && types.isIdentifier(property.key) && property.key.name === 'get'
+    );
+    if (getFunction) {
+      computedPropertyFunctionBody = getFunction.value.body;
+    }
+  }
+  return computedPropertyFunctionBody;
+}


Test patch:
diff --git a/tests/lib/rules/require-computed-property-dependencies.js b/tests/lib/rules/require-computed-property-dependencies.js
index 7d1ee2d3..c34f16eb 100644
--- a/tests/lib/rules/require-computed-property-dependencies.js
+++ b/tests/lib/rules/require-computed-property-dependencies.js
@@ -136,7 +136,18 @@ ruleTester.run('require-computed-property-dependencies', rule, {
     `,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
     },
-
+    // Explicit getter function:
+    {
+      code: `
+        computed('firstName', 'lastName', {
+          get() {
+            return this.firstName + ' ' + this.lastName;
+          },
+          set(key, value) {}
+        })
+    `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    },
     // Decorator:
     {
       // TODO: this should be an invalid test case.
@@ -772,5 +783,68 @@ ruleTester.run('require-computed-property-dependencies', rule, {
         },
       ],
     },
+    {
+      // Explicit getter function:
+      code: `
+        computed('firstName', {
+          get() {
+            return this.firstName + ' ' + this.lastName;
+          },
+          set(key, value) {}
+        })
+      `,
+      output: `
+        computed('firstName', 'lastName', {
+          get() {
+            return this.firstName + ' ' + this.lastName;
+          },
+          set(key, value) {}
+        })
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: lastName',
+          type: 'CallExpression',
+        },
+      ],
+    },
+    // Decorator with getter inside object parameter:
+    {
+      code: `
+        class Test {
+          @computed('firstName', {
+            get() {
+              return this.firstName + ' ' + this.lastName;
+            },
+            set(key, value) {}
+          })
+          fullName
+        }
+      `,
+      output: `
+        class Test {
+          @computed('firstName', 'lastName', {
+            get() {
+              return this.firstName + ' ' + this.lastName;
+            },
+            set(key, value) {}
+          })
+          fullName
+        }
+      `,
+      errors: [
+        {
+          message: 'Use of undeclared dependencies in computed property: lastName',
+          type: 'CallExpression',
+        },
+      ],
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
   ],
 });
diff --git a/tests/lib/utils/computed-properties-test.js b/tests/lib/utils/computed-properties-test.js
new file mode 100644
index 00000000..68f34340
--- /dev/null
+++ b/tests/lib/utils/computed-properties-test.js
@@ -0,0 +1,39 @@
+const babelEslint = require('babel-eslint');
+const computedPropertyUtils = require('../../../lib/utils/computed-properties');
+
+function parse(code) {
+  return babelEslint.parse(code).body[0].expression;
+}
+
+describe('getComputedPropertyFunctionBody', () => {
+  it('gets the result with no dependent keys and no function', () => {
+    const node = parse('computed()');
+    expect(computedPropertyUtils.getComputedPropertyFunctionBody(node)).toBeUndefined();
+  });
+
+  it('gets the result with one dependent key and no function', () => {
+    const node = parse('computed("dep")');
+    expect(computedPropertyUtils.getComputedPropertyFunctionBody(node)).toBeUndefined();
+  });
+
+  it('gets the result when using FunctionExpression and no dependent keys', () => {
+    const node = parse('computed(function() {})');
+    expect(computedPropertyUtils.getComputedPropertyFunctionBody(node).type).toStrictEqual(
+      'BlockStatement'
+    );
+  });
+
+  it('gets the result when using FunctionExpression and one dependent key', () => {
+    const node = parse('computed("dep1", function() {})');
+    expect(computedPropertyUtils.getComputedPropertyFunctionBody(node).type).toStrictEqual(
+      'BlockStatement'
+    );
+  });
+
+  it('gets the result when using explicit getter', () => {
+    const node = parse('computed("dep1", { get() {}})');
+    expect(computedPropertyUtils.getComputedPropertyFunctionBody(node).type).toStrictEqual(
+      'BlockStatement'
+    );
+  });
+});