Rewrite require-render-return rule (fixes #542, fixes #543)

jsx-eslint · Apr 17, 2016 · 68ed8af · 68ed8af
1 parent 42cfd68
commit 68ed8af
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 27 deletions.
diff --git a/lib/rules/require-render-return.js b/lib/rules/require-render-return.js
@@ -10,41 +10,45 @@ var Components = require('../util/Components');
 // Rule Definition
 // ------------------------------------------------------------------------------
 
-module.exports = Components.detect(function(context) {
-  /**
-   * Helper for checking if parent node is render method.
-   * @param {ASTNode} node to check
-   * @returns {boolean} If previous node is method and name is render returns true, otherwise false;
-   */
-  function checkParent(node) {
-    return (node.parent.type === 'Property' || node.parent.type === 'MethodDefinition')
-        && node.parent.key
-        && node.parent.key.name === 'render';
-  }
+module.exports = Components.detect(function(context, components) {
 
   /**
-   * Helper for checking if child node exists and if it's ReturnStatement
-   * @param {ASTNode} node to check
-   * @returns {boolean} True if ReturnStatement exists, otherwise false
+   * Mark a return statement as present
+   * @param {ASTNode} node The AST node being checked.
    */
-  function checkReturnStatementExistence(node) {
-    if (!node.body && !node.body.body && !node.body.body.length) {
-      return false;
-    }
-
-    var hasReturnStatement = node.body.body.some(function(elem) {
-      return elem.type === 'ReturnStatement';
+  function markReturnStatementPresent(node) {
+    components.set(node, {
+      hasReturnStatement: true
     });
-
-    return hasReturnStatement;
   }
 
-
   return {
-    FunctionExpression: function(node) {
-      if (checkParent(node) && !checkReturnStatementExistence(node)) {
+    ReturnStatement: function(node) {
+      var ancestors = context.getAncestors(node).reverse();
+      var depth = 0;
+      for (var i = 0, j = ancestors.length; i < j; i++) {
+        if (/Function(Expression|Declaration)$/.test(ancestors[i].type)) {
+          depth++;
+        }
+        if (
+          (ancestors[i].type !== 'Property' && ancestors[i].type !== 'MethodDefinition') ||
+          ancestors[i].key.name !== 'render' ||
+          depth > 1
+        ) {
+          continue;
+        }
+        markReturnStatementPresent(node);
+      }
+    },
+
+    'Program:exit': function() {
+      var list = components.list();
+      for (var component in list) {
+        if (!list.hasOwnProperty(component) || list[component].hasReturnStatement) {
+          continue;
+        }
         context.report({
-          node: node,
+          node: list[component].node,
           message: 'Your render method should have return statement'
         });
       }

diff --git a/tests/lib/rules/require-render-return.js b/tests/lib/rules/require-render-return.js
@@ -28,6 +28,7 @@ var ruleTester = new RuleTester();
 ruleTester.run('require-render-return', rule, {
 
   valid: [{
+    // ES6 class
     code: [
       'class Hello extends React.Component {',
       '  render() {',
@@ -37,6 +38,7 @@ ruleTester.run('require-render-return', rule, {
     ].join('\n'),
     parserOptions: parserOptions
   }, {
+    // ES5 class
     code: [
       'var Hello = React.createClass({',
       '  displayName: \'Hello\',',
@@ -46,9 +48,47 @@ ruleTester.run('require-render-return', rule, {
       '});'
     ].join('\n'),
     parserOptions: parserOptions
+  }, {
+    // Return in a switch...case
+    code: [
+      'var Hello = React.createClass({',
+      '  render: function() {',
+      '    switch (this.props.name) {',
+      '      case \'Foo\':',
+      '        return <div>Hello Foo</div>;',
+      '      default:',
+      '        return <div>Hello {this.props.name}</div>;',
+      '    }',
+      '  }',
+      '});'
+    ].join('\n'),
+    parserOptions: parserOptions
+  }, {
+    // Return in a if...else
+    code: [
+      'var Hello = React.createClass({',
+      '  render: function() {',
+      '    if (this.props.name === \'Foo\') {',
+      '      return <div>Hello Foo</div>;',
+      '    } else {',
+      '      return <div>Hello {this.props.name}</div>;',
+      '    }',
+      '  }',
+      '});'
+    ].join('\n'),
+    parserOptions: parserOptions
+  }, {
+    // Not a React component
+    code: [
+      'class Hello {',
+      '  render() {}',
+      '}'
+    ].join('\n'),
+    parserOptions: parserOptions
   }],
 
   invalid: [{
+    // Missing return in ES5 class
     code: [
       'var Hello = React.createClass({',
       '  displayName: \'Hello\',',
@@ -60,6 +100,7 @@ ruleTester.run('require-render-return', rule, {
       message: 'Your render method should have return statement'
     }]
   }, {
+    // Missing return in ES6 class
     code: [
       'class Hello extends React.Component {',
       '  render() {} ',
@@ -69,5 +110,20 @@ ruleTester.run('require-render-return', rule, {
     errors: [{
       message: 'Your render method should have return statement'
     }]
+  }, {
+    // Missing return (but one is present in a sub-function)
+    code: [
+      'class Hello extends React.Component {',
+      '  render() {',
+      '    const names = this.props.names.map(function(name) {',
+      '      return <div>{name}</div>',
+      '    });',
+      '  } ',
+      '}'
+    ].join('\n'),
+    parserOptions: parserOptions,
+    errors: [{
+      message: 'Your render method should have return statement'
+    }]
   }
 ]});