Repository: ember-cli/eslint-plugin-ember (js)

Issue:
Rule no-side-effects misses setProperties
Given the following computed property:

```js
foo: computed('bar', {
  get() {
    this.setProperties({ baz: 'BAZ' }); // SIDE EFFECT
  }
})
```

This should be captured by the `no-side-effects` rule. 

Pull request:
Updating no-side-effects to also report on setProperties.
Fixes #329.

Patch:
diff --git a/lib/rules/no-side-effects.js b/lib/rules/no-side-effects.js
index dde6d375..31be4da8 100644
--- a/lib/rules/no-side-effects.js
+++ b/lib/rules/no-side-effects.js
@@ -7,6 +7,10 @@ const ember = require('../utils/ember');
 // General rule - Don't introduce side-effects in computed properties
 //------------------------------------------------------------------------------
 
+function isUnallowedMethod(name) {
+  return ['set', 'setProperties'].indexOf(name) > -1;
+}
+
 module.exports = {
   meta: {
     docs: {
@@ -38,12 +42,12 @@ module.exports = {
         const callee = node.callee;
         const isEmberSet = (
           utils.isIdentifier(callee) &&
-          callee.name === 'set'
+          isUnallowedMethod(callee.name)
         ) || (
           utils.isMemberExpression(callee) &&
           (utils.isThisExpression(callee.object) || callee.object.name === 'Ember' || callee.object.name === emberImportAliasName) &&
           utils.isIdentifier(callee.property) &&
-          callee.property.name === 'set'
+          isUnallowedMethod(callee.property.name)
         );
 
         if (isEmberSet) {


Test patch:
diff --git a/tests/lib/rules/no-side-effects.js b/tests/lib/rules/no-side-effects.js
index c41da6c2..9805e75a 100644
--- a/tests/lib/rules/no-side-effects.js
+++ b/tests/lib/rules/no-side-effects.js
@@ -24,10 +24,23 @@ eslintTester.run('no-side-effects', rule, {
       code: 'let foo = computed("test", function() { someMap.set(this, "testAmount", test.length); return ""; })',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
     },
+    {
+      code: 'testAmount: computed("test.length", { get() { return ""; }, set() { setProperties(); }})',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    },
+    {
+      code: 'let foo = computed("test", function() { someMap.setProperties(); return ""; })',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    },
     {
       code: 'import Ember from "ember"; import Foo from "some-other-thing"; let foo = computed("test", function() { Foo.set(this, "testAmount", test.length); return ""; });',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' }
-    }
+    },
+
+    {
+      code: 'import Ember from "ember"; import Foo from "some-other-thing"; let foo = computed("test", function() { Foo.setProperties(); return ""; });',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' }
+    },
   ],
   invalid: [
     {
@@ -36,8 +49,9 @@ eslintTester.run('no-side-effects', rule, {
       errors: [{
         message: 'Don\'t introduce side-effects in computed properties',
       }],
-    }, {
-      code: 'prop: computed("test", function() {set(this, "testAmount", test.length); return "";})',
+    },
+    {
+      code: 'prop: computed("test", function() { this.setProperties("testAmount", test.length); return "";})',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'Don\'t introduce side-effects in computed properties',
@@ -50,6 +64,13 @@ eslintTester.run('no-side-effects', rule, {
         message: 'Don\'t introduce side-effects in computed properties'
       }]
     },
+    {
+      code: 'prop: computed("test", function() {if (get(this, "testAmount")) { setProperties(this, "testAmount", test.length); } return "";})',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'Don\'t introduce side-effects in computed properties'
+      }]
+    },
     {
       code: 'testAmount: computed("test.length", { get() { set(this, "testAmount", test.length); }, set() { set(this, "testAmount", test.length); }})',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
@@ -57,6 +78,13 @@ eslintTester.run('no-side-effects', rule, {
         message: 'Don\'t introduce side-effects in computed properties'
       }]
     },
+    {
+      code: 'testAmount: computed("test.length", { get() { setProperties(this, "testAmount", test.length); }, set() { setProperties(this, "testAmount", test.length); }})',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'Don\'t introduce side-effects in computed properties'
+      }]
+    },
     {
       code: 'testAmount: computed("test.length", function() { const setSomething = () => { set(this, "testAmount", test.length); }; setSomething(); })',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
@@ -64,6 +92,13 @@ eslintTester.run('no-side-effects', rule, {
         message: 'Don\'t introduce side-effects in computed properties'
       }]
     },
+    {
+      code: 'testAmount: computed("test.length", function() { const setSomething = () => { setProperties(this, "testAmount", test.length); }; setSomething(); })',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'Don\'t introduce side-effects in computed properties'
+      }]
+    },
     {
       code: 'let foo = computed("test", function() { Ember.set(this, "testAmount", test.length); return ""; })',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
@@ -71,6 +106,13 @@ eslintTester.run('no-side-effects', rule, {
         message: 'Don\'t introduce side-effects in computed properties'
       }]
     },
+    {
+      code: 'let foo = computed("test", function() { Ember.setProperties(this, "testAmount", test.length); return ""; })',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'Don\'t introduce side-effects in computed properties'
+      }]
+    },
     {
       code: 'import Foo from "ember"; let foo = computed("test", function() { Foo.set(this, "testAmount", test.length); return ""; })',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
@@ -78,12 +120,26 @@ eslintTester.run('no-side-effects', rule, {
         message: 'Don\'t introduce side-effects in computed properties'
       }]
     },
+    {
+      code: 'import Foo from "ember"; let foo = computed("test", function() { Foo.setProperties(this, "testAmount", test.length); return ""; })',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'Don\'t introduce side-effects in computed properties'
+      }]
+    },
     {
       code: 'import EmberFoo from "ember"; import Foo from "some-other-thing"; let foo = computed("test", function() { EmberFoo.set(this, "testAmount", test.length); return ""; });',
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       errors: [{
         message: 'Don\'t introduce side-effects in computed properties'
       }]
+    },
+    {
+      code: 'import EmberFoo from "ember"; import Foo from "some-other-thing"; let foo = computed("test", function() { EmberFoo.setProperties(this, "testAmount", test.length); return ""; });',
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [{
+        message: 'Don\'t introduce side-effects in computed properties'
+      }]
     }
   ],
 });