Skip to content

Commit 51114c7

Browse files
authored
Merge pull request #790 from bmish/no-side-effects-es5-setter
Catch assignment in `no-side-effects` rule
2 parents 80777e8 + 1347abd commit 51114c7

File tree

3 files changed

+25
-2
lines changed

3 files changed

+25
-2
lines changed

docs/rules/no-side-effects.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ This rule currently disallows the following side-effect-causing statements insid
88

99
* `this.set('x', 123);`
1010
* `this.setProperties({ x: 123 });`
11+
* `this.x = 123;`
1112

1213
Note that other side effects like network requests should be avoided as well.
1314

lib/rules/no-side-effects.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@ function isEmberSet(node, emberImportAliasName) {
2727
);
2828
}
2929

30+
function isThisSet(node) {
31+
return (
32+
types.isAssignmentExpression(node) &&
33+
types.isMemberExpression(node.left) &&
34+
types.isThisExpression(node.left.object) &&
35+
types.isIdentifier(node.left.property)
36+
);
37+
}
38+
3039
/**
3140
* Recursively finds calls that could be side effects in a computed property function body.
3241
*
@@ -39,7 +48,7 @@ function findSideEffects(computedPropertyBody, emberImportAliasName) {
3948

4049
new Traverser().traverse(computedPropertyBody, {
4150
enter(child) {
42-
if (isEmberSet(child, emberImportAliasName)) {
51+
if (isEmberSet(child, emberImportAliasName) || isThisSet(child)) {
4352
results.push(child);
4453
}
4554
},

tests/lib/rules/no-side-effects.js

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ eslintTester.run('no-side-effects', rule, {
2323
'testAmount: computed("test.length", { get() { return ""; }, set() { setProperties(); }})',
2424
'let foo = computed("test", function() { someMap.setProperties(); return ""; })',
2525
'import Ember from "ember"; import Foo from "some-other-thing"; let foo = computed("test", function() { Foo.set(this, "testAmount", test.length); return ""; });',
26-
2726
'import Ember from "ember"; import Foo from "some-other-thing"; let foo = computed("test", function() { Foo.setProperties(); return ""; });',
27+
'computed("test", function() { this.test; })',
28+
'computed("test", function() { this.myFunction(); })',
29+
'computed("test", function() { let x = 123; x = 456; someObject.x = 123; })',
2830

2931
// Decorators:
3032
{
@@ -256,5 +258,16 @@ eslintTester.run('no-side-effects', rule, {
256258
ecmaFeatures: { legacyDecorators: true },
257259
},
258260
},
261+
262+
{
263+
code: 'computed("test", function() { this.x = 123; })',
264+
output: null,
265+
errors: [
266+
{
267+
message: ERROR_MESSAGE,
268+
type: 'AssignmentExpression',
269+
},
270+
],
271+
},
259272
],
260273
});

0 commit comments

Comments
 (0)