From 290c6e9ce69fa185313d816dd49b11f7264eec9e Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Wed, 9 Oct 2019 09:52:57 -0700 Subject: [PATCH 1/3] no-arrow-function-computed-properties: allow arrow functions when is not present --- .../no-arrow-function-computed-properties.js | 19 +++++++++++++++---- .../no-arrow-function-computed-properties.js | 14 ++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/rules/no-arrow-function-computed-properties.js b/lib/rules/no-arrow-function-computed-properties.js index aaf40283b0..5b8e1f726e 100644 --- a/lib/rules/no-arrow-function-computed-properties.js +++ b/lib/rules/no-arrow-function-computed-properties.js @@ -17,13 +17,24 @@ module.exports = { }, create(context) { + let isThisPresent = false; + return { - CallExpression(node) { - if ( + ThisExpression() { + isThisPresent = true; + }, + CallExpression() { + isThisPresent = false; + }, + 'CallExpression:exit'(node) { + const isComputedArrow = emberUtils.isComputedProp(node) && node.arguments.length > 0 && - types.isArrowFunctionExpression(node.arguments[node.arguments.length - 1]) - ) { + types.isArrowFunctionExpression(node.arguments[node.arguments.length - 1]); + + const isLintApplicable = isComputedArrow && isThisPresent; + + if (isLintApplicable) { context.report(node.arguments[node.arguments.length - 1], ERROR_MESSAGE); } }, diff --git a/tests/lib/rules/no-arrow-function-computed-properties.js b/tests/lib/rules/no-arrow-function-computed-properties.js index e457a1488b..a4f8c10155 100644 --- a/tests/lib/rules/no-arrow-function-computed-properties.js +++ b/tests/lib/rules/no-arrow-function-computed-properties.js @@ -26,13 +26,12 @@ ruleTester.run('no-arrow-function-computed-properties', rule, { "computed.map('products', function(product) { return someFunction(product); })", 'other(() => {})', 'other.computed(() => {})', + 'computed(() => { return 123; })', + 'computed(() => { return "string stuff"; })', + 'computed(() => [])', + "computed.map('products', product => { return someFunction(product); })", ], invalid: [ - { - code: 'computed(() => { return 123; })', - errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], - }, - { code: "computed('prop', () => { return this.prop; })", errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], @@ -42,10 +41,5 @@ ruleTester.run('no-arrow-function-computed-properties', rule, { code: "computed('prop', () => { return this.prop; }).volatile()", errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], }, - - { - code: "computed.map('products', product => { return someFunction(product); })", - errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], - }, ], }); From e9a798fcbd384693fe3e85bfd32f83a823955b1e Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Wed, 9 Oct 2019 14:56:32 -0700 Subject: [PATCH 2/3] add options, keeping the current behavior, but allowing opt-in to the correct behavior ;) --- .../no-arrow-function-computed-properties.js | 13 ++++++- .../no-arrow-function-computed-properties.js | 39 +++++++++++++------ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/lib/rules/no-arrow-function-computed-properties.js b/lib/rules/no-arrow-function-computed-properties.js index 5b8e1f726e..539b31d055 100644 --- a/lib/rules/no-arrow-function-computed-properties.js +++ b/lib/rules/no-arrow-function-computed-properties.js @@ -17,6 +17,9 @@ module.exports = { }, create(context) { + const options = context.options[0] || {}; + const onlyThisContexts = options.onlyThisContexts || false; + let isThisPresent = false; return { @@ -32,9 +35,15 @@ module.exports = { node.arguments.length > 0 && types.isArrowFunctionExpression(node.arguments[node.arguments.length - 1]); - const isLintApplicable = isComputedArrow && isThisPresent; + if (!isComputedArrow) { + return; + } - if (isLintApplicable) { + if (onlyThisContexts) { + if (isThisPresent) { + context.report(node.arguments[node.arguments.length - 1], ERROR_MESSAGE); + } + } else { context.report(node.arguments[node.arguments.length - 1], ERROR_MESSAGE); } }, diff --git a/tests/lib/rules/no-arrow-function-computed-properties.js b/tests/lib/rules/no-arrow-function-computed-properties.js index a4f8c10155..1bb6e77b7c 100644 --- a/tests/lib/rules/no-arrow-function-computed-properties.js +++ b/tests/lib/rules/no-arrow-function-computed-properties.js @@ -19,19 +19,32 @@ const ruleTester = new RuleTester({ }); ruleTester.run('no-arrow-function-computed-properties', rule, { valid: [ - 'computed()', - 'computed(function() { return 123; })', - "computed('prop', function() { return this.prop; })", - "computed('prop', function() { return this.prop; }).volatile()", - "computed.map('products', function(product) { return someFunction(product); })", - 'other(() => {})', - 'other.computed(() => {})', - 'computed(() => { return 123; })', - 'computed(() => { return "string stuff"; })', - 'computed(() => [])', - "computed.map('products', product => { return someFunction(product); })", + { + code: ` + computed('prop', function() { return this.prop; }); + computed('prop', function() { return this.prop; }).volatile(); + computed(() => { return 123; }); + computed(() => { return "string stuff"; }); + computed(() => []); + computed.map('products', product => { return someFunction(product); }); + `, + options: [{ onlyThisContexts: true }], + }, + ` + computed(); + computed(function() { return 123; }); + computed('prop', function() { return this.prop; }); + computed('prop', function() { return this.prop; }).volatile(); + computed.map('products', function(product) { return someFunction(product); }); + other(() => {}); + other.computed(() => {}); + `, ], invalid: [ + { + code: 'computed(() => { return 123; })', + errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], + }, { code: "computed('prop', () => { return this.prop; })", errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], @@ -41,5 +54,9 @@ ruleTester.run('no-arrow-function-computed-properties', rule, { code: "computed('prop', () => { return this.prop; }).volatile()", errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], }, + { + code: "computed.map('products', product => { return someFunction(product); })", + errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], + }, ], }); From cdff5032e11e19c700c26505cc6d50aa797fa259 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli Date: Thu, 10 Oct 2019 05:28:07 -0700 Subject: [PATCH 3/3] split out tests, add configuration documentation and add invalid scenarios --- .../no-arrow-function-computed-properties.md | 6 ++ .../no-arrow-function-computed-properties.js | 64 +++++++++++++------ 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/docs/rules/no-arrow-function-computed-properties.md b/docs/rules/no-arrow-function-computed-properties.md index 492854189f..b7851ec3c6 100644 --- a/docs/rules/no-arrow-function-computed-properties.md +++ b/docs/rules/no-arrow-function-computed-properties.md @@ -32,6 +32,12 @@ const Person = EmberObject.extend({ }); ``` +## Configuration + +This rule takes an optional object containing: + +* `boolean` -- `onlyThisContexts` -- whether the rule should allow or disallow computed properties where the arrow function body does not contain a `this` reference (default: `false`) + ## References * [Arrow function spec](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions) diff --git a/tests/lib/rules/no-arrow-function-computed-properties.js b/tests/lib/rules/no-arrow-function-computed-properties.js index 1bb6e77b7c..1af47dbd2a 100644 --- a/tests/lib/rules/no-arrow-function-computed-properties.js +++ b/tests/lib/rules/no-arrow-function-computed-properties.js @@ -19,26 +19,37 @@ const ruleTester = new RuleTester({ }); ruleTester.run('no-arrow-function-computed-properties', rule, { valid: [ + 'computed()', + 'computed(function() { return 123; })', + 'computed("prop", function() { return this.prop; })', + 'computed("prop", function() { return this.prop; }).volatile()', + 'computed.map("products", function(product) { return someFunction(product); })', + 'other(() => {})', + 'other.computed(() => {})', { - code: ` - computed('prop', function() { return this.prop; }); - computed('prop', function() { return this.prop; }).volatile(); - computed(() => { return 123; }); - computed(() => { return "string stuff"; }); - computed(() => []); - computed.map('products', product => { return someFunction(product); }); - `, - options: [{ onlyThisContexts: true }], - }, - ` - computed(); - computed(function() { return 123; }); - computed('prop', function() { return this.prop; }); - computed('prop', function() { return this.prop; }).volatile(); - computed.map('products', function(product) { return someFunction(product); }); - other(() => {}); - other.computed(() => {}); - `, + code: `computed('prop', function() { return this.prop; });`, + options: [{ onlyThisContexts: true }], + }, + { + code: `computed('prop', function() { return this.prop; }).volatile();`, + options: [{ onlyThisContexts: true }], + }, + { + code: `computed(() => { return 123; });`, + options: [{ onlyThisContexts: true }], + }, + { + code: `computed(() => { return "string stuff"; });`, + options: [{ onlyThisContexts: true }], + }, + { + code: `computed(() => []);`, + options: [{ onlyThisContexts: true }], + }, + { + code: `computed.map('products', product => { return someFunction(product); });`, + options: [{ onlyThisContexts: true }], + }, ], invalid: [ { @@ -58,5 +69,20 @@ ruleTester.run('no-arrow-function-computed-properties', rule, { code: "computed.map('products', product => { return someFunction(product); })", errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], }, + { + code: 'computed(() => { return 123; })', + errors: [], + options: [{ onlyThisContexts: true }], + }, + { + code: "computed('prop', () => { return this.prop; })", + errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], + options: [{ onlyThisContexts: true }], + }, + { + code: "computed('prop', () => { return this.prop; }).volatile()", + errors: [{ message: ERROR_MESSAGE, type: 'ArrowFunctionExpression' }], + options: [{ onlyThisContexts: true }], + }, ], });