diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index c652e5bc12..aa53691567 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -103,7 +103,7 @@ module.exports = { return true; } // Consider every children as declared - if (propType.children === true || propType.containsSpread || propType.containsIndexers) { + if (propType.children === true || propType.containsUnresolvedSpread || propType.containsIndexers) { return true; } if (propType.acceptedProperties) { diff --git a/lib/util/ast.js b/lib/util/ast.js index 13d9ff8997..1111c992d9 100644 --- a/lib/util/ast.js +++ b/lib/util/ast.js @@ -165,6 +165,9 @@ function getKeyValue(context, node) { stripQuotes(tokens[0].value) ); } + if (node.type === 'GenericTypeAnnotation') { + return node.id.name; + } const key = node.key || node.argument; return key.type === 'Identifier' ? key.name : key.value; } diff --git a/lib/util/propTypes.js b/lib/util/propTypes.js index b03607b73c..c60f92c767 100644 --- a/lib/util/propTypes.js +++ b/lib/util/propTypes.js @@ -32,13 +32,19 @@ function isSuperTypeParameterPropsDeclaration(node) { * @param {Object[]} properties Array of properties to iterate. * @param {Function} fn Function to call on each property, receives property key and property value. (key, value) => void + * @param {Function} [handleSpreadFn] Function to call on each ObjectTypeSpreadProperty, receives the + argument */ -function iterateProperties(context, properties, fn) { +function iterateProperties(context, properties, fn, handleSpreadFn) { if (properties && properties.length && typeof fn === 'function') { for (let i = 0, j = properties.length; i < j; i++) { const node = properties[i]; const key = getKeyValue(context, node); + if (node.type === 'ObjectTypeSpreadProperty' && typeof handleSpreadFn === 'function') { + handleSpreadFn(node.argument); + } + const value = node.value; fn(key, value, node); } @@ -121,7 +127,8 @@ module.exports = function propTypesInstructions(context, components, utils) { }, ObjectTypeAnnotation(annotation, parentName, seen) { - let containsObjectTypeSpread = false; + let containsUnresolvedObjectTypeSpread = false; + let containsSpread = false; const containsIndexers = Boolean(annotation.indexers && annotation.indexers.length); const shapeTypeDefinition = { type: 'shape', @@ -129,9 +136,7 @@ module.exports = function propTypesInstructions(context, components, utils) { }; iterateProperties(context, annotation.properties, (childKey, childValue, propNode) => { const fullName = [parentName, childKey].join('.'); - if (!childKey && !childValue) { - containsObjectTypeSpread = true; - } else { + if (childKey || childValue) { const types = buildTypeAnnotationDeclarationTypes(childValue, fullName, seen); types.fullName = fullName; types.name = childKey; @@ -139,12 +144,24 @@ module.exports = function propTypesInstructions(context, components, utils) { types.isRequired = !childValue.optional; shapeTypeDefinition.children[childKey] = types; } + }, + (spreadNode) => { + const key = getKeyValue(context, spreadNode); + const types = buildTypeAnnotationDeclarationTypes(spreadNode, key, seen); + if (!types.children) { + containsUnresolvedObjectTypeSpread = true; + } else { + Object.assign(shapeTypeDefinition, types.children); + } + containsSpread = true; }); // Mark if this shape has spread or an indexer. We will know to consider all props from this shape as having propTypes, // but still have the ability to detect unused children of this shape. - shapeTypeDefinition.containsSpread = containsObjectTypeSpread; + shapeTypeDefinition.containsUnresolvedSpread = containsUnresolvedObjectTypeSpread; shapeTypeDefinition.containsIndexers = containsIndexers; + // Deprecated: containsSpread is not used anymore in the codebase, ensure to keep API backward compatibility + shapeTypeDefinition.containsSpread = containsSpread; return shapeTypeDefinition; }, @@ -241,7 +258,7 @@ module.exports = function propTypesInstructions(context, components, utils) { } /** - * Marks all props found inside ObjectTypeAnnotaiton as declared. + * Marks all props found inside ObjectTypeAnnotation as declared. * * Modifies the declaredProperties object * @param {ASTNode} propTypes @@ -253,7 +270,7 @@ module.exports = function propTypesInstructions(context, components, utils) { iterateProperties(context, propTypes.properties, (key, value, propNode) => { if (!value) { - ignorePropsValidation = true; + ignorePropsValidation = ignorePropsValidation || propNode.type !== 'ObjectTypeSpreadProperty'; return; } @@ -263,6 +280,15 @@ module.exports = function propTypesInstructions(context, components, utils) { types.node = propNode; types.isRequired = !propNode.optional; declaredPropTypes[key] = types; + }, (spreadNode) => { + const key = getKeyValue(context, spreadNode); + const spreadAnnotation = getInTypeScope(key); + if (!spreadAnnotation) { + ignorePropsValidation = true; + } else { + const spreadIgnoreValidation = declarePropTypesForObjectTypeAnnotation(spreadAnnotation, declaredPropTypes); + ignorePropsValidation = ignorePropsValidation || spreadIgnoreValidation; + } }); return ignorePropsValidation; diff --git a/tests/lib/rules/default-props-match-prop-types.js b/tests/lib/rules/default-props-match-prop-types.js index 0e6c6d0be6..5e244cbec7 100644 --- a/tests/lib/rules/default-props-match-prop-types.js +++ b/tests/lib/rules/default-props-match-prop-types.js @@ -733,6 +733,58 @@ ruleTester.run('default-props-match-prop-types', rule, { ].join('\n'), parser: parsers.BABEL_ESLINT }, + { + code: [ + 'type DefaultProps1 = {|', + ' bar1?: string', + '|};', + 'type DefaultProps2 = {|', + ' ...DefaultProps1,', + ' bar2?: string', + '|};', + 'type Props = {', + ' foo: string,', + ' ...DefaultProps2', + '};', + + 'function Hello(props: Props) {', + ' return
Hello {props.foo}
;', + '}', + + 'Hello.defaultProps = {', + ' bar1: "bar1",', + ' bar2: "bar2",', + '};' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, + { + code: [ + 'type DefaultProps1 = {|', + ' bar1?: string', + '|};', + 'type DefaultProps2 = {|', + ' ...DefaultProps1,', + ' bar2?: string', + '|};', + 'type Props = {', + ' foo: string,', + ' ...DefaultProps2', + '};', + + 'class Hello extends React.Component {', + ' render() {', + ' return
Hello {props.foo}
;', + ' }', + '}', + + 'Hello.defaultProps = {', + ' bar1: "bar1",', + ' bar2: "bar2",', + '};' + ].join('\n'), + parser: parsers.BABEL_ESLINT + }, // don't error when variable is not in scope { code: [ @@ -1460,7 +1512,6 @@ ruleTester.run('default-props-match-prop-types', rule, { column: 3 }] }, - // Investigate why this test fails. Flow type not finding foo? { code: [ 'function Hello(props: { foo: string }) {', @@ -1590,6 +1641,70 @@ ruleTester.run('default-props-match-prop-types', rule, { message: 'defaultProp "firstProperty" defined for isRequired propType.' } ] + }, + { + code: [ + 'type DefaultProps = {', + ' baz?: string,', + ' bar?: string', + '};', + + 'type Props = {', + ' foo: string,', + ' ...DefaultProps', + '}', + + 'function Hello(props: Props) {', + ' return
Hello {props.foo}
;', + '}', + 'Hello.defaultProps = { foo: "foo", frob: "frob", baz: "bar" };' + ].join('\n'), + parser: parsers.BABEL_ESLINT, + errors: [ + { + message: 'defaultProp "foo" defined for isRequired propType.', + line: 12, + column: 24 + }, + { + message: 'defaultProp "frob" has no corresponding propTypes declaration.', + line: 12, + column: 36 + } + ] + }, + { + code: [ + 'type DefaultProps = {', + ' baz?: string,', + ' bar?: string', + '};', + + 'type Props = {', + ' foo: string,', + ' ...DefaultProps', + '}', + + 'class Hello extends React.Component {', + ' render() {', + ' return
Hello {props.foo}
;', + ' }', + '}', + 'Hello.defaultProps = { foo: "foo", frob: "frob", baz: "bar" };' + ].join('\n'), + parser: parsers.BABEL_ESLINT, + errors: [ + { + message: 'defaultProp "foo" defined for isRequired propType.', + line: 14, + column: 24 + }, + { + message: 'defaultProp "frob" has no corresponding propTypes declaration.', + line: 14, + column: 36 + } + ] } ] }); diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js index b5ba74ea25..df76b3117e 100644 --- a/tests/lib/rules/no-unused-prop-types.js +++ b/tests/lib/rules/no-unused-prop-types.js @@ -4619,6 +4619,29 @@ ruleTester.run('no-unused-prop-types', rule, { errors: [{ message: '\'aProp\' PropType is defined but prop is never used' }] + }, { + // issue #2138 + code: ` + type UsedProps = {| + usedProp: number, + |}; + + type UnusedProps = {| + unusedProp: number, + |}; + + type Props = {| ...UsedProps, ...UnusedProps |}; + + function MyComponent({ usedProp, notOne }: Props) { + return
{usedProp}
; + } + `, + parser: parsers.BABEL_ESLINT, + errors: [{ + message: "'unusedProp' PropType is defined but prop is never used", + line: 7, + column: 23 + }] }, { code: ` type Props = { diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index dedf5ff727..7f533b4d26 100755 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -1632,6 +1632,23 @@ ruleTester.run('prop-types', rule, { '}' ].join('\n'), parser: parsers.BABEL_ESLINT + }, { + code: [ + 'type OtherProps = {', + ' firstname: string,', + '};', + 'type Props = {', + ' ...OtherProps,', + ' lastname: string', + '};', + 'class Hello extends React.Component {', + ' props: Props;', + ' render () {', + ' return
Hello {this.props.firstname}
;', + ' }', + '}' + ].join('\n'), + parser: parsers.BABEL_ESLINT }, { code: [ 'type Person = {', @@ -2335,6 +2352,30 @@ ruleTester.run('prop-types', rule, { `, parser: parsers.BABEL_ESLINT }, + { + // issue #2138 + code: ` + type UsedProps = {| + usedProp: number, + |}; + + type UnusedProps = {| + unusedProp: number, + |}; + + type Props = {| ...UsedProps, ...UnusedProps |}; + + function MyComponent({ usedProp }: Props) { + return
{usedProp}
; + } + `, + parser: parsers.BABEL_ESLINT, + errors: [{ + message: "'notOne' is missing in props validation", + line: 8, + column: 34 + }] + }, { // issue #1259 code: ` @@ -4728,6 +4769,30 @@ ruleTester.run('prop-types', rule, { message: '\'initialValues\' is missing in props validation' }] }, + { + // issue #2138 + code: ` + type UsedProps = {| + usedProp: number, + |}; + + type UnusedProps = {| + unusedProp: number, + |}; + + type Props = {| ...UsedProps, ...UnusedProps |}; + + function MyComponent({ usedProp, notOne }: Props) { + return
{usedProp}
; + } + `, + parser: parsers.BABEL_ESLINT, + errors: [{ + message: "'notOne' is missing in props validation", + line: 12, + column: 42 + }] + }, { // issue #2298 code: `