Repository: ember-cli/eslint-plugin-ember
Pull request #429 (2019-06-13 06:55:28)

Add new `require-computed-macros` rule
The simplest / most common computed property macros are included to begin with.


Patch:
diff --git a/README.md b/README.md
index 57c7bfed..3a769961 100644
--- a/README.md
+++ b/README.md
@@ -100,6 +100,7 @@ The `--fix` option on the command line automatically fixes problems reported by
 |  | [no-unnecessary-index-route](./docs/rules/no-unnecessary-index-route.md) | Disallow unnecessary `index` route definition |
 | :wrench: | [no-unnecessary-route-path-option](./docs/rules/no-unnecessary-route-path-option.md) | Disallow unnecessary route `path` option |
 | :wrench: | [no-unnecessary-service-injection-argument](./docs/rules/no-unnecessary-service-injection-argument.md) | Disallow unnecessary argument when injecting service |
+| :wrench: | [require-computed-macros](./docs/rules/require-computed-macros.md) | Requires using computed property macros when possible |
 |  | [route-path-style](./docs/rules/route-path-style.md) | Enforces usage of kebab-case (instead of snake_case or camelCase) in route paths |
 | :wrench: | [use-ember-get-and-set](./docs/rules/use-ember-get-and-set.md) | Enforces usage of Ember.get and Ember.set |
 
