From de6e7b5bebfe4676671ce3021e1d7b03930af304 Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Sun, 18 Oct 2015 17:19:45 +0200 Subject: [PATCH] Add support for stateless function components (fixes #237) --- lib/rules/display-name.js | 4 ++ lib/rules/prop-types.js | 18 ++++-- lib/util/component.js | 107 ++++++++++++++++---------------- tests/lib/rules/display-name.js | 38 +++++++++--- tests/lib/rules/prop-types.js | 43 +++++++++++++ 5 files changed, 143 insertions(+), 67 deletions(-) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 9fe4152d79..697277755a 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -156,6 +156,10 @@ module.exports = function(context) { markDisplayNameAsDeclared(node); }, + ReturnStatement: function(node) { + componentList.set(context, node); + }, + 'Program:exit': function() { var list = componentList.getList(); // Report missing display name for all components diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index b894f1c24f..5192d32a70 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -31,10 +31,9 @@ module.exports = function(context) { * @returns {Boolean} True if we are using a prop, false if not. */ function isPropTypesUsage(node) { - return Boolean( - node.object.type === 'ThisExpression' && - node.property.name === 'props' - ); + var isClassUsage = node.object.type === 'ThisExpression' && node.property.name === 'props'; + var isStatelessFunctionUsage = node.object.name === 'props'; + return isClassUsage || isStatelessFunctionUsage; } /** @@ -314,6 +313,9 @@ module.exports = function(context) { * @return {string} the name of the property or undefined if not found */ function getPropertyName(node) { + if (componentUtil.getNode(context, node)) { + node = node.parent; + } var property = node.property; if (property) { switch (property.type) { @@ -351,7 +353,7 @@ module.exports = function(context) { var properties; switch (node.type) { case 'MemberExpression': - name = getPropertyName(node.parent); + name = getPropertyName(node); if (name) { allNames = parentNames.concat(name); if (node.parent.type === 'MemberExpression') { @@ -397,7 +399,7 @@ module.exports = function(context) { usedPropTypes.push({ name: name, allNames: allNames, - node: node.parent.property + node: node.object.name !== 'props' ? node.parent.property : node.property }); break; case 'destructuring': @@ -589,6 +591,10 @@ module.exports = function(context) { }); }, + ReturnStatement: function(node) { + componentList.set(context, node); + }, + 'Program:exit': function() { var list = componentList.getList(); // Report undeclared proptypes for all classes diff --git a/lib/util/component.js b/lib/util/component.js index 7fa1724479..a638e9208a 100644 --- a/lib/util/component.js +++ b/lib/util/component.js @@ -23,7 +23,7 @@ function isComponentDefinition(context, node) { break; case 'ClassDeclaration': var superClass = node.superClass && context.getSource(node.superClass); - if (superClass === 'Component' && superClass === 'React.Component') { + if (superClass === 'Component' || superClass === 'React.Component') { return true; } break; @@ -34,44 +34,51 @@ function isComponentDefinition(context, node) { } /** - * Detect if the node is rendering some JSX + * Check if we are in a stateless function component * @param {Object} context The current rule context. * @param {ASTNode} node The AST node being checked. - * @returns {Boolean} True the node is rendering some JSX, false if not. + * @returns {Boolean} True if we are in a stateless function component, false if not. */ -function isRenderingJSX(context, node) { - var tokens = context.getTokens(node); - for (var i = 0, j = tokens.length; i < j; i++) { - var hasJSX = /^JSX/.test(tokens[i].type); - var hasReact = - tokens[i].type === 'Identifier' && tokens[i].value === 'React' && - tokens[i + 2] && tokens[i + 2].type === 'Identifier' && tokens[i + 2].value === 'createElement'; - if (!hasJSX && !hasReact) { - continue; +function isStatelessFunctionComponent(context, node) { + if (node.type !== 'ReturnStatement') { + return false; + } + + var scope = context.getScope(); + while (scope) { + if (scope.type === 'class') { + return false; } - return true; + scope = scope.upper; } - return false; + + var returnsJSX = + node.argument && + node.argument.type === 'JSXElement' + ; + var returnsReactCreateElement = + node.argument && + node.argument.callee && + node.argument.callee.property && + node.argument.callee.property.name === 'createElement' + ; + + return Boolean(returnsJSX || returnsReactCreateElement); } /** - * Check if a class has a valid render method - * @param {Object} context The current rule context. - * @param {ASTNode} node The AST node being checked. - * @returns {Boolean} True the class has a valid render method, false if not. + * Get the identifiers of a React component ASTNode + * @param {ASTNode} node The React component ASTNode being checked. + * @returns {Object} The component identifiers. */ -function isClassWithRender(context, node) { - if (node.type !== 'ClassDeclaration') { - return false; - } - for (var i = 0, j = node.body.body.length; i < j; i++) { - var declaration = node.body.body[i]; - if (declaration.type !== 'MethodDefinition' || declaration.key.name !== 'render') { - continue; - } - return isRenderingJSX(context, declaration); - } - return false; +function getIdentifiers(node) { + var name = node.id && node.id.name || DEFAULT_COMPONENT_NAME; + var id = name + ':' + node.loc.start.line + ':' + node.loc.start.column; + + return { + id: id, + name: name + }; } /** @@ -80,40 +87,31 @@ function isClassWithRender(context, node) { * @param {ASTNode} node The AST node being checked. * @returns {ASTNode} The ASTNode of the React component. */ -function getNode(context, node) { - var componentNode = null; +function getNode(context, node, list) { var ancestors = context.getAncestors().reverse(); ancestors.unshift(node); for (var i = 0, j = ancestors.length; i < j; i++) { if (isComponentDefinition(context, ancestors[i])) { - componentNode = ancestors[i]; - break; + return ancestors[i]; } - if (isClassWithRender(context, ancestors[i])) { - componentNode = ancestors[i]; - break; + // Node is already in the component list + var identifiers = getIdentifiers(ancestors[i]); + if (list && list[identifiers.id]) { + return ancestors[i]; } - } - return componentNode; -} - -/** - * Get the identifiers of a React component ASTNode - * @param {ASTNode} node The React component ASTNode being checked. - * @returns {Object} The component identifiers. - */ -function getIdentifiers(node) { - var name = node.id && node.id.name || DEFAULT_COMPONENT_NAME; - var id = name + ':' + node.loc.start.line + ':' + node.loc.start.column; + if (isStatelessFunctionComponent(context, node)) { + var scope = context.getScope(); + while (scope.upper && scope.type !== 'function') { + scope = scope.upper; + } + return scope.block; + } - return { - id: id, - name: name - }; + return null; } /** @@ -171,10 +169,11 @@ List.prototype.getList = function() { * @returns {Object} The added component. */ List.prototype.set = function(context, node, customProperties) { - var componentNode = getNode(context, node); + var componentNode = getNode(context, node, this._list); if (!componentNode) { return null; } + var identifiers = getIdentifiers(componentNode); var component = util._extend({ diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 0f1f147817..f1af72f2c5 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -169,6 +169,15 @@ ruleTester.run('display-name', rule, { experimentalObjectRestSpread: true, jsx: true } + }, { + code: [ + 'export default class {', + ' render() {', + ' return
Hello {this.props.name}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint' }], invalid: [{ @@ -237,16 +246,31 @@ ruleTester.run('display-name', rule, { }] }, { code: [ - 'export default class {', - ' render() {', - ' return
Hello {this.props.name}
;', - ' }', + 'var Hello = function() {', + ' return
Hello {this.props.name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: 'Component definition is missing display name' + }] + }, { + code: [ + 'function Hello() {', + ' return
Hello {this.props.name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: 'Hello component definition is missing display name' + }] + }, { + code: [ + 'var Hello = () => {', + ' return
Hello {this.props.name}
;', '}' ].join('\n'), parser: 'babel-eslint', - options: [{ - acceptTranspilerName: true - }], errors: [{ message: 'Component definition is missing display name' }] diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index e504166c50..6934260020 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -1167,6 +1167,49 @@ ruleTester.run('prop-types', rule, { errors: [ {message: '\'firstname\' is missing in props validation for Hello'} ] + }, { + code: [ + 'var Hello = function(props) {', + ' return
Hello {props.name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'name\' is missing in props validation' + }] + }, { + code: [ + 'function Hello(props) {', + ' return
Hello {props.name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'name\' is missing in props validation for Hello' + }] + }, { + code: [ + 'var Hello = (props) => {', + ' return
Hello {props.name}
;', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [{ + message: '\'name\' is missing in props validation' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' var props = {firstname: \'John\'};', + ' return
Hello {props.firstname} {this.props.lastname}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + errors: [ + {message: '\'lastname\' is missing in props validation for Hello'} + ] } ] });