Repository: ember-cli/eslint-plugin-ember
Pull request #581 (2019-11-09 21:03:30)

Update `no-get` rule to ignore `get()` usages inside objects implementing `unknownProperty()`
`get()` is still needed inside objects implementing `unknownProperty()`.

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


Patch:
diff --git a/docs/rules/no-get.md b/docs/rules/no-get.md
index 5847b09b..6b23f47a 100644
--- a/docs/rules/no-get.md
+++ b/docs/rules/no-get.md
@@ -9,7 +9,7 @@ This rule disallows:
 * `this.get('someProperty')` when `this.someProperty` can be used
 * `this.getProperties('prop1', 'prop2')` when `{ prop1: this.prop1, prop2: this.prop2 }` can be used
 
-**WARNING**: there are a number of circumstances where `get` / `getProperties` still need to be used, and you may need to manually disable the rule for these:
+**WARNING**: there are a number of circumstances where `get` / `getProperties` still need to be used, and you may need to manually disable the rule for these (although the rule will attempt to ignore them):
 
 * Ember proxy objects (`ObjectProxy`, `ArrayProxy`)
 * Objects implementing the `unknownProperty` method
@@ -64,6 +64,16 @@ export default ObjectProxy.extend({
 });
 ```
 
+```js
+import EmberObject from '@ember/object';
+export default EmberObject.extend({
+  unknownProperty(key) {},
+  someFunction() {
+    const foo = this.get('property'); // Allowed because inside object implementing `unknownProperty()`.
+  }
+});
+```
+
 ## Configuration
 
 This rule takes an optional object containing:
diff --git a/lib/rules/no-get.js b/lib/rules/no-get.js
index 83a4ae7e..b4594932 100644
--- a/lib/rules/no-get.js
+++ b/lib/rules/no-get.js
@@ -45,33 +45,52 @@ module.exports = {
     const ignoreNestedPaths = !context.options[0] || context.options[0].ignoreNestedPaths;
 
     let currentProxyObject = null;
+    let currentClassWithUnknownPropertyMethod = null;
 
     return {
       'CallExpression:exit'(node) {
         if (currentProxyObject === node) {
           currentProxyObject = null;
         }
+        if (currentClassWithUnknownPropertyMethod === node) {
+          currentClassWithUnknownPropertyMethod = null;
+        }
       },
 
       'ClassDeclaration:exit'(node) {
         if (currentProxyObject === node) {
           currentProxyObject = null;
         }
+        if (currentClassWithUnknownPropertyMethod === node) {
+          currentClassWithUnknownPropertyMethod = null;
+        }
       },
 
       ClassDeclaration(node) {
         if (emberUtils.isEmberProxy(context, node)) {
           currentProxyObject = node; // Keep track of being inside a proxy object.
         }
+        if (emberUtils.isEmberObjectImplementingUnknownProperty(node)) {
+          currentClassWithUnknownPropertyMethod = node; // Keep track of being inside an object implementing `unknownProperty`.
+        }
       },
 
       // eslint-disable-next-line complexity
       CallExpression(node) {
+        // **************************
+        // Check for situations which the rule should ignore.
+        // **************************
+
         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.
+        if (emberUtils.isEmberObjectImplementingUnknownProperty(node)) {
+          currentClassWithUnknownPropertyMethod = node;
+        }
+        if (currentProxyObject || currentClassWithUnknownPropertyMethod) {
+          // Proxy objects and objects implementing `unknownProperty()`
+          // still require using `get()`, so ignore any code inside them.
+          return;
         }
 
         // **************************
diff --git a/lib/utils/ember.js b/lib/utils/ember.js
index 2eef67a1..5667302f 100644
--- a/lib/utils/ember.js
+++ b/lib/utils/ember.js
@@ -53,6 +53,8 @@ module.exports = {
 
   isReopenObject,
   isReopenClassObject,
+
+  isEmberObjectImplementingUnknownProperty,
 };
 
 // Private
@@ -501,3 +503,27 @@ function unwrapBraceExpressions(dependentKeys) {
 
   return unwrappedExpressions.reduce((acc, cur) => acc.concat(cur), []);
 }
+
+function isEmberObjectImplementingUnknownProperty(node) {
+  if (types.isCallExpression(node)) {
+    if (!isEmberObject(node) && !isReopenObject(node)) {
+      return false;
+    }
+    // Classic class.
+    const properties = getModuleProperties(node);
+    return properties.some(
+      property => types.isIdentifier(property.key) && property.key.name === 'unknownProperty'
+    );
+  } else if (types.isClassDeclaration(node)) {
+    // Native class.
+    return node.body.body.some(
+      n =>
+        types.isMethodDefinition(n) && types.isIdentifier(n.key) && n.key.name === 'unknownProperty'
+    );
+  } else {
+    assert(
+      'Function should only be called on a `CallExpression` (classic class) or `ClassDeclaration` (native class)'
+    );
+  }
+  return false;
+}


Test patch:
diff --git a/tests/lib/rules/no-get.js b/tests/lib/rules/no-get.js
index 79509ce5..14a9711c 100644
--- a/tests/lib/rules/no-get.js
+++ b/tests/lib/rules/no-get.js
@@ -101,7 +101,7 @@ ruleTester.run('no-get', rule, {
       options: [{ ignoreGetProperties: true }],
     },
 
-    // Ignores code inside proxy objects (which still require using `get()`):
+    // Ignores `get()` inside proxy objects (which still require using `get()`):
     `
     import ObjectProxy from '@ember/object/proxy';
     export default ObjectProxy.extend({
@@ -129,6 +129,26 @@ ruleTester.run('no-get', rule, {
       }
     }
     `,
+
+    // Ignores `get()` inside classes with `unknownProperty`:
+    `
+    import EmberObject from '@ember/object';
+    export default EmberObject.extend({
+      unknownProperty() {},
+      someFunction() {
+        console.log(this.get('propertyInsideClassWithUnknownProperty'));
+      }
+    });
+    `,
+    `
+    import EmberObject from '@ember/object';
+    class MyClass extends EmberObject {
+      unknownProperty() {}
+      someFunction() {
+        console.log(this.get('propertyInsideClassWithUnknownProperty'));
+      }
+    }
+    `,
   ],
   invalid: [
     // **************************
@@ -232,5 +252,36 @@ ruleTester.run('no-get', rule, {
       output: null,
       errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
     },
+
+    {
+      // Reports violation after (classic) class with `unknownProperty()`.
+      code: `
+      import EmberObject from '@ember/object';
+      export default EmberObject.extend({
+        unknownProperty() {},
+        someFunction() {
+          console.log(this.get('propertyInsideClassWithUnknownProperty'));
+        }
+      });
+      this.get('propertyOutsideClass');
+      `,
+      output: null,
+      errors: [{ message: makeErrorMessageForGet('propertyOutsideClass'), type: 'CallExpression' }],
+    },
+    {
+      // Reports violation after (native) class with `unknownProperty()`.
+      code: `
+      import EmberObject from '@ember/object';
+      class MyClass extends EmberObject {
+        unknownProperty() {}
+        someFunction() {
+          console.log(this.get('propertyInsideClassWithUnknownProperty'));
+        }
+      }
+      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 02e36e4c..497af93c 100644
--- a/tests/lib/utils/ember-test.js
+++ b/tests/lib/utils/ember-test.js
@@ -1134,3 +1134,29 @@ describe('getEmberImportAliasName', () => {
     expect(emberUtils.getEmberImportAliasName(node)).toEqual('foo');
   });
 });
