From 1ecb622532fdb8e649cfcb559867f41c07c28134 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 15 Jun 2018 21:27:03 -0700 Subject: [PATCH] Don't depend on state parameter name in no-unused-state Resolves #1759 --- lib/rules/no-unused-state.js | 43 ++++++++++--- tests/lib/rules/no-unused-state.js | 98 ++++++++++++++++++++++++++++-- 2 files changed, 127 insertions(+), 14 deletions(-) diff --git a/lib/rules/no-unused-state.js b/lib/rules/no-unused-state.js index 4d8348419c..7d771fe303 100644 --- a/lib/rules/no-unused-state.js +++ b/lib/rules/no-unused-state.js @@ -77,7 +77,38 @@ module.exports = { // 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`, `prevState` or `nextState`. + function isStateParameterReference(node) { + const classMethods = [ + 'shouldComponentUpdate', + 'componentWillUpdate', + 'UNSAFE_componentWillUpdate', + 'getSnapshotBeforeUpdate', + 'componentDidUpdate' + ]; + + let scope = context.getScope(); + while (scope) { + const parent = scope.block && scope.block.parent; + if ( + parent && + parent.type === 'MethodDefinition' && ( + parent.static && parent.key.name === 'getDerivedStateFromProps' || + classMethods.indexOf(parent.key.name !== -1) + ) && + parent.value.type === 'FunctionExpression' && + parent.value.params[1] && + parent.value.params[1].name === node.name + ) { + return true; + } + scope = scope.upper; + } + + return false; + } + + // Returns true if the given node is possibly a reference to `this.state` or the state parameter of + // a lifecycle method. function isStateReference(node) { node = uncast(node); @@ -91,15 +122,7 @@ module.exports = { classInfo.aliases && classInfo.aliases.has(node.name); - const isPrevStateReference = - node.type === 'Identifier' && - node.name === 'prevState'; - - const isNextStateReference = - node.type === 'Identifier' && - node.name === 'nextState'; - - return isDirectStateReference || isAliasedStateReference || isPrevStateReference || isNextStateReference; + return isDirectStateReference || isAliasedStateReference || isStateParameterReference(node); } // Takes an ObjectExpression node and adds all named Property nodes to the diff --git a/tests/lib/rules/no-unused-state.js b/tests/lib/rules/no-unused-state.js index e26244be56..8261ae708c 100644 --- a/tests/lib/rules/no-unused-state.js +++ b/tests/lib/rules/no-unused-state.js @@ -492,15 +492,15 @@ eslintTester.run('no-unused-state', rule, { parser: 'babel-eslint' }, { - code: `class ESLintExample extends Component { + code: `class GetDerivedStateFromPropsTest extends Component { constructor(props) { super(props); this.state = { id: 123, }; } - static getDerivedStateFromProps(nextProps, prevState) { - if (prevState.id === nextProps.id) { + static getDerivedStateFromProps(nextProps, otherState) { + if (otherState.id === nextProps.id) { return { selected: true, }; @@ -516,7 +516,29 @@ eslintTester.run('no-unused-state', rule, { parser: 'babel-eslint' }, { - code: `class ESLintExample extends Component { + code: `class ComponentDidUpdateTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123, + }; + } + + componentDidUpdate(someProps, someState) { + if (someState.id === someProps.id) { + doStuff(); + } + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + parser: 'babel-eslint' + }, + { + code: `class ShouldComponentUpdateTest extends Component { constructor(props) { super(props); this.state = { @@ -533,6 +555,27 @@ eslintTester.run('no-unused-state', rule, { } }`, parser: 'babel-eslint' + }, + { + code: `class NestedScopesTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123, + }; + } + shouldComponentUpdate(nextProps, nextState) { + return (function() { + return nextState.id === nextProps.id; + })(); + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + parser: 'babel-eslint' } ], @@ -824,6 +867,53 @@ eslintTester.run('no-unused-state', rule, { }`, errors: getErrorMessages(['bar']), parser: 'babel-eslint' + }, + { + code: `class FakePrevStateVariableTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123, + foo: 456 + }; + } + + componentDidUpdate(someProps, someState) { + if (someState.id === someProps.id) { + const prevState = { foo: 789 }; + console.log(prevState.foo); + } + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + errors: getErrorMessages(['foo']), + parser: 'babel-eslint' + }, + { + code: `class MissingStateParameterTest extends Component { + constructor(props) { + super(props); + this.state = { + id: 123 + }; + } + + componentDidUpdate(someProps) { + const prevState = { id: 456 }; + console.log(prevState.id); + } + render() { + return ( +

{this.state.selected ? 'Selected' : 'Not selected'}

+ ); + } + }`, + errors: getErrorMessages(['id']), + parser: 'babel-eslint' } ] });