diff --git a/docs/rules/require-computed-macros.md b/docs/rules/require-computed-macros.md
new file mode 100644
index 00000000..14a3ffc5
--- /dev/null
+++ b/docs/rules/require-computed-macros.md
@@ -0,0 +1,92 @@
+# require-computed-macros
+
+It is preferred to use Ember's computed property macros as opposed to manually writing out logic in a computed property function. Reasons include:
+
+* Conciseness
+* Readability
+* Reduced chance of typos
+* Reduced chance of missing dependencies
+
+## Rule Details
+
+This rule requires using Ember's computed property macros when possible.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```js
+import Component from '@ember/component';
+import { computed } from '@ember/object';
+
+export default Component.extend({
+  propReadOnly: computed('x', function() {
+    return this.x;
+  }),
+
+  propAnd: computed('x', 'y', function() {
+    return this.x && this.y;
+  }),
+
+  propOr: computed('x', 'y', function() {
+    return this.x || this.y;
+  }),
+
+  propGt: computed('x', function() {
+    return this.x > 123;
+  }),
+
+  propGte: computed('x', function() {
+    return this.x >= 123;
+  }),
+
+  propLt: computed('x', function() {
+    return this.x < 123;
+  }),
+
+  propLte: computed('x', function() {
+    return this.x <= 123;
+  }),
+
+  propNot: computed('x', function() {
+    return !this.x;
+  }),
+
+  propEqual: computed('x', function() {
+    return this.x === 123;
+  }),
+});
+```
+
+Examples of **correct** code for this rule:
+
+
+```js
+import Component from '@ember/component';
+import { readOnly, and, or, gt, gte, lt, lte, not, equal } from '@ember/object/computed';
+
+export default Component.extend({
+  propReadOnly: readOnly('x'),
+
+  propAnd: and('x', 'y'),
+
+  propOr: or('x', 'y'),
+
+  propGt: gt('x', 123),
+
+  propGte: gte('x', 123),
+
+  propLt: lt('x', 123),
+
+  propLte: lte('x', 123),
+
+  propNot: not('x'),
+
+  propEqual: equal('x', 123)
+});
+```
+
+## References
+
+* [Guide](https://guides.emberjs.com/release/object-model/computed-properties/) for computed properties
+* [Spec](http://api.emberjs.com/ember/release/modules/@ember%2Fobject#functions-computed) for computed property macros
diff --git a/lib/index.js b/lib/index.js
index a1baf5e5..252582d9 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -42,6 +42,7 @@ module.exports = {
     'order-in-models': require('./rules/order-in-models'),
     'order-in-routes': require('./rules/order-in-routes'),
     'route-path-style': require('./rules/route-path-style'),
+    'require-computed-macros': require('./rules/require-computed-macros'),
     'require-return-from-computed': require('./rules/require-return-from-computed'),
     'require-super-in-init': require('./rules/require-super-in-init'),
     'routes-segments-snake-case': require('./rules/routes-segments-snake-case'),
diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js
index 06bb6a36..881636e7 100644
--- a/lib/recommended-rules.js
+++ b/lib/recommended-rules.js
@@ -44,6 +44,7 @@ module.exports = {
   "ember/order-in-controllers": "off",
   "ember/order-in-models": "off",
   "ember/order-in-routes": "off",
+  "ember/require-computed-macros": "off",
   "ember/require-return-from-computed": "off",
   "ember/require-super-in-init": "error",
   "ember/route-path-style": "off",
diff --git a/lib/rules/require-computed-macros.js b/lib/rules/require-computed-macros.js
new file mode 100644
index 00000000..205d8136
--- /dev/null
+++ b/lib/rules/require-computed-macros.js
@@ -0,0 +1,237 @@
+'use strict';
+
+const utils = require('../utils/utils');
+const emberUtils = require('../utils/ember');
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+function makeErrorMessage(macro) {
+  return `Use the \`${macro}\` computed property macro.`;
+}
+
+const ERROR_MESSAGE_READ_ONLY = makeErrorMessage('readOnly');
+const ERROR_MESSAGE_AND = makeErrorMessage('and');
+const ERROR_MESSAGE_OR = makeErrorMessage('or');
+const ERROR_MESSAGE_GT = makeErrorMessage('gt');
+const ERROR_MESSAGE_GTE = makeErrorMessage('gte');
+const ERROR_MESSAGE_LT = makeErrorMessage('lt');
+const ERROR_MESSAGE_LTE = makeErrorMessage('lte');
+const ERROR_MESSAGE_NOT = makeErrorMessage('not');
+const ERROR_MESSAGE_EQUAL = makeErrorMessage('equal');
+
+/**
+ * Checks if a MemberExpression node looks like `this.x` or `this.x.y`.
+ *
+ * @param {Node} node The MemberExpression node to check.
+ * @returns {boolean} Whether the node looks like `this.x` or `this.x.y`.
+ */
+function isSimpleThisExpression(node) {
+  if (!utils.isMemberExpression(node)) {
+    return false;
+  }
+
+  let current = node;
+  while (current !== null) {
+    if (utils.isMemberExpression(current)) {
+      if (!utils.isIdentifier(current.property)) {
+        return false;
+      }
+      current = current.object;
+    } else if (utils.isThisExpression(current)) {
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  return false;
+}
+
+/**
+ * Checks if a LogicalExpression looks like `this.x && this.y && this.z`
+ * containing only simple `this.x` MemberExpression nodes.
+ *
+ * @param {Node} node The LogicalExpression node to check.
+ * @returns {boolean} Whether the node looks like `this.x && this.y && this.z`.
+ */
+function isSimpleThisExpressionsInsideLogicalExpression(node, operator) {
+  if (!utils.isLogicalExpression(node)) {
+    return false;
+  }
+
+  let current = node;
+  while (current !== null) {
+    if (utils.isLogicalExpression(current)) {
+      if (!isSimpleThisExpression(current.right)) {
+        return false;
+      }
+      if (current.operator !== operator) {
+        return false;
+      }
+      current = current.left;
+    } else if (isSimpleThisExpression(current)) {
+      return true;
+    } else {
+      return false;
+    }
+  }
+  return false;
+}
+
+/**
+ * Converts a simple LogicalExpression into an array of the MemberExpression nodes in it.
+ *
+ * Example Input:   A LogicalExpression node with this source: `this.x && this.y.z && this.w`
+ * Example Output:  An array of these MemberExpression nodes: [this.x, this.y.z, this.w]
+ *
+ * @param {Node} node The LogicalExpression node containing nodes that look like `this.x`.
+ * @returns {Node[]} An array of MemberExpression nodes contained in the LogicalExpression.
+ */
+function getThisExpressions(nodeLogicalExpression) {
+  const arrayOfThisExpressions = [];
+  let current = nodeLogicalExpression;
+  while (current !== null) {
+    if (utils.isLogicalExpression(current)) {
+      arrayOfThisExpressions.push(current.right);
+      current = current.left;
+    } else {
+      arrayOfThisExpressions.push(current);
+      break;
+    }
+  }
+  return arrayOfThisExpressions.reverse();
+}
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Requires using computed property macros when possible',
+      category: 'Best Practices',
+      recommended: false,
+    },
+    fixable: 'code'
+  },
+
+  ERROR_MESSAGE_READ_ONLY,
+  ERROR_MESSAGE_AND,
+  ERROR_MESSAGE_OR,
+  ERROR_MESSAGE_GT,
+  ERROR_MESSAGE_GTE,
+  ERROR_MESSAGE_LT,
+  ERROR_MESSAGE_LTE,
+  ERROR_MESSAGE_NOT,
+  ERROR_MESSAGE_EQUAL,
+
+  create(context) {
+    /**
+     * Converts a Node containing a ThisExpression to its dependent key.
+     *
+     * Example Input:   A Node with this source code: `this.x.y`
+     * Example Output:  'x.y'
+     *
+     * @param {Node} node a MemberExpression node that looks like `this.x` or `this.x.y`.
+     * @returns {String} The dependent key of the input node (without `this.`).
+     */
+    function nodeToDependentKey(nodeWithThisExpression) {
+      const sourceCode = context.getSourceCode();
+      return sourceCode.getText(nodeWithThisExpression).replace(/^this\./, '');
+    }
+
+    function reportSingleArg(nodeComputedProperty, nodeWithThisExpression, macro) {
+      context.report({
+        node: nodeComputedProperty,
+        message: makeErrorMessage(macro),
+        fix(fixer) {
+          const text = nodeToDependentKey(nodeWithThisExpression);
+          return fixer.replaceText(nodeComputedProperty, `computed.${macro}('${text}')`);
+        }
+      });
+    }
+
+    function reportBinaryExpression(nodeComputedProperty, nodeBinaryExpression, macro) {
+      context.report({
+        node: nodeComputedProperty,
+        message: makeErrorMessage(macro),
+        fix(fixer) {
+          const sourceCode = context.getSourceCode();
+          const textLeft = nodeToDependentKey(nodeBinaryExpression.left);
+          const textRight = sourceCode.getText(nodeBinaryExpression.right);
+          return fixer.replaceText(nodeComputedProperty, `computed.${macro}('${textLeft}', ${textRight})`);
+        }
+      });
+    }
+
+    function reportLogicalExpression(nodeComputedProperty, nodeLogicalExpression, macro) {
+      context.report({
+        node: nodeComputedProperty,
+        message: makeErrorMessage(macro),
+        fix(fixer) {
+          const text = getThisExpressions(nodeLogicalExpression)
+            .map(nodeToDependentKey)
+            .join("', '");
+          return fixer.replaceText(nodeComputedProperty, `computed.${macro}('${text}')`);
+        }
+      });
+    }
+
+    return {
+      CallExpression(node) {
+        if (!emberUtils.isComputedProp(node)) {
+          return;
+        }
+
+        if (node.arguments.length === 0) {
+          return;
+        }
+
+        const lastArg = node.arguments[node.arguments.length - 1];
+        if (!utils.isFunctionExpression(lastArg)) {
+          return;
+        }
+
+        if (lastArg.body.body.length !== 1) {
+          return;
+        }
+
+        if (!utils.isReturnStatement(lastArg.body.body[0])) {
+          return;
+        }
+
+        const statement = lastArg.body.body[0].argument;
+
+        if (
+          utils.isUnaryExpression(statement) &&
+          statement.operator === '!' &&
+          isSimpleThisExpression(statement.argument)
+        ) {
+          reportSingleArg(node, statement.argument, 'not');
+        } else if (utils.isLogicalExpression(statement)) {
+          if (isSimpleThisExpressionsInsideLogicalExpression(statement, '&&')) {
+            reportLogicalExpression(node, statement, 'and');
+          } else if (isSimpleThisExpressionsInsideLogicalExpression(statement, '||')) {
+            reportLogicalExpression(node, statement, 'or');
+          }
+        } else if (
+          utils.isBinaryExpression(statement) &&
+          utils.isLiteral(statement.right) &&
+          isSimpleThisExpression(statement.left)) {
+          if (statement.operator === '===') {
+            reportBinaryExpression(node, statement, 'equal');
+          } else if (statement.operator === '>') {
+            reportBinaryExpression(node, statement, 'gt');
+          } else if (statement.operator === '>=') {
+            reportBinaryExpression(node, statement, 'gte');
+          } else if (statement.operator === '<') {
+            reportBinaryExpression(node, statement, 'lt');
+          } else if (statement.operator === '<=') {
+            reportBinaryExpression(node, statement, 'lte');
+          }
+        } else if (isSimpleThisExpression(statement)) {
+          reportSingleArg(node, statement, 'readOnly');
+        }
+      }
+    };
+  }
+};
diff --git a/lib/utils/utils.js b/lib/utils/utils.js
index 8b3aa1ad..4b317bd3 100644
--- a/lib/utils/utils.js
+++ b/lib/utils/utils.js
@@ -30,6 +30,7 @@ module.exports = {
   collectObjectPatternBindings,
   isEmptyMethod,
   isBinaryExpression,
+  isLogicalExpression,
   isReturnStatement,
   getParent
 };
