Repository: ember-cli/eslint-plugin-ember
Pull request #800 (2020-04-27 22:35:11)

Handle `@computed` decorator without parentheses in `no-side-effects` and `require-computed-property-dependencies` rules
If you did `@computed` but omitted the parentheses, the linting wouldn't recognize it as a computed property, so it would be ignored. This effectively makes `@computed` treated the same as `@computed()`.


Patch:
diff --git a/lib/rules/no-side-effects.js b/lib/rules/no-side-effects.js
index 11afef3b..51b98aa2 100644
--- a/lib/rules/no-side-effects.js
+++ b/lib/rules/no-side-effects.js
@@ -98,6 +98,16 @@ module.exports = {
 
         findSideEffects(computedPropertyBody, emberImportAliasName).forEach(report);
       },
+
+      Identifier(node) {
+        if (!emberUtils.isComputedProp(node)) {
+          return;
+        }
+
+        const computedPropertyBody = computedPropertyUtils.getComputedPropertyFunctionBody(node);
+
+        findSideEffects(computedPropertyBody, emberImportAliasName).forEach(report);
+      },
     };
   },
 };
diff --git a/lib/rules/require-computed-property-dependencies.js b/lib/rules/require-computed-property-dependencies.js
index e257e1bc..c519e7c9 100644
--- a/lib/rules/require-computed-property-dependencies.js
+++ b/lib/rules/require-computed-property-dependencies.js
@@ -368,132 +368,152 @@ module.exports = {
 
     let serviceNames = [];
 
+    function checkComputedDependencies(node, nodeArguments) {
+      const declaredDependencies = parseComputedDependencies(nodeArguments);
+
+      if (!allowDynamicKeys) {
+        declaredDependencies.dynamicKeys.forEach((key) => {
+          context.report({
+            node: key,
+            message: ERROR_MESSAGE_NON_STRING_VALUE,
+          });
+        });
+      }
+
+      const computedPropertyFunctionBody = computedPropertyUtils.getComputedPropertyFunctionBody(
+        node
+      );
+
+      const usedKeys1 = javascriptUtils.flatMap(
+        findEmberGetCalls(computedPropertyFunctionBody),
+        extractEmberGetDependencies
+      );
+      const usedKeys2 = javascriptUtils.flatMap(
+        findThisGetCalls(computedPropertyFunctionBody),
+        (node) => {
+          return extractThisGetDependencies(node, context);
+        }
+      );
+      const usedKeys = [...usedKeys1, ...usedKeys2];
+
+      const expandedDeclaredKeys = computedPropertyDependentKeyUtils.expandKeys(
+        declaredDependencies.keys.map(nodeToStringValue)
+      );
+
+      const undeclaredKeysBeforeServiceCheck = removeRedundantKeys(
+        usedKeys
+          .filter((usedKey) =>
+            expandedDeclaredKeys.every(
+              (declaredKey) =>
+                declaredKey !== usedKey &&
+                !computedPropertyDependencyMatchesKeyPath(declaredKey, usedKey)
+            )
+          )
+          .reduce((keys, key) => {
+            if (!keys.includes(key)) {
+              keys.push(key);
+            }
+            return keys;
+          }, [])
+          .sort()
+      );
+
+      const undeclaredKeys = requireServiceNames
+        ? undeclaredKeysBeforeServiceCheck
+        : removeServiceNames(undeclaredKeysBeforeServiceCheck, serviceNames);
+
+      if (undeclaredKeys.length > 0) {
+        context.report({
+          node,
+          message: 'Use of undeclared dependencies in computed property: {{undeclaredKeys}}',
+          data: { undeclaredKeys: undeclaredKeys.join(', ') },
+          fix(fixer) {
+            const sourceCode = context.getSourceCode();
+
+            const missingDependenciesAsArgumentsForDynamicKeys = declaredDependencies.dynamicKeys.map(
+              (dynamicKey) => sourceCode.getText(dynamicKey)
+            );
+            const missingDependenciesAsArgumentsForStringKeys = computedPropertyDependentKeyUtils.collapseKeys(
+              removeRedundantKeys([...undeclaredKeys, ...expandedDeclaredKeys])
+            );
+
+            const missingDependenciesAsArguments = [
+              ...missingDependenciesAsArgumentsForDynamicKeys,
+              ...missingDependenciesAsArgumentsForStringKeys,
+            ].join(', ');
+
+            if (nodeArguments.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 = nodeArguments[0];
+                const lastDependency = lastArg;
+                return fixer.replaceTextRange(
+                  [firstDependency.range[0], lastDependency.range[1]],
+                  missingDependenciesAsArguments
+                );
+              }
+            } else {
+              const nodeText = sourceCode.getText(node);
+              if (types.isCallExpression(node)) {
+                // Insert dependencies inside empty parenthesis.
+                // Before: @computed()
+                // After: @computed('first')
+                const positionAfterParenthesis = node.range[0] + nodeText.indexOf('(') + 1;
+                return fixer.insertTextAfterRange(
+                  [node.range[0], positionAfterParenthesis],
+                  missingDependenciesAsArguments
+                );
+              } else {
+                // Add dependencies with parentheses.
+                // Before: @computed
+                // After: @computed('first')
+                return fixer.insertTextAfterRange(
+                  [node.range[0], node.range[0] + nodeText.length],
+                  `(${missingDependenciesAsArguments})`
+                );
+              }
+            }
+          },
+        });
+      }
+    }
+
     return {
       Program(node) {
         // If service names aren't required dependencies, then we need to keep track of them so that we can ignore them.
         serviceNames = requireServiceNames ? [] : findInjectedServiceNames(node);
       },
 
+      Identifier(node) {
+        if (isEmberComputed(node)) {
+          checkComputedDependencies(node, []);
+        }
+      },
+
       CallExpression(node) {
         if (isEmberComputed(node.callee)) {
-          const declaredDependencies = parseComputedDependencies(node.arguments);
-
-          if (!allowDynamicKeys) {
-            declaredDependencies.dynamicKeys.forEach((key) => {
-              context.report({
-                node: key,
-                message: ERROR_MESSAGE_NON_STRING_VALUE,
-              });
-            });
-          }
-
-          const computedPropertyFunctionBody = computedPropertyUtils.getComputedPropertyFunctionBody(
-            node
-          );
-
-          const usedKeys1 = javascriptUtils.flatMap(
-            findEmberGetCalls(computedPropertyFunctionBody),
-            extractEmberGetDependencies
-          );
-          const usedKeys2 = javascriptUtils.flatMap(
-            findThisGetCalls(computedPropertyFunctionBody),
-            (node) => {
-              return extractThisGetDependencies(node, context);
-            }
-          );
-          const usedKeys = [...usedKeys1, ...usedKeys2];
-
-          const expandedDeclaredKeys = computedPropertyDependentKeyUtils.expandKeys(
-            declaredDependencies.keys.map(nodeToStringValue)
-          );
-
-          const undeclaredKeysBeforeServiceCheck = removeRedundantKeys(
-            usedKeys
-              .filter((usedKey) =>
-                expandedDeclaredKeys.every(
-                  (declaredKey) =>
-                    declaredKey !== usedKey &&
-                    !computedPropertyDependencyMatchesKeyPath(declaredKey, usedKey)
-                )
-              )
-              .reduce((keys, key) => {
-                if (!keys.includes(key)) {
-                  keys.push(key);
-                }
-                return keys;
-              }, [])
-              .sort()
-          );
-
-          const undeclaredKeys = requireServiceNames
-            ? undeclaredKeysBeforeServiceCheck
-            : removeServiceNames(undeclaredKeysBeforeServiceCheck, serviceNames);
-
-          if (undeclaredKeys.length > 0) {
-            context.report({
-              node,
-              message: 'Use of undeclared dependencies in computed property: {{undeclaredKeys}}',
-              data: { undeclaredKeys: undeclaredKeys.join(', ') },
-              fix(fixer) {
-                const sourceCode = context.getSourceCode();
-
-                const missingDependenciesAsArgumentsForDynamicKeys = declaredDependencies.dynamicKeys.map(
-                  (dynamicKey) => sourceCode.getText(dynamicKey)
-                );
-                const missingDependenciesAsArgumentsForStringKeys = computedPropertyDependentKeyUtils.collapseKeys(
-                  removeRedundantKeys([...undeclaredKeys, ...expandedDeclaredKeys])
-                );
-
-                const missingDependenciesAsArguments = [
-                  ...missingDependenciesAsArgumentsForDynamicKeys,
-                  ...missingDependenciesAsArgumentsForStringKeys,
-                ].join(', ');
-
-                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
-                  );
-                }
-              },
-            });
-          }
+          checkComputedDependencies(node, node.arguments);
         }
       },
     };
