[Fix] no-unused-prop-types: false positive with callback

Author: chiawendt

lib/util/Components.js

@@ -577,50 +577,67 @@ function componentRule(rule, context) {
     },
 
     /**
-     * Get the parent stateless component node from the current scope
-     *
-     * @returns {ASTNode} component node, null if we are not in a component
+     * @param {ASTNode} node
+     * @returns {boolean}
      */
-    getParentStatelessComponent() {
-      let scope = context.getScope();
-      while (scope) {
-        const node = scope.block;
-        const isFunction = /Function/.test(node.type); // Functions
-        const isArrowFunction = astUtil.isArrowFunction(node);
-        const enclosingScope = isArrowFunction ? utils.getArrowFunctionScope(scope) : scope;
-        const enclosingScopeParent = enclosingScope && enclosingScope.block.parent;
-        const isClass = enclosingScope && astUtil.isClass(enclosingScope.block);
-        const isMethod = enclosingScopeParent && enclosingScopeParent.type === 'MethodDefinition'; // Classes methods
-        const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.)
-        // Attribute Expressions inside JSX Elements (<button onClick={() => props.handleClick()}></button>)
-        const isJSXExpressionContainer = node.parent && node.parent.type === 'JSXExpressionContainer';
-        const pragmaComponentWrapper = this.getPragmaComponentWrapper(node);
-        if (isFunction && pragmaComponentWrapper) {
-          return pragmaComponentWrapper;
+    isInAllowedPositionForComponent(node) {
+      switch (node.parent.type) {
+        case 'VariableDeclarator':
+        case 'AssignmentExpression':
+        case 'Property':
+        case 'ReturnStatement':
+        case 'ExportDefaultDeclaration': {
+          return true;
         }
-        // Stop moving up if we reach a class or an argument (like a callback)
-        if (isClass || isArgument) {
-          return null;
+        case 'SequenceExpression': {
+          return utils.isInAllowedPositionForComponent(node.parent) &&
+            node === node.parent.expressions[node.parent.expressions.length - 1];
         }
-        // Return the node if it is a function that is not a class method and is not inside a JSX Element
-        if (isFunction && !isMethod && !isJSXExpressionContainer && utils.isReturningJSXOrNull(node)) {
+        default:
+          return false;
+      }
+    },
+
+    /**
+     * Get node if node is a stateless component, or node.parent in cases like
+     * `React.memo` or `React.forwardRef`. Otherwise returns `undefined`.
+     * @param {ASTNode} node
+     * @returns {ASTNode | undefined}
+     */
+    getStatelessComponent(node) {
+      if (node.type === 'FunctionDeclaration') {
+        if (utils.isReturningJSXOrNull(node)) {
           return node;
         }
-        scope = scope.upper;
       }
-      return null;
+
+      if (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') {
+        if (utils.isInAllowedPositionForComponent(node) && utils.isReturningJSXOrNull(node)) {
+          return node;
+        }
+
+        // Case like `React.memo(() => <></>)` or `React.forwardRef(...)`
+        const pragmaComponentWrapper = utils.getPragmaComponentWrapper(node);
+        if (pragmaComponentWrapper) {
+          return pragmaComponentWrapper;
+        }
+      }
+
+      return undefined;
     },
 
     /**
-     * Get an enclosing scope used to find `this` value by an arrow function
-     * @param {Scope} scope Current scope
-     * @returns {Scope} An enclosing scope used by an arrow function
+     * Get the parent stateless component node from the current scope
+     *
+     * @returns {ASTNode} component node, null if we are not in a component
      */
-    getArrowFunctionScope(scope) {
-      scope = scope.upper;
+    getParentStatelessComponent() {
+      let scope = context.getScope();
       while (scope) {
-        if (astUtil.isFunction(scope.block) || astUtil.isClass(scope.block)) {
-          return scope;
+        const node = scope.block;
+        const statelessComponent = utils.getStatelessComponent(node);
+        if (statelessComponent) {
+          return statelessComponent;
         }
         scope = scope.upper;
       }

lib/util/ast.js

@@ -133,15 +133,6 @@ function isFunction(node) {
   return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration';
 }
 
-/**
- * Checks if the node is an arrow function.
- * @param {ASTNode} node The node to check
- * @return {Boolean} true if it's an arrow function
- */
-function isArrowFunction(node) {
-  return node.type === 'ArrowFunctionExpression';
-}
-
 /**
  * Checks if the node is a class.
  * @param {ASTNode} node The node to check
@@ -198,7 +189,6 @@ module.exports = {
   getPropertyNameNode,
   getComponentProperties,
   getKeyValue,
-  isArrowFunction,
   isAssignmentLHS,
   isClass,
   isFunction,

tests/lib/rules/destructuring-assignment.js

@@ -256,6 +256,29 @@ ruleTester.run('destructuring-assignment', rule, {
     errors: [
       {message: 'Must use destructuring props assignment'}
     ]
+  }, {
+    code: `
+      module.exports = {
+        Foo(props) {
+          return <p>{props.a}</p>;
+        }
+      }
+    `,
+    errors: [{message: 'Must use destructuring props assignment'}]
+  }, {
+    code: `
+      export default function Foo(props) {
+        return <p>{props.a}</p>;
+      }
+    `,
+    errors: [{message: 'Must use destructuring props assignment'}]
+  }, {
+    code: `
+      function hof() {
+        return (props) => <p>{props.a}</p>;
+      }
+    `,
+    errors: [{message: 'Must use destructuring props assignment'}]
   }, {
     code: `const Foo = class extends React.PureComponent {
       render() {

tests/lib/rules/display-name.js

@@ -661,6 +661,18 @@ ruleTester.run('display-name', rule, {
     errors: [{
       message: 'Component definition is missing display name'
     }]
+  }, {
+    code: `
+      function Hof() {
+        return function () {
+          return <div />
+        }
+      }
+    `,
+    parser: parsers.BABEL_ESLINT,
+    errors: [{
+      message: 'Component definition is missing display name'
+    }]
   }, {
     code: `
       import React, { createElement } from "react";

tests/lib/rules/no-multi-comp.js

@@ -312,7 +312,26 @@ ruleTester.run('no-multi-comp', rule, {
       message: 'Declare only one React component per file',
       line: 6
     }]
-  }, {
+  },
+  {
+    code: `
+      exports.Foo = function Foo() {
+        return <></>
+      }
+
+      exports.createSomeComponent = function createSomeComponent(opts) {
+        return function Foo() {
+          return <>{opts.a}</>
+        }
+      }
+    `,
+    parser: parsers.BABEL_ESLINT,
+    errors: [{
+      message: 'Declare only one React component per file',
+      line: 7
+    }]
+  },
+  {
     code: `
   class StoreListItem extends React.PureComponent {
     // A bunch of stuff here

tests/lib/rules/no-this-in-sfc.js

@@ -217,13 +217,6 @@ ruleTester.run('no-this-in-sfc', rule, {
       return <div onClick={onClick}>{this.props.foo}</div>;
     }`,
     errors: [{message: ERROR_MESSAGE}, {message: ERROR_MESSAGE}]
-  }, {
-    code: `
-    () => {
-      this.something();
-      return null;
-    }`,
-    errors: [{message: ERROR_MESSAGE}]
   }, {
     code: `
     class Foo {

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

@@ -1856,6 +1856,22 @@ ruleTester.run('no-unused-prop-types', rule, {
         '  previousPage: PropTypes.func.isRequired,',
         '};'
       ].join('\n')
+    }, {
+      // issue 2350
+      code: `
+        function Foo(props) {
+          useEffect(() => {
+            const { a } = props;
+            document.title = a;
+          });
+          
+          return <p/>;
+        }
+
+        Foo.propTypes = {
+          a: PropTypes.string,
+        }
+      `
     }, {
       code: [
         'class Hello extends Component {',

tests/lib/rules/require-default-props.js

@@ -2225,6 +2225,20 @@ ruleTester.run('require-default-props', rule, {
       errors: [{
         message: 'propType "usedProp" is not required, but has no corresponding defaultProps declaration.'
       }]
+    },
+    {
+      code: `
+        Foo.propTypes = {
+          a: PropTypes.string,
+        }
+
+        export default function Foo(props) {
+          return <p>{props.a}</p>
+        };
+      `,
+      errors: [{
+        message: 'propType "a" is not required, but has no corresponding defaultProps declaration.'
+      }]
     }
   ]
 });