From e8205386a2c0234793621c21f4612c3acac23fd2 Mon Sep 17 00:00:00 2001 From: William Binns-Smith Date: Tue, 7 Mar 2017 12:20:53 -0800 Subject: [PATCH 1/6] Add no-unused-state 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 @jlharb'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()`. --- index.js | 3 +- lib/rules/no-unused-state.js | 299 +++++++++++++++++++++ tests/lib/rules/no-unused-state.js | 414 +++++++++++++++++++++++++++++ 3 files changed, 715 insertions(+), 1 deletion(-) create mode 100644 lib/rules/no-unused-state.js create mode 100644 tests/lib/rules/no-unused-state.js 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..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']) + } + ] +}); From b3e911fe98ccfab1aa8574c4d118af45261e8d24 Mon Sep 17 00:00:00 2001 From: William Binns-Smith Date: Thu, 9 Mar 2017 16:33:07 -0800 Subject: [PATCH 2/6] Port to node 0.10 --- lib/rules/no-unused-state.js | 80 ++-- tests/lib/rules/no-unused-state.js | 721 +++++++++++++++-------------- 2 files changed, 403 insertions(+), 398 deletions(-) diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js index c58bbf242a..248e49a2ea 100644 --- a/lib/rules/no-unused-state.js +++ b/lib/rules/no-unused-state.js @@ -59,21 +59,21 @@ module.exports = { // 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; + var classInfo = null; // Returns true if the given node is possibly a reference to `this.state`. function isStateReference(node) { node = uncast(node); - const isDirectStateReference = + var isDirectStateReference = node.type === 'MemberExpression' && isThisExpression(node.object) && node.property.name === 'state'; - const isAliasedStateReference = + var isAliasedStateReference = node.type === 'Identifier' && classInfo.aliases && - classInfo.aliases.has(node.name); + classInfo.aliases.indexOf(node.name) >= 0; return isDirectStateReference || isAliasedStateReference; } @@ -81,35 +81,35 @@ module.exports = { // 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) { + node.properties.forEach(function(prop) { if (prop.type === 'Property' && getName(prop.key) !== null) { - classInfo.stateFields.add(prop); + classInfo.stateFields.push(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); + var name = getName(node); if (name) { - classInfo.usedStateFields.add(name); + classInfo.usedStateFields.push(name); } } // Records used state fields and new aliases for an ObjectPattern which // destructures `this.state`. function handleStateDestructuring(node) { - for (let prop of node.properties) { + node.properties.forEach(function(prop) { if (prop.type === 'Property') { addUsedStateField(prop.key); } else if ( prop.type === 'ExperimentalRestProperty' && classInfo.aliases ) { - classInfo.aliases.add(getName(prop.argument)); + classInfo.aliases.push(getName(prop.argument)); } - } + }); } // Used to record used state fields and new aliases for both @@ -118,23 +118,23 @@ module.exports = { switch (left.type) { case 'Identifier': if (isStateReference(right) && classInfo.aliases) { - classInfo.aliases.add(left.name); + classInfo.aliases.push(left.name); } break; case 'ObjectPattern': if (isStateReference(right)) { handleStateDestructuring(left); } else if (isThisExpression(right) && classInfo.aliases) { - for (let prop of left.properties) { + left.properties.forEach(function(prop) { if (prop.type === 'Property' && getName(prop.key) === 'state') { - let name = getName(prop.value); + var name = getName(prop.value); if (name) { - classInfo.aliases.add(name); + classInfo.aliases.push(name); } else if (prop.value.type === 'ObjectPattern') { handleStateDestructuring(prop.value); } } - } + }); } break; default: @@ -143,19 +143,19 @@ module.exports = { } return { - ClassDeclaration(node) { + ClassDeclaration: function(node) { // Simple heuristic for determining whether we're in a React component. - const isReactComponent = node.body.body.some( - child => isMethodDefinitionWithName(child, 'render') - ); + var isReactComponent = node.body.body.some(function(child) { + return isMethodDefinitionWithName(child, 'render'); + }); if (isReactComponent) { classInfo = { // Set of nodes where state fields were defined. - stateFields: new Set(), + stateFields: [], // Set of names of state fields that we've seen used. - usedStateFields: new Set(), + 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, @@ -167,21 +167,21 @@ module.exports = { } }, - 'ClassDeclaration:exit'() { + 'ClassDeclaration:exit': function() { 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.stateFields.forEach(function(node) { + var name = getName(node.key); + if (classInfo.usedStateFields.indexOf(name) < 0) { + context.report(node, 'Unused state field: \'' + name + '\''); } - } + }); classInfo = null; }, - CallExpression(node) { + CallExpression: function(node) { if (!classInfo) { return; } @@ -198,7 +198,7 @@ module.exports = { } }, - ClassProperty(node) { + ClassProperty: function(node) { if (!classInfo) { return; } @@ -214,15 +214,15 @@ module.exports = { } }, - MethodDefinition() { + MethodDefinition: function() { if (!classInfo) { return; } // Create a new set for this.state aliases local to this method. - classInfo.aliases = new Set(); + classInfo.aliases = []; }, - 'MethodDefinition:exit'() { + 'MethodDefinition:exit': function() { if (!classInfo) { return; } @@ -230,7 +230,7 @@ module.exports = { classInfo.aliases = null; }, - AssignmentExpression(node) { + AssignmentExpression: function(node) { if (!classInfo) { return; } @@ -242,7 +242,7 @@ module.exports = { node.right.type === 'ObjectExpression' ) { // Find the nearest function expression containing this assignment. - let fn = node; + var fn = node; while (fn.type !== 'FunctionExpression' && fn.parent) { fn = fn.parent; } @@ -261,14 +261,14 @@ module.exports = { } }, - VariableDeclarator(node) { + VariableDeclarator: function(node) { if (!classInfo || !node.init) { return; } handleAssignment(node.id, node.init); }, - MemberExpression(node) { + MemberExpression: function(node) { if (!classInfo) { return; } @@ -283,13 +283,13 @@ module.exports = { } }, - JSXSpreadAttribute(node) { + JSXSpreadAttribute: function(node) { if (classInfo && isStateReference(node.argument)) { classInfo = null; } }, - ExperimentalSpreadProperty(node) { + ExperimentalSpreadProperty: function(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 index ed685aa21f..5c767a5624 100644 --- a/tests/lib/rules/no-unused-state.js +++ b/tests/lib/rules/no-unused-state.js @@ -4,410 +4,415 @@ 'use strict'; -const rule = require('../../../lib/rules/no-unused-state'); -const RuleTester = require('eslint').RuleTester; +var rule = require('../../../lib/rules/no-unused-state'); +var RuleTester = require('eslint').RuleTester; -const parserOptions = { +var parserOptions = { ecmaVersion: 6, ecmaFeatures: { jsx: true } }; -const eslintTester = new RuleTester({parserOptions, parser: 'babel-eslint'}); +var eslintTester = new RuleTester({ + parserOptions: parserOptions, + parser: 'babel-eslint' +}); function getErrorMessages(unusedFields) { - return unusedFields.map(field => ({ - message: `Unused state field: '${field}'` - })); + return unusedFields.map(function(field) { + return { + 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 ; - } - } - `, + [ + 'class NoStateTest extends React.Component {', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class CtorStateTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class SetStateTest extends React.Component {', + ' onFooChange(newFoo) {', + ' this.setState({ foo: newFoo });', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class ClassPropertyStateTest extends React.Component {', + ' state = { foo: 0 };', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class VariableDeclarationTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const foo = this.state.foo;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class DestructuringTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const {foo: myFoo} = this.state;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class ShorthandDestructuringTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const {foo} = this.state;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class AliasDeclarationTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const state = this.state;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class AliasAssignmentTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' let state;', + ' state = this.state;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class DestructuringAliasTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const {state: myState} = this;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class ShorthandDestructuringAliasTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const {state} = this;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class RestPropertyTest extends React.Component {', + ' constructor() {', + ' this.state = {', + ' foo: 0,', + ' bar: 1,', + ' };', + ' }', + ' render() {', + ' const {foo, ...others} = this.state;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class DeepDestructuringTest extends React.Component {', + ' state = { foo: 0, bar: 0 };', + ' render() {', + ' const {state: {foo, ...others}} = this;', + ' return ;', + ' }', + '}' + ].join('\n'), // 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 ; - } - } - ` + [ + 'class MethodArgFalseNegativeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' consumeFoo(foo) {}', + ' render() {', + ' this.consumeFoo(this.state.foo)', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class AssignedToObjectFalseNegativeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const obj = { foo: this.state.foo, bar: 0 };', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class ComputedAccessFalseNegativeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0, bar: 1 };', + ' }', + ' render() {', + ' const bar = \'bar\';', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class JsxSpreadFalseNegativeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class AliasedJsxSpreadFalseNegativeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const state = this.state;', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + 'class ObjectSpreadFalseNegativeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const attrs = { ...this.state, foo: 1 };', + ' return ;', + ' }', + '}' + ].join('\n'), + [ + '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 ;', + ' }', + '}' + ].join('\n'), + [ + 'class TypeCastExpressionSpreadFalseNegativeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n') ], invalid: [ { - code: ` - class UnusedCtorStateTest extends React.Component { - constructor() { - this.state = { foo: 0 }; - } - render() { - return ; - } - } - `, + code: [ + 'class UnusedCtorStateTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), errors: getErrorMessages(['foo']) }, { - code: ` - class UnusedSetStateTest extends React.Component { - onFooChange(newFoo) { - this.setState({ foo: newFoo }); - } - render() { - return ; - } - } - `, + code: [ + 'class UnusedSetStateTest extends React.Component {', + ' onFooChange(newFoo) {', + ' this.setState({ foo: newFoo });', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), errors: getErrorMessages(['foo']) }, { - code: ` - class UnusedClassPropertyStateTest extends React.Component { - state = { foo: 0 }; - render() { - return ; - } - } - `, + code: [ + 'class UnusedClassPropertyStateTest extends React.Component {', + ' state = { foo: 0 };', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), errors: getErrorMessages(['foo']) }, { - code: ` - class UnusedStateWhenPropsAreSpreadTest extends React.Component { - constructor() { - this.state = { foo: 0 }; - } - render() { - return ; - } - } - `, + code: [ + 'class UnusedStateWhenPropsAreSpreadTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), 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; - } - } - `, + code: [ + 'class AliasOutOfScopeTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' render() {', + ' const state = this.state;', + ' return ;', + ' }', + ' someMethod() {', + ' const outOfScope = state.foo;', + ' }', + '}' + ].join('\n'), 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 ; - } - } - `, + code: [ + 'class MultipleErrorsTest extends React.Component {', + ' constructor() {', + ' this.state = {', + ' foo: 0,', + ' bar: 1,', + ' baz: 2,', + ' qux: 3,', + ' };', + ' }', + ' render() {', + ' let {state} = this;', + ' return ;', + ' }', + '}' + ].join('\n'), errors: getErrorMessages(['foo', 'bar']) }, { - code: ` - class MultipleErrorsForSameKeyTest extends React.Component { - constructor() { - this.state = { foo: 0 }; - } - onFooChange(newFoo) { - this.setState({ foo: newFoo }); - } - render() { - return ; - } - } - `, + code: [ + 'class MultipleErrorsForSameKeyTest extends React.Component {', + ' constructor() {', + ' this.state = { foo: 0 };', + ' }', + ' onFooChange(newFoo) {', + ' this.setState({ foo: newFoo });', + ' }', + ' render() {', + ' return ;', + ' }', + '}' + ].join('\n'), errors: getErrorMessages(['foo', 'foo']) }, { - code: ` - class UnusedRestPropertyFieldTest extends React.Component { - constructor() { - this.state = { - foo: 0, - bar: 1, - }; - } - render() { - const {bar, ...others} = this.state; - return ; - } - } - `, + code: [ + 'class UnusedRestPropertyFieldTest extends React.Component {', + ' constructor() {', + ' this.state = {', + ' foo: 0,', + ' bar: 1,', + ' };', + ' }', + ' render() {', + ' const {bar, ...others} = this.state;', + ' return ;', + ' }', + '}' + ].join('\n'), 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 ; - } - } - `, + 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 ;', + ' }', + '}' + ].join('\n'), errors: getErrorMessages(['qux']) }, { - code: ` - class UnusedDeepDestructuringTest extends React.Component { - state = { foo: 0, bar: 0 }; - render() { - const {state: {foo}} = this; - return ; - } - } - `, + code: [ + 'class UnusedDeepDestructuringTest extends React.Component {', + ' state = { foo: 0, bar: 0 };', + ' render() {', + ' const {state: {foo}} = this;', + ' return ;', + ' }', + '}' + ].join('\n'), errors: getErrorMessages(['bar']) } ] From 1022c24d45ca6195186a222bb8d4e8e2082cebb8 Mon Sep 17 00:00:00 2001 From: William Binns-Smith Date: Sun, 12 Mar 2017 18:02:00 -0700 Subject: [PATCH 3/6] 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 {', From d0973b4b227047e249ea89c4339ecb6bf2e4a53c Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Thu, 6 Jul 2017 23:21:50 -0700 Subject: [PATCH 4/6] Re-modernize and cleanup with node 4-compatible features --- lib/rules/no-unused-state.js | 92 ++-- tests/lib/rules/no-unused-state.js | 814 +++++++++++++---------------- 2 files changed, 412 insertions(+), 494 deletions(-) diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js index d8bd433505..ca26d18973 100644 --- a/lib/rules/no-unused-state.js +++ b/lib/rules/no-unused-state.js @@ -9,7 +9,7 @@ 'use strict'; -var Components = require('../util/Components'); +const Components = require('../util/Components'); // Descend through all wrapping TypeCastExpressions and return the expression // that was cast. @@ -40,10 +40,10 @@ function isThisExpression(node) { function getInitialClassInfo() { return { // Set of nodes where state fields were defined. - stateFields: [], + stateFields: new Set(), // Set of names of state fields that we've seen used. - usedStateFields: [], + 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, @@ -69,21 +69,21 @@ module.exports = { // 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. - var classInfo = null; + let classInfo = null; // Returns true if the given node is possibly a reference to `this.state`. function isStateReference(node) { node = uncast(node); - var isDirectStateReference = + const isDirectStateReference = node.type === 'MemberExpression' && isThisExpression(node.object) && node.property.name === 'state'; - var isAliasedStateReference = + const isAliasedStateReference = node.type === 'Identifier' && classInfo.aliases && - classInfo.aliases.indexOf(node.name) >= 0; + classInfo.aliases.has(node.name); return isDirectStateReference || isAliasedStateReference; } @@ -91,35 +91,35 @@ module.exports = { // Takes an ObjectExpression node and adds all named Property nodes to the // current set of state fields. function addStateFields(node) { - node.properties.forEach(function(prop) { + for (const prop of node.properties) { if (prop.type === 'Property' && getName(prop.key) !== null) { - classInfo.stateFields.push(prop); + 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) { - var name = getName(node); + const name = getName(node); if (name) { - classInfo.usedStateFields.push(name); + classInfo.usedStateFields.add(name); } } // Records used state fields and new aliases for an ObjectPattern which // destructures `this.state`. function handleStateDestructuring(node) { - node.properties.forEach(function(prop) { + for (const prop of node.properties) { if (prop.type === 'Property') { addUsedStateField(prop.key); } else if ( prop.type === 'ExperimentalRestProperty' && classInfo.aliases ) { - classInfo.aliases.push(getName(prop.argument)); + classInfo.aliases.add(getName(prop.argument)); } - }); + } } // Used to record used state fields and new aliases for both @@ -128,54 +128,54 @@ module.exports = { switch (left.type) { case 'Identifier': if (isStateReference(right) && classInfo.aliases) { - classInfo.aliases.push(left.name); + classInfo.aliases.add(left.name); } break; case 'ObjectPattern': if (isStateReference(right)) { handleStateDestructuring(left); } else if (isThisExpression(right) && classInfo.aliases) { - left.properties.forEach(function(prop) { + for (const prop of left.properties) { if (prop.type === 'Property' && getName(prop.key) === 'state') { - var name = getName(prop.value); + const name = getName(prop.value); if (name) { - classInfo.aliases.push(name); + classInfo.aliases.add(name); } else if (prop.value.type === 'ObjectPattern') { handleStateDestructuring(prop.value); } } - }); + } } break; default: - // pass + // pass } } 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 + '\''); + 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: function(node) { + ClassDeclaration(node) { if (utils.isES6Component(node)) { classInfo = getInitialClassInfo(); } }, - ObjectExpression: function(node) { + ObjectExpression(node) { if (utils.isES5Component(node)) { classInfo = getInitialClassInfo(); } }, - 'ObjectExpression:exit': function(node) { + 'ObjectExpression:exit'(node) { if (!classInfo) { return; } @@ -186,7 +186,7 @@ module.exports = { } }, - 'ClassDeclaration:exit': function() { + 'ClassDeclaration:exit'() { if (!classInfo) { return; } @@ -194,7 +194,7 @@ module.exports = { classInfo = null; }, - CallExpression: function(node) { + CallExpression(node) { if (!classInfo) { return; } @@ -211,7 +211,7 @@ module.exports = { } }, - ClassProperty: function(node) { + ClassProperty(node) { if (!classInfo) { return; } @@ -227,15 +227,15 @@ module.exports = { } }, - MethodDefinition: function() { + MethodDefinition() { if (!classInfo) { return; } // Create a new set for this.state aliases local to this method. - classInfo.aliases = []; + classInfo.aliases = new Set(); }, - 'MethodDefinition:exit': function() { + 'MethodDefinition:exit'() { if (!classInfo) { return; } @@ -243,19 +243,19 @@ module.exports = { classInfo.aliases = null; }, - FunctionExpression: function(node) { + FunctionExpression(node) { if (!classInfo) { return; } - var parent = node.parent; + const 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]; + const body = node.body.body; + const lastBodyNode = body[body.length - 1]; if ( lastBodyNode.type === 'ReturnStatement' && @@ -265,11 +265,11 @@ module.exports = { } } else { // Create a new set for this.state aliases local to this method. - classInfo.aliases = []; + classInfo.aliases = new Set(); } }, - AssignmentExpression: function(node) { + AssignmentExpression(node) { if (!classInfo) { return; } @@ -281,7 +281,7 @@ module.exports = { node.right.type === 'ObjectExpression' ) { // Find the nearest function expression containing this assignment. - var fn = node; + let fn = node; while (fn.type !== 'FunctionExpression' && fn.parent) { fn = fn.parent; } @@ -300,14 +300,14 @@ module.exports = { } }, - VariableDeclarator: function(node) { + VariableDeclarator(node) { if (!classInfo || !node.init) { return; } handleAssignment(node.id, node.init); }, - MemberExpression: function(node) { + MemberExpression(node) { if (!classInfo) { return; } @@ -322,13 +322,13 @@ module.exports = { } }, - JSXSpreadAttribute: function(node) { + JSXSpreadAttribute(node) { if (classInfo && isStateReference(node.argument)) { classInfo = null; } }, - ExperimentalSpreadProperty: function(node) { + 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 index d0b9c40640..6465950c12 100644 --- a/tests/lib/rules/no-unused-state.js +++ b/tests/lib/rules/no-unused-state.js @@ -4,17 +4,17 @@ 'use strict'; -var rule = require('../../../lib/rules/no-unused-state'); -var RuleTester = require('eslint').RuleTester; +const rule = require('../../../lib/rules/no-unused-state'); +const RuleTester = require('eslint').RuleTester; -var parserOptions = { +const parserOptions = { ecmaVersion: 6, ecmaFeatures: { jsx: true } }; -var eslintTester = new RuleTester({ +const eslintTester = new RuleTester({ parserOptions: parserOptions, parser: 'babel-eslint' }); @@ -22,498 +22,416 @@ var eslintTester = new RuleTester({ function getErrorMessages(unusedFields) { return unusedFields.map(function(field) { return { - message: 'Unused state field: \'' + field + '\'' + message: `Unused state field: '${field}'` }; }); } 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() {', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class CtorStateTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class SetStateTest extends React.Component {', - ' onFooChange(newFoo) {', - ' this.setState({ foo: newFoo });', - ' }', - ' render() {', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class ClassPropertyStateTest extends React.Component {', - ' state = { foo: 0 };', - ' render() {', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class VariableDeclarationTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const foo = this.state.foo;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class DestructuringTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const {foo: myFoo} = this.state;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class ShorthandDestructuringTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const {foo} = this.state;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class AliasDeclarationTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const state = this.state;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class AliasAssignmentTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' let state;', - ' state = this.state;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class DestructuringAliasTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const {state: myState} = this;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class ShorthandDestructuringAliasTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const {state} = this;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class RestPropertyTest extends React.Component {', - ' constructor() {', - ' this.state = {', - ' foo: 0,', - ' bar: 1,', - ' };', - ' }', - ' render() {', - ' const {foo, ...others} = this.state;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class DeepDestructuringTest extends React.Component {', - ' state = { foo: 0, bar: 0 };', - ' render() {', - ' const {state: {foo, ...others}} = this;', - ' return ;', - ' }', - '}' - ].join('\n'), + `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 ; + } + }`, + `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 ;', - ' }', - '}' - ].join('\n'), - [ - 'class AssignedToObjectFalseNegativeTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const obj = { foo: this.state.foo, bar: 0 };', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class ComputedAccessFalseNegativeTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0, bar: 1 };', - ' }', - ' render() {', - ' const bar = \'bar\';', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class JsxSpreadFalseNegativeTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class AliasedJsxSpreadFalseNegativeTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const state = this.state;', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - 'class ObjectSpreadFalseNegativeTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' const attrs = { ...this.state, foo: 1 };', - ' return ;', - ' }', - '}' - ].join('\n'), - [ - '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 ;', - ' }', - '}' - ].join('\n'), - [ - 'class TypeCastExpressionSpreadFalseNegativeTest extends React.Component {', - ' constructor() {', - ' this.state = { foo: 0 };', - ' }', - ' render() {', - ' return ;', - ' }', - '}' - ].join('\n') + `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: [ - 'var UnusedGetInitialStateTest = React.createClass({', - ' getInitialState: function() {', - ' return { foo: 0 };', - ' },', - ' render: function() {', - ' return ;', - ' }', - '})' - ].join('\n'), + code: `var UnusedGetInitialStateTest = createReactClass({ + getInitialState: function() { + return { foo: 0 }; + }, + render: function() { + return ; + } + })`, errors: getErrorMessages(['foo']) }, { - code: [ - 'var UnusedGetInitialStateMethodTest = React.createClass({', - ' getInitialState() {', - ' return { foo: 0 };', - ' },', - ' render() {', - ' return ;', - ' }', - '})' - ].join('\n'), + code: `var UnusedGetInitialStateMethodTest = createReactClass({ + getInitialState() { + return { foo: 0 }; + }, + render() { + return ; + } + })`, errors: getErrorMessages(['foo']) }, { - code: [ - 'var UnusedSetStateTest = React.createClass({', - ' onFooChange(newFoo) {', - ' this.setState({ foo: newFoo });', - ' },', - ' render() {', - ' return ;', - ' }', - '});' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + 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 ;', - ' }', - '}' - ].join('\n'), + code: `class UnusedDeepDestructuringTest extends React.Component { + state = { foo: 0, bar: 0 }; + render() { + const {state: {foo}} = this; + return ; + } + }`, errors: getErrorMessages(['bar']) } ] From 0dd50b22348334feedee487e248fde5737e38171 Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Tue, 11 Jul 2017 20:19:38 -0700 Subject: [PATCH 5/6] Only use babel-eslint when necessary for tests --- tests/lib/rules/no-unused-state.js | 44 ++++++++++++++++++------------ 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js index 6465950c12..a4bf1d8761 100644 --- a/tests/lib/rules/no-unused-state.js +++ b/tests/lib/rules/no-unused-state.js @@ -10,14 +10,12 @@ const RuleTester = require('eslint').RuleTester; const parserOptions = { ecmaVersion: 6, ecmaFeatures: { - jsx: true + jsx: true, + experimentalObjectRestSpread: true } }; -const eslintTester = new RuleTester({ - parserOptions: parserOptions, - parser: 'babel-eslint' -}); +const eslintTester = new RuleTester({parserOptions}); function getErrorMessages(unusedFields) { return unusedFields.map(function(field) { @@ -98,12 +96,15 @@ eslintTester.run('no-unused-state', rule, { return ; } }`, - `class ClassPropertyStateTest extends React.Component { - state = { foo: 0 }; - 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 }; @@ -180,13 +181,16 @@ eslintTester.run('no-unused-state', rule, { return ; } }`, - `class DeepDestructuringTest extends React.Component { + { + 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 { @@ -257,14 +261,17 @@ eslintTester.run('no-unused-state', rule, { return ; } }`, - `class TypeCastExpressionSpreadFalseNegativeTest extends React.Component { + { + code: `class TypeCastExpressionSpreadFalseNegativeTest extends React.Component { constructor() { this.state = { foo: 0 }; } render() { return ; } - }` + }`, + parser: 'babel-eslint' + } ], invalid: [ @@ -330,7 +337,8 @@ eslintTester.run('no-unused-state', rule, { return ; } }`, - errors: getErrorMessages(['foo']) + errors: getErrorMessages(['foo']), + parser: 'babel-eslint' }, { code: `class UnusedStateWhenPropsAreSpreadTest extends React.Component { @@ -422,7 +430,8 @@ eslintTester.run('no-unused-state', rule, { return ; } }`, - errors: getErrorMessages(['qux']) + errors: getErrorMessages(['qux']), + parser: 'babel-eslint' }, { code: `class UnusedDeepDestructuringTest extends React.Component { @@ -432,7 +441,8 @@ eslintTester.run('no-unused-state', rule, { return ; } }`, - errors: getErrorMessages(['bar']) + errors: getErrorMessages(['bar']), + parser: 'babel-eslint' } ] }); From ba1dfc1fd66f85d2391fa38d4e51ea0680c14dc2 Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Fri, 21 Jul 2017 10:41:41 -0700 Subject: [PATCH 6/6] use arrow fns --- lib/rules/no-unused-state.js | 2 +- tests/lib/rules/no-unused-state.js | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js index ca26d18973..41f333af9a 100644 --- a/lib/rules/no-unused-state.js +++ b/lib/rules/no-unused-state.js @@ -64,7 +64,7 @@ module.exports = { schema: [] }, - create: Components.detect(function(context, components, utils) { + 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 diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js index a4bf1d8761..c5e60d89a2 100644 --- a/tests/lib/rules/no-unused-state.js +++ b/tests/lib/rules/no-unused-state.js @@ -18,11 +18,9 @@ const parserOptions = { const eslintTester = new RuleTester({parserOptions}); function getErrorMessages(unusedFields) { - return unusedFields.map(function(field) { - return { - message: `Unused state field: '${field}'` - }; - }); + return unusedFields.map(field => ({ + message: `Unused state field: '${field}'` + })); } eslintTester.run('no-unused-state', rule, {