Fix no-direct-mutation-state to report only in React components (fixes …

…#229)
jsx-eslint · Sep 30, 2015 · b42f681 · b42f681
1 parent 00f716a
commit b42f681
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 1 deletion.
diff --git a/lib/rules/no-direct-mutation-state.js b/lib/rules/no-direct-mutation-state.js
@@ -4,12 +4,41 @@
  */
 'use strict';
 
+var componentUtil = require('../util/component');
+var ComponentList = componentUtil.List;
+
 // ------------------------------------------------------------------------------
 // Rule Definition
 // ------------------------------------------------------------------------------
 
 module.exports = function(context) {
 
+  var componentList = new ComponentList();
+
+  /**
+   * Checks if the component is valid
+   * @param {Object} component The component to process
+   * @returns {Boolean} True if the component is valid, false if not.
+   */
+  function isValid(component) {
+    var isNotReactComponent = Boolean(component && !component.isReactComponent);
+    var doNotMutateSetState = Boolean(component && !component.mutateSetState);
+
+    return isNotReactComponent || doNotMutateSetState;
+  }
+
+  /**
+   * Reports undeclared proptypes for a given component
+   * @param {Object} component The component to process
+   */
+  function reportMutations(component) {
+    var mutation;
+    for (var i = 0, j = component.mutations.length; i < j; i++) {
+      mutation = component.mutations[i];
+      context.report(mutation, 'Do not mutate state directly. Use setState().');
+    }
+  }
+
   // --------------------------------------------------------------------------
   // Public
   // --------------------------------------------------------------------------
@@ -29,8 +58,33 @@ module.exports = function(context) {
         item.object.type === 'ThisExpression' &&
         item.property.name === 'state'
       ) {
-        context.report(node.left.object, 'Do not mutate state directly. Use setState().');
+        var component = componentList.getByNode(context, node);
+        var mutations = component && component.mutations || [];
+        mutations.push(node.left.object);
+        componentList.set(context, node, {
+          mutateSetState: true,
+          mutations: mutations
+        });
+      }
+    },
+
+    'Program:exit': function() {
+      var list = componentList.getList();
+      for (var component in list) {
+        if (!list.hasOwnProperty(component) || isValid(list[component])) {
+          continue;
+        }
+        reportMutations(list[component]);
+      }
+    },
+
+    ReturnStatement: function(node) {
+      if (!componentUtil.isReactComponent(context, node)) {
+        return;
       }
+      componentList.set(context, node, {
+        isReactComponent: true
+      });
     }
   };
 

diff --git a/tests/lib/rules/no-direct-mutation-state.js b/tests/lib/rules/no-direct-mutation-state.js
@@ -52,6 +52,20 @@ ruleTester.run('no-direct-mutation-state', rule, {
     ecmaFeatures: {
       jsx: true
     }
+  }, {
+    code: [
+      'class Hello {',
+      '  getFoo() {',
+      '    this.state.foo = \'bar\'',
+      '    return this.state.foo;',
+      '  }',
+      '}'
+    ].join('\n'),
+    ecmaFeatures: {
+      classes: true,
+      modules: true,
+      jsx: true
+    }
   }],
 
   invalid: [{
@@ -99,6 +113,28 @@ ruleTester.run('no-direct-mutation-state', rule, {
     errors: [{
       message: 'Do not mutate state directly. Use setState().'
     }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  render: function() {',
+      '    this.state.person.name.first = "bar"',
+      '    this.state.person.name.last = "baz"',
+      '    return <div>Hello</div>;',
+      '  }',
+      '});'
+    ].join('\n'),
+    ecmaFeatures: {
+      jsx: true
+    },
+    errors: [{
+      message: 'Do not mutate state directly. Use setState().',
+      line: 3,
+      column: 5
+    }, {
+      message: 'Do not mutate state directly. Use setState().',
+      line: 4,
+      column: 5
+    }]
   }
   /**
    * Would be nice to prevent this too