@@ -111,6 +112,16 @@ function isBinaryExpression(node) {
   return node !== undefined && node.type === 'BinaryExpression';
 }
 
+/**
+ * Check whether or not a node is an LogicalExpression.
+ *
+ * @param {Object} node The node to check.
+ * @returns {boolean} Whether or not the node is an LogicalExpression.
+ */
+function isLogicalExpression(node) {
+  return node !== undefined && node.type === 'LogicalExpression';
+}
+
 /**
  * Check whether or not a node is an ObjectExpression.
  *


Test patch:
diff --git a/tests/lib/rules/require-computed-macros.js b/tests/lib/rules/require-computed-macros.js
new file mode 100644
index 00000000..dc616148
--- /dev/null
+++ b/tests/lib/rules/require-computed-macros.js
@@ -0,0 +1,157 @@
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/require-computed-macros');
+const RuleTester = require('eslint').RuleTester;
+
+const {
+  ERROR_MESSAGE_READ_ONLY,
+  ERROR_MESSAGE_AND,
+  ERROR_MESSAGE_OR,
+  ERROR_MESSAGE_GT,
+  ERROR_MESSAGE_GTE,
+  ERROR_MESSAGE_LT,
+  ERROR_MESSAGE_LTE,
+  ERROR_MESSAGE_NOT,
+  ERROR_MESSAGE_EQUAL
+} = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester();
+
+ruleTester.run('require-computed-macros', rule, {
+  valid: [
+    'computed()',
+    'computed(true)',
+    'computed(function() {})',
+    'computed(function() { someCall(); return this.x; })', // Multiple statements in function body.
+    'computed(function() { return this.x; }, SOME_OTHER_ARG)', // Function isn't last arg.
+    'other(function() { return this.x; })',
+
+    // READ_ONLY
+    "readOnly('x')",
+    'computed(function() { return this; })',
+    'computed(function() { return SOME_VAR; })',
+    'computed(function() { return this.prop[123]; })',
+    'computed(function() { return this.someFunction(); })',
+    'computed(function() { return this.prop.someFunction(); })',
+
+    // AND
+    "and('x', 'y')",
+    'computed(function() { return SOME_VAR && OTHER_VAR; })',
+    'computed(function() { return this.x && this.y || this.z; })', // Mixed operators.
+
+    // OR
+    "or('x', 'y')",
+    'computed(function() { return SOME_VAR || OTHER_VAR; })',
+
+    // GT
+    "gt('x', 123)",
+    'computed(function() { return SOME_VAR > OTHER_VAR; })',
+
+    // GTE
+    "gte('x', 123)",
+    'computed(function() { return SOME_VAR >= OTHER_VAR; })',
+
+    // LT
+    "lt('x', 123)",
+    'computed(function() { return SOME_VAR < OTHER_VAR; })',
+
+    // LTE
+    "lte('x', 123)",
+    'computed(function() { return SOME_VAR <= OTHER_VAR; })',
+
+    // NOT
+    "not('x')",
+    'computed(function() { return !SOME_VAR; })',
+
+    // EQUAL
+    "equal('x', 123)",
+    'computed(function() { return SOME_VAR === 123; })',
+    'computed(function() { return SOME_VAR === "Hello"; })',
+    'computed(function() { return this.prop === MY_VAR; })',
+  ],
+  invalid: [
+    // READ_ONLY
+    {
+      code: 'computed(function() { return this.x; })',
+      output: "computed.readOnly('x')",
+      errors: [{ message: ERROR_MESSAGE_READ_ONLY, type: 'CallExpression' }]
+    },
+    {
+      code: 'computed(function() { return this.x.y; })', // Nested path.
+      output: "computed.readOnly('x.y')",
+      errors: [{ message: ERROR_MESSAGE_READ_ONLY, type: 'CallExpression' }]
+    },
+
+    // AND
+    {
+      code: 'computed(function() { return this.x && this.y; })',
+      output: "computed.and('x', 'y')",
+      errors: [{ message: ERROR_MESSAGE_AND, type: 'CallExpression' }]
+    },
+    {
+      code: 'computed(function() { return this.x && this.y && this.z; })', // Three parts.
+      output: "computed.and('x', 'y', 'z')",
+      errors: [{ message: ERROR_MESSAGE_AND, type: 'CallExpression' }]
+    },
+    {
+      code: 'computed(function() { return this.x && this.y.z && this.w; })', // Three parts with a nested path.
+      output: "computed.and('x', 'y.z', 'w')",
+      errors: [{ message: ERROR_MESSAGE_AND, type: 'CallExpression' }]
+    },
+
+    // OR
+    {
+      code: 'computed(function() { return this.x || this.y; })',
+      output: "computed.or('x', 'y')",
+      errors: [{ message: ERROR_MESSAGE_OR, type: 'CallExpression' }]
+    },
+
+    // GT
+    {
+      code: 'computed(function() { return this.x > 123; })',
+      output: "computed.gt('x', 123)",
+      errors: [{ message: ERROR_MESSAGE_GT, type: 'CallExpression' }]
+    },
+
+    // GTE
+    {
+      code: 'computed(function() { return this.x >= 123; })',
+      output: "computed.gte('x', 123)",
+      errors: [{ message: ERROR_MESSAGE_GTE, type: 'CallExpression' }]
+    },
+
+    // LT
+    {
+      code: 'computed(function() { return this.x < 123; })',
+      output: "computed.lt('x', 123)",
+      errors: [{ message: ERROR_MESSAGE_LT, type: 'CallExpression' }]
+    },
+
+    // LTE
+    {
+      code: 'computed(function() { return this.x <= 123; })',
+      output: "computed.lte('x', 123)",
+      errors: [{ message: ERROR_MESSAGE_LTE, type: 'CallExpression' }]
+    },
+
+    // NOT
+    {
+      code: 'computed(function() { return !this.x; })',
+      output: "computed.not('x')",
+      errors: [{ message: ERROR_MESSAGE_NOT, type: 'CallExpression' }]
+    },
+
+    // EQUAL
+    {
+      code: 'computed(function() { return this.x === 123; })',
+      output: "computed.equal('x', 123)",
+      errors: [{ message: ERROR_MESSAGE_EQUAL, type: 'CallExpression' }]
+    },
+  ]
+});