Merge pull request #1945 from lukyth/lukyth/state-in-constructor

[New] Add `state-in-constructor` rule
jsx-eslint · Jan 21, 2019 · 790dd9f · 790dd9f
2 parents 57f0127 + 9415814
commit 790dd9f
Show file tree

Hide file tree

Showing 8 changed files with 516 additions and 28 deletions.
diff --git a/README.md b/README.md
@@ -134,6 +134,7 @@ Enable the rules that you would like to use.
 * [react/self-closing-comp](docs/rules/self-closing-comp.md): Prevent extra closing tags for components without children (fixable)
 * [react/sort-comp](docs/rules/sort-comp.md): Enforce component methods order (fixable)
 * [react/sort-prop-types](docs/rules/sort-prop-types.md): Enforce propTypes declarations alphabetical sorting
+* [react/state-in-constructor](docs/rules/state-in-constructor.md): Enforce the state initialization style to be either in a constructor or with a class property
 * [react/style-prop-object](docs/rules/style-prop-object.md): Enforce style prop value being an object
 * [react/void-dom-elements-no-children](docs/rules/void-dom-elements-no-children.md): Prevent void DOM elements (e.g. `<img />`, `<br />`) from receiving children
 

diff --git a/docs/rules/state-in-constructor.md b/docs/rules/state-in-constructor.md
@@ -0,0 +1,74 @@
+# Enforce state initialization style (react/state-in-constructor)
+
+This rule will enforce the state initialization style to be either in a constructor or with a class property.
+
+## Rule Options
+
+```js
+...
+"react/state-in-constructor": [<enabled>, <mode>]
+...
+```
+
+### `always` mode
+
+Will enforce the state initialization style to be in a constructor. This is the default mode.
+
+The following patterns are considered warnings:
+
+```jsx
+class Foo extends React.Component {
+  state = { bar: 0 }
+  render() {
+    return <div>Foo</div>
+  }
+}
+```
+
+The following patterns are **not** considered warnings:
+
+```jsx
+class Foo extends React.Component {
+  constructor(props) {
+    super(props)
+    this.state = { bar: 0 }
+  }
+  render() {
+    return <div>Foo</div>
+  }
+}
+```
+
+### `never` mode
+
+Will enforce the state initialization style to be with a class property.
+
+The following patterns are considered warnings:
+
+```jsx
+class Foo extends React.Component {
+  constructor(props) {
+    super(props)
+    this.state = { bar: 0 }
+  }
+  render() {
+    return <div>Foo</div>
+  }
+}
+```
+
+The following patterns are **not** considered warnings:
+
+```jsx
+class Foo extends React.Component {
+  state = { bar: 0 }
+  render() {
+    return <div>Foo</div>
+  }
+}
+```
+
+
+## When Not To Use It
+
+When the way a component state is being initialized doesn't matter.
diff --git a/index.js b/index.js
@@ -80,6 +80,7 @@ const allRules = {
   'self-closing-comp': require('./lib/rules/self-closing-comp'),
   'sort-comp': require('./lib/rules/sort-comp'),
   'sort-prop-types': require('./lib/rules/sort-prop-types'),
+  'state-in-constructor': require('./lib/rules/state-in-constructor'),
   'style-prop-object': require('./lib/rules/style-prop-object'),
   'void-dom-elements-no-children': require('./lib/rules/void-dom-elements-no-children')
 };

diff --git a/lib/rules/no-direct-mutation-state.js b/lib/rules/no-direct-mutation-state.js
@@ -59,15 +59,6 @@ module.exports = {
       return node;
     }
 
