[Fix] no-unused-prop-types/prop-types: typescript interface suppo…

…rt literal type and only FunctionComponent should have propTypes validation Fixes #2686. Fixes #2689. Co-authored-by: Hank Chen <hank121314@gmail.com> Co-authored-by: Jordan Harband <ljharb@gmail.com>
jsx-eslint · Jun 30, 2020 · 4ee6f8e · 4ee6f8e
1 parent 830bde7
commit 4ee6f8e
Show file tree

Hide file tree

Showing 4 changed files with 221 additions and 40 deletions.
diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js
@@ -12,6 +12,8 @@ const variableUtil = require('./variable');
 const versionUtil = require('./version');
 const propWrapperUtil = require('./propWrapper');
 const getKeyValue = require('./ast').getKeyValue;
+const findReturnStatement = require('./ast').findReturnStatement;
+const isJSX = require('./jsx').isJSX;
 
 /**
  * Checks if we are declaring a props as a generic type in a flow-annotated class.
@@ -67,7 +69,25 @@ function isInsideClassBody(node) {
     }
     parent = parent.parent;
   }
+  return false;
+}
 
+/**
+ * Checks if a node is a Function and return JSXElement
+ *
+ * @param {ASTNode} node  the AST node being checked.
+ * @returns {Boolean} True if the node is a Function and return JSXElement.
+ */
+function isJSXFunctionComponent(node) {
+  if (node.type === 'ArrowFunctionExpression' || node.type === 'FunctionDeclaration' || node.type === 'FunctionExpression') {
+    const res = findReturnStatement(node);
+    // If function return JSXElement with return keyword.
+    if (res) {
+      return isJSX(res.argument);
+    }
+    // If function return JSXElement without return keyword;
+    return isJSX(node.body);
+  }
   return false;
 }
 
@@ -215,7 +235,6 @@ module.exports = function propTypesInstructions(context, components, utils) {
     if (annotation.type === 'GenericTypeAnnotation' && getInTypeScope(annotation.id.name)) {
       return getInTypeScope(annotation.id.name);
     }
-
     return annotation;
   }
 
@@ -313,13 +332,21 @@ module.exports = function propTypesInstructions(context, components, utils) {
       return true;
     }
 
