Skip to content

Commit 409fe31

Browse files
authored
(Fixes #420) Fix false positive in no-side-effect-in-cp (#464)
* Parse Member and CallExpression to simplified versions for better side-effects detection * Update parseMemberOrCallExpression * Add extra test case
1 parent 552ea4f commit 409fe31

File tree

4 files changed

+77
-1
lines changed

4 files changed

+77
-1
lines changed

Diff for: lib/rules/no-side-effects-in-computed-properties.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ module.exports = {
4141
},
4242
// this.xxx.func()
4343
'CallExpression' (node) {
44-
const code = context.getSourceCode().getText(node)
44+
const code = utils.parseMemberOrCallExpression(node)
4545
const MUTATION_REGEX = /(this.)((?!(concat|slice|map|filter)\().)[^\)]*((push|pop|shift|unshift|reverse|splice|sort|copyWithin|fill)\()/g
4646

4747
if (MUTATION_REGEX.test(code)) {

Diff for: lib/utils/index.js

+37
Original file line numberDiff line numberDiff line change
@@ -617,5 +617,42 @@ module.exports = {
617617
return
618618
}
619619
return body.errors.some(error => typeof error.code === 'string' && error.code.startsWith('eof-'))
620+
},
621+
622+
/**
623+
* Parse CallExpression or MemberExpression to get simplified version without arguments
624+
*
625+
* @param {Object} node The node to parse (MemberExpression | CallExpression)
626+
* @return {String} eg. 'this.asd.qwe().map().filter().test.reduce()'
627+
*/
628+
parseMemberOrCallExpression (node) {
629+
const parsedCallee = []
630+
let n = node
631+
let isFunc
632+
633+
while (n.type === 'MemberExpression' || n.type === 'CallExpression') {
634+
if (n.type === 'CallExpression') {
635+
n = n.callee
636+
isFunc = true
637+
} else {
638+
if (n.computed) {
639+
parsedCallee.push('[]')
640+
} else if (n.property.type === 'Identifier') {
641+
parsedCallee.push(n.property.name + (isFunc ? '()' : ''))
642+
}
643+
isFunc = false
644+
n = n.object
645+
}
646+
}
647+
648+
if (n.type === 'Identifier') {
649+
parsedCallee.push(n.name)
650+
}
651+
652+
if (n.type === 'ThisExpression') {
653+
parsedCallee.push('this')
654+
}
655+
656+
return parsedCallee.reverse().join('.').replace(/\.\[/g, '[')
620657
}
621658
}

Diff for: tests/lib/rules/no-side-effects-in-computed-properties.js

+19
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,25 @@ ruleTester.run('no-side-effects-in-computed-properties', rule, {
8181
get() {
8282
return Object.keys(this.a).sort()
8383
}
84+
},
85+
test11() {
86+
const categories = {}
87+
88+
this.types.forEach(c => {
89+
categories[c.category] = categories[c.category] || []
90+
categories[c.category].push(c)
91+
})
92+
93+
return categories
94+
},
95+
test12() {
96+
return this.types.map(t => {
97+
// [].push('xxx')
98+
return t
99+
})
100+
},
101+
test13 () {
102+
this.someArray.forEach(arr => console.log(arr))
84103
}
85104
}
86105
})`,

Diff for: tests/lib/utils/index.js

+20
Original file line numberDiff line numberDiff line change
@@ -166,3 +166,23 @@ describe('getStaticPropertyName', () => {
166166
assert.ok(parsed === 'computed')
167167
})
168168
})
169+
170+
describe('parseMemberOrCallExpression', () => {
171+
let node
172+
173+
const parse = function (code) {
174+
return babelEslint.parse(code).body[0].declarations[0].init
175+
}
176+
177+
it('should parse CallExpression', () => {
178+
node = parse(`const test = this.lorem['ipsum'].map(d => d.id).filter((a, b) => a > b).reduce((acc, d) => acc + d, 0)`)
179+
const parsed = utils.parseMemberOrCallExpression(node)
180+
assert.equal(parsed, 'this.lorem[].map().filter().reduce()')
181+
})
182+
183+
it('should parse MemberExpression', () => {
184+
node = parse(`const test = this.lorem['ipsum'][0].map(d => d.id).dolor.reduce((acc, d) => acc + d, 0).sit`)
185+
const parsed = utils.parseMemberOrCallExpression(node)
186+
assert.equal(parsed, 'this.lorem[][].map().dolor.reduce().sit')
187+
})
188+
})

0 commit comments

Comments
 (0)