-    /**
-     * Determine if this MemberExpression is for `this.state`
-     * @param {Object} node The node to process
-     * @returns {Boolean}
-     */
-    function isStateMemberExpression(node) {
-      return node.object.type === 'ThisExpression' && node.property.name === 'state';
-    }
-
     /**
      * Determine if we should currently ignore assignments in this component.
      * @param {?Object} component The component to process
@@ -101,7 +92,7 @@ module.exports = {
           return;
         }
         const item = getOuterMemberExpression(node.left);
-        if (isStateMemberExpression(item)) {
+        if (utils.isStateMemberExpression(item)) {
           const mutations = (component && component.mutations) || [];
           mutations.push(node.left.object);
           components.set(node, {
@@ -117,7 +108,7 @@ module.exports = {
           return;
         }
         const item = getOuterMemberExpression(node.argument);
-        if (isStateMemberExpression(item)) {
+        if (utils.isStateMemberExpression(item)) {
           const mutations = (component && component.mutations) || [];
           mutations.push(item);
           components.set(node, {

diff --git a/lib/rules/state-in-constructor.js b/lib/rules/state-in-constructor.js
@@ -0,0 +1,58 @@
+/**
+ * @fileoverview Enforce the state initialization style to be either in a constructor or with a class property
+ * @author Kanitkorn Sujautra
+ */
+'use strict';
+
+const Components = require('../util/Components');
+const docsUrl = require('../util/docsUrl');
+
+// ------------------------------------------------------------------------------
+// Rule Definition
+// ------------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'State initialization in an ES6 class component should be in a constructor',
+      category: 'Stylistic Issues',
+      recommended: false,
+      url: docsUrl('state-in-constructor')
+    },
+    schema: [{
+      enum: ['always', 'never']
+    }]
+  },
+
+  create: Components.detect((context, components, utils) => {
+    const option = context.options[0] || 'always';
+    return {
+      ClassProperty: function (node) {
+        if (
+          option === 'always' &&
+          !node.static &&
+          node.key.name === 'state' &&
+          utils.getParentES6Component()
+        ) {
+          context.report({
+            node,
+            message: 'State initialization should be in a constructor'
+          });
+        }
+      },
+      AssignmentExpression(node) {
+        if (
+          option === 'never' &&
+          utils.isStateMemberExpression(node.left) &&
+          utils.inConstructor() &&
+          utils.getParentES6Component()
+        ) {
+          context.report({
+            node,
+            message: 'State initialization should be in a class property'
+          });
+        }
+      }
+    };
+  })
+};
diff --git a/lib/util/Components.js b/lib/util/Components.js
@@ -298,6 +298,30 @@ function componentRule(rule, context) {
       return calledOnPragma;
     },
 
+    /**
+     * Check if we are in a class constructor
+     * @return {boolean} true if we are in a class constructor, false if not
+     */
+    inConstructor: function() {
+      let scope = context.getScope();
+      while (scope) {
+        if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') {
+          return true;
+        }
+        scope = scope.upper;
+      }
+      return false;
+    },
+
+    /**
+     * Determine if the node is MemberExpression of `this.state`
+     * @param {Object} node The node to process
+     * @returns {Boolean}
+     */
+    isStateMemberExpression: function(node) {
+      return node.type === 'MemberExpression' && node.object.type === 'ThisExpression' && node.property.name === 'state';
+    },
+
     getReturnPropertyAndNode(ASTnode) {
       let property;
       let node = ASTnode;

diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js
@@ -200,21 +200,6 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
     return key.type === 'Identifier' ? key.name : key.value;
   }
 
-  /**
-   * Check if we are in a class constructor
-   * @return {boolean} true if we are in a class constructor, false if not
-   */
-  function inConstructor() {
-    let scope = context.getScope();
-    while (scope) {
-      if (scope.block && scope.block.parent && scope.block.parent.kind === 'constructor') {
-        return true;
-      }
-      scope = scope.upper;
-    }
-    return false;
-  }
-
   /**
    * Retrieve the name of a property node
    * @param {ASTNode} node The AST node with the property.
@@ -226,7 +211,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
     const isDirectPrevProp = DIRECT_PREV_PROPS_REGEX.test(sourceCode.getText(node));
     const isDirectSetStateProp = isPropArgumentInSetStateUpdater(node);
     const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component();
-    const isNotInConstructor = !inConstructor(node);
+    const isNotInConstructor = !utils.inConstructor(node);
     const isNotInLifeCycleMethod = !inLifeCycleMethod();
     const isNotInSetStateUpdater = !inSetStateUpdater();
     if ((isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp)
@@ -383,7 +368,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
           || DIRECT_NEXT_PROPS_REGEX.test(nodeSource)
           || DIRECT_PREV_PROPS_REGEX.test(nodeSource);
         const reportedNode = (
-          !isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ?
+          !isDirectProp && !utils.inConstructor() && !inComponentWillReceiveProps() ?
             node.parent.property :
             node.property
         );