Repository: ember-cli/eslint-plugin-ember
Pull request #580 (2019-11-09 19:56:40)

Update `no-get` rule to ignore `get()` usages inside proxy objects
`get()` is still needed inside proxy objects.

This fix will help avoid false positives and enable future autofixing (#577).

More info:
* https://github.com/emberjs/rfcs/blob/master/text/0281-es5-getters.md
* https://github.com/emberjs/ember.js/issues/16148


Patch:
diff --git a/docs/rules/no-get.md b/docs/rules/no-get.md
index 56b4e49d..5847b09b 100644
--- a/docs/rules/no-get.md
+++ b/docs/rules/no-get.md
@@ -55,6 +55,15 @@ const { prop1, prop2 } = this;
 const foo = { prop1: this.prop1, prop2: this.prop2 };
 ```
 
+```js
+import ObjectProxy from '@ember/object/proxy';
+export default ObjectProxy.extend({
+  someFunction() {
+    const foo = this.get('propertyInsideProxyObject'); // Allowed because inside proxy object.
+  }
+});
+```
+
 ## Configuration
 
 This rule takes an optional object containing:
diff --git a/lib/rules/no-get.js b/lib/rules/no-get.js
index c180374e..83a4ae7e 100644
--- a/lib/rules/no-get.js
+++ b/lib/rules/no-get.js
@@ -1,6 +1,7 @@
 'use strict';
 
 const types = require('../utils/types');
+const emberUtils = require('../utils/ember');
 
 function makeErrorMessageForGet(property, isImportedGet) {
   return isImportedGet
@@ -43,9 +44,36 @@ module.exports = {
     const ignoreGetProperties = context.options[0] && context.options[0].ignoreGetProperties;
     const ignoreNestedPaths = !context.options[0] || context.options[0].ignoreNestedPaths;
 
+    let currentProxyObject = null;
+
     return {
+      'CallExpression:exit'(node) {
+        if (currentProxyObject === node) {
+          currentProxyObject = null;
+        }
+      },
+
+      'ClassDeclaration:exit'(node) {
+        if (currentProxyObject === node) {
+          currentProxyObject = null;
+        }
+      },
+
+      ClassDeclaration(node) {
+        if (emberUtils.isEmberProxy(context, node)) {
+          currentProxyObject = node; // Keep track of being inside a proxy object.
+        }
+      },
+
       // eslint-disable-next-line complexity
       CallExpression(node) {
+        if (emberUtils.isEmberProxy(context, node)) {
+          currentProxyObject = node; // Keep track of being inside a proxy object.
+        }
+        if (currentProxyObject) {
+          return; // Proxy objects still require using `get()` so ignore any code inside them.
+        }
+
         // **************************
         // get
         // **************************
diff --git a/lib/utils/ember.js b/lib/utils/ember.js
index 4c3b8db5..2eef67a1 100644
--- a/lib/utils/ember.js
+++ b/lib/utils/ember.js
@@ -16,6 +16,9 @@ module.exports = {
   isEmberMixin,
   isEmberRoute,
   isEmberService,
+  isEmberArrayProxy,
+  isEmberObjectProxy,
+  isEmberProxy,
 
   isSingleLineFn,
   isMultiLineFn,
@@ -59,6 +62,8 @@ const CORE_MODULE_IMPORT_PATHS = {
   Mixin: '@ember/object/mixin',
   Route: '@ember/routing/route',
   Service: '@ember/service',
+  ArrayProxy: '@ember/array/proxy',
+  ObjectProxy: '@ember/object/proxy',
 };
 
 function isClassicEmberCoreModule(node, module, filePath) {
@@ -201,6 +206,18 @@ function isEmberService(context, node) {
   return isEmberCoreModule(context, node, 'Service');
 }
 
+function isEmberArrayProxy(context, node) {
+  return isEmberCoreModule(context, node, 'ArrayProxy');
+}
+
+function isEmberObjectProxy(context, node) {
+  return isEmberCoreModule(context, node, 'ObjectProxy');
+}
+
+function isEmberProxy(context, node) {
+  return isEmberArrayProxy(context, node) || isEmberObjectProxy(context, node);
+}
+
 function isInjectedServiceProp(node) {
   return isPropOfType(node, 'service') || isPropOfType(node, 'inject');
 }


Test patch:
diff --git a/tests/lib/rules/no-get.js b/tests/lib/rules/no-get.js
index 4655f52b..79509ce5 100644
--- a/tests/lib/rules/no-get.js
+++ b/tests/lib/rules/no-get.js
@@ -100,6 +100,35 @@ ruleTester.run('no-get', rule, {
       code: "getProperties(this, ['prop1', 'prop2']);", // With parameters in array.
       options: [{ ignoreGetProperties: true }],
     },
+
+    // Ignores code inside proxy objects (which still require using `get()`):
+    `
+    import ObjectProxy from '@ember/object/proxy';
+    export default ObjectProxy.extend({
+      someFunction() {
+        test();
+        console.log(this.get('propertyInsideProxyObject'));
+      }
+    });
+    `,
+    `
+    import ArrayProxy from '@ember/array/proxy';
+    export default ArrayProxy.extend({
+      someFunction() {
+        test();
+        console.log(this.get('propertyInsideProxyObject'));
+      }
+    });
+    `,
+    `
+    import ArrayProxy from '@ember/array/proxy';
+    class MyProxy extends ArrayProxy {
+      someFunction() {
+        test();
+        console.log(this.get('propertyInsideProxyObject'));
+      }
+    }
+    `,
   ],
   invalid: [
     // **************************
@@ -172,5 +201,36 @@ ruleTester.run('no-get', rule, {
       options: [{ ignoreNestedPaths: false }],
       errors: [{ message: ERROR_MESSAGE_GET_PROPERTIES, type: 'CallExpression' }],
     },
+
+    {
+      // Reports violation after (classic) proxy class.
+      code: `
+      import ArrayProxy from '@ember/array/proxy';
+      export default ArrayProxy.extend({
+        someFunction() {
+          test();
+          console.log(this.get('propertyInsideProxyObject'));
+        }
+      });
+      this.get('propertyOutsideClass');
+      `,
+      output: null,
+      errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
+    },
+    {
+      // Reports violation after (native) proxy class.
+      code: `
+      import ArrayProxy from '@ember/array/proxy';
+      class MyProxy extends ArrayProxy {
+        someFunction() {
+          test();
+          console.log(this.get('propertyInsideProxyObject'));
+        }
+      }
+      this.get('propertyOutsideClass');
+      `,
+      output: null,
+      errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
+    },
   ],
 });
diff --git a/tests/lib/utils/ember-test.js b/tests/lib/utils/ember-test.js
index 287fe266..02e36e4c 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -427,6 +427,138 @@ describe('isEmberService', () => {
   });
 });
 
+describe('isEmberArrayProxy', () => {
+  it('should detect using old module style', () => {
+    const context = new FauxContext('Ember.ArrayProxy.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberArrayProxy(context, node)).toBeTruthy();
+  });
+
+  it('should not detect using old module style with wrong name', () => {
+    const context = new FauxContext('Ember.SomethingElse.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberArrayProxy(context, node)).toBeFalsy();
+  });
+
+  it('should detect when using local module', () => {
+    const context = new FauxContext('ArrayProxy.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberArrayProxy(context, node)).toBeTruthy();
+  });
+
+  it('should not detect when using local module with wrong name', () => {
+    const context = new FauxContext('SomethingElse.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberArrayProxy(context, node)).toBeFalsy();
+  });
+
+  it('should detect when using native classes', () => {
+    const context = new FauxContext(`
+      import ArrayProxy from '@ember/array/proxy';
+      class MyProxy extends ArrayProxy {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberArrayProxy(context, node)).toBeTruthy();
+  });
+
+  it('should detect when using native classes with other name but correct import path', () => {
+    const context = new FauxContext(`
+      import OtherName from '@ember/array/proxy';
+      class MyProxy extends OtherName {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberArrayProxy(context, node)).toBeTruthy();
+  });
+
+  it('should not detect when using native classes if the import path is incorrect', () => {
+    const context = new FauxContext(`
+      import ArrayProxy from '@something-else/service';
+      class MyProxy extends ArrayProxy {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberArrayProxy(context, node)).toBeFalsy();
+  });
+});
+
+describe('isEmberObjectProxy', () => {
+  it('should detect using old module style', () => {
+    const context = new FauxContext('Ember.ObjectProxy.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberObjectProxy(context, node)).toBeTruthy();
+  });
+
+  it('should not detect using old module style with wrong name', () => {
+    const context = new FauxContext('Ember.SomethingElse.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberObjectProxy(context, node)).toBeFalsy();
+  });
+
+  it('should detect when using local module', () => {
+    const context = new FauxContext('ObjectProxy.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberObjectProxy(context, node)).toBeTruthy();
+  });
+
+  it('should not detect when using local module with wrong name', () => {
+    const context = new FauxContext('SomethingElse.extend()');
+    const node = context.ast.body[0].expression;
+    expect(emberUtils.isEmberObjectProxy(context, node)).toBeFalsy();
+  });
+
+  it('should detect when using native classes', () => {
+    const context = new FauxContext(`
+      import ObjectProxy from '@ember/object/proxy';
+      class MyProxy extends ObjectProxy {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberObjectProxy(context, node)).toBeTruthy();
+  });
+
+  it('should detect when using native classes with other name but correct import path', () => {
+    const context = new FauxContext(`
+      import OtherName from '@ember/object/proxy';
+      class MyProxy extends OtherName {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberObjectProxy(context, node)).toBeTruthy();
+  });
+
+  it('should not detect when using native classes if the import path is incorrect', () => {
+    const context = new FauxContext(`
+      import ObjectProxy from '@something-else/service';
+      class MyProxy extends ObjectProxy {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberObjectProxy(context, node)).toBeFalsy();
+  });
+});
+
+describe('isEmberProxy', () => {
+  it('should detect ArrayProxy example', () => {
+    const context = new FauxContext(`
+      import ArrayProxy from '@ember/array/proxy';
+      class MyProxy extends ArrayProxy {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberProxy(context, node)).toBeTruthy();
+  });
+
+  it('should detect ObjectProxy example', () => {
+    const context = new FauxContext(`
+      import ObjectProxy from '@ember/object/proxy';
+      class MyProxy extends ObjectProxy {}
+    `);
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberProxy(context, node)).toBeTruthy();
+  });
+
+  it('should not detect random code', () => {
+    const context = new FauxContext('const x = 123;');
+    const node = context.ast.body[1];
+    expect(emberUtils.isEmberProxy(context, node)).toBeFalsy();
+  });
+});
+
 describe('isInjectedServiceProp', () => {
   describe('classic classes', () => {
     it("should check if it's an injected service prop with renamed import", () => {