Repository: ember-cli/eslint-plugin-ember
Pull request #99 (2017-07-14 19:01:46)

Adding new rule: no-attrs-snapshot
@michalsnik 

Addresses #51 

This adds the first rule from [eslint-plugin-ember-best-practices](https://github.com/ember-best-practices/eslint-plugin-ember-best-practices). The structure has been adapted to adhere to the existing structure of this repo's rules, with one exception:

The message has been added to the `meta` of the rule. This allows the message to be reused during `invalid` test verifications. 

scalvert: @michalsnik can you take another look?

@chadhietala @rwjblue @twillis

Patch:
diff --git a/README.md b/README.md
index 272895d2..f3a5fe0c 100644
--- a/README.md
+++ b/README.md
@@ -2,7 +2,7 @@
 
 [![NPM version](https://img.shields.io/npm/v/eslint-plugin-ember.svg?style=flat)](https://npmjs.org/package/eslint-plugin-ember)
 [![NPM downloads](https://img.shields.io/npm/dm/eslint-plugin-ember.svg?style=flat)](https://npmjs.org/package/eslint-plugin-ember)
-[![Build Status](https://travis-ci.org/netguru/eslint-plugin-ember.svg?branch=master)](https://travis-ci.org/netguru/eslint-plugin-ember)
+[![Build Status](https://travis-ci.org/ember-cli/eslint-plugin-ember.svg?branch=master)](https://travis-ci.org/ember-cli/eslint-plugin-ember)
 
 > An ESlint plugin that provides set of rules for Ember Applications based on commonly known good practices.
 
@@ -37,8 +37,8 @@ module.exports = {
 ```
 
 Possible configurations:
-- [plugin:ember/base](https://github.com/netguru/eslint-plugin-ember/blob/master/config/base.js) - contains only recommended settings for custom rules defined in this plugin.
-- [plugin:ember/recommended](https://github.com/netguru/eslint-plugin-ember/blob/master/config/recommended.js) - extends base configuration with extra rules' settings provided by eslint
+- [plugin:ember/base](https://github.com/ember-cli/eslint-plugin-ember/blob/master/config/base.js) - contains only recommended settings for custom rules defined in this plugin.
+- [plugin:ember/recommended](https://github.com/ember-cli/eslint-plugin-ember/blob/master/config/recommended.js) - extends base configuration with extra rules' settings provided by eslint
 
 #### Use plain plugin:
 
@@ -144,7 +144,7 @@ In order to add a new rule, you should:
 
 Please be aware that we're using `yarn` in this repository, so if you plan to add some dependencies - make sure you commit `yarn.lock` file too.
 
-If you have any suggestions, ideas or problems feel free to add new [issue](https://github.com/netguru/eslint-plugin-ember/issues), but first please make sure your question does not repeat previous ones.
+If you have any suggestions, ideas or problems feel free to add new [issue](https://github.com/ember-cli/eslint-plugin-ember/issues), but first please make sure your question does not repeat previous ones.
 
 ## ⭐️ Contributors
 - [Michał Sajnóg](http://github.com/michalsnik)
diff --git a/docs/rules/no-attrs-snapshot.md b/docs/rules/no-attrs-snapshot.md
new file mode 100644
index 00000000..8ccd07cf
--- /dev/null
+++ b/docs/rules/no-attrs-snapshot.md
@@ -0,0 +1,98 @@
+# Disallow use of attrs snapshot in `didReceiveAttrs` and `didUpdateAttrs` (no-attrs-snapshot)
+
+Do not use the arguments (attrs) that are passed in `didReceiveAttrs` and `didUpdateAttrs`. Using the arguments (attrs) in these hooks can result in performance degredation in your application.
+
+```js
+
+// Bad
+export default Ember.Component({
+  init() {
+    this._super(...arguments);
+    this.updated = false;
+  },
+  didReceiveAttrs(attrs) {
+    let { newAttrs, oldAttrs } = attrs;
+    if ((newAttrs && oldAttrs) && newAttrs.value !== oldAttrs.value) {
+      this.set('updated', true);
+    } else {
+      this.set('updated', false);
+    }
+  }
+});
+
+// Good
+export default Ember.Component({
+  init() {
+    this._super(...arguments);
+    this._valueCache = this.value;
+    this.updated = false;
+  },
+  didReceiveAttrs() {
+    if (this._valueCache !== this.value) {
+      this._valueCache = this.value;
+      this.set('updated', true);
+    } else {
+      this.set('updated', false);
+    }
+  }
+});
+
+```
+
+## Rule Details
+
+In 2.0.0, `didReceiveAttrs` and `didUpdateAttrs` hooks were introduced. These hooks are called whenever the references of arguments to a component change. These hooks receive arguments, however one should not use them as they force those objects to reify, which can be very costly when you have a lot of components on the page. These arguments are also purposely undocumented. 
+
+If for some reason you need to do a comparison of arguments we suggest that you simply keep a cache on the component.
+
+Examples of **incorrect** code for this rule:
+
+```js
+
+export default Ember.Component({
+  init() {
+    this._super(...arguments);
+    this.updated = false;
+  },
+  didReceiveAttrs(attrs) {
+    let { newAttrs, oldAttrs } = attrs;
+    if ((newAttrs && oldAttrs) && newAttrs.value !== oldAttrs.value) {
+      this.set('updated', true);
+    } else {
+      this.set('updated', false);
+    }
+  }
+});
+
+```
+
+Examples of **correct** code for this rule:
+
+```js
+
+export default Ember.Component({
+  init() {
+    this._super(...arguments);
+    this._valueCache = this.value;
+    this.updated = false;
+  },
+  didReceiveAttrs() {
+    if (this._valueCache !== this.value) {
+      this._valueCache = this.value;
+      this.set('updated', true);
+    } else {
+      this.set('updated', false);
+    }
+  }
+});
+
+```
+
+## When Not To Use It
+
+You can turn this rule of if you aren't worried about the negative performance impacts of using params in these hooks.
+
+## Further Reading
+
+- [`didReceiveAttrs`](https://guides.emberjs.com/v2.9.0/components/the-component-lifecycle/#toc_formatting-component-attributes-with-code-didreceiveattrs-code)
+- [`didUpdateAttrs`](https://guides.emberjs.com/v2.9.0/components/the-component-lifecycle/#toc_resetting-presentation-state-on-attribute-change-with-code-didupdateattrs-code)
diff --git a/lib/rules/no-attrs-snapshot.js b/lib/rules/no-attrs-snapshot.js
new file mode 100644
index 00000000..9af02dbd
--- /dev/null
+++ b/lib/rules/no-attrs-snapshot.js
@@ -0,0 +1,42 @@
+'use strict';
+
+const utils = require('../utils/utils');
+
+//------------------------------------------------------------------------------
+// General rule -  Disallow use of attrs snapshot in `didReceiveAttrs`
+// and `didUpdateAttrs`.
+//------------------------------------------------------------------------------
+
+const message = 'Do not use the attrs snapshot that is passed in `didReceiveAttrs` and `didUpdateAttrs`. Please see the following guide for more information: https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-attrs-snapshot.md';
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Disallow use of attrs snapshot in `didReceiveAttrs` and `didUpdateAttrs`',
+      category: 'Components',
+      recommended: true
+    },
+    fixable: null, // or "code" or "whitespace"
+    message
+  },
+  create(context) {
+    const hasAttrsSnapShot = function (node) {
+      const methodName = utils.getPropertyValue(node, 'parent.key.name');
+      const hasParams = node.params.length > 0;
+
+      return (methodName === 'didReceiveAttrs' || methodName === 'didUpdateAttrs') && hasParams;
+    };
+
+    const report = function (node) {
+      context.report(node, message);
+    };
+
+    return {
+      FunctionExpression(node) {
+        if (hasAttrsSnapShot(node)) {
+          report(node);
+        }
+      }
+    };
+  }
+};
diff --git a/lib/utils/ember.js b/lib/utils/ember.js
index 2bd96438..ff92cd91 100644
--- a/lib/utils/ember.js
+++ b/lib/utils/ember.js
@@ -129,6 +129,12 @@ function isObserverProp(node) {
 }
 
 function isComputedProp(node) {
+  const allowedMemberExpNames = ['volatile', 'readOnly', 'property', 'meta'];
+  if (utils.isMemberExpression(node.callee) && utils.isCallExpression(node.callee.object)) {
+    return isModule(node.callee.object, 'computed') &&
+      utils.isIdentifier(node.callee.property) &&
+      allowedMemberExpNames.indexOf(node.callee.property.name) > -1;
+  }
   return isModule(node, 'computed');
 }
 
diff --git a/lib/utils/utils.js b/lib/utils/utils.js
index aecb87cb..bc82b514 100644
--- a/lib/utils/utils.js
+++ b/lib/utils/utils.js
@@ -21,6 +21,7 @@ module.exports = {
   parseCallee,
   parseArgs,
   findUnorderedProperty,
+  getPropertyValue,
 };
 
 /**
@@ -270,3 +271,31 @@ function findUnorderedProperty(arr) {
 
   return null;
 }
+
+/**
+ * Gets a property's value either by property path.
+ *
+ * @example
+ * getPropertyValue('name');
+ * getPropertyValue('parent.key.name');
+ *
+ * @param {Object} node
+ * @param {String} path
+ * @returns
+ */
+function getPropertyValue(node, path) {
+  const parts = typeof path === 'string' ? path.split('.') : path;
+
+  if (parts.length === 1) {
+    return node[path];
+  }
+
+  const property = node[parts[0]];
+
+  if (property && parts.length > 1) {
+    parts.shift();
+    return getPropertyValue(property, parts);
+  }
+
+  return property;
+}


Test patch:
diff --git a/tests/lib/rules/no-attrs-snapshot.js b/tests/lib/rules/no-attrs-snapshot.js
new file mode 100644
index 00000000..d945f936
--- /dev/null
+++ b/tests/lib/rules/no-attrs-snapshot.js
@@ -0,0 +1,92 @@
+const rule = require('../../../lib/rules/no-attrs-snapshot');
+const RuleTester = require('eslint').RuleTester;
+
+const message = rule.meta.message;
+const eslintTester = new RuleTester();
+
+eslintTester.run('no-attrs-snapshot', rule, {
+  valid: [
+    {
+      code: `
+        export default Ember.Component({
+          init() {
+            this._super(...arguments);
+            this._valueCache = this.value;
+            this.updated = false;
+          },
+          didReceiveAttrs() {
+            if (this._valueCache !== this.value) {
+              this._valueCache = this.value;
+              this.set('updated', true);
+            } else {
+              this.set('updated', false);
+            }
+          }
+        });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+    },
+    {
+      code: `
+        export default Ember.Component({
+          init() {
+            this._super(...arguments);
+            this._valueCache = this.value;
+            this.updated = false;
+          },
+          didUpdateAttrs() {
+            if (this._valueCache !== this.value) {
+              this._valueCache = this.value;
+              this.set('updated', true);
+            } else {
+              this.set('updated', false);
+            }
+          }
+        });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+    }
+  ],
+  invalid: [
+    {
+      code: `
+        export default Ember.Component({
+          init() {
+            this._super(...arguments);
+            this.updated = false;
+          },
+          didReceiveAttrs(attrs) {
+            let { newAttrs, oldAttrs } = attrs;
+            if ((newAttrs && oldAttrs) && newAttrs.value !== oldAttrs.value) {
+              this.set('updated', true);
+            } else {
+              this.set('updated', false);
+            }
+          }
+        });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message
+      }]
+    },
+    {
+      code: `
+        export default Ember.Component({
+          init() {
+            this._super(...arguments);
+            this.updated = false;
+          },
+          didUpdateAttrs(attrs) {
+            let { newAttrs, oldAttrs } = attrs;
+            if ((newAttrs && oldAttrs) && newAttrs.value !== oldAttrs.value) {
+              this.set('updated', true);
+            } else {
+              this.set('updated', false);
+            }
+          }
+        });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message
+      }]
+    }
+  ]
+});
diff --git a/tests/lib/rules/order-in-components.js b/tests/lib/rules/order-in-components.js
index b8ec9252..b3c63206 100644
--- a/tests/lib/rules/order-in-components.js
+++ b/tests/lib/rules/order-in-components.js
@@ -330,6 +330,15 @@ eslintTester.run('order-in-components', rule, {
       });`,
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
     },
+    {
+      code: `export default Component.extend({
+        foo: computed(function() {
+        }).volatile(),
+        bar: computed(function() {
+        })
+      });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    }
   ],
   invalid: [
     {
@@ -624,5 +633,17 @@ eslintTester.run('order-in-components', rule, {
         line: 5,
       }],
     },
+    {
+      code: `export default Component.extend({
+        foo: computed(function() {
+        }).volatile(),
+        name: "Jon Snow",
+      });`,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'The "name" property should be above the "foo" multi-line function on line 2',
+        line: 4,
+      }],
+    },
   ],
 });
diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js
index b683f5fb..be112554 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -229,6 +229,27 @@ describe('isComputedProp', () => {
     node = parse('Ember.computed()');
     expect(emberUtils.isComputedProp(node)).toBeTruthy();
   });
+
+  it(
+    'should detect whitelisted computed props with MemberExpressions',
+    () => {
+      ['volatile', 'meta', 'readOnly', 'property'].forEach((prop) => {
+        node = parse(`computed().${prop}()`);
+        expect(emberUtils.isComputedProp(node)).toBeTruthy();
+
+        node = parse(`Ember.computed().${prop}()`);
+        expect(emberUtils.isComputedProp(node)).toBeTruthy();
+      });
+    }
+  );
+
+  it('shouldn\'t allow other MemberExpressions', () => {
+    node = parse('computed().foo()');
+    expect(emberUtils.isComputedProp(node)).not.toBeTruthy();
+
+    node = parse('Ember.computed().foo()');
+    expect(emberUtils.isComputedProp(node)).not.toBeTruthy();
+  });
 });
 
 describe('isObserverProp', () => {
diff --git a/tests/lib/utils/utils-test.js b/tests/lib/utils/utils-test.js
index 3855a012..514e657b 100644
--- a/tests/lib/utils/utils-test.js
+++ b/tests/lib/utils/utils-test.js
@@ -186,3 +186,63 @@ describe('parseArgs', () => {
     expect(parsedArgs).toEqual(['asd', 'qwe', 'zxc']);
   });
 });
+
+describe('getPropertyValue', () => {
+  const simpleObject = {
+    foo: true,
+    bar: {
+      baz: 1,
+      fizz: {
+        buzz: 'buzz'
+      }
+    }
+  };
+
+  const node = babelEslint.parse(`
+    export default Ember.Component({
+      init() {
+        this._super(...arguments);
+        this._valueCache = this.value;
+        this.updated = false;
+      },
+      didReceiveAttrs() {
+        if (this._valueCache !== this.value) {
+          this._valueCache = this.value;
+          this.set('updated', true);
+        } else {
+          this.set('updated', false);
+        }
+      }
+    });
+  `).body[0].declaration;
+
+  it('should return null when property value not found for simpleObject', () => {
+    const value = utils.getPropertyValue(simpleObject, 'blah');
+    expect(value).toEqual(undefined);
+  });
+
+  it('should return value when using a simple property path for simpleObject', () => {
+    const value = utils.getPropertyValue(simpleObject, 'foo');
+    expect(value).toEqual(true);
+  });
+
+  it('should return value when using a full property path for simpleObject', () => {
+    const buzz = utils.getPropertyValue(simpleObject, 'bar.fizz.buzz');
+    expect(buzz).toEqual('buzz');
+  });
+
+  it('should return null when property value not found for node', () => {
+    const value = utils.getPropertyValue(node, 'blah');
+    expect(value).toEqual(undefined);
+  });
+
+  it('should return value when using a simple property path for node', () => {
+    const type = utils.getPropertyValue(node, 'type');
+    expect(type).toEqual('CallExpression');
+  });
+
+  it('should return value when using a full property path for node', () => {
+    const name = utils.getPropertyValue(node, 'callee.object.name');
+    expect(name).toEqual('Ember');
+  });
+});