From 70155b4f85c85165e157e5141053906c084f1398 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 16:31:27 -0700 Subject: [PATCH 1/5] Extract isRequired detection from default-props-match-prop-types --- lib/rules/default-props-match-prop-types.js | 416 +------------------- lib/util/propTypes.js | 23 +- 2 files changed, 24 insertions(+), 415 deletions(-) diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index fb9c082a26..6270d2042d 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -6,10 +6,6 @@ 'use strict'; const Components = require('../util/Components'); -const variableUtil = require('../util/variable'); -const annotations = require('../util/annotations'); -const astUtil = require('../util/ast'); -const propsUtil = require('../util/props'); const docsUrl = require('../util/docsUrl'); // ------------------------------------------------------------------------------ @@ -36,219 +32,9 @@ module.exports = { }] }, - create: Components.detect((context, components, utils) => { + create: Components.detect((context, components) => { const configuration = context.options[0] || {}; const allowRequiredDefaults = configuration.allowRequiredDefaults || false; - const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); - // Used to track the type annotations in scope. - // Necessary because babel's scopes do not track type annotations. - let stack = null; - - /** - * Try to resolve the node passed in to a variable in the current scope. If the node passed in is not - * an Identifier, then the node is simply returned. - * @param {ASTNode} node The node to resolve. - * @returns {ASTNode|null} Return null if the value could not be resolved, ASTNode otherwise. - */ - function resolveNodeValue(node) { - if (node.type === 'Identifier') { - return variableUtil.findVariableByName(context, node.name); - } - if ( - node.type === 'CallExpression' && - propWrapperFunctions.has(node.callee.name) && - node.arguments && node.arguments[0] - ) { - return resolveNodeValue(node.arguments[0]); - } - return node; - } - - /** - * Helper for accessing the current scope in the stack. - * @param {string} key The name of the identifier to access. If omitted, returns the full scope. - * @param {ASTNode} value If provided sets the new value for the identifier. - * @returns {Object|ASTNode} Either the whole scope or the ASTNode associated with the given identifier. - */ - function typeScope(key, value) { - if (arguments.length === 0) { - return stack[stack.length - 1]; - } else if (arguments.length === 1) { - return stack[stack.length - 1][key]; - } - stack[stack.length - 1][key] = value; - return value; - } - - /** - * Tries to find the definition of a GenericTypeAnnotation in the current scope. - * @param {ASTNode} node The node GenericTypeAnnotation node to resolve. - * @return {ASTNode|null} Return null if definition cannot be found, ASTNode otherwise. - */ - function resolveGenericTypeAnnotation(node) { - if (node.type !== 'GenericTypeAnnotation' || node.id.type !== 'Identifier') { - return null; - } - - return variableUtil.findVariableByName(context, node.id.name) || typeScope(node.id.name); - } - - function resolveUnionTypeAnnotation(node) { - // Go through all the union and resolve any generic types. - return node.types.map(annotation => { - if (annotation.type === 'GenericTypeAnnotation') { - return resolveGenericTypeAnnotation(annotation); - } - - return annotation; - }); - } - - /** - * Extracts a PropType from an ObjectExpression node. - * @param {ASTNode} objectExpression ObjectExpression node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromObjectExpression(objectExpression) { - const props = objectExpression.properties.filter(property => property.type !== 'ExperimentalSpreadProperty' && property.type !== 'SpreadElement'); - - return props.map(property => ({ - name: property.key.name, - isRequired: propsUtil.isRequiredPropType(property.value), - node: property - })); - } - - /** - * Handles Props defined in IntersectionTypeAnnotation nodes - * e.g. type Props = PropsA & PropsB - * @param {ASTNode} intersectionTypeAnnotation ObjectExpression node. - * @returns {Object[]} - */ - function getPropertiesFromIntersectionTypeAnnotationNode(annotation) { - return annotation.types.reduce((properties, type) => { - annotation = resolveGenericTypeAnnotation(type); - - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - if (!annotation || !annotation.properties) { - return properties; - } - - return properties.concat(annotation.properties); - }, []); - } - - /** - * Extracts a PropType from a TypeAnnotation node. - * @param {ASTNode} node TypeAnnotation node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromTypeAnnotation(node) { - let properties = []; - - switch (node.typeAnnotation.type) { - case 'GenericTypeAnnotation': - let annotation = resolveGenericTypeAnnotation(node.typeAnnotation); - - if (annotation && annotation.type === 'IntersectionTypeAnnotation') { - properties = getPropertiesFromIntersectionTypeAnnotationNode(annotation); - } else { - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - properties = annotation ? (annotation.properties || []) : []; - } - - break; - - case 'UnionTypeAnnotation': - const union = resolveUnionTypeAnnotation(node.typeAnnotation); - properties = union.reduce((acc, curr) => { - if (!curr) { - return acc; - } - - return acc.concat(curr.properties); - }, []); - break; - - case 'ObjectTypeAnnotation': - properties = node.typeAnnotation.properties; - break; - - default: - properties = []; - break; - } - - const props = properties.filter(property => property.type === 'ObjectTypeProperty'); - - return props.map(property => { - // the `key` property is not present in ObjectTypeProperty nodes, so we need to get the key name manually. - const tokens = context.getFirstTokens(property, 1); - const name = tokens[0].value; - - return { - name: name, - isRequired: !property.optional, - node: property - }; - }); - } - - /** - * Adds propTypes to the component passed in. - * @param {ASTNode} component The component to add the propTypes to. - * @param {Object[]} propTypes propTypes to add to the component. - * @returns {void} - */ - function addPropTypesToComponent(component, propTypes) { - const props = component.propTypes || []; - - components.set(component.node, { - propTypes: props.concat(propTypes) - }); - } - - /** - * Tries to find a props type annotation in a stateless component. - * @param {ASTNode} node The AST node to look for a props type annotation. - * @return {void} - */ - function handleStatelessComponent(node) { - if (!node.params || !node.params.length || !annotations.isAnnotatedFunctionPropsDeclaration(node, context)) { - return; - } - - // find component this props annotation belongs to - const component = components.get(utils.getParentStatelessComponent()); - if (!component) { - return; - } - - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.params[0].typeAnnotation, context)); - } - - function handlePropTypeAnnotationClassProperty(node) { - // find component this props annotation belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.typeAnnotation, context)); - } - - function isPropTypeAnnotation(node) { - return (astUtil.getPropertyName(node) === 'props' && !!node.typeAnnotation); - } - - function propFromName(propTypes, name) { - return propTypes.find(prop => prop.name === name); - } /** * Reports all defaultProps passed in that don't have an appropriate propTypes counterpart. @@ -260,13 +46,13 @@ module.exports = { // If this defaultProps is "unresolved" or the propTypes is undefined, then we should ignore // this component and not report any errors for it, to avoid false-positives with e.g. // external defaultProps/propTypes declarations or spread operators. - if (defaultProps === 'unresolved' || !propTypes) { + if (defaultProps === 'unresolved' || !propTypes || Object.keys(propTypes).length === 0) { return; } Object.keys(defaultProps).forEach(defaultPropName => { const defaultProp = defaultProps[defaultPropName]; - const prop = propFromName(propTypes, defaultPropName); + const prop = propTypes[defaultPropName]; if (prop && (allowRequiredDefaults || !prop.isRequired)) { return; @@ -293,201 +79,7 @@ module.exports = { // -------------------------------------------------------------------------- return { - MemberExpression: function(node) { - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = utils.getRelatedComponent(node); - if (!component) { - return; - } - - // e.g.: - // MyComponent.propTypes = { - // foo: React.PropTypes.string.isRequired, - // bar: React.PropTypes.string - // }; - // - // or: - // - // MyComponent.propTypes = myPropTypes; - if (node.parent.type === 'AssignmentExpression') { - const expression = resolveNodeValue(node.parent.right); - if (expression && expression.type === 'ObjectExpression') { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - } - - return; - } - - // e.g.: - // MyComponent.propTypes.baz = React.PropTypes.string; - if (node.parent.type === 'MemberExpression' && node.parent.parent && - node.parent.parent.type === 'AssignmentExpression') { - addPropTypesToComponent(component, [{ - name: node.parent.property.name, - isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), - node: node.parent.parent - }]); - } - }, - - // e.g.: - // class Hello extends React.Component { - // static get propTypes() { - // return { - // name: React.PropTypes.string - // }; - // } - // static get defaultProps() { - // return { - // name: 'Dean' - // }; - // } - // render() { - // return
Hello {this.props.name}
; - // } - // } - MethodDefinition: function(node) { - if (!node.static || node.kind !== 'get') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const returnStatement = utils.findReturnStatement(node); - if (!returnStatement) { - return; - } - - const expression = resolveNodeValue(returnStatement.argument); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // class Greeting extends React.Component { - // render() { - // return ( - //

