Repository: ember-cli/eslint-plugin-ember
Pull request #575 (2019-11-07 19:46:45)

Update `avoid-leaking-state-in-ember-objects` rule to handle logical expressions
Very similar to #571 

Much like a ternary expression should be valid if either option are valid, a logical expression (`&&` or `||`) should be valid if both options are valid.


Patch:
diff --git a/lib/rules/avoid-leaking-state-in-ember-objects.js b/lib/rules/avoid-leaking-state-in-ember-objects.js
index e72ed955..3743186e 100644
--- a/lib/rules/avoid-leaking-state-in-ember-objects.js
+++ b/lib/rules/avoid-leaking-state-in-ember-objects.js
@@ -23,6 +23,10 @@ function isAllowedTernary(value) {
   );
 }
 
+function isAllowedLogicalExpression(value) {
+  return types.isLogicalExpression(value) && isAllowed(value.left) && isAllowed(value.right);
+}
+
 function isAllowed(value) {
   return (
     ember.isFunctionExpression(value) ||
@@ -34,7 +38,8 @@ function isAllowed(value) {
     types.isTaggedTemplateExpression(value) ||
     types.isMemberExpression(value) ||
     types.isUnaryExpression(value) ||
-    isAllowedTernary(value)
+    isAllowedTernary(value) ||
+    isAllowedLogicalExpression(value)
   );
 }
 
diff --git a/lib/rules/require-computed-macros.js b/lib/rules/require-computed-macros.js
index df31e387..29f7dbd7 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 or `this.get('x')` CallExpression nodes.
+ * 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` or `this.get('x') && this.get('y') && this.get('z')`.
+ * @returns {boolean} Whether the node looks like `this.x && this.y && this.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/CallExpression nodes in it.
+ * 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` or `this.get('x')`.
- * @returns {Node[]} An array of MemberExpression/CallExpression nodes contained in the LogicalExpression.
+ * @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 = [];
diff --git a/lib/utils/property-getter.js b/lib/utils/property-getter.js
index 60993823..ec61e94f 100644
--- a/lib/utils/property-getter.js
+++ b/lib/utils/property-getter.js
@@ -1,39 +1,12 @@
 const types = require('./types');
 
 /**
- * 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')`.
+ * 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` or `this.get('x')`.
+ * @returns {boolean} Whether the node looks like `this.x` or `this.x.y`.
  */
 function isSimpleThisExpression(node) {
-  if (isThisGetCall(node)) {
-    return true;
-  }
-
   if (!types.isMemberExpression(node)) {
     return false;
   }
@@ -58,24 +31,18 @@ function isSimpleThisExpression(node) {
 /**
  * Converts a Node containing a ThisExpression to its dependent key.
  *
- * Example Input:   A Node with this source code: `this.x.y` or `this.get('x')`
+ * 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.y` or `this.get('x')`.
+ * @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, 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 = {
-  nodeToDependentKey,
   isSimpleThisExpression,
-  isThisGetCall,
+  nodeToDependentKey,
 };


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..a80ab28c 100644
--- a/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
+++ b/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
@@ -64,6 +64,8 @@ eslintTester.run('avoid-leaking-state-in-ember-objects', rule, {
     'export default Foo.extend({ foo: !true });',
     'export default Foo.extend({ ...props });',
     'export default Foo.extend({ foo: condition ? "foo" : "bar" });',
+    'export default Foo.extend({ foo: "foo" && "bar" });',
+    'export default Foo.extend({ foo: "foo" || "bar" });',
   ],
   invalid: [
     {
@@ -187,5 +189,45 @@ eslintTester.run('avoid-leaking-state-in-ember-objects', rule, {
         },
       ],
     },
+    {
+      code: 'export default Foo.extend({ foo: 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: {} && 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: 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: {} || 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 1186bd2a..2bdaed3d 100644
--- a/tests/lib/rules/require-computed-macros.js
+++ b/tests/lib/rules/require-computed-macros.js
@@ -37,7 +37,6 @@ 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(); })',
@@ -47,7 +46,6 @@ 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')",
@@ -78,7 +76,6 @@ 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
@@ -92,11 +89,6 @@ 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
     {
@@ -114,11 +106,6 @@ 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
     {
@@ -168,10 +155,5 @@ 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 76b96ac7..705a1868 100644
--- a/tests/lib/utils/property-getter-test.js
+++ b/tests/lib/utils/property-getter-test.js
@@ -9,40 +9,14 @@ function parse(code) {
 
 describe('isSimpleThisExpression', () => {
   it('behaves correctly', () => {
-    // False:
+    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x'))).toBeTruthy();
+    expect(propertyGetterUtils.isSimpleThisExpression(parse('this.x.y'))).toBeTruthy();
     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();
   });
 });