From 5be7ae9b2c58b743baf7a8a6dd9148cb3dc3e5b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Sat, 21 Apr 2018 22:53:48 +0800 Subject: [PATCH 1/3] Parse Member and CallExpression to simplified versions for better side-effects detection --- .../no-side-effects-in-computed-properties.js | 2 +- lib/utils/index.js | 37 +++++++++++++++++++ .../no-side-effects-in-computed-properties.js | 16 ++++++++ tests/lib/utils/index.js | 26 +++++++++++++ 4 files changed, 80 insertions(+), 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 149abac56..9ef2d8b7c 100644 --- a/lib/rules/no-side-effects-in-computed-properties.js +++ b/lib/rules/no-side-effects-in-computed-properties.js @@ -41,7 +41,7 @@ module.exports = { }, // this.xxx.func() 'CallExpression' (node) { - const code = context.getSourceCode().getText(node) + const code = utils.parseMemberOrCallExpression(node) const MUTATION_REGEX = /(this.)((?!(concat|slice|map|filter)\().)[^\)]*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g if (MUTATION_REGEX.test(code)) { diff --git a/lib/utils/index.js b/lib/utils/index.js index 677fff6b5..ef2a85473 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -617,5 +617,42 @@ module.exports = { return } return body.errors.some(error => typeof error.code === 'string' && error.code.startsWith('eof-')) + }, + + /** + * Parse CallExpression or MemberExpression to get simplified version without arguments + * + * @param {Object} node The node to parse (MemberExpression | CallExpression) + * @return {String} eg. 'this.asd.qwe().map().filter().test.reduce()' + */ + parseMemberOrCallExpression (node) { + const parsedCallee = [] + let n = node + let isFunc + + while (n.type === 'MemberExpression' || n.type === 'CallExpression') { + if (n.type === 'CallExpression') { + n = n.callee + isFunc = true + } else { + if (!n.computed && n.property.type === 'Identifier') { + parsedCallee.push(n.property.name + (isFunc ? '()' : '')) + } else if (n.property.type === 'Literal') { + parsedCallee.push(n.property.value + (isFunc ? '()' : '')) + } + isFunc = false + n = n.object + } + } + + if (n.type === 'Identifier') { + parsedCallee.push(n.name) + } + + if (n.type === 'ThisExpression') { + parsedCallee.push('this') + } + + return parsedCallee.reverse().join('.') } } 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 cfd71c7d1..45b69e595 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -81,6 +81,22 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, { get() { return Object.keys(this.a).sort() } + }, + test11() { + const categories = {} + + this.types.forEach(c => { + categories[c.category] = categories[c.category] || [] + categories[c.category].push(c) + }) + + return categories + }, + test12() { + return this.types.map(t => { + // [].push('xxx') + return t + }) } } })`, diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index 2189f8960..38f55ec04 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -166,3 +166,29 @@ describe('getStaticPropertyName', () => { assert.ok(parsed === 'computed') }) }) + +describe('parseMemberOrCallExpression', () => { + let node + + const parse = function (code) { + return babelEslint.parse(code).body[0].declarations[0].init + } + + it('should parse CallExpression', () => { + node = parse(`const test = this.lorem['ipsum'].map(d => d.id).filter((a, b) => a > b).reduce((acc, d) => acc + d, 0)`) + const parsed = utils.parseMemberOrCallExpression(node) + assert.equal(parsed, 'this.lorem.ipsum.map().filter().reduce()') + }) + + it('should parse MemberExpression', () => { + node = parse(`const test = this.lorem['ipsum'].map(d => d.id).dolor.reduce((acc, d) => acc + d, 0).sit`) + const parsed = utils.parseMemberOrCallExpression(node) + assert.equal(parsed, 'this.lorem.ipsum.map().dolor.reduce().sit') + }) + + it('should ignore computed values', () => { + node = parse(`const test = this.lorem[ipsum].dolor.map(d => d.id)['asd']()`) + const parsed = utils.parseMemberOrCallExpression(node) + assert.equal(parsed, 'this.lorem.dolor.map().asd()') + }) +}) From 0c946635a084e291abcda2fffaf18ac0b5442b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Sat, 21 Apr 2018 23:22:26 +0800 Subject: [PATCH 2/3] Update parseMemberOrCallExpression --- lib/utils/index.js | 8 ++++---- tests/lib/utils/index.js | 12 +++--------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/utils/index.js b/lib/utils/index.js index ef2a85473..7494178c4 100644 --- a/lib/utils/index.js +++ b/lib/utils/index.js @@ -635,10 +635,10 @@ module.exports = { n = n.callee isFunc = true } else { - if (!n.computed && n.property.type === 'Identifier') { + if (n.computed) { + parsedCallee.push('[]') + } else if (n.property.type === 'Identifier') { parsedCallee.push(n.property.name + (isFunc ? '()' : '')) - } else if (n.property.type === 'Literal') { - parsedCallee.push(n.property.value + (isFunc ? '()' : '')) } isFunc = false n = n.object @@ -653,6 +653,6 @@ module.exports = { parsedCallee.push('this') } - return parsedCallee.reverse().join('.') + return parsedCallee.reverse().join('.').replace(/\.\[/g, '[') } } diff --git a/tests/lib/utils/index.js b/tests/lib/utils/index.js index 38f55ec04..c222142d8 100644 --- a/tests/lib/utils/index.js +++ b/tests/lib/utils/index.js @@ -177,18 +177,12 @@ describe('parseMemberOrCallExpression', () => { it('should parse CallExpression', () => { node = parse(`const test = this.lorem['ipsum'].map(d => d.id).filter((a, b) => a > b).reduce((acc, d) => acc + d, 0)`) const parsed = utils.parseMemberOrCallExpression(node) - assert.equal(parsed, 'this.lorem.ipsum.map().filter().reduce()') + assert.equal(parsed, 'this.lorem[].map().filter().reduce()') }) it('should parse MemberExpression', () => { - node = parse(`const test = this.lorem['ipsum'].map(d => d.id).dolor.reduce((acc, d) => acc + d, 0).sit`) + node = parse(`const test = this.lorem['ipsum'][0].map(d => d.id).dolor.reduce((acc, d) => acc + d, 0).sit`) const parsed = utils.parseMemberOrCallExpression(node) - assert.equal(parsed, 'this.lorem.ipsum.map().dolor.reduce().sit') - }) - - it('should ignore computed values', () => { - node = parse(`const test = this.lorem[ipsum].dolor.map(d => d.id)['asd']()`) - const parsed = utils.parseMemberOrCallExpression(node) - assert.equal(parsed, 'this.lorem.dolor.map().asd()') + assert.equal(parsed, 'this.lorem[][].map().dolor.reduce().sit') }) }) From f5d7ddd83d3768438355673b3cb326db6212d7c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Sajn=C3=B3g?= Date: Sat, 21 Apr 2018 23:29:24 +0800 Subject: [PATCH 3/3] Add extra test case --- tests/lib/rules/no-side-effects-in-computed-properties.js | 3 +++ 1 file changed, 3 insertions(+) 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 45b69e595..3eadc45c0 100644 --- a/tests/lib/rules/no-side-effects-in-computed-properties.js +++ b/tests/lib/rules/no-side-effects-in-computed-properties.js @@ -97,6 +97,9 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, { // [].push('xxx') return t }) + }, + test13 () { + this.someArray.forEach(arr => console.log(arr)) } } })`,