-    foundDeclaredPropertiesList.forEach((tsPropertySignature) => {
-      if (tsPropertySignature.type !== 'TSIndexSignature') {
-        declaredPropTypes[tsPropertySignature.key.name] = {
-          fullName: tsPropertySignature.key.name,
-          name: tsPropertySignature.key.name,
-          node: tsPropertySignature,
-          isRequired: !tsPropertySignature.optional
+    foundDeclaredPropertiesList.forEach((tsInterfaceBody) => {
+      if (tsInterfaceBody.type === 'TSPropertySignature') {
+        let accessor = 'name';
+        if (tsInterfaceBody.key.type === 'Literal') {
+          if (typeof tsInterfaceBody.key.value === 'number') {
+            accessor = 'raw';
+          } else {
+            accessor = 'value';
+          }
+        }
+        declaredPropTypes[tsInterfaceBody.key[accessor]] = {
+          fullName: tsInterfaceBody.key[accessor],
+          name: tsInterfaceBody.key[accessor],
+          node: tsInterfaceBody,
+          isRequired: !tsInterfaceBody.optional
         };
       }
     });
@@ -621,6 +648,11 @@ module.exports = function propTypesInstructions(context, components, utils) {
       return;
     }
 
+    // Should ignore function that not return JSXElement
+    if (!isJSXFunctionComponent(node)) {
+      return;
+    }
+
     const param = node.params[0];
     if (param.typeAnnotation && param.typeAnnotation.typeAnnotation && param.typeAnnotation.typeAnnotation.type === 'UnionTypeAnnotation') {
       param.typeAnnotation.typeAnnotation.types.forEach((annotation) => {

diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js
@@ -208,37 +208,6 @@ function hasSpreadOperator(context, node) {
   return tokens.length && tokens[0].value === '...';
 }
 
-/**
- * Retrieve the name of a property node
- * @param {ASTNode} node The AST node with the property.
- * @return {string|undefined} the name of the property or undefined if not found
- */
-function getPropertyName(node) {
-  const property = node.property;
-  if (property) {
-    switch (property.type) {
-      case 'Identifier':
-        if (node.computed) {
-          return '__COMPUTED_PROP__';
-        }
-        return property.name;
-      case 'MemberExpression':
-        return;
-      case 'Literal':
-        // Accept computed properties that are literal strings
-        if (typeof property.value === 'string') {
-          return property.value;
-        }
-        // falls through
-      default:
-        if (node.computed) {
-          return '__COMPUTED_PROP__';
-        }
-        break;
-    }
-  }
-}
-
 /**
  * Checks if the node is a propTypes usage of the form `this.props.*`, `props.*`, `prevProps.*`, or `nextProps.*`.
  * @param {ASTNode} node
@@ -272,6 +241,46 @@ function isPropTypesUsageByMemberExpression(node, context, utils, checkAsyncSafe
   return unwrappedObjectNode.name === 'props' && !ast.isAssignmentLHS(node);
 }
 
+/**
+ * Retrieve the name of a property node
+ * @param {ASTNode} node The AST node with the property.
+ * @param {Context} context
+ * @param {Object} utils
+ * @param {boolean} checkAsyncSafeLifeCycles
+ * @return {string|undefined} the name of the property or undefined if not found
+ */
+function getPropertyName(node, context, utils, checkAsyncSafeLifeCycles) {
+  const property = node.property;
+  if (property) {
+    switch (property.type) {
+      case 'Identifier':
+        if (node.computed) {
+          return '__COMPUTED_PROP__';
+        }
+        return property.name;
+      case 'MemberExpression':
+        return;
+      case 'Literal':
+        // Accept computed properties that are literal strings
+        if (typeof property.value === 'string') {
+          return property.value;
+        }
+        // Accept number as well but only accept props[123]
+        if (typeof property.value === 'number') {
+          if (isPropTypesUsageByMemberExpression(node, context, utils, checkAsyncSafeLifeCycles)) {
+            return property.raw;
+          }
+        }
+        // falls through
+      default:
+        if (node.computed) {
+          return '__COMPUTED_PROP__';
+        }
+        break;
+    }
+  }
+}
+
 module.exports = function usedPropTypesInstructions(context, components, utils) {
   const checkAsyncSafeLifeCycles = versionUtil.testReactVersion(context, '16.3.0');
 
@@ -293,7 +302,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
     switch (node.type) {
       case 'OptionalMemberExpression':
       case 'MemberExpression':
-        name = getPropertyName(node);
+        name = getPropertyName(node, context, utils, checkAsyncSafeLifeCycles);
         if (name) {
           allNames = parentNames.concat(name);
           if (

diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js
@@ -3333,6 +3333,99 @@ ruleTester.run('no-unused-prop-types', rule, {
         '}'
       ].join('\n'),
       parser: parsers.TYPESCRIPT_ESLINT
+    }, {
+      code: [
+        'interface Props {',
+        ' \'aria-label\': string;',
+        '}',
+        'export default function Component({',
+        ' \'aria-label\': ariaLabel,',
+        '}: Props): JSX.Element {',
+        '  return <div aria-label={ariaLabel} />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT
+    }, {
+      code: [
+        'interface Props {',
+        ' [\'aria-label\']: string;',
+        '}',
+        'export default function Component({',
+        ' [\'aria-label\']: ariaLabel,',
+        '}: Props): JSX.Element {',
+        '  return <div aria-label={ariaLabel} />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT
+    }, {
+      code: [
+        'interface Props {',
+        ' [1234]: string;',
+        '}',
+        'export default function Component(',
+        ' props ',
+        ': Props): JSX.Element {',
+        '  return <div aria-label={props[1234]} />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT
+    }, {
+      code: [
+        'interface Props {',
+        ' [\'1234\']: string;',
+        '}',
+        'export default function Component(',
+        ' props ',
+        ': Props): JSX.Element {',
+        '  return <div aria-label={props[1234]} />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT
+    }, {
+      code: [
+        'interface Props {',
+        ' [1234]: string;',
+        '}',
+        'export default function Component(',
+        ' props ',
+        ': Props): JSX.Element {',
+        '  return <div aria-label={props[\'1234\']} />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT
+    },
+    {
+      code: [
+        'interface Props {',
+        ' [1234]: string;',
+        '}',
+        'export default function Component(',
+        ' props ',
+        ': Props): JSX.Element {',
+        'const handleVerifySubmit = ({',
+        '  otp,',
+        ' }) => {',
+        '  dispatch(',
+        '    verifyOTPPhone({',
+        '      otp,',
+        '    }),',
+        '  );',
+        '};',
+        'return <div aria-label={props[\'1234\']} />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT
+    }, {
+      code: [
+        'interface Props {',
+        ' foo: string;',
+        '}',
+        'const Component = (props: Props) => (',
+        ' <div>{(()=> {return props.foo})()}</div>',
+        ')',
+        'export default Component'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT
     }
   ],
 
@@ -5572,6 +5665,38 @@ ruleTester.run('no-unused-prop-types', rule, {
         }
       ]
     },
+    {
+      code: [
+        'interface Props {',
+        ' \'aria-label\': string;',
+        '}',
+        'export default function Component(props: Props): JSX.Element {',
+        '  return <div />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT,
+      errors: [
+        {
+          message: '\'aria-label\' PropType is defined but prop is never used'
+        }
+      ]
+    },
+    {
+      code: [
+        'interface Props {',
+        ' [1234]: string;',
+        '}',
+        'export default function Component(props: Props): JSX.Element {',
+        '  return <div />;',
+        '}'
+      ].join('\n'),
+      parser: parsers.TYPESCRIPT_ESLINT,
+      errors: [
+        {
+          message: '\'1234\' PropType is defined but prop is never used'
+        }
+      ]
+    },
     {
       code: [
         'const Hello = ({firstname}: {firstname: string, lastname: string}) => {',

diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js
@@ -2489,6 +2489,21 @@ ruleTester.run('prop-types', rule, {
           ordering: PropTypes.array
         };
       `
+    },
+    {
+      code: `
+        interface Props {
+          'aria-label': string // 'undefined' PropType is defined but prop is never used eslint(react/no-unused-prop-types)
+          // 'undefined' PropType is defined but prop is never used eslint(react-redux/no-unused-prop-types)
+        }
+
+        export default function Component({
+          'aria-label': ariaLabel, // 'aria-label' is missing in props validation eslint(react/prop-types)
+        }: Props): JSX.Element {
+          return <div aria-label={ariaLabel} />
+        }
+      `,
+      parser: parsers.TYPESCRIPT_ESLINT
     }
   ],