Hello, {this.props.foo} {this.props.bar}

- // ); - // } - // static propTypes = { - // foo: React.PropTypes.string, - // bar: React.PropTypes.string.isRequired - // }; - // } - ClassProperty: function(node) { - if (isPropTypeAnnotation(node)) { - handlePropTypeAnnotationClassProperty(node); - return; - } - - if (!node.static) { - return; - } - - if (!node.value) { - return; - } - - const propName = astUtil.getPropertyName(node); - const isPropType = propName === 'propTypes'; - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const expression = resolveNodeValue(node.value); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // React.createClass({ - // render: function() { - // return
{this.props.foo}
; - // }, - // propTypes: { - // foo: React.PropTypes.string.isRequired, - // }, - // getDefaultProps: function() { - // return { - // foo: 'default' - // }; - // } - // }); - ObjectExpression: function(node) { - // find component this propTypes/defaultProps belongs to - const component = utils.isES5Component(node) && components.get(node); - if (!component) { - return; - } - - // Search for the proptypes declaration - node.properties.forEach(property => { - if (property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(property); - - if (isPropType && property.value.type === 'ObjectExpression') { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(property.value)); - return; - } - }); - }, - - TypeAlias: function(node) { - typeScope(node.id.name, node.right); - }, - - Program: function() { - stack = [{}]; - }, - - BlockStatement: function () { - stack.push(Object.create(typeScope())); - }, - - 'BlockStatement:exit': function () { - stack.pop(); - }, - - // Check for type annotations in stateless components - FunctionDeclaration: handleStatelessComponent, - ArrowFunctionExpression: handleStatelessComponent, - FunctionExpression: handleStatelessComponent, - 'Program:exit': function() { - stack = null; const list = components.list(); Object.keys(list).forEach(component => { @@ -497,7 +89,7 @@ module.exports = { } reportInvalidDefaultProps( - list[component].propTypes, + list[component].declaredPropTypes, list[component].defaultProps || {} ); }); diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index f0ca66ee8e..68c2641891 100644 --- a/lib/util/propTypes.js +++ b/lib/util/propTypes.js @@ -61,7 +61,7 @@ function iterateProperties(context, properties, fn) { const key = getKeyValue(context, node); const value = node.value; - fn(key, value); + fn(key, value, node); } } } @@ -139,6 +139,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.fullName = fullName; types.name = childKey; types.node = childValue; + types.isRequired = !childValue.optional; shapeTypeDefinition.children[childKey] = types; } }); @@ -209,6 +210,7 @@ module.exports = function propTypesInstructions(context, components, utils) { if (annotation.type === 'GenericTypeAnnotation' && getInTypeScope(annotation.id.name)) { return getInTypeScope(annotation.id.name); } + return annotation; } @@ -248,7 +250,7 @@ module.exports = function propTypesInstructions(context, components, utils) { function declarePropTypesForObjectTypeAnnotation(propTypes, declaredPropTypes) { let ignorePropsValidation = false; - iterateProperties(context, propTypes.properties, (key, value) => { + iterateProperties(context, propTypes.properties, (key, value, propNode) => { if (!value) { ignorePropsValidation = true; return; @@ -258,6 +260,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.fullName = key; types.name = key; types.node = value; + types.isRequired = !propNode.optional; declaredPropTypes[key] = types; }); @@ -444,6 +447,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.fullName = key; types.name = key; types.node = value; + types.isRequired = propsUtil.isRequiredPropType(value); declaredPropTypes[key] = types; }); break; @@ -482,6 +486,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.name = propTypes.property.name; types.fullName = [parentProp, propTypes.property.name].join('.'); types.node = propTypes.property; + types.isRequired = propsUtil.isRequiredPropType(propTypes.parent.right); curDeclaredPropTypes[propTypes.property.name] = types; } else { let isUsedInPropTypes = false; @@ -546,7 +551,19 @@ module.exports = function propTypesInstructions(context, components, utils) { if (!node.params || !node.params.length || !annotations.isAnnotatedFunctionPropsDeclaration(node, context)) { return; } - markPropTypesAsDeclared(node, resolveTypeAnnotation(node.params[0])); + + const param = node.params[0]; + if (param.typeAnnotation && param.typeAnnotation.typeAnnotation && param.typeAnnotation.typeAnnotation.type === 'UnionTypeAnnotation') { + param.typeAnnotation.typeAnnotation.types.forEach(annotation => { + if (annotation.type === 'GenericTypeAnnotation') { + markPropTypesAsDeclared(node, resolveTypeAnnotation(annotation)); + } else { + markPropTypesAsDeclared(node, annotation); + } + }); + } else { + markPropTypesAsDeclared(node, resolveTypeAnnotation(param)); + } } /** From 2e4f91afdc8a089936a2ff49e6b4c83cccb3432a Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Sat, 11 Aug 2018 17:22:59 -0700 Subject: [PATCH 2/5] Make require-default-props use isRequired detection --- lib/rules/no-unused-prop-types.js | 2 +- lib/rules/require-default-props.js | 459 +---------------------------- lib/util/propTypes.js | 28 +- lib/util/usedPropTypes.js | 4 +- 4 files changed, 27 insertions(+), 466 deletions(-) diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js index 0e80cfd179..8aaf9f9d2f 100644 --- a/lib/rules/no-unused-prop-types.js +++ b/lib/rules/no-unused-prop-types.js @@ -103,7 +103,7 @@ module.exports = { if (prop.node && !isPropUsed(component, prop)) { context.report( - prop.node, + prop.node.value || prop.node, UNUSED_MESSAGE, { name: prop.fullName } diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index 6b418cf86a..4daf0ba3ad 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -5,13 +5,8 @@ 'use strict'; const Components = require('../util/Components'); -const variableUtil = require('../util/variable'); -const annotations = require('../util/annotations'); -const astUtil = require('../util/ast'); -const propsUtil = require('../util/props'); const docsUrl = require('../util/docsUrl'); -const QUOTES_REGEX = /^["']|["']$/g; // ------------------------------------------------------------------------------ // Rule Definition @@ -36,191 +31,10 @@ module.exports = { }] }, - create: Components.detect((context, components, utils) => { - const sourceCode = context.getSourceCode(); - const propWrapperFunctions = new Set(context.settings.propWrapperFunctions || []); + create: Components.detect((context, components) => { const configuration = context.options[0] || {}; const forbidDefaultForRequired = configuration.forbidDefaultForRequired || false; - // Used to track the type annotations in scope. - // Necessary because babel's scopes do not track type annotations. - let stack = null; - /** - * Try to resolve the node passed in to a variable in the current scope. If the node passed in is not - * an Identifier, then the node is simply returned. - * @param {ASTNode} node The node to resolve. - * @returns {ASTNode|null} Return null if the value could not be resolved, ASTNode otherwise. - */ - function resolveNodeValue(node) { - if (node.type === 'Identifier') { - return variableUtil.findVariableByName(context, node.name); - } - if ( - node.type === 'CallExpression' && - propWrapperFunctions.has(node.callee.name) && - node.arguments && node.arguments[0] - ) { - return resolveNodeValue(node.arguments[0]); - } - - return node; - } - - /** - * Helper for accessing the current scope in the stack. - * @param {string} key The name of the identifier to access. If omitted, returns the full scope. - * @param {ASTNode} value If provided sets the new value for the identifier. - * @returns {Object|ASTNode} Either the whole scope or the ASTNode associated with the given identifier. - */ - function typeScope(key, value) { - if (arguments.length === 0) { - return stack[stack.length - 1]; - } else if (arguments.length === 1) { - return stack[stack.length - 1][key]; - } - stack[stack.length - 1][key] = value; - return value; - } - - /** - * Tries to find the definition of a GenericTypeAnnotation in the current scope. - * @param {ASTNode} node The node GenericTypeAnnotation node to resolve. - * @return {ASTNode|null} Return null if definition cannot be found, ASTNode otherwise. - */ - function resolveGenericTypeAnnotation(node) { - if (node.type !== 'GenericTypeAnnotation' || node.id.type !== 'Identifier') { - return null; - } - - return variableUtil.findVariableByName(context, node.id.name) || typeScope(node.id.name); - } - - function resolveUnionTypeAnnotation(node) { - // Go through all the union and resolve any generic types. - return node.types.map(annotation => { - if (annotation.type === 'GenericTypeAnnotation') { - return resolveGenericTypeAnnotation(annotation); - } - - return annotation; - }); - } - - /** - * Extracts a PropType from an ObjectExpression node. - * @param {ASTNode} objectExpression ObjectExpression node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromObjectExpression(objectExpression) { - const props = objectExpression.properties.filter(property => property.type !== 'ExperimentalSpreadProperty' && property.type !== 'SpreadElement'); - - return props.map(property => ({ - name: sourceCode.getText(property.key).replace(QUOTES_REGEX, ''), - isRequired: propsUtil.isRequiredPropType(property.value), - node: property - })); - } - - /** - * Extracts a PropType from a TypeAnnotation node. - * @param {ASTNode} node TypeAnnotation node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromTypeAnnotation(node) { - let properties; - - switch (node.typeAnnotation.type) { - case 'GenericTypeAnnotation': - let annotation = resolveGenericTypeAnnotation(node.typeAnnotation); - - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - properties = annotation ? (annotation.properties || []) : []; - break; - - case 'UnionTypeAnnotation': - const union = resolveUnionTypeAnnotation(node.typeAnnotation); - properties = union.reduce((acc, curr) => { - if (!curr) { - return acc; - } - - return acc.concat(curr.properties); - }, []); - break; - - case 'ObjectTypeAnnotation': - properties = node.typeAnnotation.properties; - break; - - default: - properties = []; - break; - } - - const props = properties.filter(property => property.type === 'ObjectTypeProperty'); - - return props.map(property => { - // the `key` property is not present in ObjectTypeProperty nodes, so we need to get the key name manually. - const tokens = context.getFirstTokens(property, 1); - const name = tokens[0].value; - - return { - name: name, - isRequired: !property.optional, - node: property - }; - }); - } - - /** - * Adds propTypes to the component passed in. - * @param {ASTNode} component The component to add the propTypes to. - * @param {Object[]} propTypes propTypes to add to the component. - * @returns {void} - */ - function addPropTypesToComponent(component, propTypes) { - const props = component.propTypes || []; - - components.set(component.node, { - propTypes: props.concat(propTypes) - }); - } - - /** - * Tries to find a props type annotation in a stateless component. - * @param {ASTNode} node The AST node to look for a props type annotation. - * @return {void} - */ - function handleStatelessComponent(node) { - if (!node.params || !node.params.length || !annotations.isAnnotatedFunctionPropsDeclaration(node, context)) { - return; - } - - // find component this props annotation belongs to - const component = components.get(utils.getParentStatelessComponent()); - if (!component) { - return; - } - - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.params[0].typeAnnotation, context)); - } - - function handlePropTypeAnnotationClassProperty(node) { - // find component this props annotation belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - addPropTypesToComponent(component, getPropTypesFromTypeAnnotation(node.typeAnnotation, context)); - } - - function isPropTypeAnnotation(node) { - return (astUtil.getPropertyName(node) === 'props' && !!node.typeAnnotation); - } /** * Reports all propTypes passed in that don't have a defaultProp counterpart. @@ -235,297 +49,42 @@ module.exports = { return; } - propTypes.forEach(prop => { + Object.keys(propTypes).forEach(propName => { + const prop = propTypes[propName]; if (prop.isRequired) { - if (forbidDefaultForRequired && defaultProps[prop.name]) { + if (forbidDefaultForRequired && defaultProps[propName]) { context.report( prop.node, 'propType "{{name}}" is required and should not have a defaultProp declaration.', - {name: prop.name} + {name: propName} ); } return; } - if (defaultProps[prop.name]) { + if (defaultProps[propName]) { return; } context.report( prop.node, 'propType "{{name}}" is not required, but has no corresponding defaultProp declaration.', - {name: prop.name} + {name: propName} ); }); } - /** - * Extracts a PropType from a TypeAnnotation contained in generic node. - * @param {ASTNode} node TypeAnnotation node. - * @returns {Object[]} Array of PropType object representations, to be consumed by `addPropTypesToComponent`. - */ - function getPropTypesFromGeneric(node) { - let annotation = resolveGenericTypeAnnotation(node); - - if (annotation && annotation.id) { - annotation = variableUtil.findVariableByName(context, annotation.id.name); - } - - const properties = annotation ? (annotation.properties || []) : []; - - const props = properties.filter(property => property.type === 'ObjectTypeProperty'); - - return props.map(property => { - // the `key` property is not present in ObjectTypeProperty nodes, so we need to get the key name manually. - const tokens = context.getFirstTokens(property, 1); - const name = tokens[0].value; - - return { - name: name, - isRequired: !property.optional, - node: property - }; - }); - } - - function hasPropTypesAsGeneric(node) { - return node && node.parent && node.parent.type === 'ClassDeclaration'; - } - - function handlePropTypesAsGeneric(node) { - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - if (node.params[0]) { - addPropTypesToComponent(component, getPropTypesFromGeneric(node.params[0], context)); - } - } - // -------------------------------------------------------------------------- // Public API // -------------------------------------------------------------------------- return { - MemberExpression: function(node) { - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = utils.getRelatedComponent(node); - if (!component) { - return; - } - - // e.g.: - // MyComponent.propTypes = { - // foo: PropTypes.string.isRequired, - // bar: PropTypes.string - // }; - // - // or: - // - // MyComponent.propTypes = myPropTypes; - if (node.parent.type === 'AssignmentExpression') { - const expression = resolveNodeValue(node.parent.right); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - return; - } - - // e.g.: - // MyComponent.propTypes.baz = PropTypes.string; - if (node.parent.type === 'MemberExpression' && node.parent.parent.type === 'AssignmentExpression') { - addPropTypesToComponent(component, [{ - name: node.parent.property.name, - isRequired: propsUtil.isRequiredPropType(node.parent.parent.right), - node: node.parent.parent - }]); - } - }, - - // e.g.: - // class Hello extends React.Component { - // static get propTypes() { - // return { - // name: PropTypes.string - // }; - // } - // static get defaultProps() { - // return { - // name: 'Dean' - // }; - // } - // render() { - // return
Hello {this.props.name}
; - // } - // } - MethodDefinition: function(node) { - if (!node.static || node.kind !== 'get') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(node); - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const returnStatement = utils.findReturnStatement(node); - if (!returnStatement) { - return; - } - - const expression = resolveNodeValue(returnStatement.argument); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // class Greeting extends React.Component { - // render() { - // return ( - //