diff --git a/lib/utils/computed-properties.js b/lib/utils/computed-properties.js
index 7e34ae07..0648fcee 100644
--- a/lib/utils/computed-properties.js
+++ b/lib/utils/computed-properties.js
@@ -33,37 +33,46 @@ function isComputedPropertyBodyArg(node) {
  * * computed('prop1', 'prop2', function() { ... })
  * * computed('prop1', 'prop2', () => { ... })
  * * computed('prop1', 'prop2', { get() { ... } })
+ * * @computed('prop1', 'prop2') get fullName() { ... }
+ * * @computed get fullName() { ... }
  *
  * @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');
+  assert(
+    types.isCallExpression(node) || types.isIdentifier(node),
+    'Should only call this function on a CallExpression or Identifier'
+  );
 
-  const lastArg = node.arguments[node.arguments.length - 1];
+  if (types.isCallExpression(node)) {
+    const lastArg = node.arguments[node.arguments.length - 1];
 
-  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;
+    if (types.isArrowFunctionExpression(lastArg) || types.isFunctionExpression(lastArg)) {
+      // Example: computed('prop1', 'prop2', function() { ... })
+      // Example: computed('prop1', 'prop2', () => { ... })
+      return 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) {
+        return getFunction.value.body;
+      }
     }
-  } else if (
+  }
+
+  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;
+    // Example: @computed get fullName() {}
+    return node.parent.parent.value.body;
   }
 
