Repository: yannickcr/eslint-plugin-react
Pull request #1103 (2017-03-07 20:23:40)

Add no-unused-state
Link to significant commit [is here](https://github.com/wbinnssmith/eslint-plugin-react/commit/1e5b989cf7299962e62e65574241b7288120dd61)

This adds a new rule, react/no-unused-state, which discovers state fields in a React component and warns if any of them are never read. It was developed internally at Facebook by @rjbailey and has been in use throughout Facebook's JS for a couple months now. It was written against a modern version of node, so has been rebased on @ljharb's branch dropping support for node < 4.

It currently supports es2015 classes extending `React.Component`  (no support currently for `React.createClass()`) and can detect when state is as `this.state = {}`, assigning state in a property initializer, and when calling `this.setState()`.

wbinnssmith: I'm happy to port down to node 0.10 to get this merged sooner, but would rather #1038 be merged :)
ljharb: Yes, please do that. It will take much longer if it needs to wait for a major release, and this PR's diff will be cleaner.
wbinnssmith: Rebased and ported back to node 0.10

How do you feel about the lack of support for `React.createClass()`?
ianobermiller: > It currently supports es2015 classes extending React.Component

To clarify, it actually supports ANY class that has a `render` method, no matter what it extends from.
wbinnssmith: Addressed the feedback and implemented createClass support. Components util made quick work of it -- awesome.
ljharb: @yannickcr @lencioni @EvNaverniouk ping for review
danielheyman: Any update on this rule?
ljharb: @wbinnssmith would you mind freshly rebasing this? after that, i'll give it a rereview, and then merge ASAP if there's no objections from other collabs.
wbinnssmith: @ljharb no problem :)
wbinnssmith: okay @ljharb, just rebased, cleaned things up, and moved some stuff back to node 4 compatible features (where this all started 😂)
wbinnssmith: I can split the tests up using multiple RuleTesters, separating those that parse js compatible with espree + es2015, and those that require babylon with support for experimental features enabled (property initializers and object spread, mainly). Does that sound useful?
ljharb: You don't need to create a new RuleTester instance - the constructor argument is just the default parser options; each valid/invalid example can also provide its own parser options.
wbinnssmith: @ljharb that's exactly what I ended up doing :)

ping @yannickcr @lencioni @EvHaus
wbinnssmith: ping! can @yannickcr @lencioni or @EvHaus look at this please?
ljharb: Sorry for the inconvenience; I've just merged a refactor to start using arrow functions. Would you mind rebasing?

I'll merge within the next day or two, if no objections, after that's done.

