diff --git a/index.js b/index.js index 28d904646f..2849bb6495 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 0000000000..41f333af9a --- /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; + } + } + }; + }) +}; diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js new file mode 100644 index 0000000000..c5e60d89a2 --- /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 ; + };`, + `var NoStateTest = createReactClass({ + render: function() { + return ; + } + });`, + `var NoStateMethodTest = createReactClass({ + render() { + return ; + } + });`, + `var GetInitialStateTest = createReactClass({ + getInitialState: function() { + return { foo: 0 }; + }, + render: function() { + return ; + } + });`, + `var GetInitialStateMethodTest = createReactClass({ + getInitialState() { + return { foo: 0 }; + }, + render() { + return ; + } + });`, + `var SetStateTest = createReactClass({ + onFooChange(newFoo) { + this.setState({ foo: newFoo }); + }, + render() { + return ; + } + });`, + `var MultipleSetState = createReactClass({ + getInitialState() { + return { foo: 0 }; + }, + update() { + this.setState({foo: 1}); + }, + render() { + return ; + } + });`, + `class NoStateTest extends React.Component { + render() { + return ; + } + }`, + `class CtorStateTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + return ; + } + }`, + `class SetStateTest extends React.Component { + onFooChange(newFoo) { + this.setState({ foo: newFoo }); + } + render() { + return ; + } + }`, + { + code: `class ClassPropertyStateTest extends React.Component { + state = { foo: 0 }; + render() { + return ; + } + }`, + parser: 'babel-eslint' + }, + `class VariableDeclarationTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const foo = this.state.foo; + return ; + } + }`, + `class DestructuringTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const {foo: myFoo} = this.state; + return ; + } + }`, + `class ShorthandDestructuringTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const {foo} = this.state; + return ; + } + }`, + `class AliasDeclarationTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const state = this.state; + return ; + } + }`, + `class AliasAssignmentTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + let state; + state = this.state; + return ; + } + }`, + `class DestructuringAliasTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const {state: myState} = this; + return ; + } + }`, + `class ShorthandDestructuringAliasTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const {state} = this; + return ; + } + }`, + `class RestPropertyTest extends React.Component { + constructor() { + this.state = { + foo: 0, + bar: 1, + }; + } + render() { + const {foo, ...others} = this.state; + return ; + } + }`, + { + code: `class DeepDestructuringTest extends React.Component { + state = { foo: 0, bar: 0 }; + render() { + const {state: {foo, ...others}} = this; + return ; + } + }`, + 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 ; + } + }`, + `class AssignedToObjectFalseNegativeTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const obj = { foo: this.state.foo, bar: 0 }; + return ; + } + }`, + `class ComputedAccessFalseNegativeTest extends React.Component { + constructor() { + this.state = { foo: 0, bar: 1 }; + } + render() { + const bar = \'bar\'; + return ; + } + }`, + `class JsxSpreadFalseNegativeTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + return ; + } + }`, + `class AliasedJsxSpreadFalseNegativeTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const state = this.state; + return ; + } + }`, + `class ObjectSpreadFalseNegativeTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const attrs = { ...this.state, foo: 1 }; + return ; + } + }`, + `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 ; + } + }`, + { + code: `class TypeCastExpressionSpreadFalseNegativeTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + return ; + } + }`, + parser: 'babel-eslint' + } + ], + + invalid: [ + { + code: `var UnusedGetInitialStateTest = createReactClass({ + getInitialState: function() { + return { foo: 0 }; + }, + render: function() { + return ; + } + })`, + errors: getErrorMessages(['foo']) + }, + { + code: `var UnusedGetInitialStateMethodTest = createReactClass({ + getInitialState() { + return { foo: 0 }; + }, + render() { + return ; + } + })`, + errors: getErrorMessages(['foo']) + }, + { + code: `var UnusedSetStateTest = createReactClass({ + onFooChange(newFoo) { + this.setState({ foo: newFoo }); + }, + render() { + return ; + } + });`, + errors: getErrorMessages(['foo']) + }, + { + code: `class UnusedCtorStateTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + return ; + } + }`, + errors: getErrorMessages(['foo']) + }, + { + code: `class UnusedSetStateTest extends React.Component { + onFooChange(newFoo) { + this.setState({ foo: newFoo }); + } + render() { + return ; + } + }`, + errors: getErrorMessages(['foo']) + }, + { + code: `class UnusedClassPropertyStateTest extends React.Component { + state = { foo: 0 }; + render() { + return ; + } + }`, + errors: getErrorMessages(['foo']), + parser: 'babel-eslint' + }, + { + code: `class UnusedStateWhenPropsAreSpreadTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + return ; + } + }`, + errors: getErrorMessages(['foo']) + }, + { + code: `class AliasOutOfScopeTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + const state = this.state; + return ; + } + 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 ; + } + }`, + errors: getErrorMessages(['foo', 'bar']) + }, + { + code: `class MultipleErrorsForSameKeyTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + onFooChange(newFoo) { + this.setState({ foo: newFoo }); + } + render() { + return ; + } + }`, + errors: getErrorMessages(['foo', 'foo']) + }, + { + code: `class UnusedRestPropertyFieldTest extends React.Component { + constructor() { + this.state = { + foo: 0, + bar: 1, + }; + } + render() { + const {bar, ...others} = this.state; + return ; + } + }`, + 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 ; + } + }`, + errors: getErrorMessages(['qux']), + parser: 'babel-eslint' + }, + { + code: `class UnusedDeepDestructuringTest extends React.Component { + state = { foo: 0, bar: 0 }; + render() { + const {state: {foo}} = this; + return ; + } + }`, + errors: getErrorMessages(['bar']), + parser: 'babel-eslint' + } + ] +});