From c471d775310ca7f0fb4f5341f44bd38efc79395a Mon Sep 17 00:00:00 2001 From: David Petersen Date: Sun, 5 Nov 2017 15:46:26 -0600 Subject: [PATCH] Fix no-unused-prop-types setState updater Handle when props are used in the setState update callback Fix #1506 --- lib/rules/no-unused-prop-types.js | 75 +++++++++++-- tests/lib/rules/no-unused-prop-types.js | 139 +++++++++++++++++++++--- 2 files changed, 191 insertions(+), 23 deletions(-) diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index 0471913737..f3c764be48 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -100,6 +100,48 @@ module.exports = { return false; } + /** + * Check if the current node is in a setState updater method + * @return {boolean} true if we are in a setState updater, false if not + */ + function inSetStateUpdater() { + let scope = context.getScope(); + while (scope) { + if ( + scope.block && scope.block.parent + && scope.block.parent.type === 'CallExpression' + && scope.block.parent.callee.property + && scope.block.parent.callee.property.name === 'setState' + // Make sure we are in the updater not the callback + && scope.block.parent.arguments[0].start === scope.block.start + ) { + return true; + } + scope = scope.upper; + } + return false; + } + + function isPropArgumentInSetStateUpdater(node) { + let scope = context.getScope(); + while (scope) { + if ( + scope.block && scope.block.parent + && scope.block.parent.type === 'CallExpression' + && scope.block.parent.callee.property + && scope.block.parent.callee.property.name === 'setState' + // Make sure we are in the updater not the callback + && scope.block.parent.arguments[0].start === scope.block.start + && scope.block.parent.arguments[0].params + && scope.block.parent.arguments[0].params.length > 0 + ) { + return scope.block.parent.arguments[0].params[1].name === node.object.name; + } + scope = scope.upper; + } + return false; + } + /** * Checks if we are using a prop * @param {ASTNode} node The AST node being checked. @@ -108,7 +150,8 @@ module.exports = { function isPropTypesUsage(node) { const isClassUsage = ( (utils.getParentES6Component() || utils.getParentES5Component()) && - node.object.type === 'ThisExpression' && node.property.name === 'props' + ((node.object.type === 'ThisExpression' && node.property.name === 'props') + || isPropArgumentInSetStateUpdater(node)) ); const isStatelessFunctionUsage = node.object.name === 'props'; return isClassUsage || isStatelessFunctionUsage || inLifeCycleMethod(); @@ -558,16 +601,20 @@ module.exports = { const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); const isDirectNextProp = DIRECT_NEXT_PROPS_REGEX.test(sourceCode.getText(node)); const isDirectPrevProp = DIRECT_PREV_PROPS_REGEX.test(sourceCode.getText(node)); + const isDirectSetStateProp = isPropArgumentInSetStateUpdater(node); const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component(); const isNotInConstructor = !inConstructor(node); const isNotInLifeCycleMethod = !inLifeCycleMethod(); - if ((isDirectProp || isDirectNextProp || isDirectPrevProp) + const isNotInSetStateUpdater = !inSetStateUpdater(); + if ((isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp) && isInClassComponent && isNotInConstructor - && isNotInLifeCycleMethod) { + && isNotInLifeCycleMethod + && isNotInSetStateUpdater + ) { return void 0; } - if (!isDirectProp && !isDirectNextProp && !isDirectPrevProp) { + if (!isDirectProp && !isDirectNextProp && !isDirectPrevProp && !isDirectSetStateProp) { node = node.parent; } const property = node.property; @@ -631,6 +678,9 @@ module.exports = { case 'FunctionExpression': type = 'destructuring'; properties = node.params[0].properties; + if (inSetStateUpdater()) { + properties = node.params[1].properties; + } break; case 'VariableDeclarator': for (let i = 0, j = node.id.properties.length; i < j; i++) { @@ -915,11 +965,20 @@ module.exports = { markPropTypesAsDeclared(node, resolveTypeAnnotation(node.params[0])); } + function handleSetStateUpdater(node) { + if (!node.params || !node.params.length || !inSetStateUpdater()) { + return; + } + markPropTypesAsUsed(node); + } + /** + * Handle both stateless functions and setState updater functions. * @param {ASTNode} node We expect either an ArrowFunctionExpression, * FunctionDeclaration, or FunctionExpression */ - function handleStatelessComponent(node) { + function handleFunctionLikeExpressions(node) { + handleSetStateUpdater(node); markDestructuredFunctionArgumentsAsUsed(node); markAnnotatedFunctionArgumentsAsDeclared(node); } @@ -959,11 +1018,11 @@ module.exports = { markPropTypesAsUsed(node); }, - FunctionDeclaration: handleStatelessComponent, + FunctionDeclaration: handleFunctionLikeExpressions, - ArrowFunctionExpression: handleStatelessComponent, + ArrowFunctionExpression: handleFunctionLikeExpressions, - FunctionExpression: handleStatelessComponent, + FunctionExpression: handleFunctionLikeExpressions, MemberExpression: function(node) { let type; diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js index 62ef6094c0..f7de810736 100644 --- a/tests/lib/rules/no-unused-prop-types.js +++ b/tests/lib/rules/no-unused-prop-types.js @@ -829,10 +829,10 @@ ruleTester.run('no-unused-prop-types', rule, { type PropsA = { a: string } type PropsB = { b: string } type Props = PropsA & PropsB; - + class MyComponent extends React.Component { props: Props; - + render() { return
{this.props.a} - {this.props.b}
} @@ -848,7 +848,7 @@ ruleTester.run('no-unused-prop-types', rule, { class Bar extends React.Component { props: Props & PropsC; - + render() { return
{this.props.foo} - {this.props.bar} - {this.props.zap}
} @@ -864,7 +864,7 @@ ruleTester.run('no-unused-prop-types', rule, { class Bar extends React.Component { props: Props & PropsC; - + render() { return
{this.props.foo} - {this.props.bar} - {this.props.zap}
} @@ -876,12 +876,12 @@ ruleTester.run('no-unused-prop-types', rule, { type PropsB = { foo: string }; type PropsC = { bar: string }; type Props = PropsB & { - zap: string + zap: string }; class Bar extends React.Component { props: Props & PropsC; - + render() { return
{this.props.foo} - {this.props.bar} - {this.props.zap}
} @@ -893,12 +893,12 @@ ruleTester.run('no-unused-prop-types', rule, { type PropsB = { foo: string }; type PropsC = { bar: string }; type Props = { - zap: string + zap: string } & PropsB; class Bar extends React.Component { props: Props & PropsC; - + render() { return
{this.props.foo} - {this.props.bar} - {this.props.zap}
} @@ -2110,6 +2110,93 @@ ruleTester.run('no-unused-prop-types', rule, { ].join('\n'), parser: 'babel-eslint', options: [{skipShapeProps: false}] + }, { + // issue #1506 + code: [ + 'class MyComponent extends React.Component {', + ' onFoo() {', + ' this.setState((prevState, props) => {', + ' props.doSomething();', + ' });', + ' }', + ' render() {', + ' return (', + '
Test
', + ' );', + ' }', + '}', + 'MyComponent.propTypes = {', + ' doSomething: PropTypes.func', + '};', + 'var tempVar2;' + ].join('\n'), + parser: 'babel-eslint', + options: [{skipShapeProps: false}] + }, { + // issue #1506 + code: [ + 'class MyComponent extends React.Component {', + ' onFoo() {', + ' this.setState((prevState, { doSomething }) => {', + ' doSomething();', + ' });', + ' }', + ' render() {', + ' return (', + '
Test
', + ' );', + ' }', + '}', + 'MyComponent.propTypes = {', + ' doSomething: PropTypes.func', + '};' + ].join('\n'), + parser: 'babel-eslint', + options: [{skipShapeProps: false}] + }, { + // issue #1506 + code: [ + 'class MyComponent extends React.Component {', + ' onFoo() {', + ' this.setState((prevState, obj) => {', + ' obj.doSomething();', + ' });', + ' }', + ' render() {', + ' return (', + '
Test
', + ' );', + ' }', + '}', + 'MyComponent.propTypes = {', + ' doSomething: PropTypes.func', + '};', + 'var tempVar2;' + ].join('\n'), + parser: 'babel-eslint', + options: [{skipShapeProps: false}] + }, { + // issue #1506 + code: [ + 'class MyComponent extends React.Component {', + ' onFoo() {', + ' this.setState(() => {', + ' this.props.doSomething();', + ' });', + ' }', + ' render() {', + ' return (', + '
Test
', + ' );', + ' }', + '}', + 'MyComponent.propTypes = {', + ' doSomething: PropTypes.func', + '};', + 'var tempVar;' + ].join('\n'), + parser: 'babel-eslint', + options: [{skipShapeProps: false}] }, { // issue #106 code: ` @@ -2796,10 +2883,10 @@ ruleTester.run('no-unused-prop-types', rule, { type PropsA = { a: string } type PropsB = { b: string } type Props = PropsA & PropsB; - + class MyComponent extends React.Component { props: Props; - + render() { return
{this.props.a}
} @@ -2818,7 +2905,7 @@ ruleTester.run('no-unused-prop-types', rule, { class Bar extends React.Component { props: Props & PropsC; - + render() { return
{this.props.foo} - {this.props.bar}
} @@ -2833,12 +2920,12 @@ ruleTester.run('no-unused-prop-types', rule, { type PropsB = { foo: string }; type PropsC = { bar: string }; type Props = PropsB & { - zap: string + zap: string }; class Bar extends React.Component { props: Props & PropsC; - + render() { return
{this.props.foo} - {this.props.bar}
} @@ -2853,12 +2940,12 @@ ruleTester.run('no-unused-prop-types', rule, { type PropsB = { foo: string }; type PropsC = { bar: string }; type Props = { - zap: string + zap: string } & PropsB; class Bar extends React.Component { props: Props & PropsC; - + render() { return
{this.props.foo} - {this.props.bar}
} @@ -3716,6 +3803,28 @@ ruleTester.run('no-unused-prop-types', rule, { errors: [{ message: '\'lastname\' PropType is defined but prop is never used' }] + }, { + // issue #1506 + code: [ + 'class MyComponent extends React.Component {', + ' onFoo() {', + ' this.setState(({ doSomething }, props) => {', + ' return { doSomething: doSomething + 1 };', + ' });', + ' }', + ' render() {', + ' return (', + '
Test
', + ' );', + ' }', + '}', + 'MyComponent.propTypes = {', + ' doSomething: PropTypes.func', + '};' + ].join('\n'), + errors: [{ + message: '\'doSomething\' PropType is defined but prop is never used' + }] }, { code: ` type Props = {