From 6b175fd07fdac11dc3ed74cbb54e916a8e2e9487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Fri, 30 Jun 2017 10:36:57 +0200 Subject: [PATCH 01/11] Add no-side-effects-in-computed-properties rule --- .../no-side-effects-in-computed-properties.md | 41 +++++++++++++++++ .../no-side-effects-in-computed-properties.js | 44 +++++++++++++++++++ .../no-side-effects-in-computed-properties.js | 37 ++++++++++++++++ 3 files changed, 122 insertions(+) create mode 100644 docs/rules/no-side-effects-in-computed-properties.md create mode 100644 lib/rules/no-side-effects-in-computed-properties.js create mode 100644 tests/lib/rules/no-side-effects-in-computed-properties.js diff --git a/docs/rules/no-side-effects-in-computed-properties.md b/docs/rules/no-side-effects-in-computed-properties.md new file mode 100644 index 000000000..d908435f0 --- /dev/null +++ b/docs/rules/no-side-effects-in-computed-properties.md @@ -0,0 +1,41 @@ +# Do not introduce side effects in computed properties (no-side-effects-in-computed-properties) + +It is considered a very bad practice to introduce side effects inside computed properties. It makes the code not predictable and hard to understand. + + +## Rule Details + +Examples of **incorrect** code for this rule: + +```js + +export default { + computed: { + fullName() { + this.firstName = 'lorem' // <- side effect + return `${this.firstName} ${this.lastName}` + }, + somethingReversed() { + return this.something.reverse() // <- side effect + } + } +} + +``` + +Examples of **correct** code for this rule: + +```js + +export default { + computed: { + fullName() { + return `${this.firstName} ${this.lastName}` + }, + somethingReversed() { + return this.something.slice(0).reverse() + } + } +} + +``` diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js new file mode 100644 index 000000000..3cb10f8d9 --- /dev/null +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -0,0 +1,44 @@ +/** + * @fileoverview Don't introduce side effects in computed properties + * @author Michał Sajnóg + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: "Don't introduce side effects in computed properties", + category: "Fill me in", + recommended: false + }, + fixable: null, // or "code" or "whitespace" + schema: [ + // fill in your schema + ] + }, + + create: function(context) { + + // variables should be defined here + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + // any helper functions should go here or else delete this section + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + return { + + // give me methods + + }; + } +}; diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js new file mode 100644 index 000000000..4258816bf --- /dev/null +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -0,0 +1,37 @@ +/** + * @fileoverview Don't introduce side effects in computed properties + * @author Michał Sajnóg + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require("../../../lib/rules/no-side-effects-in-computed-properties"), + + RuleTester = require("eslint").RuleTester; + + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +var ruleTester = new RuleTester(); +ruleTester.run("no-side-effects-in-computed-properties", rule, { + + valid: [ + + // give me some code that won't trigger a warning + ], + + invalid: [ + { + code: "", + errors: [{ + message: "Fill me in.", + type: "Me too" + }] + } + ] +}); From 34fb5daccde2e6bccdc3c357098f0f6e18e48595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Wed, 5 Jul 2017 00:09:17 +0200 Subject: [PATCH 02/11] Prepare example tests for no-side-effects-in-computed-properties rule, Move component detection logic to utils --- .../no-side-effects-in-computed-properties.js | 55 +++---- lib/rules/order-in-components.js | 55 +------ lib/utils/index.js | 77 +++++++++ .../no-side-effects-in-computed-properties.js | 155 +++++++++++++++--- 4 files changed, 232 insertions(+), 110 deletions(-) diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 3cb10f8d9..2c1e712bb 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -2,43 +2,28 @@ * @fileoverview Don't introduce side effects in computed properties * @author Michał Sajnóg */ -"use strict"; +'use strict' -//------------------------------------------------------------------------------ +const utils = require('../utils') + +function create (context) { + return utils.executeOnVueComponent(context, (properties) => { + }) +} + +// ------------------------------------------------------------------------------ // Rule Definition -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ module.exports = { - meta: { - docs: { - description: "Don't introduce side effects in computed properties", - category: "Fill me in", - recommended: false - }, - fixable: null, // or "code" or "whitespace" - schema: [ - // fill in your schema - ] + create, + meta: { + docs: { + description: 'Don\'t introduce side effects in computed properties', + category: 'Best Practices', + recommended: false }, - - create: function(context) { - - // variables should be defined here - - //---------------------------------------------------------------------- - // Helpers - //---------------------------------------------------------------------- - - // any helper functions should go here or else delete this section - - //---------------------------------------------------------------------- - // Public - //---------------------------------------------------------------------- - - return { - - // give me methods - - }; - } -}; + fixable: null, + schema: [] + } +} diff --git a/lib/rules/order-in-components.js b/lib/rules/order-in-components.js index 56cf55d0f..82f02b860 100644 --- a/lib/rules/order-in-components.js +++ b/lib/rules/order-in-components.js @@ -4,6 +4,8 @@ */ 'use strict' +const utils = require('../utils') + const defaultOrder = [ ['name', 'delimiters', 'functional', 'model'], ['components', 'directives', 'filters'], @@ -36,38 +38,6 @@ const groups = { ] } -function isComponentFile (node, path) { - const isVueFile = path.endsWith('.vue') || path.endsWith('.jsx') - return isVueFile && node.declaration.type === 'ObjectExpression' -} - -function isVueComponent (node) { - const callee = node.callee - - const isFullVueComponent = node.type === 'CallExpression' && - callee.type === 'MemberExpression' && - callee.object.type === 'Identifier' && - callee.object.name === 'Vue' && - callee.property.type === 'Identifier' && - callee.property.name === 'component' && - node.arguments.length && - node.arguments.slice(-1)[0].type === 'ObjectExpression' - - const isDestructedVueComponent = callee.type === 'Identifier' && - callee.name === 'component' - - return isFullVueComponent || isDestructedVueComponent -} - -function isVueInstance (node) { - const callee = node.callee - return node.type === 'NewExpression' && - callee.type === 'Identifier' && - callee.name === 'Vue' && - node.arguments.length && - node.arguments[0].type === 'ObjectExpression' -} - function getOrderMap (order) { const orderMap = new Map() @@ -106,28 +76,13 @@ function checkOrder (propertiesNodes, orderMap, context) { function create (context) { const options = context.options[0] || {} const order = options.order || defaultOrder - const filePath = context.getFilename() const extendedOrder = order.map(property => groups[property] || property) const orderMap = getOrderMap(extendedOrder) - return { - ExportDefaultDeclaration (node) { - // export default {} in .vue || .jsx - if (!isComponentFile(node, filePath)) return - checkOrder(node.declaration.properties, orderMap, context) - }, - CallExpression (node) { - // Vue.component('xxx', {}) || component('xxx', {}) - if (!isVueComponent(node)) return - checkOrder(node.arguments.slice(-1)[0].properties, orderMap, context) - }, - NewExpression (node) { - // new Vue({}) - if (!isVueInstance(node)) return - checkOrder(node.arguments[0].properties, orderMap, context) - } - } + return utils.executeOnVueComponent(context, (properties) => { + checkOrder(properties, orderMap, context) + }) } // ------------------------------------------------------------------------------ diff --git a/lib/utils/index.js b/lib/utils/index.js index cd01df744..f317827b1 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -241,5 +241,82 @@ module.exports = { assert(typeof name === 'string') return VOID_ELEMENT_NAMES.has(name.toLowerCase()) + }, + + /** + * Check whether the given node is a Vue component based + * on the filename and default export type + * export default {} in .vue || .jsx + * @param {ASTNode} node Node to check + * @param {string} path File name with extension + * @returns {boolean} + */ + isVueComponentFile (node, path) { + const isVueFile = path.endsWith('.vue') || path.endsWith('.jsx') + return isVueFile && + node.type === 'ExportDefaultDeclaration' && + node.declaration.type === 'ObjectExpression' + }, + + /** + * Check whether given node is Vue component + * Vue.component('xxx', {}) || component('xxx', {}) + * @param {ASTNode} node Node to check + * @returns {boolean} + */ + isVueComponent (node) { + const callee = node.callee + + const isFullVueComponent = node.type === 'CallExpression' && + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && + callee.object.name === 'Vue' && + callee.property.type === 'Identifier' && + callee.property.name === 'component' && + node.arguments.length && + node.arguments.slice(-1)[0].type === 'ObjectExpression' + + const isDestructedVueComponent = callee.type === 'Identifier' && + callee.name === 'component' + + return isFullVueComponent || isDestructedVueComponent + }, + + /** + * Check whether given node is new Vue instance + * new Vue({}) + * @param {ASTNode} node Node to check + * @returns {boolean} + */ + isVueInstance (node) { + const callee = node.callee + return node.type === 'NewExpression' && + callee.type === 'Identifier' && + callee.name === 'Vue' && + node.arguments.length && + node.arguments[0].type === 'ObjectExpression' + }, + + executeOnVueComponent (context, cb) { + const filePath = context.getFilename() + const _this = this + + return { + ExportDefaultDeclaration (node) { + // export default {} in .vue || .jsx + if (!_this.isVueComponentFile(node, filePath)) return + cb(node.declaration.properties) + }, + CallExpression (node) { + // Vue.component('xxx', {}) || component('xxx', {}) + if (!_this.isVueComponent(node)) return + cb(node.arguments.slice(-1)[0].properties) + }, + NewExpression (node) { + // new Vue({}) + if (!_this.isVueInstance(node)) return + cb(node.arguments[0].properties) + } + } } } diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js index 4258816bf..14e1581c9 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -2,36 +2,141 @@ * @fileoverview Don't introduce side effects in computed properties * @author Michał Sajnóg */ -"use strict"; +'use strict' -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ // Requirements -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ -var rule = require("../../../lib/rules/no-side-effects-in-computed-properties"), +const rule = require('../../../lib/rules/no-side-effects-in-computed-properties') +const { RuleTester } = require('eslint') - RuleTester = require("eslint").RuleTester; +const parserOptions = { + ecmaVersion: 6, + sourceType: 'module', + ecmaFeatures: { experimentalObjectRestSpread: true } +} - -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ // Tests -//------------------------------------------------------------------------------ - -var ruleTester = new RuleTester(); -ruleTester.run("no-side-effects-in-computed-properties", rule, { - - valid: [ +// ------------------------------------------------------------------------------ - // give me some code that won't trigger a warning - ], - - invalid: [ - { - code: "", - errors: [{ - message: "Fill me in.", - type: "Me too" - }] +const ruleTester = new RuleTester() +ruleTester.run('no-side-effects-in-computed-properties', rule, { + valid: [ + { + code: `Vue.component('test', { + computed: { + test1() { + return this.firstName + ' ' + this.lastName + }, + test2() { + return this.something.slice(0).reverse() + }, + test3() { + const example = this.something * 2 + return example + 'test' + }, + test4() { + return { + ...this.something, + test: 'example' + } + }, + test5: { + get() { + return this.firstName + ' ' + this.lastName + }, + set(newValue) { + const names = newValue.split(' ') + this.firstName = names[0] + this.lastName = names[names.length - 1] + } + }, + test6: { + get() { + return this.something.slice(0).reverse() + } + }, + test7: { + get() { + const example = this.something * 2 + return example + 'test' + } + }, + test8: { + get() { + return { + ...this.something, + test: 'example' + } + } + } + } + })`, + parserOptions + } + ], + invalid: [ + { + code: `Vue.component('test', { + computed: { + test1() { + this.firstName = 'lorem' + return this.firstName + ' ' + this.lastName + }, + test2() { + return this.something.reverse() + }, + test3() { + const test = this.another.something.push('example') + return 'something' + }, + test4: { + get() { + this.firstName = 'lorem' + return this.firstName + ' ' + this.lastName + } + }, + test5: { + get() { + return this.something.reverse() + } + }, + test6: { + get() { + const test = this.another.something.push('example') + return 'something' + }, + set(newValue) { + this.something = newValue + } + }, } - ] -}); + })`, + parserOptions, + errors: [{ + line: 4, + message: 'Unexpected side effect in "test1" computed property' + }, { + line: 8, + message: 'Unexpected side effect in "test2" computed property' + }, { + line: 11, + message: 'Unexpected side effect in "test3" computed property' + }, { + line: 15, + message: 'Unexpected side effect in "test4" computed property' + }, { + line: 21, + message: 'Unexpected side effect in "test5" computed property' + }, { + line: 26, + message: 'Unexpected side effect in "test6" computed property' + }, { + line: 31, + message: 'Unexpected side effect in "test7" computed property' + }] + } + ] +}) From 20deabd6c1002dbc979212366afd46472e729850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Wed, 5 Jul 2017 00:46:32 +0200 Subject: [PATCH 03/11] Add getComputedProperties logic --- .../no-side-effects-in-computed-properties.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 2c1e712bb..d22896627 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -6,8 +6,42 @@ const utils = require('../utils') +function getComputedProperties (componentProperties) { + const computedPropertiesNode = componentProperties + .filter(p => + p.key.type === 'Identifier' && + p.key.name === 'computed' && + p.value.type === 'ObjectExpression' + )[0] + + if (!computedPropertiesNode) { return [] } + + const computedProperties = computedPropertiesNode.value.properties + + return computedProperties.map(cp => { + const key = cp.key.name + let value + + if (cp.value.type === 'FunctionExpression') { + value = cp.value.body.body + } else if (cp.value.type === 'ObjectExpression') { + value = cp.value.properties + .filter(p => + p.key.type === 'Identifier' && + p.key.name === 'get' && + p.value.type === 'FunctionExpression' + ) + .map(p => p.value.body.body)[0] + } + + return { key, value } + }) +} + function create (context) { return utils.executeOnVueComponent(context, (properties) => { + const computedProperties = getComputedProperties(properties) + // to be continued... }) } From 774a169faa0136fabdfe05777ce4f953e796296c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Thu, 6 Jul 2017 09:55:22 +0200 Subject: [PATCH 04/11] Finalize initial logic for no-side-effects-in-computed-properties rule --- .../no-side-effects-in-computed-properties.js | 76 +++++++++++++++++-- lib/utils/index.js | 6 +- .../no-side-effects-in-computed-properties.js | 68 ++++++++++++----- 3 files changed, 122 insertions(+), 28 deletions(-) diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index d22896627..4aa4fce95 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -6,6 +6,30 @@ const utils = require('../utils') +function parseMemberExpression (node) { + const members = [] + let memberExpression + + if (node.type === 'MemberExpression') { + memberExpression = node + + while (memberExpression.type === 'MemberExpression') { + if (memberExpression.property.type === 'Identifier') { + members.push(memberExpression.property.name) + } + memberExpression = memberExpression.object + } + + if (memberExpression.type === 'ThisExpression') { + members.push('this') + } else if (memberExpression.type === 'Identifier') { + members.push(memberExpression.name) + } + } + + return members.reverse() +} + function getComputedProperties (componentProperties) { const computedPropertiesNode = componentProperties .filter(p => @@ -23,7 +47,7 @@ function getComputedProperties (componentProperties) { let value if (cp.value.type === 'FunctionExpression') { - value = cp.value.body.body + value = cp.value.body } else if (cp.value.type === 'ObjectExpression') { value = cp.value.properties .filter(p => @@ -31,7 +55,7 @@ function getComputedProperties (componentProperties) { p.key.name === 'get' && p.value.type === 'FunctionExpression' ) - .map(p => p.value.body.body)[0] + .map(p => p.value.body)[0] } return { key, value } @@ -39,10 +63,50 @@ function getComputedProperties (componentProperties) { } function create (context) { - return utils.executeOnVueComponent(context, (properties) => { - const computedProperties = getComputedProperties(properties) - // to be continued... - }) + const forbiddenNodes = [] + + return Object.assign({}, + { + // this.xxx <=|+=|-=> + 'AssignmentExpression > MemberExpression' (node) { + if (parseMemberExpression(node)[0] === 'this') { + forbiddenNodes.push(node) + } + }, + // this.xxx <++|--> + 'UpdateExpression > MemberExpression' (node) { + if (parseMemberExpression(node)[0] === 'this') { + forbiddenNodes.push(node) + } + }, + // this.xxx.func() + 'CallExpression' (node) { + const code = context.getSourceCode().getText(node) + const MUTATION_REGEX = /(this.)((?!(concat|slice)\().)*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g + + if (MUTATION_REGEX.test(code)) { + forbiddenNodes.push(node) + } + } + }, + utils.executeOnVueComponent(context, (properties) => { + const computedProperties = getComputedProperties(properties) + + computedProperties.forEach(cp => { + forbiddenNodes.forEach(node => { + if ( + node.loc.start.line >= cp.value.loc.start.line && + node.loc.end.line <= cp.value.loc.end.line + ) { + context.report({ + node: node, + message: `Unexpected side effect in "${cp.key}" computed property.` + }) + } + }) + }) + }) + ) } // ------------------------------------------------------------------------------ diff --git a/lib/utils/index.js b/lib/utils/index.js index f317827b1..62d954f74 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -302,17 +302,17 @@ module.exports = { const _this = this return { - ExportDefaultDeclaration (node) { + 'ExportDefaultDeclaration:exit' (node) { // export default {} in .vue || .jsx if (!_this.isVueComponentFile(node, filePath)) return cb(node.declaration.properties) }, - CallExpression (node) { + 'CallExpression:exit' (node) { // Vue.component('xxx', {}) || component('xxx', {}) if (!_this.isVueComponent(node)) return cb(node.arguments.slice(-1)[0].properties) }, - NewExpression (node) { + 'NewExpression:exit' (node) { // new Vue({}) if (!_this.isVueInstance(node)) return cb(node.arguments[0].properties) diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js index 14e1581c9..24bf61a71 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -83,27 +83,63 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, { computed: { test1() { this.firstName = 'lorem' + asd.qwe.zxc = 'lorem' return this.firstName + ' ' + this.lastName }, test2() { - return this.something.reverse() + this.count += 2; + this.count++; + return this.count; }, test3() { + return this.something.reverse() + }, + test4() { const test = this.another.something.push('example') return 'something' }, - test4: { + } + })`, + parserOptions, + errors: [{ + line: 4, + message: 'Unexpected side effect in "test1" computed property.' + }, { + line: 9, + message: 'Unexpected side effect in "test2" computed property.' + }, { + line: 10, + message: 'Unexpected side effect in "test2" computed property.' + }, { + line: 14, + message: 'Unexpected side effect in "test3" computed property.' + }, { + line: 17, + message: 'Unexpected side effect in "test4" computed property.' + }] + }, + { + code: `Vue.component('test', { + computed: { + test1: { get() { this.firstName = 'lorem' return this.firstName + ' ' + this.lastName } }, - test5: { + test2: { + get() { + this.count += 2; + this.count++; + return this.count; + } + }, + test3: { get() { return this.something.reverse() } }, - test6: { + test4: { get() { const test = this.another.something.push('example') return 'something' @@ -116,26 +152,20 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, { })`, parserOptions, errors: [{ - line: 4, - message: 'Unexpected side effect in "test1" computed property' - }, { - line: 8, - message: 'Unexpected side effect in "test2" computed property' + line: 5, + message: 'Unexpected side effect in "test1" computed property.' }, { line: 11, - message: 'Unexpected side effect in "test3" computed property' - }, { - line: 15, - message: 'Unexpected side effect in "test4" computed property' + message: 'Unexpected side effect in "test2" computed property.' }, { - line: 21, - message: 'Unexpected side effect in "test5" computed property' + line: 12, + message: 'Unexpected side effect in "test2" computed property.' }, { - line: 26, - message: 'Unexpected side effect in "test6" computed property' + line: 18, + message: 'Unexpected side effect in "test3" computed property.' }, { - line: 31, - message: 'Unexpected side effect in "test7" computed property' + line: 23, + message: 'Unexpected side effect in "test4" computed property.' }] } ] From 2d5201f4e85ccb82a672ffc820589c4c31253585 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Thu, 6 Jul 2017 10:24:44 +0200 Subject: [PATCH 05/11] Fix test --- tests/lib/rules/no-side-effects-in-computed-properties.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js index 24bf61a71..a8b1acc2c 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -9,7 +9,7 @@ // ------------------------------------------------------------------------------ const rule = require('../../../lib/rules/no-side-effects-in-computed-properties') -const { RuleTester } = require('eslint') +const RuleTester = require('eslint').RuleTester const parserOptions = { ecmaVersion: 6, From 01b2b4063122043142c05530bef47dd8fa13d960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Thu, 6 Jul 2017 12:58:47 +0200 Subject: [PATCH 06/11] Add more methods in regex --- lib/rules/no-side-effects-in-computed-properties.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 4aa4fce95..59e6e0435 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -82,7 +82,7 @@ function create (context) { // this.xxx.func() 'CallExpression' (node) { const code = context.getSourceCode().getText(node) - const MUTATION_REGEX = /(this.)((?!(concat|slice)\().)*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g + const MUTATION_REGEX = /(this.)((?!(concat|slice|map|filter)\().)*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g if (MUTATION_REGEX.test(code)) { forbiddenNodes.push(node) From 99b25991e63cc5fecf69c2c0fec94cc48a522a55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Fri, 30 Jun 2017 10:36:57 +0200 Subject: [PATCH 07/11] Add no-side-effects-in-computed-properties rule --- .../no-side-effects-in-computed-properties.md | 41 +++++++++++++++++ .../no-side-effects-in-computed-properties.js | 44 +++++++++++++++++++ .../no-side-effects-in-computed-properties.js | 37 ++++++++++++++++ 3 files changed, 122 insertions(+) create mode 100644 docs/rules/no-side-effects-in-computed-properties.md create mode 100644 lib/rules/no-side-effects-in-computed-properties.js create mode 100644 tests/lib/rules/no-side-effects-in-computed-properties.js diff --git a/docs/rules/no-side-effects-in-computed-properties.md b/docs/rules/no-side-effects-in-computed-properties.md new file mode 100644 index 000000000..d908435f0 --- /dev/null +++ b/docs/rules/no-side-effects-in-computed-properties.md @@ -0,0 +1,41 @@ +# Do not introduce side effects in computed properties (no-side-effects-in-computed-properties) + +It is considered a very bad practice to introduce side effects inside computed properties. It makes the code not predictable and hard to understand. + + +## Rule Details + +Examples of **incorrect** code for this rule: + +```js + +export default { + computed: { + fullName() { + this.firstName = 'lorem' // <- side effect + return `${this.firstName} ${this.lastName}` + }, + somethingReversed() { + return this.something.reverse() // <- side effect + } + } +} + +``` + +Examples of **correct** code for this rule: + +```js + +export default { + computed: { + fullName() { + return `${this.firstName} ${this.lastName}` + }, + somethingReversed() { + return this.something.slice(0).reverse() + } + } +} + +``` diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js new file mode 100644 index 000000000..3cb10f8d9 --- /dev/null +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -0,0 +1,44 @@ +/** + * @fileoverview Don't introduce side effects in computed properties + * @author Michał Sajnóg + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: "Don't introduce side effects in computed properties", + category: "Fill me in", + recommended: false + }, + fixable: null, // or "code" or "whitespace" + schema: [ + // fill in your schema + ] + }, + + create: function(context) { + + // variables should be defined here + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + // any helper functions should go here or else delete this section + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + return { + + // give me methods + + }; + } +}; diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js new file mode 100644 index 000000000..4258816bf --- /dev/null +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -0,0 +1,37 @@ +/** + * @fileoverview Don't introduce side effects in computed properties + * @author Michał Sajnóg + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require("../../../lib/rules/no-side-effects-in-computed-properties"), + + RuleTester = require("eslint").RuleTester; + + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +var ruleTester = new RuleTester(); +ruleTester.run("no-side-effects-in-computed-properties", rule, { + + valid: [ + + // give me some code that won't trigger a warning + ], + + invalid: [ + { + code: "", + errors: [{ + message: "Fill me in.", + type: "Me too" + }] + } + ] +}); From be7dfa075a588c4632664a310db744648aa176d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Wed, 5 Jul 2017 00:09:17 +0200 Subject: [PATCH 08/11] Prepare example tests for no-side-effects-in-computed-properties rule, Move component detection logic to utils --- .../no-side-effects-in-computed-properties.js | 55 +++---- lib/rules/order-in-components.js | 55 +------ lib/utils/index.js | 77 +++++++++ .../no-side-effects-in-computed-properties.js | 155 +++++++++++++++--- 4 files changed, 232 insertions(+), 110 deletions(-) diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 3cb10f8d9..2c1e712bb 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -2,43 +2,28 @@ * @fileoverview Don't introduce side effects in computed properties * @author Michał Sajnóg */ -"use strict"; +'use strict' -//------------------------------------------------------------------------------ +const utils = require('../utils') + +function create (context) { + return utils.executeOnVueComponent(context, (properties) => { + }) +} + +// ------------------------------------------------------------------------------ // Rule Definition -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ module.exports = { - meta: { - docs: { - description: "Don't introduce side effects in computed properties", - category: "Fill me in", - recommended: false - }, - fixable: null, // or "code" or "whitespace" - schema: [ - // fill in your schema - ] + create, + meta: { + docs: { + description: 'Don\'t introduce side effects in computed properties', + category: 'Best Practices', + recommended: false }, - - create: function(context) { - - // variables should be defined here - - //---------------------------------------------------------------------- - // Helpers - //---------------------------------------------------------------------- - - // any helper functions should go here or else delete this section - - //---------------------------------------------------------------------- - // Public - //---------------------------------------------------------------------- - - return { - - // give me methods - - }; - } -}; + fixable: null, + schema: [] + } +} diff --git a/lib/rules/order-in-components.js b/lib/rules/order-in-components.js index 0c4d23bab..febe26f4f 100644 --- a/lib/rules/order-in-components.js +++ b/lib/rules/order-in-components.js @@ -4,6 +4,8 @@ */ 'use strict' +const utils = require('../utils') + const defaultOrder = [ ['name', 'delimiters', 'functional', 'model'], ['components', 'directives', 'filters'], @@ -36,38 +38,6 @@ const groups = { ] } -function isComponentFile (node, path) { - const isVueFile = path.endsWith('.vue') || path.endsWith('.jsx') - return isVueFile && node.declaration.type === 'ObjectExpression' -} - -function isVueComponent (node) { - const callee = node.callee - - const isFullVueComponent = node.type === 'CallExpression' && - callee.type === 'MemberExpression' && - callee.object.type === 'Identifier' && - callee.object.name === 'Vue' && - callee.property.type === 'Identifier' && - callee.property.name === 'component' && - node.arguments.length && - node.arguments.slice(-1)[0].type === 'ObjectExpression' - - const isDestructedVueComponent = callee.type === 'Identifier' && - callee.name === 'component' - - return isFullVueComponent || isDestructedVueComponent -} - -function isVueInstance (node) { - const callee = node.callee - return node.type === 'NewExpression' && - callee.type === 'Identifier' && - callee.name === 'Vue' && - node.arguments.length && - node.arguments[0].type === 'ObjectExpression' -} - function getOrderMap (order) { const orderMap = new Map() @@ -106,28 +76,13 @@ function checkOrder (propertiesNodes, orderMap, context) { function create (context) { const options = context.options[0] || {} const order = options.order || defaultOrder - const filePath = context.getFilename() const extendedOrder = order.map(property => groups[property] || property) const orderMap = getOrderMap(extendedOrder) - return { - ExportDefaultDeclaration (node) { - // export default {} in .vue || .jsx - if (!isComponentFile(node, filePath)) return - checkOrder(node.declaration.properties, orderMap, context) - }, - CallExpression (node) { - // Vue.component('xxx', {}) || component('xxx', {}) - if (!isVueComponent(node)) return - checkOrder(node.arguments.slice(-1)[0].properties, orderMap, context) - }, - NewExpression (node) { - // new Vue({}) - if (!isVueInstance(node)) return - checkOrder(node.arguments[0].properties, orderMap, context) - } - } + return utils.executeOnVueComponent(context, (properties) => { + checkOrder(properties, orderMap, context) + }) } // ------------------------------------------------------------------------------ diff --git a/lib/utils/index.js b/lib/utils/index.js index cd01df744..f317827b1 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -241,5 +241,82 @@ module.exports = { assert(typeof name === 'string') return VOID_ELEMENT_NAMES.has(name.toLowerCase()) + }, + + /** + * Check whether the given node is a Vue component based + * on the filename and default export type + * export default {} in .vue || .jsx + * @param {ASTNode} node Node to check + * @param {string} path File name with extension + * @returns {boolean} + */ + isVueComponentFile (node, path) { + const isVueFile = path.endsWith('.vue') || path.endsWith('.jsx') + return isVueFile && + node.type === 'ExportDefaultDeclaration' && + node.declaration.type === 'ObjectExpression' + }, + + /** + * Check whether given node is Vue component + * Vue.component('xxx', {}) || component('xxx', {}) + * @param {ASTNode} node Node to check + * @returns {boolean} + */ + isVueComponent (node) { + const callee = node.callee + + const isFullVueComponent = node.type === 'CallExpression' && + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && + callee.object.name === 'Vue' && + callee.property.type === 'Identifier' && + callee.property.name === 'component' && + node.arguments.length && + node.arguments.slice(-1)[0].type === 'ObjectExpression' + + const isDestructedVueComponent = callee.type === 'Identifier' && + callee.name === 'component' + + return isFullVueComponent || isDestructedVueComponent + }, + + /** + * Check whether given node is new Vue instance + * new Vue({}) + * @param {ASTNode} node Node to check + * @returns {boolean} + */ + isVueInstance (node) { + const callee = node.callee + return node.type === 'NewExpression' && + callee.type === 'Identifier' && + callee.name === 'Vue' && + node.arguments.length && + node.arguments[0].type === 'ObjectExpression' + }, + + executeOnVueComponent (context, cb) { + const filePath = context.getFilename() + const _this = this + + return { + ExportDefaultDeclaration (node) { + // export default {} in .vue || .jsx + if (!_this.isVueComponentFile(node, filePath)) return + cb(node.declaration.properties) + }, + CallExpression (node) { + // Vue.component('xxx', {}) || component('xxx', {}) + if (!_this.isVueComponent(node)) return + cb(node.arguments.slice(-1)[0].properties) + }, + NewExpression (node) { + // new Vue({}) + if (!_this.isVueInstance(node)) return + cb(node.arguments[0].properties) + } + } } } diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js index 4258816bf..14e1581c9 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -2,36 +2,141 @@ * @fileoverview Don't introduce side effects in computed properties * @author Michał Sajnóg */ -"use strict"; +'use strict' -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ // Requirements -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ -var rule = require("../../../lib/rules/no-side-effects-in-computed-properties"), +const rule = require('../../../lib/rules/no-side-effects-in-computed-properties') +const { RuleTester } = require('eslint') - RuleTester = require("eslint").RuleTester; +const parserOptions = { + ecmaVersion: 6, + sourceType: 'module', + ecmaFeatures: { experimentalObjectRestSpread: true } +} - -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ // Tests -//------------------------------------------------------------------------------ - -var ruleTester = new RuleTester(); -ruleTester.run("no-side-effects-in-computed-properties", rule, { - - valid: [ +// ------------------------------------------------------------------------------ - // give me some code that won't trigger a warning - ], - - invalid: [ - { - code: "", - errors: [{ - message: "Fill me in.", - type: "Me too" - }] +const ruleTester = new RuleTester() +ruleTester.run('no-side-effects-in-computed-properties', rule, { + valid: [ + { + code: `Vue.component('test', { + computed: { + test1() { + return this.firstName + ' ' + this.lastName + }, + test2() { + return this.something.slice(0).reverse() + }, + test3() { + const example = this.something * 2 + return example + 'test' + }, + test4() { + return { + ...this.something, + test: 'example' + } + }, + test5: { + get() { + return this.firstName + ' ' + this.lastName + }, + set(newValue) { + const names = newValue.split(' ') + this.firstName = names[0] + this.lastName = names[names.length - 1] + } + }, + test6: { + get() { + return this.something.slice(0).reverse() + } + }, + test7: { + get() { + const example = this.something * 2 + return example + 'test' + } + }, + test8: { + get() { + return { + ...this.something, + test: 'example' + } + } + } + } + })`, + parserOptions + } + ], + invalid: [ + { + code: `Vue.component('test', { + computed: { + test1() { + this.firstName = 'lorem' + return this.firstName + ' ' + this.lastName + }, + test2() { + return this.something.reverse() + }, + test3() { + const test = this.another.something.push('example') + return 'something' + }, + test4: { + get() { + this.firstName = 'lorem' + return this.firstName + ' ' + this.lastName + } + }, + test5: { + get() { + return this.something.reverse() + } + }, + test6: { + get() { + const test = this.another.something.push('example') + return 'something' + }, + set(newValue) { + this.something = newValue + } + }, } - ] -}); + })`, + parserOptions, + errors: [{ + line: 4, + message: 'Unexpected side effect in "test1" computed property' + }, { + line: 8, + message: 'Unexpected side effect in "test2" computed property' + }, { + line: 11, + message: 'Unexpected side effect in "test3" computed property' + }, { + line: 15, + message: 'Unexpected side effect in "test4" computed property' + }, { + line: 21, + message: 'Unexpected side effect in "test5" computed property' + }, { + line: 26, + message: 'Unexpected side effect in "test6" computed property' + }, { + line: 31, + message: 'Unexpected side effect in "test7" computed property' + }] + } + ] +}) From f218407e9131dd93ddcc77e2123f841122968e6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Wed, 5 Jul 2017 00:46:32 +0200 Subject: [PATCH 09/11] Add getComputedProperties logic --- .../no-side-effects-in-computed-properties.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 2c1e712bb..d22896627 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -6,8 +6,42 @@ const utils = require('../utils') +function getComputedProperties (componentProperties) { + const computedPropertiesNode = componentProperties + .filter(p => + p.key.type === 'Identifier' && + p.key.name === 'computed' && + p.value.type === 'ObjectExpression' + )[0] + + if (!computedPropertiesNode) { return [] } + + const computedProperties = computedPropertiesNode.value.properties + + return computedProperties.map(cp => { + const key = cp.key.name + let value + + if (cp.value.type === 'FunctionExpression') { + value = cp.value.body.body + } else if (cp.value.type === 'ObjectExpression') { + value = cp.value.properties + .filter(p => + p.key.type === 'Identifier' && + p.key.name === 'get' && + p.value.type === 'FunctionExpression' + ) + .map(p => p.value.body.body)[0] + } + + return { key, value } + }) +} + function create (context) { return utils.executeOnVueComponent(context, (properties) => { + const computedProperties = getComputedProperties(properties) + // to be continued... }) } From 5e6330a7a8213c0d3568b6eebcc3053511d1c8e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Wed, 19 Jul 2017 13:07:32 +0200 Subject: [PATCH 10/11] Add more test, move functions to utils --- .../no-side-effects-in-computed-properties.js | 64 +-------- lib/rules/order-in-components.js | 4 +- lib/utils/index.js | 72 +++++++++- package.json | 2 + .../no-side-effects-in-computed-properties.js | 28 ++++ tests/lib/utils/index.js | 123 ++++++++++++++++++ 6 files changed, 228 insertions(+), 65 deletions(-) create mode 100644 tests/lib/utils/index.js diff --git a/lib/rules/no-side-effects-in-computed-properties.js b/lib/rules/no-side-effects-in-computed-properties.js index 59e6e0435..968f7a814 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -6,62 +6,6 @@ const utils = require('../utils') -function parseMemberExpression (node) { - const members = [] - let memberExpression - - if (node.type === 'MemberExpression') { - memberExpression = node - - while (memberExpression.type === 'MemberExpression') { - if (memberExpression.property.type === 'Identifier') { - members.push(memberExpression.property.name) - } - memberExpression = memberExpression.object - } - - if (memberExpression.type === 'ThisExpression') { - members.push('this') - } else if (memberExpression.type === 'Identifier') { - members.push(memberExpression.name) - } - } - - return members.reverse() -} - -function getComputedProperties (componentProperties) { - const computedPropertiesNode = componentProperties - .filter(p => - p.key.type === 'Identifier' && - p.key.name === 'computed' && - p.value.type === 'ObjectExpression' - )[0] - - if (!computedPropertiesNode) { return [] } - - const computedProperties = computedPropertiesNode.value.properties - - return computedProperties.map(cp => { - const key = cp.key.name - let value - - if (cp.value.type === 'FunctionExpression') { - value = cp.value.body - } else if (cp.value.type === 'ObjectExpression') { - value = cp.value.properties - .filter(p => - p.key.type === 'Identifier' && - p.key.name === 'get' && - p.value.type === 'FunctionExpression' - ) - .map(p => p.value.body)[0] - } - - return { key, value } - }) -} - function create (context) { const forbiddenNodes = [] @@ -69,13 +13,13 @@ function create (context) { { // this.xxx <=|+=|-=> 'AssignmentExpression > MemberExpression' (node) { - if (parseMemberExpression(node)[0] === 'this') { + if (utils.parseMemberExpression(node)[0] === 'this') { forbiddenNodes.push(node) } }, // this.xxx <++|--> 'UpdateExpression > MemberExpression' (node) { - if (parseMemberExpression(node)[0] === 'this') { + if (utils.parseMemberExpression(node)[0] === 'this') { forbiddenNodes.push(node) } }, @@ -89,8 +33,8 @@ function create (context) { } } }, - utils.executeOnVueComponent(context, (properties) => { - const computedProperties = getComputedProperties(properties) + utils.executeOnVueComponent(context, (obj) => { + const computedProperties = utils.getComputedProperties(obj) computedProperties.forEach(cp => { forbiddenNodes.forEach(node => { diff --git a/lib/rules/order-in-components.js b/lib/rules/order-in-components.js index 82f02b860..82815e254 100644 --- a/lib/rules/order-in-components.js +++ b/lib/rules/order-in-components.js @@ -80,8 +80,8 @@ function create (context) { const extendedOrder = order.map(property => groups[property] || property) const orderMap = getOrderMap(extendedOrder) - return utils.executeOnVueComponent(context, (properties) => { - checkOrder(properties, orderMap, context) + return utils.executeOnVueComponent(context, (obj) => { + checkOrder(obj.properties, orderMap, context) }) } diff --git a/lib/utils/index.js b/lib/utils/index.js index 62d954f74..4dd38d790 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -243,6 +243,72 @@ module.exports = { return VOID_ELEMENT_NAMES.has(name.toLowerCase()) }, + /** + * Parse member expression node to get array with all of it's parts + * @param {ASTNode} MemberExpression + * @returns {Array} + */ + parseMemberExpression (node) { + const members = [] + let memberExpression + + if (node.type === 'MemberExpression') { + memberExpression = node + + while (memberExpression.type === 'MemberExpression') { + if (memberExpression.property.type === 'Identifier') { + members.push(memberExpression.property.name) + } + memberExpression = memberExpression.object + } + + if (memberExpression.type === 'ThisExpression') { + members.push('this') + } else if (memberExpression.type === 'Identifier') { + members.push(memberExpression.name) + } + } + + return members.reverse() + }, + + /** + * Get all computed properties by looking at all component's properties + * @param {ObjectExpression} Object with component definition + * @return {Array} Array of computed properties in format: [{key: String, value: ASTNode}] + */ + getComputedProperties (componentObject) { + const computedPropertiesNode = componentObject.properties + .filter(p => + p.key.type === 'Identifier' && + p.key.name === 'computed' && + p.value.type === 'ObjectExpression' + )[0] + + if (!computedPropertiesNode) { return [] } + + return computedPropertiesNode.value.properties + .filter(cp => cp.type === 'Property') + .map(cp => { + const key = cp.key.name + let value + + if (cp.value.type === 'FunctionExpression') { + value = cp.value.body + } else if (cp.value.type === 'ObjectExpression') { + value = cp.value.properties + .filter(p => + p.key.type === 'Identifier' && + p.key.name === 'get' && + p.value.type === 'FunctionExpression' + ) + .map(p => p.value.body)[0] + } + + return { key, value } + }) + }, + /** * Check whether the given node is a Vue component based * on the filename and default export type @@ -305,17 +371,17 @@ module.exports = { 'ExportDefaultDeclaration:exit' (node) { // export default {} in .vue || .jsx if (!_this.isVueComponentFile(node, filePath)) return - cb(node.declaration.properties) + cb(node.declaration) }, 'CallExpression:exit' (node) { // Vue.component('xxx', {}) || component('xxx', {}) if (!_this.isVueComponent(node)) return - cb(node.arguments.slice(-1)[0].properties) + cb(node.arguments.slice(-1)[0]) }, 'NewExpression:exit' (node) { // new Vue({}) if (!_this.isVueInstance(node)) return - cb(node.arguments[0].properties) + cb(node.arguments[0]) } } } diff --git a/package.json b/package.json index 955b0c5b8..48f7e68fb 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,8 @@ }, "devDependencies": { "@types/node": "^4.2.6", + "babel-eslint": "^7.2.3", + "chai": "^4.1.0", "eslint": "^3.18.0", "eslint-plugin-eslint-plugin": "^0.7.1", "eslint-plugin-vue-libs": "^1.2.0", diff --git a/tests/lib/rules/no-side-effects-in-computed-properties.js b/tests/lib/rules/no-side-effects-in-computed-properties.js index a8b1acc2c..5a58de081 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -75,6 +75,34 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, { } })`, parserOptions + }, + { + code: `Vue.component('test', { + computed: { + ...mapGetters(['example']), + test1() { + const num = 0 + const something = { + a: 'val', + b: ['1', '2'] + } + num++ + something.a = 'something' + something.b.reverse() + return something.b + } + } + })`, + parserOptions + }, + { + code: `Vue.component('test', { + name: 'something', + data() { + return {} + } + })`, + parserOptions } ], invalid: [ diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js new file mode 100644 index 000000000..4a600e8a9 --- /dev/null +++ b/tests/lib/utils/index.js @@ -0,0 +1,123 @@ +'use strict' + +const babelEslint = require('babel-eslint') +const utils = require('../../../lib/utils/index') +const chai = require('chai') + +const { assert } = chai + +describe('parseMemberExpression', () => { + let node + + const parse = function (code) { + return babelEslint.parse(code).body[0].expression + } + + it('should parse member expression', () => { + node = parse('this.some.nested.property') + assert.deepEqual( + utils.parseMemberExpression(node), + ['this', 'some', 'nested', 'property'] + ) + + node = parse('another.property') + assert.deepEqual( + utils.parseMemberExpression(node), + ['another', 'property'] + ) + + node = parse('this.something') + assert.deepEqual( + utils.parseMemberExpression(node), + ['this', 'something'] + ) + }) +}) + +describe('getComputedProperties', () => { + let node + + const parse = function (code) { + return babelEslint.parse(code).body[0].declarations[0].init + } + + it('should return empty array when there is no computed property', () => { + node = parse(`const test = { + name: 'test', + data() { + return {} + } + }`) + + assert.equal( + utils.getComputedProperties(node).length, + 0 + ) + }) + + it('should return computed properties', () => { + node = parse(`const test = { + name: 'test', + data() { + return {} + }, + computed: { + a: 'bad', + b: function () { + return 'b' + }, + c() { + return 'c' + }, + d: {}, + e: { + set(val) { + this.something = val + } + }, + f: { + get() { + return 'f' + } + } + } + }`) + + const computedProperties = utils.getComputedProperties(node) + + assert.equal( + computedProperties.length, + 6, + 'it detects all computed properties' + ) + + assert.notOk(computedProperties[0].value) + assert.ok(computedProperties[1].value) + assert.ok(computedProperties[2].value) + assert.notOk(computedProperties[3].value) + assert.notOk(computedProperties[4].value) + assert.ok(computedProperties[5].value) + }) + + it('should not collide with object spread operator', () => { + node = parse(`const test = { + name: 'test', + computed: { + ...mapGetters(['test']), + a() { + return 'a' + } + } + }`) + + const computedProperties = utils.getComputedProperties(node) + + assert.equal( + computedProperties.length, + 1, + 'it detects all computed properties' + ) + + assert.ok(computedProperties[0].value) + }) +}) From c036ba92577c1b8502922a39586f7d9347d6f128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Wed, 19 Jul 2017 13:28:55 +0200 Subject: [PATCH 11/11] Make it work with Node 4 --- tests/lib/utils/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index 4a600e8a9..13293fb3a 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -4,7 +4,7 @@ const babelEslint = require('babel-eslint') const utils = require('../../../lib/utils/index') const chai = require('chai') -const { assert } = chai +const assert = chai.assert describe('parseMemberExpression', () => { let node