Skip to content

Commit c471d77

Browse files
petersendiditpetersendidit
committed
Fix no-unused-prop-types setState updater
Handle when props are used in the setState update callback Fix #1506
1 parent 17a7e47 commit c471d77

File tree

2 files changed

+191
-23
lines changed

2 files changed

+191
-23
lines changed

‎lib/rules/no-unused-prop-types.js‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,48 @@ module.exports = {
100100
return false;
101101
}
102102

103+
/**
104+
* Check if the current node is in a setState updater method
105+
* @return {boolean} true if we are in a setState updater, false if not
106+
*/
107+
function inSetStateUpdater() {
108+
let scope = context.getScope();
109+
while (scope) {
110+
if (
111+
scope.block && scope.block.parent
112+
&& scope.block.parent.type === 'CallExpression'
113+
&& scope.block.parent.callee.property
114+
&& scope.block.parent.callee.property.name === 'setState'
115+
// Make sure we are in the updater not the callback
116+
&& scope.block.parent.arguments[0].start === scope.block.start
117+
) {
118+
return true;
119+
}
120+
scope = scope.upper;
121+
}
122+
return false;
123+
}
124+
125+
function isPropArgumentInSetStateUpdater(node) {
126+
let scope = context.getScope();
127+
while (scope) {
128+
if (
129+
scope.block && scope.block.parent
130+
&& scope.block.parent.type === 'CallExpression'
131+
&& scope.block.parent.callee.property
132+
&& scope.block.parent.callee.property.name === 'setState'
133+
// Make sure we are in the updater not the callback
134+
&& scope.block.parent.arguments[0].start === scope.block.start
135+
&& scope.block.parent.arguments[0].params
136+
&& scope.block.parent.arguments[0].params.length > 0
137+
) {
138+
return scope.block.parent.arguments[0].params[1].name === node.object.name;
139+
}
140+
scope = scope.upper;
141+
}
142+
return false;
143+
}
144+
103145
/**
104146
* Checks if we are using a prop
105147
* @param {ASTNode} node The AST node being checked.
@@ -108,7 +150,8 @@ module.exports = {
108150
function isPropTypesUsage(node) {
109151
const isClassUsage = (
110152
(utils.getParentES6Component() || utils.getParentES5Component()) &&
111-
node.object.type === 'ThisExpression' && node.property.name === 'props'
153+
((node.object.type === 'ThisExpression' && node.property.name === 'props')
154+
|| isPropArgumentInSetStateUpdater(node))
112155
);
113156
const isStatelessFunctionUsage = node.object.name === 'props';
114157
return isClassUsage || isStatelessFunctionUsage || inLifeCycleMethod();
@@ -558,16 +601,20 @@ module.exports = {
558601
const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node));
559602
const isDirectNextProp = DIRECT_NEXT_PROPS_REGEX.test(sourceCode.getText(node));
560603
const isDirectPrevProp = DIRECT_PREV_PROPS_REGEX.test(sourceCode.getText(node));
604+
const isDirectSetStateProp = isPropArgumentInSetStateUpdater(node);
561605
const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component();
562606
const isNotInConstructor = !inConstructor(node);
563607
const isNotInLifeCycleMethod = !inLifeCycleMethod();
564-
if ((isDirectProp || isDirectNextProp || isDirectPrevProp)
608+
const isNotInSetStateUpdater = !inSetStateUpdater();
609+
if ((isDirectProp || isDirectNextProp || isDirectPrevProp || isDirectSetStateProp)
565610
&& isInClassComponent
566611
&& isNotInConstructor
567-
&& isNotInLifeCycleMethod) {
612+
&& isNotInLifeCycleMethod
613+
&& isNotInSetStateUpdater
614+
) {
568615
return void 0;
569616
}
570-
if (!isDirectProp && !isDirectNextProp && !isDirectPrevProp) {
617+
if (!isDirectProp && !isDirectNextProp && !isDirectPrevProp && !isDirectSetStateProp) {
571618
node = node.parent;
572619
}
573620
const property = node.property;
@@ -631,6 +678,9 @@ module.exports = {
631678
case 'FunctionExpression':
632679
type = 'destructuring';
633680
properties = node.params[0].properties;
681+
if (inSetStateUpdater()) {
682+
properties = node.params[1].properties;
683+
}
634684
break;
635685
case 'VariableDeclarator':
636686
for (let i = 0, j = node.id.properties.length; i < j; i++) {
@@ -915,11 +965,20 @@ module.exports = {
915965
markPropTypesAsDeclared(node, resolveTypeAnnotation(node.params[0]));
916966
}
917967

968+
function handleSetStateUpdater(node) {
969+
if (!node.params || !node.params.length || !inSetStateUpdater()) {
970+
return;
971+
}
972+
markPropTypesAsUsed(node);
973+
}
974+
918975
/**
976+
* Handle both stateless functions and setState updater functions.
919977
* @param {ASTNode} node We expect either an ArrowFunctionExpression,
920978
* FunctionDeclaration, or FunctionExpression
921979
*/
922-
function handleStatelessComponent(node) {
980+
function handleFunctionLikeExpressions(node) {
981+
handleSetStateUpdater(node);
923982
markDestructuredFunctionArgumentsAsUsed(node);
924983
markAnnotatedFunctionArgumentsAsDeclared(node);
925984
}
@@ -959,11 +1018,11 @@ module.exports = {
9591018
markPropTypesAsUsed(node);
9601019
},
9611020

962-
FunctionDeclaration: handleStatelessComponent,
1021+
FunctionDeclaration: handleFunctionLikeExpressions,
9631022

964-
ArrowFunctionExpression: handleStatelessComponent,
1023+
ArrowFunctionExpression: handleFunctionLikeExpressions,
9651024

966-
FunctionExpression: handleStatelessComponent,
1025+
FunctionExpression: handleFunctionLikeExpressions,
9671026

9681027
MemberExpression: function(node) {
9691028
let type;

‎tests/lib/rules/no-unused-prop-types.js‎

Lines changed: 124 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -829,10 +829,10 @@ ruleTester.run('no-unused-prop-types', rule, {
829829
type PropsA = { a: string }
830830
type PropsB = { b: string }
831831
type Props = PropsA & PropsB;
832-
832+
833833
class MyComponent extends React.Component {
834834
props: Props;
835-
835+
836836
render() {
837837
return <div>{this.props.a} - {this.props.b}</div>
838838
}
@@ -848,7 +848,7 @@ ruleTester.run('no-unused-prop-types', rule, {
848848
849849
class Bar extends React.Component {
850850
props: Props & PropsC;
851-
851+
852852
render() {
853853
return <div>{this.props.foo} - {this.props.bar} - {this.props.zap}</div>
854854
}
@@ -864,7 +864,7 @@ ruleTester.run('no-unused-prop-types', rule, {
864864
865865
class Bar extends React.Component {
866866
props: Props & PropsC;
867-
867+
868868
render() {
869869
return <div>{this.props.foo} - {this.props.bar} - {this.props.zap}</div>
870870
}
@@ -876,12 +876,12 @@ ruleTester.run('no-unused-prop-types', rule, {
876876
type PropsB = { foo: string };
877877
type PropsC = { bar: string };
878878
type Props = PropsB & {
879-
zap: string
879+
zap: string
880880
};
881881
882882
class Bar extends React.Component {
883883
props: Props & PropsC;
884-
884+
885885
render() {
886886
return <div>{this.props.foo} - {this.props.bar} - {this.props.zap}</div>
887887
}
@@ -893,12 +893,12 @@ ruleTester.run('no-unused-prop-types', rule, {
893893
type PropsB = { foo: string };
894894
type PropsC = { bar: string };
895895
type Props = {
896-
zap: string
896+
zap: string
897897
} & PropsB;
898898
899899
class Bar extends React.Component {
900900
props: Props & PropsC;
901-
901+
902902
render() {
903903
return <div>{this.props.foo} - {this.props.bar} - {this.props.zap}</div>
904904
}
@@ -2110,6 +2110,93 @@ ruleTester.run('no-unused-prop-types', rule, {
21102110
].join('\n'),
21112111
parser: 'babel-eslint',
21122112
options: [{skipShapeProps: false}]
2113+
}, {
2114+
// issue #1506
2115+
code: [
2116+
'class MyComponent extends React.Component {',
2117+
' onFoo() {',
2118+
' this.setState((prevState, props) => {',
2119+
' props.doSomething();',
2120+
' });',
2121+
' }',
2122+
' render() {',
2123+
' return (',
2124+
' <div onClick={this.onFoo}>Test</div>',
2125+
' );',
2126+
' }',
2127+
'}',
2128+
'MyComponent.propTypes = {',
2129+
' doSomething: PropTypes.func',
2130+
'};',
2131+
'var tempVar2;'
2132+
].join('\n'),
2133+
parser: 'babel-eslint',
2134+
options: [{skipShapeProps: false}]
2135+
}, {
2136+
// issue #1506
2137+
code: [
2138+
'class MyComponent extends React.Component {',
2139+
' onFoo() {',
2140+
' this.setState((prevState, { doSomething }) => {',
2141+
' doSomething();',
2142+
' });',
2143+
' }',
2144+
' render() {',
2145+
' return (',
2146+
' <div onClick={this.onFoo}>Test</div>',
2147+
' );',
2148+
' }',
2149+
'}',
2150+
'MyComponent.propTypes = {',
2151+
' doSomething: PropTypes.func',
2152+
'};'
2153+
].join('\n'),
2154+
parser: 'babel-eslint',
2155+
options: [{skipShapeProps: false}]
2156+
}, {
2157+
// issue #1506
2158+
code: [
2159+
'class MyComponent extends React.Component {',
2160+
' onFoo() {',
2161+
' this.setState((prevState, obj) => {',
2162+
' obj.doSomething();',
2163+
' });',
2164+
' }',
2165+
' render() {',
2166+
' return (',
2167+
' <div onClick={this.onFoo}>Test</div>',
2168+
' );',
2169+
' }',
2170+
'}',
2171+
'MyComponent.propTypes = {',
2172+
' doSomething: PropTypes.func',
2173+
'};',
2174+
'var tempVar2;'
2175+
].join('\n'),
2176+
parser: 'babel-eslint',
2177+
options: [{skipShapeProps: false}]
2178+
}, {
2179+
// issue #1506
2180+
code: [
2181+
'class MyComponent extends React.Component {',
2182+
' onFoo() {',
2183+
' this.setState(() => {',
2184+
' this.props.doSomething();',
2185+
' });',
2186+
' }',
2187+
' render() {',
2188+
' return (',
2189+
' <div onClick={this.onFoo}>Test</div>',
2190+
' );',
2191+
' }',
2192+
'}',
2193+
'MyComponent.propTypes = {',
2194+
' doSomething: PropTypes.func',
2195+
'};',
2196+
'var tempVar;'
2197+
].join('\n'),
2198+
parser: 'babel-eslint',
2199+
options: [{skipShapeProps: false}]
21132200
}, {
21142201
// issue #106
21152202
code: `
@@ -2796,10 +2883,10 @@ ruleTester.run('no-unused-prop-types', rule, {
27962883
type PropsA = { a: string }
27972884
type PropsB = { b: string }
27982885
type Props = PropsA & PropsB;
2799-
2886+
28002887
class MyComponent extends React.Component {
28012888
props: Props;
2802-
2889+
28032890
render() {
28042891
return <div>{this.props.a}</div>
28052892
}
@@ -2818,7 +2905,7 @@ ruleTester.run('no-unused-prop-types', rule, {
28182905
28192906
class Bar extends React.Component {
28202907
props: Props & PropsC;
2821-
2908+
28222909
render() {
28232910
return <div>{this.props.foo} - {this.props.bar}</div>
28242911
}
@@ -2833,12 +2920,12 @@ ruleTester.run('no-unused-prop-types', rule, {
28332920
type PropsB = { foo: string };
28342921
type PropsC = { bar: string };
28352922
type Props = PropsB & {
2836-
zap: string
2923+
zap: string
28372924
};
28382925
28392926
class Bar extends React.Component {
28402927
props: Props & PropsC;
2841-
2928+
28422929
render() {
28432930
return <div>{this.props.foo} - {this.props.bar}</div>
28442931
}
@@ -2853,12 +2940,12 @@ ruleTester.run('no-unused-prop-types', rule, {
28532940
type PropsB = { foo: string };
28542941
type PropsC = { bar: string };
28552942
type Props = {
2856-
zap: string
2943+
zap: string
28572944
} & PropsB;
28582945
28592946
class Bar extends React.Component {
28602947
props: Props & PropsC;
2861-
2948+
28622949
render() {
28632950
return <div>{this.props.foo} - {this.props.bar}</div>
28642951
}
@@ -3716,6 +3803,28 @@ ruleTester.run('no-unused-prop-types', rule, {
37163803
errors: [{
37173804
message: '\'lastname\' PropType is defined but prop is never used'
37183805
}]
3806+
}, {
3807+
// issue #1506
3808+
code: [
3809+
'class MyComponent extends React.Component {',
3810+
' onFoo() {',
3811+
' this.setState(({ doSomething }, props) => {',
3812+
' return { doSomething: doSomething + 1 };',
3813+
' });',
3814+
' }',
3815+
' render() {',
3816+
' return (',
3817+
' <div onClick={this.onFoo}>Test</div>',
3818+
' );',
3819+
' }',
3820+
'}',
3821+
'MyComponent.propTypes = {',
3822+
' doSomething: PropTypes.func',
3823+
'};'
3824+
].join('\n'),
3825+
errors: [{
3826+
message: '\'doSomething\' PropType is defined but prop is never used'
3827+
}]
37193828
}, {
37203829
code: `
37213830
type Props = {

0 commit comments

Comments
 (0)