From cb1a28ce056f624c9a58049e217c0f131c23116b Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Sun, 14 Jun 2015 22:51:26 -0400 Subject: [PATCH] Support nested prop types and use react propTypes to make further analysis. Minimal analyse of primitive propTypes. --- lib/rules/prop-types.js | 277 +++++++++++++++++++++++++++-- tests/lib/rules/prop-types.js | 323 ++++++++++++++++++++++++++++++++++ 2 files changed, 581 insertions(+), 19 deletions(-) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 421ff96cd9..b4aa245cfd 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -83,16 +83,74 @@ module.exports = function(context) { ); } + /** + * Internal: Checks if the prop is declared + * @param {Object} declaredPropTypes Description of propTypes declared in the current component + * @param {String[]} keyList Dot separated name of the prop to check. + * @returns {Boolean} True if the prop is declared, false if not. + */ + function _isDeclaredInComponent(declaredPropTypes, keyList) { + for (var i = 0, j = keyList.length; i < j; i++) { + var key = keyList[i]; + var propType = ( + // Check if this key is declared + declaredPropTypes[key] || + // If not, check if this type accepts any key + declaredPropTypes.__ANY_KEY__ + ); + + if (!propType) { + // If it's a computed property, we can't make any further analysis, but is valid + return key === '__COMPUTED_PROP__'; + } + if (propType === true) { + return true; + } + // Consider every children as declared + if (propType.children === true) { + return true; + } + if (propType.acceptedProperties) { + return key in propType.acceptedProperties; + } + if (propType.type === 'union') { + // If we fall in this case, we know there is at least one complex type in the union + if (i + 1 >= j) { + // this is the last key, accept everything + return true; + } + // non trivial, check all of them + var unionTypes = propType.children; + var unionPropType = {}; + for (var k = 0, z = unionTypes.length; k < z; k++) { + unionPropType[key] = unionTypes[k]; + var isValid = _isDeclaredInComponent( + unionPropType, + keyList.slice(i) + ); + if (isValid) { + return true; + } + } + + // every possible union were invalid + return false; + } + declaredPropTypes = propType.children; + } + return true; + } + /** * Checks if the prop is declared - * @param {String} name Name of the prop to check. * @param {Object} component The component to process + * @param {String} name Dot separated name of the prop to check. * @returns {Boolean} True if the prop is declared, false if not. */ function isDeclaredInComponent(component, name) { - return ( - component.declaredPropTypes && - component.declaredPropTypes.indexOf(name) !== -1 + return _isDeclaredInComponent( + component.declaredPropTypes || {}, + name.split('.') ); } @@ -106,15 +164,169 @@ module.exports = function(context) { return tokens.length && tokens[0].value === '...'; } + /** + * Iterates through a properties node, like a customized forEach. + * @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 + */ + function iterateProperties(properties, fn) { + if (properties.length && typeof fn === 'function') { + for (var i = 0, j = properties.length; i < j; i++) { + var node = properties[i]; + var key = node.key; + var keyName = key.type === 'Identifier' ? key.name : key.value; + + var value = node.value; + fn(keyName, value); + } + } + } + + /** + * Creates the representation of the React propTypes for the component. + * The representation is used to verify nested used properties. + * @param {ASTNode} value Node of the React.PropTypes for the desired propery + * @return {Object|Boolean} The representation of the declaration, true means + * the property is declared without the need for further analysis. + */ + function buildReactDeclarationTypes(value) { + if ( + value.type === 'MemberExpression' && + value.property && + value.property.name && + value.property.name === 'isRequired' + ) { + value = value.object; + } + + // Verify React.PropTypes that are functions + if ( + value.type === 'CallExpression' && + value.callee && + value.callee.property && + value.callee.property.name && + value.arguments && + value.arguments.length > 0 + ) { + var callName = value.callee.property.name; + var argument = value.arguments[0]; + switch (callName) { + case 'shape': + if (argument.type !== 'ObjectExpression') { + // Invalid proptype or cannot analyse statically + return true; + } + var shapeTypeDefinition = { + type: 'shape', + children: {} + }; + iterateProperties(argument.properties, function(childKey, childValue) { + shapeTypeDefinition.children[childKey] = buildReactDeclarationTypes(childValue); + }); + return shapeTypeDefinition; + case 'arrayOf': + return { + type: 'array', + children: { + // Accept only array prototype and computed properties + __ANY_KEY__: { + acceptedProperties: Array.prototype + }, + __COMPUTED_PROP__: buildReactDeclarationTypes(argument) + } + }; + case 'objectOf': + return { + type: 'object', + children: { + __ANY_KEY__: buildReactDeclarationTypes(argument) + } + }; + case 'oneOfType': + if ( + !argument.elements || + !argument.elements.length + ) { + // Invalid proptype or cannot analyse statically + return true; + } + var unionTypeDefinition = { + type: 'union', + children: [] + }; + for (var i = 0, j = argument.elements.length; i < j; i++) { + var type = buildReactDeclarationTypes(argument.elements[i]); + // keep only complex type + if (type !== true) { + if (type.children === true) { + // every child is accepted for one type, abort type analysis + unionTypeDefinition.children = true; + return unionTypeDefinition; + } + unionTypeDefinition.children.push(type); + } + } + if (unionTypeDefinition.length === 0) { + // no complex type found, simply accept everything + return true; + } + return unionTypeDefinition; + case 'instanceOf': + return { + type: 'instance', + // Accept all children because we can't know what type they are + children: true + }; + case 'oneOf': + default: + return true; + } + } + if ( + value.type === 'MemberExpression' && + value.property && + value.property.name + ) { + var name = value.property.name; + // React propTypes with limited possible properties + var propertiesMap = { + array: Array.prototype, + bool: Boolean.prototype, + func: Function.prototype, + number: Number.prototype, + string: String.prototype + }; + if (name in propertiesMap) { + return { + type: name, + children: { + __ANY_KEY__: { + acceptedProperties: propertiesMap[name] + } + } + }; + } + } + // Unknown property or accepts everything (any, object, ...) + return true; + } + /** * Mark a prop type as used * @param {ASTNode} node The AST node being marked. */ - function markPropTypesAsUsed(node) { - var component = componentList.getByNode(context, node); - var usedPropTypes = component && component.usedPropTypes || []; + function markPropTypesAsUsed(node, parentName) { var type; - if (node.parent.property && node.parent.property.name && !node.parent.computed) { + var name = node.parent.computed ? + '__COMPUTED_PROP__' + : node.parent.property && node.parent.property.name; + var fullName = parentName ? parentName + '.' + name : name; + + if (node.parent.type === 'MemberExpression') { + markPropTypesAsUsed(node.parent, fullName); + } + if (name && !node.parent.computed) { type = 'direct'; } else if ( node.parent.parent.declarations && @@ -123,15 +335,17 @@ module.exports = function(context) { ) { type = 'destructuring'; } + var component = componentList.getByNode(context, node); + var usedPropTypes = component && component.usedPropTypes || []; switch (type) { case 'direct': // Ignore Object methods - if (Object.prototype[node.parent.property.name]) { + if (Object.prototype[name]) { break; } usedPropTypes.push({ - name: node.parent.property.name, + name: fullName, node: node.parent.property }); break; @@ -163,18 +377,41 @@ module.exports = function(context) { */ function markPropTypesAsDeclared(node, propTypes) { var component = componentList.getByNode(context, node); - var declaredPropTypes = component && component.declaredPropTypes || []; + var declaredPropTypes = component && component.declaredPropTypes || {}; var ignorePropsValidation = false; switch (propTypes && propTypes.type) { case 'ObjectExpression': - for (var i = 0, j = propTypes.properties.length; i < j; i++) { - var key = propTypes.properties[i].key; - declaredPropTypes.push(key.type === 'Identifier' ? key.name : key.value); - } + iterateProperties(propTypes.properties, function(key, value) { + declaredPropTypes[key] = buildReactDeclarationTypes(value); + }); break; case 'MemberExpression': - declaredPropTypes.push(propTypes.property.name); + // throw new Error(require("util").inspect(propTypes)); + var curPropTypes = declaredPropTypes; + // Walk the list of properties, until we reach the assignment + // ie: ClassX.propTypes.a.b.c = ... + while ( + propTypes && + propTypes.parent.type !== 'AssignmentExpression' && + propTypes.property && + curPropTypes + ) { + var key = propTypes.property.name; + if (key in curPropTypes) { + curPropTypes = curPropTypes[key].children; + propTypes = propTypes.parent; + } else { + // This will crash at runtime because we haven't seen this key before + // stop this and do not declare it + propTypes = null; + } + } + // console.log(propTypes, propTypes.property.name, propTypes.parent.right) + if (propTypes) { + curPropTypes[propTypes.property.name] = + buildReactDeclarationTypes(propTypes.parent.right); + } break; case null: break; @@ -187,7 +424,6 @@ module.exports = function(context) { declaredPropTypes: declaredPropTypes, ignorePropsValidation: ignorePropsValidation }); - } /** @@ -198,13 +434,16 @@ module.exports = function(context) { var name; for (var i = 0, j = component.usedPropTypes.length; i < j; i++) { name = component.usedPropTypes[i].name; - if (isDeclaredInComponent(component, name) || isIgnored(name)) { + if ( + isIgnored(name.split('.').pop()) || + isDeclaredInComponent(component, name) + ) { continue; } context.report( component.usedPropTypes[i].node, component.name === componentUtil.DEFAULT_COMPONENT_NAME ? MISSING_MESSAGE : MISSING_MESSAGE_NAMED_COMP, { - name: name, + name: name.replace(/\.__COMPUTED_PROP__/g, '[]'), component: component.name } ); diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index f51fdaf311..ba11d4aeab 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -253,6 +253,168 @@ eslintTester.addRuleTest('lib/rules/prop-types', { classes: true, jsx: true } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {};', + 'Hello.propTypes.a = React.PropTypes.shape({', + ' b: React.PropTypes.string', + '});' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.shape({', + ' b: React.PropTypes.shape({', + ' })', + ' })', + '};', + 'Hello.propTypes.a.b.c = React.PropTypes.number;' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' this.props.a.__.d.length;', + ' this.props.a.anything.e[2];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.objectOf(', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' d: React.PropTypes.string,', + ' e: React.PropTypes.array', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' var i = 3;', + ' this.props.a[2].c;', + ' this.props.a[i].d.length;', + ' this.props.a[i + 2].e[2];', + ' this.props.a.length;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.arrayOf(', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' d: React.PropTypes.string,', + ' e: React.PropTypes.array', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.c;', + ' this.props.a[2] === true;', + ' this.props.a.e[2];', + ' this.props.a.length;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.oneOfType([', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' e: React.PropTypes.array', + ' }).isRequired,', + ' React.PropTypes.arrayOf(', + ' React.PropTypes.bool', + ' )', + ' ])', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.render;', + ' this.props.a.c;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.instanceOf(Hello)', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.arr;', + ' this.props.arr[3];', + ' this.props.arr.length;', + ' this.props.arr.push(3);', + ' this.props.bo;', + ' this.props.bo.toString();', + ' this.props.fu;', + ' this.props.fu.bind(this);', + ' this.props.numb;', + ' this.props.numb.toFixed();', + ' this.props.stri;', + ' this.props.stri.length();', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' arr: React.PropTypes.array,', + ' bo: React.PropTypes.bool.isRequired,', + ' fu: React.PropTypes.func,', + ' numb: React.PropTypes.number,', + ' stri: React.PropTypes.string', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } } ], @@ -396,6 +558,167 @@ eslintTester.addRuleTest('lib/rules/prop-types', { errors: [{ message: '\'firstname\' is missing in props validation for Hello' }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.shape({', + ' })', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [{ + message: '\'a.b\' is missing in props validation for Hello' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.shape({', + ' b: React.PropTypes.shape({', + ' })', + ' })', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [{ + message: '\'a.b.c\' is missing in props validation for Hello' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' this.props.a.__.d.length;', + ' this.props.a.anything.e[2];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.objectOf(', + ' React.PropTypes.shape({', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'a.b.c\' is missing in props validation for Hello'}, + {message: '\'a.__.d\' is missing in props validation for Hello'}, + {message: '\'a.__.d.length\' is missing in props validation for Hello'}, + {message: '\'a.anything.e\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' var i = 3;', + ' this.props.a[2].c;', + ' this.props.a[i].d.length;', + ' this.props.a[i + 2].e[2];', + ' this.props.a.length;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.arrayOf(', + ' React.PropTypes.shape({', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'a[].c\' is missing in props validation for Hello'}, + {message: '\'a[].d\' is missing in props validation for Hello'}, + {message: '\'a[].d.length\' is missing in props validation for Hello'}, + {message: '\'a[].e\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.length;', + ' this.props.a.b;', + ' this.props.a.e.length;', + ' this.props.a.e.anyProp;', + ' this.props.a.c.toString();', + ' this.props.a.c.someThingElse();', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.oneOfType([', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' e: React.PropTypes.array', + ' })', + ' ])', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'a.length\' is missing in props validation for Hello'}, + {message: '\'a.b\' is missing in props validation for Hello'}, + {message: '\'a.e.anyProp\' is missing in props validation for Hello'}, + {message: '\'a.c.someThingElse\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.arr.toFixed();', + ' this.props.bo.push();', + ' this.props.fu.push();', + ' this.props.numb.propX;', + ' this.props.stri.tooString();', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' arr: React.PropTypes.array,', + ' bo: React.PropTypes.bool,', + ' fu: React.PropTypes.func,', + ' numb: React.PropTypes.number,', + ' stri: React.PropTypes.string', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'arr.toFixed\' is missing in props validation for Hello'}, + {message: '\'bo.push\' is missing in props validation for Hello'}, + {message: '\'fu.push\' is missing in props validation for Hello'}, + {message: '\'numb.propX\' is missing in props validation for Hello'}, + {message: '\'stri.tooString\' is missing in props validation for Hello'} + ] } ] });