+
+describe('isEmberObjectImplementingUnknownProperty', () => {
+  it('should be true for a classic class EmberObject with `unknownProperty`', () => {
+    const node = babelEslint.parse('EmberObject.extend({ unknownProperty() {} });').body[0]
+      .expression;
+    expect(emberUtils.isEmberObjectImplementingUnknownProperty(node)).toBeTruthy();
+  });
+
+  it('should be false for a classic class EmberObject without `unknownProperty`', () => {
+    const node = babelEslint.parse('EmberObject.extend({ somethingElse() {} });').body[0]
+      .expression;
+    expect(emberUtils.isEmberObjectImplementingUnknownProperty(node)).toBeFalsy();
+  });
+
+  it('should be true for a native class EmberObject with `unknownProperty`', () => {
+    const node = babelEslint.parse('class MyClass extends EmberObject { unknownProperty() {} }')
+      .body[0];
+    expect(emberUtils.isEmberObjectImplementingUnknownProperty(node)).toBeTruthy();
+  });
+
+  it('should be false for a native class EmberObject without `unknownProperty`', () => {
+    const node = babelEslint.parse('class MyClass extends EmberObject { somethingElse() {} }')
+      .body[0];
+    expect(emberUtils.isEmberObjectImplementingUnknownProperty(node)).toBeFalsy();
+  });
+});