From 48e4be08c3afd08d4b2bef24b76893b073540d0b Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 15:16:28 -0700 Subject: [PATCH] Make require-default-props use defaultProps detection --- lib/rules/default-props-match-prop-types.js | 9 +- lib/rules/require-default-props.js | 124 ++------------------ lib/util/defaultProps.js | 13 +- 3 files changed, 26 insertions(+), 120 deletions(-) diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index ff91b28ed3..99ca165757 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -265,8 +265,9 @@ module.exports = { return; } - defaultProps.forEach(defaultProp => { - const prop = propFromName(propTypes, defaultProp.name); + Object.keys(defaultProps).forEach(defaultPropName => { + const defaultProp = defaultProps[defaultPropName]; + const prop = propFromName(propTypes, defaultPropName); if (prop && (allowRequiredDefaults || !prop.isRequired)) { return; @@ -276,13 +277,13 @@ module.exports = { context.report( defaultProp.node, 'defaultProp "{{name}}" defined for isRequired propType.', - {name: defaultProp.name} + {name: defaultPropName} ); } else { context.report( defaultProp.node, 'defaultProp "{{name}}" has no corresponding propTypes declaration.', - {name: defaultProp.name} + {name: defaultPropName} ); } }); diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index 0704e8e323..8b21a9841c 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -176,36 +176,6 @@ module.exports = { }); } - /** - * Extracts a DefaultProp from an ObjectExpression node. - * @param {ASTNode} objectExpression ObjectExpression node. - * @returns {Object|string} Object representation of a defaultProp, to be consumed by - * `addDefaultPropsToComponent`, or string "unresolved", if the defaultProps - * from this ObjectExpression can't be resolved. - */ - function getDefaultPropsFromObjectExpression(objectExpression) { - const hasSpread = objectExpression.properties.find(property => property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement'); - - if (hasSpread) { - return 'unresolved'; - } - - return objectExpression.properties.map(property => sourceCode.getText(property.key).replace(QUOTES_REGEX, '')); - } - - /** - * Marks a component's DefaultProps declaration as "unresolved". A component's DefaultProps is - * marked as "unresolved" if we cannot safely infer the values of its defaultProps declarations - * without risking false negatives. - * @param {Object} component The component to mark. - * @returns {void} - */ - function markDefaultPropsAsUnresolved(component) { - components.set(component.node, { - defaultProps: 'unresolved' - }); - } - /** * Adds propTypes to the component passed in. * @param {ASTNode} component The component to add the propTypes to. @@ -220,35 +190,6 @@ module.exports = { }); } - /** - * Adds defaultProps to the component passed in. - * @param {ASTNode} component The component to add the defaultProps to. - * @param {String[]|String} defaultProps defaultProps to add to the component or the string "unresolved" - * if this component has defaultProps that can't be resolved. - * @returns {void} - */ - function addDefaultPropsToComponent(component, defaultProps) { - // Early return if this component's defaultProps is already marked as "unresolved". - if (component.defaultProps === 'unresolved') { - return; - } - - if (defaultProps === 'unresolved') { - markDefaultPropsAsUnresolved(component); - return; - } - - const defaults = component.defaultProps || {}; - - defaultProps.forEach(defaultProp => { - defaults[defaultProp] = true; - }); - - components.set(component.node, { - defaultProps: defaults - }); - } - /** * Tries to find a props type annotation in a stateless component. * @param {ASTNode} node The AST node to look for a props type annotation. @@ -370,9 +311,8 @@ module.exports = { return { MemberExpression: function(node) { const isPropType = propsUtil.isPropTypesDeclaration(node); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(node); - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -394,39 +334,21 @@ module.exports = { if (node.parent.type === 'AssignmentExpression') { const expression = resolveNodeValue(node.parent.right); if (!expression || expression.type !== 'ObjectExpression') { - // If a value can't be found, we mark the defaultProps declaration as "unresolved", because - // we should ignore this component and not report any errors for it, to avoid false-positives - // with e.g. external defaultProps declarations. - if (isDefaultProp) { - markDefaultPropsAsUnresolved(component); - } - return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } - + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); return; } // e.g.: // MyComponent.propTypes.baz = PropTypes.string; if (node.parent.type === 'MemberExpression' && node.parent.parent.type === 'AssignmentExpression') { - if (isPropType) { - addPropTypesToComponent(component, [{ - name: node.parent.property.name, - isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), - node: node.parent.parent - }]); - } else { - addDefaultPropsToComponent(component, [node.parent.property.name]); - } - - return; + addPropTypesToComponent(component, [{ + name: node.parent.property.name, + isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), + node: node.parent.parent + }]); } }, @@ -452,9 +374,8 @@ module.exports = { } const isPropType = propsUtil.isPropTypesDeclaration(node); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(node); - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -474,11 +395,7 @@ module.exports = { return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); }, // e.g.: @@ -508,9 +425,8 @@ module.exports = { } const isPropType = astUtil.getPropertyName(node) === 'propTypes'; - const isDefaultProp = astUtil.getPropertyName(node) === 'defaultProps' || astUtil.getPropertyName(node) === 'getDefaultProps'; - if (!isPropType && !isDefaultProp) { + if (!isPropType) { return; } @@ -525,11 +441,7 @@ module.exports = { return; } - if (isPropType) { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } else { - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(expression)); - } + addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); }, // e.g.: @@ -560,25 +472,11 @@ module.exports = { } const isPropType = propsUtil.isPropTypesDeclaration(property); - const isDefaultProp = propsUtil.isDefaultPropsDeclaration(property); - - if (!isPropType && !isDefaultProp) { - return; - } if (isPropType && property.value.type === 'ObjectExpression') { addPropTypesToComponent(component, getPropTypesFromObjectExpression(property.value)); return; } - - if (isDefaultProp && property.value.type === 'FunctionExpression') { - const returnStatement = utils.findReturnStatement(property); - if (!returnStatement || returnStatement.argument.type !== 'ObjectExpression') { - return; - } - - addDefaultPropsToComponent(component, getDefaultPropsFromObjectExpression(returnStatement.argument)); - } }); }, diff --git a/lib/util/defaultProps.js b/lib/util/defaultProps.js index 00bef47f4c..8546c5e654 100644 --- a/lib/util/defaultProps.js +++ b/lib/util/defaultProps.js @@ -7,7 +7,10 @@ const astUtil = require('./ast'); const propsUtil = require('./props'); const variableUtil = require('./variable'); +const QUOTES_REGEX = /^["']|["']$/g; + module.exports = function defaultPropsInstructions(context, components, utils) { + const sourceCode = context.getSourceCode(); const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); /** @@ -45,7 +48,7 @@ module.exports = function defaultPropsInstructions(context, components, utils) { } return objectExpression.properties.map(defaultProp => ({ - name: defaultProp.key.name, + name: sourceCode.getText(defaultProp.key).replace(QUOTES_REGEX, ''), node: defaultProp })); } @@ -81,10 +84,14 @@ module.exports = function defaultPropsInstructions(context, components, utils) { return; } - const defaults = component.defaultProps || []; + const defaults = component.defaultProps || {}; + const newDefaultProps = defaultProps.reduce((acc, prop) => { + acc[prop.name] = prop; + return acc; + }, Object.assign({}, defaults)); components.set(component.node, { - defaultProps: defaults.concat(defaultProps) + defaultProps: newDefaultProps }); }