-  return computedPropertyFunctionBody;
+  return undefined;
 }
diff --git a/lib/utils/ember.js b/lib/utils/ember.js
index e8813dc0..f9621244 100644
--- a/lib/utils/ember.js
+++ b/lib/utils/ember.js
@@ -127,10 +127,15 @@ function isPropOfType(node, type) {
 function isModule(node, element, module) {
   const moduleName = module || 'Ember';
 
-  return (
-    types.isCallExpression(node) &&
-    (isLocalModule(node.callee, element) || isEmberModule(node.callee, element, moduleName))
-  );
+  if (types.isIdentifier(node)) {
+    return isLocalModule(node, element) || isEmberModule(node, element, moduleName);
+  }
+
+  if (types.isCallExpression(node)) {
+    return isLocalModule(node.callee, element) || isEmberModule(node.callee, element, moduleName);
+  }
+
+  return false;
 }
 
 function isDSModel(node, filePath) {


Test patch:
diff --git a/tests/lib/rules/no-side-effects.js b/tests/lib/rules/no-side-effects.js
index 26967464..869c3843 100644
--- a/tests/lib/rules/no-side-effects.js
+++ b/tests/lib/rules/no-side-effects.js
@@ -43,11 +43,26 @@ eslintTester.run('no-side-effects', rule, {
         ecmaFeatures: { legacyDecorators: true },
       },
     },
+    {
+      code: `
+        class Test {
+          @computed
+          get fullName() { return 'foo'; }
+        }
+      `,
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
 
     // No computed property function body;
     'computed()',
     'computed("test")',
     'computed("test", function() {})',
+    'computed',
 
     // Not in a computed property:
     "this.set('x', 123);",
@@ -258,6 +273,28 @@ eslintTester.run('no-side-effects', rule, {
         ecmaFeatures: { legacyDecorators: true },
       },
     },
+    // Decorator without parentheses:
+    {
+      code: `
+        class Test {
+          @computed
+          get someProp() { this.set('x', 123); }
+        }
+      `,
+      output: null,
+      errors: [
+        {
+          message: ERROR_MESSAGE,
+          type: 'CallExpression',
+        },
+      ],
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
 
     {
       code: 'computed("test", function() { this.x = 123; })',
diff --git a/tests/lib/rules/require-computed-property-dependencies.js b/tests/lib/rules/require-computed-property-dependencies.js
index 0e225e9e..7ef35481 100644
--- a/tests/lib/rules/require-computed-property-dependencies.js
+++ b/tests/lib/rules/require-computed-property-dependencies.js
@@ -891,6 +891,33 @@ ruleTester.run('require-computed-property-dependencies', rule, {
         ecmaFeatures: { legacyDecorators: true },
       },
     },
+    // Decorator with no parens:
+    {
+      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: 'Identifier',
+        },
+      ],
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
     // Decorator with no args:
     {
       code: `
diff --git a/tests/lib/rules/require-return-from-computed.js b/tests/lib/rules/require-return-from-computed.js
index f48eca29..d49fb122 100644
--- a/tests/lib/rules/require-return-from-computed.js
+++ b/tests/lib/rules/require-return-from-computed.js
@@ -35,6 +35,17 @@ eslintTester.run('require-return-from-computed', rule, {
         ecmaFeatures: { legacyDecorators: true },
       },
     },
+    {
+      // 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() {} }',
+      parser: require.resolve('babel-eslint'),
+      parserOptions: {
+        ecmaVersion: 6,
+        sourceType: 'module',
+        ecmaFeatures: { legacyDecorators: true },
+      },
+    },
   ],
   invalid: [
     {
diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js
index 82aae3e1..e6554fae 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -774,6 +774,18 @@ describe('isComputedProp', () => {
     node = parse('Ember.computed().foo()');
     expect(emberUtils.isComputedProp(node)).not.toBeTruthy();
   });
+
+  it('should detect the computed annotation', () => {
+    const program = babelEslint.parse('class Object { @computed() get foo() {} }');
+    node = program.body[0].body.body[0].decorators[0].expression;
+    expect(emberUtils.isComputedProp(node)).toBeTruthy();
+  });
+
+  it('should detect the computed annotation without parentheses', () => {
+    const program = babelEslint.parse('class Object { @computed get foo() {} }');
+    node = program.body[0].body.body[0].decorators[0].expression;
+    expect(emberUtils.isComputedProp(node)).toBeTruthy();
+  });
 });
 
 describe('isObserverProp', () => {