Repository: ember-cli/eslint-plugin-ember (js)

Issue:
BUG: `require-computed-macros` doesn't work with this.get()
REPRO CODE:
```
myComputed: computed('a', 'b', function () {
  return this.get('a') || this.get('b');
}),
```

EXPECTED:
`require-computed-macros` fails with a message saying the function should use `computed.or('a', 'b')`

ACTUAL:
eslint passes

Note: Same issue occurs if you use `return get(this, 'a') || get(this, 'b');` instead

Pull request description:
Update `require-computed-macros` rule to handle `this.get('property')` (in addition to `this.property`)
Fixes #572. CC: @BlueRaja 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 33281703..e52bec63 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,21 +1,3 @@
-## v7.4.0 (2019-11-06)
-
-#### :rocket: Enhancement
-* [#561](https://github.com/ember-cli/eslint-plugin-ember/pull/561) Add `octane` configuration (experimental) ([@patocallaghan](https://github.com/patocallaghan))
-* [#562](https://github.com/ember-cli/eslint-plugin-ember/pull/562) Add new rule `no-test-module-for` ([@NullVoxPopuli](https://github.com/NullVoxPopuli))
-* [#565](https://github.com/ember-cli/eslint-plugin-ember/pull/565) Add `ignoreNestedPaths` option (default true) to `no-get` rule ([@bmish](https://github.com/bmish))
-* [#564](https://github.com/ember-cli/eslint-plugin-ember/pull/564) Update `no-new-mixins` rule to handle native classes ([@bmish](https://github.com/bmish))
-
-#### :house: Internal
-* [#570](https://github.com/ember-cli/eslint-plugin-ember/pull/570) Simplify some tests by setting `parserOptions` globally instead of in each individual test case ([@bmish](https://github.com/bmish))
-* [#568](https://github.com/ember-cli/eslint-plugin-ember/pull/568) Add tests to ensure plugin exports correct configurations ([@bmish](https://github.com/bmish))
-* [#563](https://github.com/ember-cli/eslint-plugin-ember/pull/563) Lint against unnecessary template literals internally ([@bmish](https://github.com/bmish))
-
-#### Committers: 3
-- Bryan Mishkin ([@bmish](https://github.com/bmish))
-- L. Preston Sego III ([@NullVoxPopuli](https://github.com/NullVoxPopuli))
-- Pat O'Callaghan ([@patocallaghan](https://github.com/patocallaghan))
-
 ## v7.3.0 (2019-10-30)
 
 #### :rocket: Enhancement
diff --git a/lib/rules/avoid-leaking-state-in-ember-objects.js b/lib/rules/avoid-leaking-state-in-ember-objects.js
index e72ed955..c4e8aa4d 100644
--- a/lib/rules/avoid-leaking-state-in-ember-objects.js
+++ b/lib/rules/avoid-leaking-state-in-ember-objects.js
@@ -15,15 +15,8 @@ const DEFAULT_IGNORED_PROPERTIES = [
   'attrs',
 ];
 
-function isAllowedTernary(value) {
-  return (
-    types.isConditionalExpression(value) &&
-    isAllowed(value.consequent) &&
-    isAllowed(value.alternate)
-  );
-}
-
-function isAllowed(value) {
+const isAllowed = function(property) {
+  const value = property.value;
   return (
     ember.isFunctionExpression(value) ||
     types.isLiteral(value) ||
@@ -33,10 +26,9 @@ function isAllowed(value) {
     types.isTemplateLiteral(value) ||
     types.isTaggedTemplateExpression(value) ||
     types.isMemberExpression(value) ||
-    types.isUnaryExpression(value) ||
-    isAllowedTernary(value)
+    types.isUnaryExpression(value)
   );
-}
+};
 
 //------------------------------------------------------------------------------
 // Ember object rule - Avoid leaking state
@@ -83,7 +75,7 @@ module.exports = {
         properties
           .filter(property => property.key && ignoredProperties.indexOf(property.key.name) === -1)
           .forEach(property => {
-            if (!isAllowed(property.value)) {
+            if (!isAllowed(property)) {
               report(property);
             }
           });
diff --git a/lib/rules/require-computed-macros.js b/lib/rules/require-computed-macros.js
index 29f7dbd7..df31e387 100644
--- a/lib/rules/require-computed-macros.js
+++ b/lib/rules/require-computed-macros.js
@@ -24,10 +24,10 @@ const ERROR_MESSAGE_EQUAL = makeErrorMessage('equal');
 
 /**
  * Checks if a LogicalExpression looks like `this.x && this.y && this.z`
- * containing only simple `this.x` MemberExpression nodes.
+ * containing only simple `this.x` MemberExpression or `this.get('x')` CallExpression nodes.
  *
  * @param {Node} node The LogicalExpression node to check.
- * @returns {boolean} Whether the node looks like `this.x && this.y && this.z`.
+ * @returns {boolean} Whether the node looks like `this.x && this.y && this.z` or `this.get('x') && this.get('y') && this.get('z')`.
  */
 function isSimpleThisExpressionsInsideLogicalExpression(node, operator) {
   if (!types.isLogicalExpression(node)) {
@@ -54,13 +54,13 @@ function isSimpleThisExpressionsInsideLogicalExpression(node, operator) {
 }
 
 /**
- * Converts a simple LogicalExpression into an array of the MemberExpression nodes in it.
+ * Converts a simple LogicalExpression into an array of the MemberExpression/CallExpression 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.
+ * @param {Node} node The LogicalExpression node containing nodes that look like `this.x` or `this.get('x')`.
+ * @returns {Node[]} An array of MemberExpression/CallExpression nodes contained in the LogicalExpression.
  */
 function getThisExpressions(nodeLogicalExpression) {
   const arrayOfThisExpressions = [];
diff --git a/lib/utils/property-getter.js b/lib/utils/property-getter.js
index ec61e94f..60993823 100644
--- a/lib/utils/property-getter.js
+++ b/lib/utils/property-getter.js
@@ -1,12 +1,39 @@
 const types = require('./types');
 
 /**
- * Checks if a MemberExpression node looks like `this.x` or `this.x.y`.
+ * Checks if a CallExpression node looks like `this.get('property')`.
+ *
+ * @param {Node} node The CallExpression node to check.
+ * @returns {boolean} Whether the node looks like `this.get('property')`.
+ */
+function isThisGetCall(node) {
+  if (
+    types.isCallExpression(node) &&
+    types.isMemberExpression(node.callee) &&
+    types.isThisExpression(node.callee.object) &&
+    types.isIdentifier(node.callee.property) &&
+    node.callee.property.name === 'get' &&
+    node.arguments.length === 1 &&
+    types.isStringLiteral(node.arguments[0])
+  ) {
+    // Looks like: this.get('property')
+    return true;
+  }
+
+  return false;
+}
+
+/**
+ * Checks if a MemberExpression node looks like `this.x` or `this.x.y` or `this.get('x')`.
  *
  * @param {Node} node The MemberExpression node to check.
- * @returns {boolean} Whether the node looks like `this.x` or `this.x.y`.
+ * @returns {boolean} Whether the node looks like `this.x` or `this.x.y` or `this.get('x')`.
  */
 function isSimpleThisExpression(node) {
+  if (isThisGetCall(node)) {
+    return true;
+  }
+
   if (!types.isMemberExpression(node)) {
     return false;
   }
@@ -31,18 +58,24 @@ function isSimpleThisExpression(node) {
 /**
  * Converts a Node containing a ThisExpression to its dependent key.
  *
- * Example Input:   A Node with this source code: `this.x.y`
+ * Example Input:   A Node with this source code: `this.x.y` or `this.get('x')`
  * Example Output:  'x.y'
  *
- * @param {Node} node a MemberExpression node that looks like `this.x` or `this.x.y`.
+ * @param {Node} node a MemberExpression node that looks like `this.x.y` or `this.get('x')`.
  * @returns {String} The dependent key of the input node (without `this.`).
  */
 function nodeToDependentKey(nodeWithThisExpression, context) {
+  if (types.isCallExpression(nodeWithThisExpression)) {
+    // Looks like: this.get('property')
+    return nodeWithThisExpression.arguments[0].value;
+  }
+
   const sourceCode = context.getSourceCode();
   return sourceCode.getText(nodeWithThisExpression).replace(/^this\./, '');
 }
 
 module.exports = {
-  isSimpleThisExpression,
   nodeToDependentKey,
+  isSimpleThisExpression,
+  isThisGetCall,
 };
diff --git a/package.json b/package.json
index c16c71d6..f177594a 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-ember",
-  "version": "7.4.0",
+  "version": "7.3.0",
   "description": "Eslint plugin for Ember.js apps",
   "main": "lib/index.js",
   "files": [


Test patch:
diff --git a/tests/lib/rules/avoid-leaking-state-in-ember-objects.js b/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
index 6afa0b58..e87730d0 100644
--- a/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
+++ b/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
@@ -63,7 +63,6 @@ eslintTester.run('avoid-leaking-state-in-ember-objects', rule, {
     'export default Foo.extend({ foo: abc.something() });',
     'export default Foo.extend({ foo: !true });',
     'export default Foo.extend({ ...props });',
-    'export default Foo.extend({ foo: condition ? "foo" : "bar" });',
   ],
   invalid: [
     {
@@ -167,25 +166,5 @@ eslintTester.run('avoid-leaking-state-in-ember-objects', rule, {
         },
       ],
     },
-    {
-      code: 'export default Foo.extend({ foo: condition ? {} : false });',
-      output: null,
-      errors: [
-        {
-          message:
-            'Only string, number, symbol, boolean, null, undefined, and function are allowed as default properties',
-        },
-      ],
-    },
-    {
-      code: 'export default Foo.extend({ foo: condition ? false : {} });',
-      output: null,
-      errors: [
-        {
-          message:
-            'Only string, number, symbol, boolean, null, undefined, and function are allowed as default properties',
-        },
-      ],
-    },
   ],
 });
diff --git a/tests/lib/rules/require-computed-macros.js b/tests/lib/rules/require-computed-macros.js
index 2bdaed3d..1186bd2a 100644
--- a/tests/lib/rules/require-computed-macros.js
+++ b/tests/lib/rules/require-computed-macros.js
@@ -37,6 +37,7 @@ ruleTester.run('require-computed-macros', rule, {
     "reads('x')",
     'computed(function() { return this; })',
     'computed(function() { return SOME_VAR; })',
+    'computed(function() { return this.get(SOME_VAR); })',
     'computed(function() { return this.prop[123]; })',
     'computed(function() { return this.prop[i]; })',
     'computed(function() { return this.someFunction(); })',
@@ -46,6 +47,7 @@ ruleTester.run('require-computed-macros', rule, {
     "and('x', 'y')",
     'computed(function() { return SOME_VAR && OTHER_VAR; })',
     'computed(function() { return this.x && this.y || this.z; })', // Mixed operators.
+    'computed(function() { return this.get("x") && this.get("y") || this.get("z"); })', // Mixed operators (and this.get)
 
     // OR
     "or('x', 'y')",
@@ -76,6 +78,7 @@ ruleTester.run('require-computed-macros', rule, {
     'computed(function() { return SOME_VAR === 123; })',
     'computed(function() { return SOME_VAR === "Hello"; })',
     'computed(function() { return this.prop === MY_VAR; })',
+    "computed(function() { return this.get('prop') === MY_VAR; })",
   ],
   invalid: [
     // READS
@@ -89,6 +92,11 @@ ruleTester.run('require-computed-macros', rule, {
       output: "computed.reads('x.y')",
       errors: [{ message: ERROR_MESSAGE_READS, type: 'CallExpression' }],
     },
+    {
+      code: "computed(function() { return this.get('x.y'); })", // this.get()
+      output: "computed.reads('x.y')",
+      errors: [{ message: ERROR_MESSAGE_READS, type: 'CallExpression' }],
+    },
 
     // AND
     {
@@ -106,6 +114,11 @@ ruleTester.run('require-computed-macros', rule, {
       output: "computed.and('x', 'y.z', 'w')",
       errors: [{ message: ERROR_MESSAGE_AND, type: 'CallExpression' }],
     },
+    {
+      code: "computed(function() { return this.get('x') && this.get('y.z') && this.w; })", // Three parts with a nested path (and this.get).
+      output: "computed.and('x', 'y.z', 'w')",
+      errors: [{ message: ERROR_MESSAGE_AND, type: 'CallExpression' }],
+    },
 
     // OR
     {
@@ -155,5 +168,10 @@ ruleTester.run('require-computed-macros', rule, {
       output: "computed.equal('x', 123)",
       errors: [{ message: ERROR_MESSAGE_EQUAL, type: 'CallExpression' }],
     },
+    {
+      code: "computed(function() { return this.get('x') === 123; })", // this.get()
+      output: "computed.equal('x', 123)",
+      errors: [{ message: ERROR_MESSAGE_EQUAL, type: 'CallExpression' }],
+    },
   ],
 });
diff --git a/tests/lib/utils/property-getter-test.js b/tests/lib/utils/property-getter-test.js
index 705a1868..76b96ac7 100644
--- a/tests/lib/utils/property-getter-test.js
+++ b/tests/lib/utils/property-getter-test.js
@@ -9,14 +9,40 @@ function parse(code) {
 
 describe('isSimpleThisExpression', () => {
   it('behaves correctly', () => {
-    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x'))).toBeTruthy();
-    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x.y'))).toBeTruthy();
+    // False:
     expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x().y'))).toBeFalsy();
     expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x()'))).toBeFalsy();
     expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x.y()'))).toBeFalsy();
-    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.get("property")'))).toBeFalsy();
     expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x[1]'))).toBeFalsy();
     expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x[i]'))).toBeFalsy();
     expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x.y[i]'))).toBeFalsy();
+
+    // True:
+    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x'))).toBeTruthy();
+    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x.y'))).toBeTruthy();
+    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.get("property")'))).toBeTruthy();
+    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.get("x.y")'))).toBeTruthy();
+  });
+});
+
+describe('isThisGetCall', () => {
+  it('behaves correctly', () => {
+    // False:
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x.y'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x().y'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x()'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x.y()'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x[1]'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x[i]'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.x.y[i]'))).toBeFalsy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.get("property").something'))).toBeFalsy();
+    expect(
+      propertyGetterUtils.isThisGetCall(parse('this.get("unexpected", "argument").something'))
+    ).toBeFalsy();
+
+    // True:
+    expect(propertyGetterUtils.isThisGetCall(parse('this.get("property")'))).toBeTruthy();
+    expect(propertyGetterUtils.isThisGetCall(parse('this.get("x.y")'))).toBeTruthy();
   });
 });