Patch:
diff --git a/index.js b/index.js
index 28d90464..2849bb64 100644
--- a/index.js
+++ b/index.js
@@ -66,7 +66,8 @@ const allRules = {
   'jsx-tag-spacing': require('./lib/rules/jsx-tag-spacing'),
   'no-redundant-should-component-update': require('./lib/rules/no-redundant-should-component-update'),
   'boolean-prop-naming': require('./lib/rules/boolean-prop-naming'),
-  'no-typos': require('./lib/rules/no-typos')
+  'no-typos': require('./lib/rules/no-typos'),
+  'no-unused-state': require('./lib/rules/no-unused-state')
 };
 
 function filterRules(rules, predicate) {
diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js
new file mode 100644
index 00000000..41f333af
--- /dev/null
+++ b/lib/rules/no-unused-state.js
@@ -0,0 +1,338 @@
+/**
+ * @fileoverview  Attempts to discover all state fields in a React component and
+ * warn if any of them are never read.
+ *
+ * State field definitions are collected from `this.state = {}` assignments in
+ * the constructor, objects passed to `this.setState()`, and `state = {}` class
+ * property assignments.
+ */
+
+'use strict';
+
+const Components = require('../util/Components');
+
+// Descend through all wrapping TypeCastExpressions and return the expression
+// that was cast.
+function uncast(node) {
+  while (node.type === 'TypeCastExpression') {
+    node = node.expression;
+  }
+  return node;
+}
+
+// Return the name of an identifier or the string value of a literal. Useful
+// anywhere that a literal may be used as a key (e.g., member expressions,
+// method definitions, ObjectExpression property keys).
+function getName(node) {
+  node = uncast(node);
+  if (node.type === 'Identifier') {
+    return node.name;
+  } else if (node.type === 'Literal') {
+    return String(node.value);
+  }
+  return null;
+}
+
+function isThisExpression(node) {
+  return uncast(node).type === 'ThisExpression';
+}
+
+function getInitialClassInfo() {
+  return {
+    // Set of nodes where state fields were defined.
+    stateFields: new Set(),
+
+    // Set of names of state fields that we've seen used.
+    usedStateFields: new Set(),
+
+    // Names of local variables that may be pointing to this.state. To
+    // track this properly, we would need to keep track of all locals,
+    // shadowing, assignments, etc. To keep things simple, we only
+    // maintain one set of aliases per method and accept that it will
+    // produce some false negatives.
+    aliases: null
+  };
+}
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Prevent definition of unused state fields',
+      category: 'Best Practices',
+      recommended: false
+    },
+    schema: []
+  },
+
+  create: Components.detect((context, components, utils) => {
+    // Non-null when we are inside a React component ClassDeclaration and we have
+    // not yet encountered any use of this.state which we have chosen not to
+    // analyze. If we encounter any such usage (like this.state being spread as
+    // JSX attributes), then this is again set to null.
+    let classInfo = null;
+
+    // Returns true if the given node is possibly a reference to `this.state`.
+    function isStateReference(node) {
+      node = uncast(node);
+
+      const isDirectStateReference =
+        node.type === 'MemberExpression' &&
+        isThisExpression(node.object) &&
+        node.property.name === 'state';
+
+      const isAliasedStateReference =
+        node.type === 'Identifier' &&
+        classInfo.aliases &&
+        classInfo.aliases.has(node.name);
+
+      return isDirectStateReference || isAliasedStateReference;
+    }
+
+    // Takes an ObjectExpression node and adds all named Property nodes to the
+    // current set of state fields.
+    function addStateFields(node) {
+      for (const prop of node.properties) {
+        if (prop.type === 'Property' && getName(prop.key) !== null) {
+          classInfo.stateFields.add(prop);
+        }
+      }
+    }
+
+    // Adds the name of the given node as a used state field if the node is an
+    // Identifier or a Literal. Other node types are ignored.
+    function addUsedStateField(node) {
+      const name = getName(node);
+      if (name) {
+        classInfo.usedStateFields.add(name);
+      }
+    }
+
+    // Records used state fields and new aliases for an ObjectPattern which
+    // destructures `this.state`.
+    function handleStateDestructuring(node) {
+      for (const prop of node.properties) {
+        if (prop.type === 'Property') {
+          addUsedStateField(prop.key);
+        } else if (
+          prop.type === 'ExperimentalRestProperty' &&
+          classInfo.aliases
+        ) {
+          classInfo.aliases.add(getName(prop.argument));
+        }
+      }
+    }
+
+    // Used to record used state fields and new aliases for both
+    // AssignmentExpressions and VariableDeclarators.
+    function handleAssignment(left, right) {
+      switch (left.type) {
+        case 'Identifier':
+          if (isStateReference(right) && classInfo.aliases) {
+            classInfo.aliases.add(left.name);
+          }
+          break;
+        case 'ObjectPattern':
+          if (isStateReference(right)) {
+            handleStateDestructuring(left);
+          } else if (isThisExpression(right) && classInfo.aliases) {
+            for (const prop of left.properties) {
+              if (prop.type === 'Property' && getName(prop.key) === 'state') {
+                const name = getName(prop.value);
+                if (name) {
+                  classInfo.aliases.add(name);
+                } else if (prop.value.type === 'ObjectPattern') {
+                  handleStateDestructuring(prop.value);
+                }
+              }
+            }
+          }
+          break;
+        default:
+        // pass
+      }
+    }
+
+    function reportUnusedFields() {
+      // Report all unused state fields.
+      for (const node of classInfo.stateFields) {
+        const name = getName(node.key);
+        if (!classInfo.usedStateFields.has(name)) {
+          context.report(node, `Unused state field: '${name}'`);
+        }
+      }
+    }
+
+    return {
+      ClassDeclaration(node) {
+        if (utils.isES6Component(node)) {
+          classInfo = getInitialClassInfo();
+        }
+      },
+
+      ObjectExpression(node) {
+        if (utils.isES5Component(node)) {
+          classInfo = getInitialClassInfo();
+        }
+      },
+
+      'ObjectExpression:exit'(node) {
+        if (!classInfo) {
+          return;
+        }
+
+        if (utils.isES5Component(node)) {
+          reportUnusedFields();
+          classInfo = null;
+        }
+      },
+
+      'ClassDeclaration:exit'() {
+        if (!classInfo) {
+          return;
+        }
+        reportUnusedFields();
+        classInfo = null;
+      },
+
+      CallExpression(node) {
+        if (!classInfo) {
+          return;
+        }
+        // If we're looking at a `this.setState({})` invocation, record all the
+        // properties as state fields.
+        if (
+          node.callee.type === 'MemberExpression' &&
+          isThisExpression(node.callee.object) &&
+          getName(node.callee.property) === 'setState' &&
+          node.arguments.length > 0 &&
+          node.arguments[0].type === 'ObjectExpression'
+        ) {
+          addStateFields(node.arguments[0]);
+        }
+      },
+
+      ClassProperty(node) {
+        if (!classInfo) {
+          return;
+        }
+        // If we see state being assigned as a class property using an object
+        // expression, record all the fields of that object as state fields.
+        if (
+          getName(node.key) === 'state' &&
+          !node.static &&
+          node.value &&
+          node.value.type === 'ObjectExpression'
+        ) {
+          addStateFields(node.value);
+        }
+      },
+
+      MethodDefinition() {
+        if (!classInfo) {
+          return;
+        }
+        // Create a new set for this.state aliases local to this method.
+        classInfo.aliases = new Set();
+      },
+
+      'MethodDefinition:exit'() {
+        if (!classInfo) {
+          return;
+        }
+        // Forget our set of local aliases.
+        classInfo.aliases = null;
+      },
+
+      FunctionExpression(node) {
+        if (!classInfo) {
+          return;
+        }
+
+        const parent = node.parent;
+        if (!utils.isES5Component(parent.parent)) {
+          return;
+        }
+
+        if (parent.key.name === 'getInitialState') {
+          const body = node.body.body;
+          const lastBodyNode = body[body.length - 1];
+
+          if (
+            lastBodyNode.type === 'ReturnStatement' &&
+            lastBodyNode.argument.type === 'ObjectExpression'
+          ) {
+            addStateFields(lastBodyNode.argument);
+          }
+        } else {
+          // Create a new set for this.state aliases local to this method.
+          classInfo.aliases = new Set();
+        }
+      },
+
+      AssignmentExpression(node) {
+        if (!classInfo) {
+          return;
+        }
+        // Check for assignments like `this.state = {}`
+        if (
+          node.left.type === 'MemberExpression' &&
+          isThisExpression(node.left.object) &&
+          getName(node.left.property) === 'state' &&
+          node.right.type === 'ObjectExpression'
+        ) {
+          // Find the nearest function expression containing this assignment.
+          let fn = node;
+          while (fn.type !== 'FunctionExpression' && fn.parent) {
+            fn = fn.parent;
+          }
+          // If the nearest containing function is the constructor, then we want
+          // to record all the assigned properties as state fields.
+          if (
+            fn.parent &&
+            fn.parent.type === 'MethodDefinition' &&
+            fn.parent.kind === 'constructor'
+          ) {
+            addStateFields(node.right);
+          }
+        } else {
+          // Check for assignments like `alias = this.state` and record the alias.
+          handleAssignment(node.left, node.right);
+        }
+      },
+
+      VariableDeclarator(node) {
+        if (!classInfo || !node.init) {
+          return;
+        }
+        handleAssignment(node.id, node.init);
+      },
+
+      MemberExpression(node) {
+        if (!classInfo) {
+          return;
+        }
+        if (isStateReference(node.object)) {
+          // If we see this.state[foo] access, give up.
+          if (node.computed && node.property.type !== 'Literal') {
+            classInfo = null;
+            return;
+          }
+          // Otherwise, record that we saw this property being accessed.
+          addUsedStateField(node.property);
+        }
+      },
+
+      JSXSpreadAttribute(node) {
+        if (classInfo && isStateReference(node.argument)) {
+          classInfo = null;
+        }
+      },
+
+      ExperimentalSpreadProperty(node) {
+        if (classInfo && isStateReference(node.argument)) {
+          classInfo = null;
+        }
+      }
+    };
+  })
+};


