diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 16d09d8e54..b45ca15e97 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -135,17 +135,49 @@ module.exports = { /** * Checks if we are using a prop - * @param {ASTNode} node The AST node being checked. + * @param {ASTNode} node The MemberExpression node being checked. * @returns {Boolean} True if we are using a prop, false if not. */ function isPropTypesUsage(node) { - const isClassUsage = ( - (utils.getParentES6Component() || utils.getParentES5Component()) && - node.object.type === 'ThisExpression' && node.property.name === 'props' - ); - const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node); - const isNextPropsUsage = node.object.name === 'nextProps' && inComponentWillReceiveProps(); - return isClassUsage || isStatelessFunctionUsage || isNextPropsUsage; + // only return usages for the MemberExpressions at the bottom of the tree + if (node.object.type !== 'Identifier' && node.object.type !== 'ThisExpression') { + return false; + } + + const isInClassComponent = !!(utils.getParentES6Component() || utils.getParentES5Component()) + && node.object.name !== 'React'; + + + const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); + const isThisPropsUsage = node.object.type === 'ThisExpression' && node.property.name === 'props'; + const isPropsUsage = node.object.name === 'props'; + const isInStatelessComponent = !!utils.getParentStatelessComponent() && !isInClassComponent; + + if (isPropsUsage && !isInClassComponent && !isAssignmentToProp(node)) { + return true; + } + + // props.foo usages are only allowed + // in the constructor and componentWillReceiveProps in class components + if (isInClassComponent && !inConstructor() && !inComponentWillReceiveProps() && isDirectProp) { + return false; + } + + if (isInStatelessComponent) { + if (isAssignmentToProp(node)) { + return false; + } + // don't allow this.props usage in functional components + if (isThisPropsUsage) { + return false; + } + } + + if (!isInStatelessComponent && !isInClassComponent) { + return false; + } + + return true; } /** @@ -241,6 +273,7 @@ module.exports = { if (typeof propType === 'object' && Object.keys(propType).length === 0) { return true; } + // Consider every children as declared if (propType.children === true) { return true; @@ -346,11 +379,30 @@ module.exports = { const key = getKeyValue(node); const value = node.value; - fn(key, value); + fn(key, value, node); } } } + /** + * Traverse down the tree of properties in a node, build up an array of keys + * and call a function + * @param {Object[]} properties Array of properties to traverse. + * @param {Function} fn Function to call on each property, receives property key, property value + * property node and all keys above this node in the tree + * (key, value, property, keys) => void + */ + function traverseProperties(properties, fn, keys) { + keys = keys || []; + iterateProperties(properties, (key, value, property) => { + const updatedKeys = keys.concat(key); + fn(key, value, property, updatedKeys); + if (value && value.properties) { + traverseProperties(value.properties, fn, updatedKeys); + } + }); + } + /** * Creates the representation of the React propTypes for the component. * The representation is used to verify nested used properties. @@ -532,16 +584,6 @@ module.exports = { * @return {string} the name of the property or undefined if not found */ function getPropertyName(node) { - const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); - const isInClassComponent = utils.getParentES6Component() || utils.getParentES5Component(); - const isNotInConstructor = !inConstructor(); - const isNotInComponentWillReceiveProps = !inComponentWillReceiveProps(); - if (isDirectProp && isInClassComponent && isNotInConstructor && isNotInComponentWillReceiveProps) { - return void 0; - } - if (!isDirectProp) { - node = node.parent; - } const property = node.property; if (property) { switch (property.type) { @@ -569,42 +611,176 @@ module.exports = { } /** - * Mark a prop type as used - * @param {ASTNode} node The AST node being marked. + * Mark prop type usages in an array of properties + * @param {ASTNode} properties An array of properties to search. + * @param {Object} usedPropTypes An array of previous prop type usages. + * @param {Object} allNames any names that come from parent prop type usages. + */ + function markPropTypesInProperties(properties, usedPropTypes, allNames) { + allNames = allNames || []; + traverseProperties(properties, (key, value, property, allPropertyNames) => { + if (hasSpreadOperator(property) || property.computed) { + return; + } + const propName = getKeyValue(property); + const propValueName = property.value.name; + const isRenamedProperty = propName !== propValueName && property.value.type === 'Identifier'; + if (propName) { + usedPropTypes.push({ + name: isRenamedProperty ? propValueName : propName, + allNames: allNames.concat(allPropertyNames), + node: property + }); + } + }); + } + + /** + * traverse up a member expression and a call a callback + * @param {ASTNode} expression a member expression node. + * @param {Function} fn Function to call on each expression, receives property name, + * the from steps further down the tree, and the expression + * (propertyName, parentNames, expression) => void + * @param {ASTNode} parentNames the names from steps further down the tree. */ - function markPropTypesAsUsed(node, parentNames) { + function traverseMemberExpression(expression, fn, parentNames) { parentNames = parentNames || []; - let type; - let name; - let allNames; - let properties; + const propertyName = getPropertyName(expression); + if (propertyName) { + const childNames = parentNames.concat(propertyName); + fn(propertyName, childNames, expression); + if (expression.parent.type === 'MemberExpression') { + return traverseMemberExpression(expression.parent, fn, childNames); + } + } + return parentNames; + } + + /** + * Checks if the node is using destructuring + * @param {ASTNode} node a VariableDeclarator node. + * @returns {Boolean} True if destructing, false if not. + */ + function isDestructuredVariableDeclaration(node) { + return node.init && node.id && node.id.type === 'ObjectPattern'; + } + + /** + * Check if the nodes parent is using destructuring + * @param {ASTNode} node any node. + * @returns {Boolean} True if destructing, false if not. + */ + function parentIsDestructured(node) { + return node.parent.type === 'VariableDeclarator' && isDestructuredVariableDeclaration(node.parent); + } + + + /** + * Handle all of the variations of MemberExpression nodes (props, vs this.props etc) + * and return the starting node and parent usage if needed + * @param {ASTNode} node a MemberExpression node. + * @param {ASTNode} usedPropTypes an array of previous prop type usages. + * @returns {Object} An object containing the startingNode MemberExpression to traverse + * and the parentUsage if one exists, or returns void if its not a valid node + */ + function getProcessedMemberExpression(node, usedPropTypes) { + const isThisProps = (node.object.type === 'ThisExpression' && node.property.name === 'props'); + // if this is the top MemberExpression node + if (isThisProps && node.parent.type !== 'MemberExpression') { + if (!parentIsDestructured(node)) { + return void 0; + } + + // if the parent is using destructuring, mark prop types for its properties + const properties = node.parent.id.properties; + markPropTypesInProperties(properties, usedPropTypes); + return void 0; + } + + // this.props vs props + const startingNode = isThisProps ? node.parent : node; + const directlyOffOfProps = startingNode.object && + (startingNode.object.name === 'props' || startingNode.object.name === 'nextProps'); + + // if this member expression isn't coming off props in some way + // don't search for a parent usage + if (directlyOffOfProps || isThisProps) { + return { + startingNode: startingNode, + parentUsage: void 0 + }; + } + + // let { a } = this.props + //
{a.b}
+ const parentUsage = usedPropTypes.find( + propType => propType.name === node.object.name + ); + + // if we're not directly off of props + // and there isn't a parent usage, skip this node + if (!parentUsage) { + return void 0; + } + return { + startingNode: startingNode, + parentUsage: parentUsage + }; + } + + /** + * Mark a node as using a prop type + * @param {ASTNode} node The AST node being marked. + */ + function markPropTypesAsUsed(node) { + const component = components.get(utils.getParentComponent()); + const usedPropTypes = (component && component.usedPropTypes || []).slice(); switch (node.type) { case 'MemberExpression': - name = getPropertyName(node); - if (name) { - allNames = parentNames.concat(name); - if (node.parent.type === 'MemberExpression') { - markPropTypesAsUsed(node.parent, allNames); - } - // Do not mark computed props as used. - type = name !== '__COMPUTED_PROP__' ? 'direct' : null; - } else if ( - node.parent.id && - node.parent.id.properties && - node.parent.id.properties.length && - getKeyValue(node.parent.id.properties[0]) - ) { - type = 'destructuring'; - properties = node.parent.id.properties; + const processedExpression = getProcessedMemberExpression(node, usedPropTypes); + if (!processedExpression) { + break; } + const startingNames = processedExpression.parentUsage ? processedExpression.parentUsage.allNames : []; + traverseMemberExpression(processedExpression.startingNode, (propertyName, currentNames, currentNode) => { + if (propertyName === '__COMPUTED_PROP__') { + return; + } + + // Ignore Object methods + if (Object.prototype[propertyName] && typeof Object.prototype[propertyName] === 'function') { + return; + } + + // Ignore Array methods + if (Array.prototype[propertyName] && typeof Array.prototype[propertyName] === 'function') { + return; + } + + usedPropTypes.push({ + name: propertyName, + allNames: currentNames, + node: currentNode.property + }); + if (parentIsDestructured(currentNode)) { + markPropTypesInProperties(currentNode.parent.id.properties, usedPropTypes, currentNames); + } + }, startingNames); break; case 'ArrowFunctionExpression': case 'FunctionDeclaration': case 'FunctionExpression': - type = 'destructuring'; - properties = node.params[0].properties; + markPropTypesInProperties(node.params[0].properties, usedPropTypes); break; case 'VariableDeclarator': + const isInClassComponent = (utils.getParentES6Component() || utils.getParentES5Component()); + const isInStatelessComponent = !!utils.getParentStatelessComponent() & !isInClassComponent; + + // // let {firstname} = props + const directDestructuring = + PROPS_REGEX.test(node.init.name) && + (isInStatelessComponent || inConstructor() || inComponentWillReceiveProps()); + for (let i = 0, j = node.id.properties.length; i < j; i++) { // let {props: {firstname}} = this const thisDestructuring = ( @@ -612,20 +788,23 @@ module.exports = { (PROPS_REGEX.test(node.id.properties[i].key.name) || PROPS_REGEX.test(node.id.properties[i].key.value)) && node.id.properties[i].value.type === 'ObjectPattern' ); - // let {firstname} = props - const directDestructuring = - PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) - ; - - if (thisDestructuring) { - properties = node.id.properties[i].value.properties; - } else if (directDestructuring) { - properties = node.id.properties; - } else { - continue; + + const properties = thisDestructuring ? node.id.properties[i].value.properties : node.id.properties; + let allNames = []; + if (!thisDestructuring && !directDestructuring) { + // let {props: { a }} + // let { b } = a + const parentUsage = usedPropTypes.find( + propType => propType.name === node.init.name + ); + if (!parentUsage) { + continue; + } + if (parentUsage) { + allNames = parentUsage.allNames; + } } - type = 'destructuring'; + markPropTypesInProperties(properties, usedPropTypes, allNames); break; } break; @@ -633,61 +812,12 @@ module.exports = { throw new Error(`${node.type} ASTNodes are not handled by markPropTypesAsUsed`); } - const component = components.get(utils.getParentComponent()); - const usedPropTypes = (component && component.usedPropTypes || []).slice(); - - switch (type) { - case 'direct': - // Ignore Object methods - if (Object.prototype[name]) { - break; - } - - const isDirectProp = DIRECT_PROPS_REGEX.test(sourceCode.getText(node)); - - usedPropTypes.push({ - name: name, - allNames: allNames, - node: ( - !isDirectProp && !inConstructor() && !inComponentWillReceiveProps() ? - node.parent.property : - node.property - ) - }); - break; - case 'destructuring': - for (let k = 0, l = properties.length; k < l; k++) { - if (hasSpreadOperator(properties[k]) || properties[k].computed) { - continue; - } - const propName = getKeyValue(properties[k]); - - let currentNode = node; - allNames = []; - while (currentNode.property && !PROPS_REGEX.test(currentNode.property.name)) { - allNames.unshift(currentNode.property.name); - currentNode = currentNode.object; - } - allNames.push(propName); - - if (propName) { - usedPropTypes.push({ - name: propName, - allNames: allNames, - node: properties[k] - }); - } - } - break; - default: - break; - } - components.set(node, { usedPropTypes: usedPropTypes }); } + /** * Marks all props found inside ObjectTypeAnnotaiton as declared. * @@ -937,6 +1067,7 @@ module.exports = { markAnnotatedFunctionArgumentsAsDeclared(node); } + // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- @@ -964,20 +1095,9 @@ module.exports = { }, VariableDeclarator: function(node) { - const destructuring = node.init && node.id && node.id.type === 'ObjectPattern'; - // let {props: {firstname}} = this - const thisDestructuring = destructuring && node.init.type === 'ThisExpression'; - // let {firstname} = props - const directDestructuring = - destructuring && - PROPS_REGEX.test(node.init.name) && - (utils.getParentStatelessComponent() || inConstructor() || inComponentWillReceiveProps()) - ; - - if (!thisDestructuring && !directDestructuring) { - return; + if (isDestructuredVariableDeclaration(node) && node.init.type !== 'MemberExpression') { + markPropTypesAsUsed(node); } - markPropTypesAsUsed(node); }, FunctionDeclaration: handleStatelessComponent, @@ -993,10 +1113,10 @@ module.exports = { MemberExpression: function(node) { let type; - if (isPropTypesUsage(node)) { - type = 'usage'; - } else if (propsUtil.isPropTypesDeclaration(node)) { + if (propsUtil.isPropTypesDeclaration(node)) { type = 'declaration'; + } else if (isPropTypesUsage(node)) { + type = 'usage'; } switch (type) { diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index 08153a711a..9342fa2722 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -1861,7 +1861,6 @@ ruleTester.run('prop-types', rule, { parser: 'babel-eslint' } ], - invalid: [ { code: [ @@ -2445,7 +2444,6 @@ ruleTester.run('prop-types', rule, { parser: 'babel-eslint', errors: [ {message: '\'names\' is missing in props validation'}, - {message: '\'names.map\' is missing in props validation'}, {message: '\'company\' is missing in props validation'} ] }, { @@ -3533,6 +3531,116 @@ ruleTester.run('prop-types', rule, { message: '\'fooBar\' is missing in props validation' }], parser: 'babel-eslint' + }, + { + code: ` + const Foo = ({ a: { b } }) => { + const { c } = b + return
{c.d}
+ } + Foo.propTypes = { + a: PropTypes.object.shape({ + b: PropTypes.object.shape({ + c: PropTypes.object.shape({}) + }) + }), + } + `, + errors: [ + {message: '\'a.b.c.d\' is missing in props validation'} + ], + parser: 'babel-eslint' + }, + { + code: ` + const Foo = props => { + const { ignore } = this.props + const { b } = props.a + return
{b}
+ } + Foo.propTypes = { + a: PropTypes.object.shape({}), + } + `, + errors: [ + {message: '\'a.b\' is missing in props validation'} + ], + parser: 'babel-eslint' + }, + { + code: ` + class Foo extends React.Component { + handler = () => { + const { handlerIgnore } = props + } + render = () => { + const { renderIgnore } = props + const { a } = this.props + const { c } = a.b + } + } + Foo.propTypes = { + a: PropTypes.object.shape({ + b: PropTypes.object.shape({}) + }), + } + `, + errors: [ + {message: '\'a.b.c\' is missing in props validation'} + ], + parser: 'babel-eslint' + }, + { + code: ` + class Foo extends React.Component { + componentWillReceiveProps = nextProps => { + const { b } = nextProps.a + const { d } = this.props.c + } + render = () => { + return
test
+ } + } + Foo.propTypes = { + a: PropTypes.object.shape({}), + c: PropTypes.object.shape({}), + } + `, + errors: [ + {message: '\'a.b\' is missing in props validation'}, + {message: '\'c.d\' is missing in props validation'} + ], + parser: 'babel-eslint' + }, + { + code: ` + const Foo = ({ a: renamedA }) => { + const { b } = renamedA + return
test
+ } + Foo.propTypes = { + a: PropTypes.object.shape({}), + } + `, + errors: [ + {message: '\'a.b\' is missing in props validation'} + ], + parser: 'babel-eslint' + }, + { + code: ` + const Foo = ({ a: renamedA }) => { + return
{renamedA.b}
+ } + Foo.propTypes = { + a: PropTypes.object.shape({}), + } + `, + errors: [ + {message: '\'a.b\' is missing in props validation'} + ], + parser: 'babel-eslint' } + ] });