From 4ee6f8e1ff15f89596e6c0249a21275a92052e58 Mon Sep 17 00:00:00 2001 From: Hank Chen Date: Tue, 30 Jun 2020 17:26:49 +0800 Subject: [PATCH] [Fix] `no-unused-prop-types`/`prop-types`: typescript interface support literal type and only FunctionComponent should have propTypes validation Fixes #2686. Fixes #2689. Co-authored-by: Hank Chen Co-authored-by: Jordan Harband --- lib/util/propTypes.js | 48 +++++++-- lib/util/usedPropTypes.js | 73 ++++++++------ tests/lib/rules/no-unused-prop-types.js | 125 ++++++++++++++++++++++++ tests/lib/rules/prop-types.js | 15 +++ 4 files changed, 221 insertions(+), 40 deletions(-) mode change 100755 => 100644 lib/util/usedPropTypes.js diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index 11a30d4a43..af0e4f112b 100644 --- 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 old mode 100755 new mode 100644 index 6c618add32..eb11698d4c --- 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 index d688ce382a..ae472dfea3 100644 --- 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
;', + '}' + ].join('\n'), + parser: parsers.TYPESCRIPT_ESLINT + }, { + code: [ + 'interface Props {', + ' [\'aria-label\']: string;', + '}', + 'export default function Component({', + ' [\'aria-label\']: ariaLabel,', + '}: Props): JSX.Element {', + ' return
;', + '}' + ].join('\n'), + parser: parsers.TYPESCRIPT_ESLINT + }, { + code: [ + 'interface Props {', + ' [1234]: string;', + '}', + 'export default function Component(', + ' props ', + ': Props): JSX.Element {', + ' return
;', + '}' + ].join('\n'), + parser: parsers.TYPESCRIPT_ESLINT + }, { + code: [ + 'interface Props {', + ' [\'1234\']: string;', + '}', + 'export default function Component(', + ' props ', + ': Props): JSX.Element {', + ' return
;', + '}' + ].join('\n'), + parser: parsers.TYPESCRIPT_ESLINT + }, { + code: [ + 'interface Props {', + ' [1234]: string;', + '}', + 'export default function Component(', + ' props ', + ': Props): JSX.Element {', + ' return
;', + '}' + ].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
;', + '}' + ].join('\n'), + parser: parsers.TYPESCRIPT_ESLINT + }, { + code: [ + 'interface Props {', + ' foo: string;', + '}', + 'const Component = (props: Props) => (', + '
{(()=> {return props.foo})()}
', + ')', + '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
;', + '}' + ].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
;', + '}' + ].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 index e120d34f5f..bf9926c382 100755 --- 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
+ } + `, + parser: parsers.TYPESCRIPT_ESLINT } ],