Skip to content

Fix issues with IntersectionTypeAnnotation for prop-types and no-unused-prop-types #1415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 12, 2017
Merged

Fix issues with IntersectionTypeAnnotation for prop-types and no-unused-prop-types #1415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
87aca04
Add support for multiple Flow IntersectionTypeAnnotations and skip pr…
jseminck Sep 6, 2017
a771125
Add similar supprt for no-used-prop-types
jseminck Sep 6, 2017
20f75ee
Add support for TypeA & { ... }
Sep 6, 2017
19a2911
Clean up the code a bit
Sep 6, 2017
3e4d184
Aadd support also for no-unused-prop-types
Sep 6, 2017
3e9aefe
Make buildTypeAnnotationDeclarationTypes always return an object inst…
Sep 7, 2017
0ee5e93
Rewrite reduce to ignorePropsValidation as some()
Sep 7, 2017
357dcda
buildReactDeclarationTypes to always return an object
Sep 7, 2017
09c4ed7
Add tests for {...} & Props in addition to Props & {...}
jseminck Sep 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
143 changes: 76 additions & 67 deletions lib/rules/no-unused-prop-types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ module.exports = {
value.callee.object &&
hasCustomValidator(value.callee.object.name)
) {
return true;
return {};
}

if (
Expand Down Expand Up @@ -387,23 +387,20 @@ module.exports = {
switch (callName) {
case 'shape':
if (skipShapeProps) {
return true;
return {};
}

if (argument.type !== 'ObjectExpression') {
// Invalid proptype or cannot analyse statically
return true;
return {};
}
const shapeTypeDefinition = {
type: 'shape',
children: []
};
iterateProperties(argument.properties, (childKey, childValue) => {
const fullName = [parentName, childKey].join('.');
let types = buildReactDeclarationTypes(childValue, fullName);
if (types === true) {
types = {};
}
const types = buildReactDeclarationTypes(childValue, fullName);
types.fullName = fullName;
types.name = childKey;
types.node = childValue;
Expand All @@ -413,10 +410,7 @@ module.exports = {
case 'arrayOf':
case 'objectOf':
const fullName = [parentName, '*'].join('.');
let child = buildReactDeclarationTypes(argument, fullName);
if (child === true) {
child = {};
}
const child = buildReactDeclarationTypes(argument, fullName);
child.fullName = fullName;
child.name = '__ANY_KEY__';
child.node = argument;
Expand All @@ -430,7 +424,7 @@ module.exports = {
!argument.elements.length
) {
// Invalid proptype or cannot analyse statically
return true;
return {};
}
const unionTypeDefinition = {
type: 'union',
Expand All @@ -439,7 +433,7 @@ module.exports = {
for (let i = 0, j = argument.elements.length; i < j; i++) {
const type = buildReactDeclarationTypes(argument.elements[i], parentName);
// keep only complex type
if (type !== true) {
if (Object.keys(type).length > 0) {
if (type.children === true) {
// every child is accepted for one type, abort type analysis
unionTypeDefinition.children = true;
Expand All @@ -451,7 +445,7 @@ module.exports = {
}
if (unionTypeDefinition.length === 0) {
// no complex type found, simply accept everything
return true;
return {};
}
return unionTypeDefinition;
case 'instanceOf':
Expand All @@ -462,19 +456,19 @@ module.exports = {
};
case 'oneOf':
default:
return true;
return {};
}
}
// Unknown property or accepts everything (any, object, ...)
return true;
return {};
}

/**
* Creates the representation of the React props type annotation for the component.
* The representation is used to verify nested used properties.
* @param {ASTNode} annotation Type annotation for the props class property.
* @param {String} parentName Name of the parent prop node.
* @return {Object|Boolean} The representation of the declaration, true means
* @return {Object} The representation of the declaration, an empty object means
* the property is declared without the need for further analysis.
*/
function buildTypeAnnotationDeclarationTypes(annotation, parentName) {
Expand All @@ -483,21 +477,18 @@ module.exports = {
if (typeScope(annotation.id.name)) {
return buildTypeAnnotationDeclarationTypes(typeScope(annotation.id.name), parentName);
}
return true;
return {};
case 'ObjectTypeAnnotation':
if (skipShapeProps) {
return true;
return {};
}
const shapeTypeDefinition = {
type: 'shape',
children: []
};
iterateProperties(annotation.properties, (childKey, childValue) => {
const fullName = [parentName, childKey].join('.');
let types = buildTypeAnnotationDeclarationTypes(childValue, fullName);
if (types === true) {
types = {};
}
const types = buildTypeAnnotationDeclarationTypes(childValue, fullName);
types.fullName = fullName;
types.name = childKey;
types.node = childValue;
Expand All @@ -512,7 +503,7 @@ module.exports = {
for (let i = 0, j = annotation.types.length; i < j; i++) {
const type = buildTypeAnnotationDeclarationTypes(annotation.types[i], parentName);
// keep only complex type
if (type !== true) {
if (Object.keys(type).length > 0) {
if (type.children === true) {
// every child is accepted for one type, abort type analysis
unionTypeDefinition.children = true;
Expand All @@ -523,16 +514,13 @@ module.exports = {
unionTypeDefinition.children.push(type);
}
if (unionTypeDefinition.children.length === 0) {
// no complex type found, simply accept everything
return true;
// no complex type found
return {};
}
return unionTypeDefinition;
case 'ArrayTypeAnnotation':
const fullName = [parentName, '*'].join('.');
let child = buildTypeAnnotationDeclarationTypes(annotation.elementType, fullName);
if (child === true) {
child = {};
}
const child = buildTypeAnnotationDeclarationTypes(annotation.elementType, fullName);
child.fullName = fullName;
child.name = '__ANY_KEY__';
child.node = annotation;
Expand All @@ -542,7 +530,7 @@ module.exports = {
};
default:
// Unknown or accepts everything.
return true;
return {};
}
}

Expand Down Expand Up @@ -724,6 +712,60 @@ module.exports = {
});
}

/**
* Marks all props found inside ObjectTypeAnnotaiton as declared.
*
* Modifies the declaredProperties object
* @param {ASTNode} propTypes
* @param {Object} declaredPropTypes
* @returns {Boolean} True if propTypes should be ignored (e.g. when a type can't be resolved, when it is imported)
*/
function declarePropTypesForObjectTypeAnnotation(propTypes, declaredPropTypes) {
let ignorePropsValidation = false;

iterateProperties(propTypes.properties, (key, value) => {
if (!value) {
ignorePropsValidation = true;
return;
}

const types = buildTypeAnnotationDeclarationTypes(value, key);
types.fullName = key;
types.name = key;
types.node = value;
declaredPropTypes.push(types);
});

return ignorePropsValidation;
}

/**
* Marks all props found inside IntersectionTypeAnnotation as declared.
* Since InterSectionTypeAnnotations can be nested, this handles recursively.
*
* Modifies the declaredPropTypes object
* @param {ASTNode} propTypes
* @param {Object} declaredPropTypes
* @returns {Boolean} True if propTypes should be ignored (e.g. when a type can't be resolved, when it is imported)
*/
function declarePropTypesForIntersectionTypeAnnotation(propTypes, declaredPropTypes) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these two functions live somewhere they can be shared across the rules that currently duplicate them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I'm working on it 😄 The idea will be that the rule will be declared as:

create: propTypes.detect((context, components, utils) => {
...
})

And this will take care of ALL the shared code between prop-types and no-unused-prop-types. Perhaps also for the other rules.

return propTypes.types.some(annotation => {
if (annotation.type === 'ObjectTypeAnnotation') {
return declarePropTypesForObjectTypeAnnotation(annotation, declaredPropTypes);
}

const typeNode = typeScope(annotation.id.name);

if (!typeNode) {
return true;
} else if (typeNode.type === 'IntersectionTypeAnnotation') {
return declarePropTypesForIntersectionTypeAnnotation(typeNode, declaredPropTypes);
}

return declarePropTypesForObjectTypeAnnotation(typeNode, declaredPropTypes);
});
}

/**
* Mark a prop type as declared
* @param {ASTNode} node The AST node being checked.
Expand All @@ -736,31 +778,15 @@ module.exports = {

switch (propTypes && propTypes.type) {
case 'ObjectTypeAnnotation':
iterateProperties(propTypes.properties, (key, value) => {
if (!value) {
ignorePropsValidation = true;
return;
}
let types = buildTypeAnnotationDeclarationTypes(value, key);
if (types === true) {
types = {};
}
types.fullName = key;
types.name = key;
types.node = value;
declaredPropTypes.push(types);
});
ignorePropsValidation = declarePropTypesForObjectTypeAnnotation(propTypes, declaredPropTypes);
break;
case 'ObjectExpression':
iterateProperties(propTypes.properties, (key, value) => {
if (!value) {
ignorePropsValidation = true;
return;
}
let types = buildReactDeclarationTypes(value, key);
if (types === true) {
types = {};
}
const types = buildReactDeclarationTypes(value, key);
types.fullName = key;
types.name = key;
types.node = value;
Expand Down Expand Up @@ -791,24 +817,7 @@ module.exports = {
}
break;
case 'IntersectionTypeAnnotation':
propTypes.types.forEach(annotation => {
const propsType = typeScope(annotation.id.name);
iterateProperties(propsType.properties, (key, value) => {
if (!value) {
ignorePropsValidation = true;
return;
}

let types = buildTypeAnnotationDeclarationTypes(value, key);
if (types === true) {
types = {};
}
types.fullName = key;
types.name = key;
types.node = value;
declaredPropTypes.push(types);
});
});
ignorePropsValidation = declarePropTypesForIntersectionTypeAnnotation(propTypes, declaredPropTypes);
break;
case null:
break;
Expand Down
Loading