Hello, {this.props.foo} {this.props.bar}

- // ); - // } - // static propTypes = { - // foo: PropTypes.string, - // bar: PropTypes.string.isRequired - // }; - // } - ClassProperty: function(node) { - if (isPropTypeAnnotation(node)) { - handlePropTypeAnnotationClassProperty(node); - return; - } - - if (!node.static) { - return; - } - - if (!node.value) { - return; - } - - const isPropType = astUtil.getPropertyName(node) === 'propTypes'; - - if (!isPropType) { - return; - } - - // find component this propTypes/defaultProps belongs to - const component = components.get(utils.getParentES6Component()); - if (!component) { - return; - } - - const expression = resolveNodeValue(node.value); - if (!expression || expression.type !== 'ObjectExpression') { - return; - } - - addPropTypesToComponent(component, getPropTypesFromObjectExpression(expression)); - }, - - // e.g.: - // createReactClass({ - // render: function() { - // return
{this.props.foo}
; - // }, - // propTypes: { - // foo: PropTypes.string.isRequired, - // }, - // getDefaultProps: function() { - // return { - // foo: 'default' - // }; - // } - // }); - ObjectExpression: function(node) { - // find component this propTypes/defaultProps belongs to - const component = utils.isES5Component(node) && components.get(node); - if (!component) { - return; - } - - // Search for the proptypes declaration - node.properties.forEach(property => { - if (property.type === 'ExperimentalSpreadProperty' || property.type === 'SpreadElement') { - return; - } - - const isPropType = propsUtil.isPropTypesDeclaration(property); - - if (isPropType && property.value.type === 'ObjectExpression') { - addPropTypesToComponent(component, getPropTypesFromObjectExpression(property.value)); - return; - } - }); - }, - - TypeAlias: function(node) { - typeScope(node.id.name, node.right); - }, - - Program: function() { - stack = [{}]; - }, - - BlockStatement: function () { - stack.push(Object.create(typeScope())); - }, - - 'BlockStatement:exit': function () { - stack.pop(); - }, - - // e.g.: - // type HelloProps = { - // foo?: string - // }; - // class Hello extends React.Component { - // static defaultProps = { - // foo: 'default' - // } - // render() { - // return
{this.props.foo}
; - // } - // }; - TypeParameterInstantiation: function(node) { - if (hasPropTypesAsGeneric(node)) { - handlePropTypesAsGeneric(node); - return; - } - }, - - // Check for type annotations in stateless components - FunctionDeclaration: handleStatelessComponent, - ArrowFunctionExpression: handleStatelessComponent, - FunctionExpression: handleStatelessComponent, - 'Program:exit': function() { - stack = null; const list = components.list(); - Object.keys(list).filter(component => list[component].propTypes).forEach(component => { + Object.keys(list).filter(component => list[component].declaredPropTypes).forEach(component => { reportPropTypesWithoutDefault( - list[component].propTypes, + list[component].declaredPropTypes, list[component].defaultProps || {} ); }); diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index 68c2641891..d90f7d7713 100644 --- a/lib/util/propTypes.js +++ b/lib/util/propTypes.js @@ -130,7 +130,7 @@ module.exports = function propTypesInstructions(context, components, utils) { type: 'shape', children: {} }; - iterateProperties(context, annotation.properties, (childKey, childValue) => { + iterateProperties(context, annotation.properties, (childKey, childValue, propNode) => { const fullName = [parentName, childKey].join('.'); if (!childKey && !childValue) { containsObjectTypeSpread = true; @@ -138,7 +138,7 @@ module.exports = function propTypesInstructions(context, components, utils) { const types = buildTypeAnnotationDeclarationTypes(childValue, fullName, seen); types.fullName = fullName; types.name = childKey; - types.node = childValue; + types.node = propNode; types.isRequired = !childValue.optional; shapeTypeDefinition.children[childKey] = types; } @@ -259,7 +259,7 @@ module.exports = function propTypesInstructions(context, components, utils) { const types = buildTypeAnnotationDeclarationTypes(value, key); types.fullName = key; types.name = key; - types.node = value; + types.node = propNode; types.isRequired = !propNode.optional; declaredPropTypes[key] = types; }); @@ -352,13 +352,15 @@ module.exports = function propTypesInstructions(context, components, utils) { type: 'shape', children: {} }; - iterateProperties(context, argument.properties, (childKey, childValue) => { - const fullName = [parentName, childKey].join('.'); - const types = buildReactDeclarationTypes(childValue, fullName); - types.fullName = fullName; - types.name = childKey; - types.node = childValue; - shapeTypeDefinition.children[childKey] = types; + iterateProperties(context, argument.properties, (childKey, childValue, propNode) => { + if (childValue) { // skip spread propTypes + const fullName = [parentName, childKey].join('.'); + const types = buildReactDeclarationTypes(childValue, fullName); + types.fullName = fullName; + types.name = childKey; + types.node = propNode; + shapeTypeDefinition.children[childKey] = types; + } }); return shapeTypeDefinition; case 'arrayOf': @@ -438,7 +440,7 @@ module.exports = function propTypesInstructions(context, components, utils) { ignorePropsValidation = declarePropTypesForObjectTypeAnnotation(propTypes, declaredPropTypes); break; case 'ObjectExpression': - iterateProperties(context, propTypes.properties, (key, value) => { + iterateProperties(context, propTypes.properties, (key, value, propNode) => { if (!value) { ignorePropsValidation = true; return; @@ -446,7 +448,7 @@ module.exports = function propTypesInstructions(context, components, utils) { const types = buildReactDeclarationTypes(value, key); types.fullName = key; types.name = key; - types.node = value; + types.node = propNode; types.isRequired = propsUtil.isRequiredPropType(value); declaredPropTypes[key] = types; }); @@ -485,7 +487,7 @@ module.exports = function propTypesInstructions(context, components, utils) { types.name = propTypes.property.name; types.fullName = [parentProp, propTypes.property.name].join('.'); - types.node = propTypes.property; + types.node = propTypes.parent; types.isRequired = propsUtil.isRequiredPropType(propTypes.parent.right); curDeclaredPropTypes[propTypes.property.name] = types; } else { diff --git a/lib/util/usedPropTypes.js b/lib/util/usedPropTypes.js index dd3becf331..caec9ad450 100644 --- a/lib/util/usedPropTypes.js +++ b/lib/util/usedPropTypes.js @@ -460,8 +460,8 @@ module.exports = function usedPropTypesInstructions(context, components, utils) Object.keys(propTypes).forEach(key => { const node = propTypes[key].node; - if (astUtil.isFunctionLikeExpression(node)) { - markPropTypesAsUsed(node); + if (node.value && astUtil.isFunctionLikeExpression(node.value)) { + markPropTypesAsUsed(node.value); } }); } From 4d1b833427ef90c79c1614342f8429732954ed61 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Fri, 17 Aug 2018 21:36:16 -0700 Subject: [PATCH 3/5] Add test for #1908 --- .../rules/default-props-match-prop-types.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/lib/rules/default-props-match-prop-types.js b/tests/lib/rules/default-props-match-prop-types.js index d8b65302af..4b06b93da3 100644 --- a/tests/lib/rules/default-props-match-prop-types.js +++ b/tests/lib/rules/default-props-match-prop-types.js @@ -1568,6 +1568,27 @@ ruleTester.run('default-props-match-prop-types', rule, { message: 'defaultProp "foo" defined for isRequired propType.' } ] + }, + { + code: ` + class SomeComponent extends React.Component { + render() { + return
; + } + } + SomeComponent.propTypes = { + "firstProperty": PropTypes.string.isRequired, + }; + + SomeComponent.defaultProps = { + "firstProperty": () => undefined + }; + `, + errors: [ + { + message: 'defaultProp "firstProperty" defined for isRequired propType.' + } + ] } ] }); From 5211a2ba63952255572c749c56b8ee0f7f8603a1 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Wed, 26 Sep 2018 20:36:01 -0700 Subject: [PATCH 4/5] Add test for #1865 --- tests/lib/rules/require-default-props.js | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/lib/rules/require-default-props.js b/tests/lib/rules/require-default-props.js index de42f32a78..a1c0225def 100644 --- a/tests/lib/rules/require-default-props.js +++ b/tests/lib/rules/require-default-props.js @@ -834,6 +834,19 @@ ruleTester.run('require-default-props', rule, { ].join('\n'), parser: 'babel-eslint', options: [{forbidDefaultForRequired: true}] + }, { + code: ` + type Props = { + +name?: string, + }; + function Hello(props: Props) { + return
Hello {props.name}
; + } + Hello.defaultProps = { + name: 'foo' + }; + `, + parser: 'babel-eslint' } ], @@ -2174,6 +2187,20 @@ ruleTester.run('require-default-props', rule, { }, { message: 'propType "bar" is not required, but has no corresponding defaultProp declaration.' }] + }, + { + code: ` + type Props = { + +name?: string, + }; + function Hello(props: Props) { + return
Hello {props.name}
; + } + `, + parser: 'babel-eslint', + errors: [{ + message: 'propType "name" is not required, but has no corresponding defaultProp declaration.' + }] } ] }); From d8a615f5857b7627b629ae98bc734fca6bcb03d4 Mon Sep 17 00:00:00 2001 From: Alex Zherdev Date: Wed, 26 Sep 2018 20:47:06 -0700 Subject: [PATCH 5/5] Use filter instead of if/return --- lib/rules/default-props-match-prop-types.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/rules/default-props-match-prop-types.js b/lib/rules/default-props-match-prop-types.js index 6270d2042d..cf1597a311 100644 --- a/lib/rules/default-props-match-prop-types.js +++ b/lib/rules/default-props-match-prop-types.js @@ -82,12 +82,8 @@ module.exports = { 'Program:exit': function() { const list = components.list(); - Object.keys(list).forEach(component => { - // If no defaultProps could be found, we don't report anything. - if (!list[component].defaultProps) { - return; - } - + // If no defaultProps could be found, we don't report anything. + Object.keys(list).filter(component => list[component].defaultProps).forEach(component => { reportInvalidDefaultProps( list[component].declaredPropTypes, list[component].defaultProps || {}