Merge pull request #1507 from petersendidit/1506-fix

Fix no-unused-prop-types setState updater
jsx-eslint · Nov 13, 2017 · 64cf2d7 · 64cf2d7
2 parents 006441f + c471d77
commit 64cf2d7
Show file tree

Hide file tree

Showing 2 changed files with 176 additions and 8 deletions.
diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js
@@ -101,6 +101,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.
@@ -109,7 +151,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();
@@ -534,16 +577,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;
@@ -607,6 +654,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++) {
@@ -898,11 +948,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);
     }
@@ -942,11 +1001,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
@@ -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 (',
+        '       <div onClick={this.onFoo}>Test</div>',
+        '    );',
+        '  }',
+        '}',
+        '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 (',
+        '       <div onClick={this.onFoo}>Test</div>',
+        '    );',
+        '  }',
+        '}',
+        '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 (',
+        '       <div onClick={this.onFoo}>Test</div>',
+        '    );',
+        '  }',
+        '}',
+        '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 (',
+        '       <div onClick={this.onFoo}>Test</div>',
+        '    );',
+        '  }',
+        '}',
+        'MyComponent.propTypes = {',
+        '  doSomething: PropTypes.func',
+        '};',
+        'var tempVar;'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      options: [{skipShapeProps: false}]
     }, {
       // issue #106
       code: `
@@ -3788,6 +3875,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 (',
+        '       <div onClick={this.onFoo}>Test</div>',
+        '    );',
+        '  }',
+        '}',
+        'MyComponent.propTypes = {',
+        '  doSomething: PropTypes.func',
+        '};'
+      ].join('\n'),
+      errors: [{
+        message: '\'doSomething\' PropType is defined but prop is never used'
+      }]
     }, {
       code: `
         type Props = {