Skip to content

Commit

Permalink
[Fix] no-unused-prop-types/prop-types: typescript interface suppo…
Browse files Browse the repository at this point in the history
…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>
  • Loading branch information
hank121314 and ljharb committed Jun 30, 2020
1 parent 830bde7 commit 4ee6f8e
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 40 deletions.
48 changes: 40 additions & 8 deletions lib/util/propTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
};
}
});
Expand Down Expand Up @@ -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) => {
Expand Down
73 changes: 41 additions & 32 deletions lib/util/usedPropTypes.js
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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');

Expand All @@ -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 (
Expand Down
125 changes: 125 additions & 0 deletions tests/lib/rules/no-unused-prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
],

Expand Down Expand Up @@ -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}) => {',
Expand Down
15 changes: 15 additions & 0 deletions tests/lib/rules/prop-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
],

Expand Down

0 comments on commit 4ee6f8e

Please sign in to comment.