From 17f95664ad831374b7f5adf51d748c96212b45d2 Mon Sep 17 00:00:00 2001 From: William Binns-Smith Date: Sun, 12 Mar 2017 18:02:00 -0700 Subject: [PATCH] Implement React.createClass support and use Component util --- lib/rules/no-unused-state.js | 115 +++++++++++++++++++---------- tests/lib/rules/no-unused-state.js | 101 +++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 38 deletions(-) diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js index 248e49a2ea..d8bd433505 100644 --- a/lib/rules/no-unused-state.js +++ b/lib/rules/no-unused-state.js @@ -9,6 +9,8 @@ 'use strict'; +var Components = require('../util/Components'); + // Descend through all wrapping TypeCastExpressions and return the expression // that was cast. function uncast(node) { @@ -31,19 +33,27 @@ function getName(node) { 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'; } +function getInitialClassInfo() { + return { + // Set of nodes where state fields were defined. + stateFields: [], + + // Set of names of state fields that we've seen used. + usedStateFields: [], + + // 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: { @@ -54,7 +64,7 @@ module.exports = { schema: [] }, - create: function(context) { + create: Components.detect(function(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 @@ -142,28 +152,37 @@ module.exports = { } } + function reportUnusedFields() { + // Report all unused state fields. + classInfo.stateFields.forEach(function(node) { + var name = getName(node.key); + if (classInfo.usedStateFields.indexOf(name) < 0) { + context.report(node, 'Unused state field: \'' + name + '\''); + } + }); + } + return { ClassDeclaration: function(node) { - // Simple heuristic for determining whether we're in a React component. - var isReactComponent = node.body.body.some(function(child) { - return isMethodDefinitionWithName(child, 'render'); - }); - - if (isReactComponent) { - classInfo = { - // Set of nodes where state fields were defined. - stateFields: [], - - // Set of names of state fields that we've seen used. - usedStateFields: [], - - // 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 - }; + if (utils.isES6Component(node)) { + classInfo = getInitialClassInfo(); + } + }, + + ObjectExpression: function(node) { + if (utils.isES5Component(node)) { + classInfo = getInitialClassInfo(); + } + }, + + 'ObjectExpression:exit': function(node) { + if (!classInfo) { + return; + } + + if (utils.isES5Component(node)) { + reportUnusedFields(); + classInfo = null; } }, @@ -171,13 +190,7 @@ module.exports = { if (!classInfo) { return; } - // Report all unused state fields. - classInfo.stateFields.forEach(function(node) { - var name = getName(node.key); - if (classInfo.usedStateFields.indexOf(name) < 0) { - context.report(node, 'Unused state field: \'' + name + '\''); - } - }); + reportUnusedFields(); classInfo = null; }, @@ -230,6 +243,32 @@ module.exports = { classInfo.aliases = null; }, + FunctionExpression: function(node) { + if (!classInfo) { + return; + } + + var parent = node.parent; + if (!utils.isES5Component(parent.parent)) { + return; + } + + if (parent.key.name === 'getInitialState') { + var body = node.body.body; + var 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 = []; + } + }, + AssignmentExpression: function(node) { if (!classInfo) { return; @@ -295,5 +334,5 @@ module.exports = { } } }; - } + }) }; diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js index 5c767a5624..d0b9c40640 100644 --- a/tests/lib/rules/no-unused-state.js +++ b/tests/lib/rules/no-unused-state.js @@ -29,6 +29,68 @@ function getErrorMessages(unusedFields) { eslintTester.run('no-unused-state', rule, { valid: [ + [ + 'function StatelessFnUnaffectedTest(props) {', + ' return ;', + '};' + ].join('\n'), + [ + 'var NoStateTest = React.createClass({', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + [ + 'var NoStateMethodTest = React.createClass({', + ' render() {', + ' return ;', + ' }', + '});' + ].join('\n'), + [ + 'var GetInitialStateTest = React.createClass({', + ' getInitialState: function() {', + ' return { foo: 0 };', + ' },', + ' render: function() {', + ' return ;', + ' }', + '});' + ].join('\n'), + [ + 'var GetInitialStateMethodTest = React.createClass({', + ' getInitialState() {', + ' return { foo: 0 };', + ' },', + ' render() {', + ' return ;', + ' }', + '});' + ].join('\n'), + [ + 'var SetStateTest = React.createClass({', + ' onFooChange(newFoo) {', + ' this.setState({ foo: newFoo });', + ' },', + ' render() {', + ' return ;', + ' }', + '});' + ].join('\n'), + [ + 'var MultipleSetState = React.createClass({', + ' getInitialState() {', + ' return { foo: 0 };', + ' },', + ' update() {', + ' this.setState({foo: 1});', + ' },', + ' render() {', + ' return ;', + ' }', + '});' + ].join('\n'), [ 'class NoStateTest extends React.Component {', ' render() {', @@ -262,6 +324,45 @@ eslintTester.run('no-unused-state', rule, { ], invalid: [ + { + code: [ + 'var UnusedGetInitialStateTest = React.createClass({', + ' getInitialState: function() {', + ' return { foo: 0 };', + ' },', + ' render: function() {', + ' return ;', + ' }', + '})' + ].join('\n'), + errors: getErrorMessages(['foo']) + }, + { + code: [ + 'var UnusedGetInitialStateMethodTest = React.createClass({', + ' getInitialState() {', + ' return { foo: 0 };', + ' },', + ' render() {', + ' return ;', + ' }', + '})' + ].join('\n'), + errors: getErrorMessages(['foo']) + }, + { + code: [ + 'var UnusedSetStateTest = React.createClass({', + ' onFooChange(newFoo) {', + ' this.setState({ foo: newFoo });', + ' },', + ' render() {', + ' return ;', + ' }', + '});' + ].join('\n'), + errors: getErrorMessages(['foo']) + }, { code: [ 'class UnusedCtorStateTest extends React.Component {',