From 59f26d71478e651422aab59748da2fbddd6372d4 Mon Sep 17 00:00:00 2001 From: Joe Lencioni Date: Sat, 6 Feb 2016 08:47:23 -0800 Subject: [PATCH] Teach prop-types about destructing in function signatures The following should cause an error for missing propType validations, but it doesn't. const Test = ({ name }) => { return (
{name}
); }; If you replace the destructuring with using the props object, it works as expected. This commit fixes this problem. Fixes #354. --- docs/rules/prop-types.md | 11 +++++ lib/rules/prop-types.js | 27 +++++++++++ tests/lib/rules/prop-types.js | 90 +++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+) diff --git a/docs/rules/prop-types.md b/docs/rules/prop-types.md index f653399b90..acf636574a 100644 --- a/docs/rules/prop-types.md +++ b/docs/rules/prop-types.md @@ -23,6 +23,10 @@ var Hello = React.createClass({ return
Hello {this.props.firstname} {this.props.lastname}
; // lastname type is not defined in propTypes } }); + +function Hello({ name }) { + return
Hello {this.props.name}
; +} ``` The following patterns are not considered warnings: @@ -50,6 +54,13 @@ var Hello = React.createClass({ return
Hello {this.props.name}
; } }); + +function Hello({ name }) { + return
Hello {this.props.name}
; +} +Hello.propTypes = { + name: React.PropTypes.string.isRequired, +}; ``` ## Rule Options diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index b339823da6..1e322c86df 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -505,6 +505,12 @@ module.exports = Components.detect(function(context, components, utils) { properties = node.parent.id.properties; } break; + case 'ArrowFunctionExpression': + case 'FunctionDeclaration': + case 'FunctionExpression': + type = 'destructuring'; + properties = node.params[0].properties; + break; case 'VariableDeclarator': for (var i = 0, j = node.id.properties.length; i < j; i++) { // let {props: {firstname}} = this @@ -701,6 +707,21 @@ module.exports = Components.detect(function(context, components, utils) { return annotation; } + /** + * @param {ASTNode} node We expect either an ArrowFunctionExpression, + * FunctionDeclaration, or FunctionExpression + */ + function markDestructuredFunctionArgumentsAsUsed(node) { + var destructuring = node.params && + node.params.length === 1 && + node.params[0] && + node.params[0].type === 'ObjectPattern'; + + if (destructuring) { + markPropTypesAsUsed(node); + } + } + // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- @@ -727,6 +748,12 @@ module.exports = Components.detect(function(context, components, utils) { markPropTypesAsUsed(node); }, + FunctionDeclaration: markDestructuredFunctionArgumentsAsUsed, + + ArrowFunctionExpression: markDestructuredFunctionArgumentsAsUsed, + + FunctionExpression: markDestructuredFunctionArgumentsAsUsed, + MemberExpression: function(node) { var type; if (isPropTypesUsage(node)) { diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 060cead4c4..4486ca2006 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -836,6 +836,66 @@ ruleTester.run('prop-types', rule, { '};' ].join('\n'), parserOptions: parserOptions + }, { + code: [ + 'const statelessComponent = ({ someProp }) => {', + ' const subRender = () => {', + ' return {someProp};', + ' };', + ' return
{subRender()}
;', + '};', + 'statelessComponent.propTypes = {', + ' someProp: PropTypes.string', + '};' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'const statelessComponent = function({ someProp }) {', + ' const subRender = () => {', + ' return {someProp};', + ' };', + ' return
{subRender()}
;', + '};', + 'statelessComponent.propTypes = {', + ' someProp: PropTypes.string', + '};' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'function statelessComponent({ someProp }) {', + ' const subRender = () => {', + ' return {someProp};', + ' };', + ' return
{subRender()}
;', + '};', + 'statelessComponent.propTypes = {', + ' someProp: PropTypes.string', + '};' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'function notAComponent({ something }) {', + ' return something + 1;', + '};' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'const notAComponent = function({ something }) {', + ' return something + 1;', + '};' + ].join('\n'), + parserOptions: parserOptions + }, { + code: [ + 'const notAComponent = ({ something }) => {', + ' return something + 1;', + '};' + ].join('\n'), + parserOptions: parserOptions }, { // Validation is ignored on reassigned props object code: [ @@ -1446,6 +1506,36 @@ ruleTester.run('prop-types', rule, { errors: [{ message: '\'name\' is missing in props validation' }] + }, { + code: [ + 'function Hello({ name }) {', + ' return
Hello {name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'name\' is missing in props validation' + }] + }, { + code: [ + 'const Hello = function({ name }) {', + ' return
Hello {name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'name\' is missing in props validation' + }] + }, { + code: [ + 'const Hello = ({ name }) => {', + ' return
Hello {name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'name\' is missing in props validation' + }] }, { code: [ 'class Hello extends React.Component {',