diff --git a/.eslintrc b/.eslintrc index 0667d9ddb5..2c320fb92b 100644 --- a/.eslintrc +++ b/.eslintrc @@ -1,5 +1,6 @@ { "env": { + "es6": true, "node": true }, ecmaFeatures: { diff --git a/index.js b/index.js index 914e4433f3..ff568c10ab 100644 --- a/index.js +++ b/index.js @@ -63,7 +63,8 @@ var allRules = { 'no-comment-textnodes': require('./lib/rules/no-comment-textnodes'), 'require-extension': require('./lib/rules/require-extension'), 'wrap-multilines': require('./lib/rules/wrap-multilines'), - 'jsx-tag-spacing': require('./lib/rules/jsx-tag-spacing') + 'jsx-tag-spacing': require('./lib/rules/jsx-tag-spacing'), + '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..c58bbf242a --- /dev/null +++ b/lib/rules/no-unused-state.js @@ -0,0 +1,299 @@ +/** + * @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'; + +// 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 isMethodDefinitionWithName(node, name, isStatic) { + isStatic = isStatic || false; + return ( + node.type === 'MethodDefinition' && + node.static === isStatic && + getName(node.key) === name + ); +} + +function isThisExpression(node) { + return uncast(node).type === 'ThisExpression'; +} + +module.exports = { + meta: { + docs: { + description: 'Prevent definition of unused state fields', + category: 'Best Practices', + recommended: false + }, + schema: [] + }, + + create: function(context) { + // 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 (let 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) { + let 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 (let 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 (let prop of left.properties) { + if (prop.type === 'Property' && getName(prop.key) === 'state') { + let name = getName(prop.value); + if (name) { + classInfo.aliases.add(name); + } else if (prop.value.type === 'ObjectPattern') { + handleStateDestructuring(prop.value); + } + } + } + } + break; + default: + // pass + } + } + + return { + ClassDeclaration(node) { + // Simple heuristic for determining whether we're in a React component. + const isReactComponent = node.body.body.some( + child => isMethodDefinitionWithName(child, 'render') + ); + + if (isReactComponent) { + classInfo = { + // 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 + }; + } + }, + + 'ClassDeclaration:exit'() { + if (!classInfo) { + return; + } + // Report all unused state fields. + for (let node of classInfo.stateFields) { + let name = getName(node.key); + if (!classInfo.usedStateFields.has(name)) { + context.report(node, `Unused state field: '${name}'`); + } + } + 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; + }, + + 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..ed685aa21f --- /dev/null +++ b/tests/lib/rules/no-unused-state.js @@ -0,0 +1,414 @@ +/** + * @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 + } +}; + +const eslintTester = new RuleTester({parserOptions, parser: 'babel-eslint'}); + +function getErrorMessages(unusedFields) { + return unusedFields.map(field => ({ + message: `Unused state field: '${field}'` + })); +} + +eslintTester.run('no-unused-state', rule, { + valid: [ + ` + 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 ; + } + } + `, + ` + class ClassPropertyStateTest extends React.Component { + state = { foo: 0 }; + render() { + return ; + } + } + `, + ` + 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 ; + } + } + `, + ` + class DeepDestructuringTest extends React.Component { + state = { foo: 0, bar: 0 }; + render() { + const {state: {foo, ...others}} = this; + return ; + } + } + `, + // 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 ; + } + } + `, + ` + class TypeCastExpressionSpreadFalseNegativeTest extends React.Component { + constructor() { + this.state = { foo: 0 }; + } + render() { + return ; + } + } + ` + ], + + invalid: [ + { + 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']) + }, + { + 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']) + }, + { + code: ` + class UnusedDeepDestructuringTest extends React.Component { + state = { foo: 0, bar: 0 }; + render() { + const {state: {foo}} = this; + return ; + } + } + `, + errors: getErrorMessages(['bar']) + } + ] +});