Test patch:
diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js
new file mode 100644
index 00000000..c5e60d89
--- /dev/null
+++ b/tests/lib/rules/no-unused-state.js
@@ -0,0 +1,446 @@
+/**
+ * @fileoverview Tests for no-unused-state
+ */
+
+'use strict';
+
+const rule = require('../../../lib/rules/no-unused-state');
+const RuleTester = require('eslint').RuleTester;
+
+const parserOptions = {
+  ecmaVersion: 6,
+  ecmaFeatures: {
+    jsx: true,
+    experimentalObjectRestSpread: true
+  }
+};
+
+const eslintTester = new RuleTester({parserOptions});
+
+function getErrorMessages(unusedFields) {
+  return unusedFields.map(field => ({
+    message: `Unused state field: '${field}'`
+  }));
+}
+
+eslintTester.run('no-unused-state', rule, {
+  valid: [
+    `function StatelessFnUnaffectedTest(props) {
+       return <SomeComponent foo={props.foo} />;
+    };`,
+    `var NoStateTest = createReactClass({
+      render: function() {
+        return <SomeComponent />;
+      }
+    });`,
+    `var NoStateMethodTest = createReactClass({
+      render() {
+        return <SomeComponent />;
+      }
+    });`,
+    `var GetInitialStateTest = createReactClass({
+      getInitialState: function() {
+        return { foo: 0 };
+      },
+      render: function() {
+        return <SomeComponent foo={this.state.foo} />;
+      }
+    });`,
+    `var GetInitialStateMethodTest = createReactClass({
+      getInitialState() {
+        return { foo: 0 };
+      },
+      render() {
+        return <SomeComponent foo={this.state.foo} />;
+      }
+    });`,
+    `var SetStateTest = createReactClass({
+      onFooChange(newFoo) {
+        this.setState({ foo: newFoo });
+      },
+      render() {
+        return <SomeComponent foo={this.state.foo} />;
+      }
+    });`,
+    `var MultipleSetState = createReactClass({
+      getInitialState() {
+        return { foo: 0 };
+      },
+      update() {
+        this.setState({foo: 1});
+      },
+      render() {
+        return <SomeComponent onClick={this.update} foo={this.state.foo} />;
+      }
+    });`,
+    `class NoStateTest extends React.Component {
+      render() {
+        return <SomeComponent />;
+      }
+    }`,
+    `class CtorStateTest extends React.Component {
+      constructor() {
+        this.state = { foo: 0 };
+      }
+      render() {
+        return <SomeComponent foo={this.state.foo} />;
+      }
+    }`,
+    `class SetStateTest extends React.Component {
+        onFooChange(newFoo) {
+          this.setState({ foo: newFoo });
+        }
+        render() {
+          return <SomeComponent foo={this.state.foo} />;
+        }
+      }`,
+    {
+      code: `class ClassPropertyStateTest extends React.Component {
+          state = { foo: 0 };
+          render() {
+            return <SomeComponent foo={this.state.foo} />;
+          }
+        }`,
+      parser: 'babel-eslint'
+    },
+    `class VariableDeclarationTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const foo = this.state.foo;
+          return <SomeComponent foo={foo} />;
+        }
+      }`,
+    `class DestructuringTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const {foo: myFoo} = this.state;
+          return <SomeComponent foo={myFoo} />;
+        }
+      }`,
+    `class ShorthandDestructuringTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const {foo} = this.state;
+          return <SomeComponent foo={foo} />;
+        }
+      }`,
+    `class AliasDeclarationTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const state = this.state;
+          return <SomeComponent foo={state.foo} />;
+        }
+      }`,
+    `class AliasAssignmentTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          let state;
+          state = this.state;
+          return <SomeComponent foo={state.foo} />;
+        }
+      }`,
+    `class DestructuringAliasTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const {state: myState} = this;
+          return <SomeComponent foo={myState.foo} />;
+        }
+      }`,
+    `class ShorthandDestructuringAliasTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const {state} = this;
+          return <SomeComponent foo={state.foo} />;
+        }
+      }`,
+    `class RestPropertyTest extends React.Component {
+        constructor() {
+          this.state = {
+            foo: 0,
+            bar: 1,
+          };
+        }
+        render() {
+          const {foo, ...others} = this.state;
+          return <SomeComponent foo={foo} bar={others.bar} />;
+        }
+      }`,
+    {
+      code: `class DeepDestructuringTest extends React.Component {
+        state = { foo: 0, bar: 0 };
+        render() {
+          const {state: {foo, ...others}} = this;
+          return <SomeComponent foo={foo} bar={others.bar} />;
+        }
+      }`,
+      parser: 'babel-eslint'
+    },
+    // A cleverer analysis might recognize that the following should be errors,
+    // but they're out of scope for this lint rule.
+    `class MethodArgFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        consumeFoo(foo) {}
+        render() {
+          this.consumeFoo(this.state.foo);
+          return <SomeComponent />;
+        }
+      }`,
+    `class AssignedToObjectFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const obj = { foo: this.state.foo, bar: 0 };
+          return <SomeComponent bar={obj.bar} />;
+        }
+      }`,
+    `class ComputedAccessFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0, bar: 1 };
+        }
+        render() {
+          const bar = \'bar\';
+          return <SomeComponent bar={this.state[bar]} />;
+        }
+      }`,
+    `class JsxSpreadFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          return <SomeComponent {...this.state} />;
+        }
+      }`,
+    `class AliasedJsxSpreadFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const state = this.state;
+          return <SomeComponent {...state} />;
+        }
+      }`,
+    `class ObjectSpreadFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const attrs = { ...this.state, foo: 1 };
+          return <SomeComponent foo={attrs.foo} />;
+        }
+      }`,
+    `class ShadowingFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          const state = this.state;
+          let foo;
+          {
+            const state = { foo: 5 };
+            foo = state.foo;
+          }
+          return <SomeComponent foo={foo} />;
+        }
+      }`,
+    {
+      code: `class TypeCastExpressionSpreadFalseNegativeTest extends React.Component {
+        constructor() {
+          this.state = { foo: 0 };
+        }
+        render() {
+          return <SomeComponent {...(this.state: any)} />;
+        }
+      }`,
+      parser: 'babel-eslint'
+    }
+  ],
+
+  invalid: [
+    {
+      code: `var UnusedGetInitialStateTest = createReactClass({
+          getInitialState: function() {
+            return { foo: 0 };
+          },
+          render: function() {
+            return <SomeComponent />;
+          }
+        })`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `var UnusedGetInitialStateMethodTest = createReactClass({
+          getInitialState() {
+            return { foo: 0 };
+          },
+          render() {
+            return <SomeComponent />;
+          }
+        })`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `var UnusedSetStateTest = createReactClass({
+          onFooChange(newFoo) {
+            this.setState({ foo: newFoo });
+          },
+          render() {
+            return <SomeComponent />;
+          }
+        });`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `class UnusedCtorStateTest extends React.Component {
+          constructor() {
+            this.state = { foo: 0 };
+          }
+          render() {
+            return <SomeComponent />;
+          }
+        }`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `class UnusedSetStateTest extends React.Component {
+          onFooChange(newFoo) {
+            this.setState({ foo: newFoo });
+          }
+          render() {
+            return <SomeComponent />;
+          }
+        }`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `class UnusedClassPropertyStateTest extends React.Component {
+          state = { foo: 0 };
+          render() {
+            return <SomeComponent />;
+          }
+        }`,
+      errors: getErrorMessages(['foo']),
+      parser: 'babel-eslint'
+    },
+    {
+      code: `class UnusedStateWhenPropsAreSpreadTest extends React.Component {
+          constructor() {
+            this.state = { foo: 0 };
+          }
+          render() {
+            return <SomeComponent {...this.props} />;
+          }
+        }`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `class AliasOutOfScopeTest extends React.Component {
+          constructor() {
+            this.state = { foo: 0 };
+          }
+          render() {
+            const state = this.state;
+            return <SomeComponent />;
+          }
+          someMethod() {
+            const outOfScope = state.foo;
+          }
+        }`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `class MultipleErrorsTest extends React.Component {
+          constructor() {
+            this.state = {
+              foo: 0,
+              bar: 1,
+              baz: 2,
+              qux: 3,
+            };
+          }
+          render() {
+            let {state} = this;
+            return <SomeComponent baz={state.baz} qux={state.qux} />;
+          }
+        }`,
+      errors: getErrorMessages(['foo', 'bar'])
+    },
+    {
+      code: `class MultipleErrorsForSameKeyTest extends React.Component {
+          constructor() {
+            this.state = { foo: 0 };
+          }
+          onFooChange(newFoo) {
+            this.setState({ foo: newFoo });
+          }
+          render() {
+            return <SomeComponent />;
+          }
+        }`,
+      errors: getErrorMessages(['foo', 'foo'])
+    },
+    {
+      code: `class UnusedRestPropertyFieldTest extends React.Component {
+          constructor() {
+            this.state = {
+              foo: 0,
+              bar: 1,
+            };
+          }
+          render() {
+            const {bar, ...others} = this.state;
+            return <SomeComponent bar={bar} />;
+          }
+        }`,
+      errors: getErrorMessages(['foo'])
+    },
+    {
+      code: `class TypeCastExpressionTest extends React.Component {
+          constructor() {
+            this.state = {
+              foo: 0,
+              bar: 1,
+              baz: 2,
+              qux: 3,
+            };
+          }
+          render() {
+            const foo = ((this: any).state: any).foo;
+            const {bar, ...others} = (this.state: any);
+            let baz;
+            baz = (others: any)[\'baz\'];
+            return <SomeComponent foo={foo} bar={bar} baz={baz} />;
+          }
+        }`,
+      errors: getErrorMessages(['qux']),
+      parser: 'babel-eslint'
+    },
+    {
+      code: `class UnusedDeepDestructuringTest extends React.Component {
+          state = { foo: 0, bar: 0 };
+          render() {
+            const {state: {foo}} = this;
+            return <SomeComponent foo={foo} />;
+          }
+        }`,
+      errors: getErrorMessages(['bar']),
+      parser: 'babel-eslint'
+    }
+  ]
+});