Merge pull request #1459 from jackyho112/make-jsx-no-bind-only-warn-for-props

[jsx-no-bind] Only warn for props and account for variable declaration

Author: lencioni

docs/rules/jsx-no-bind.md

@@ -24,6 +24,7 @@ The following patterns are **not** considered warnings:
 "react/jsx-no-bind": [<enabled>, {
   "ignoreRefs": <boolean> || false,
   "allowArrowFunctions": <boolean> || false,
+  "allowFunctions": <boolean> || false,
   "allowBind": <boolean> || false
 }]
 ```
@@ -45,6 +46,14 @@ When `true` the following is **not** considered a warning:
 <div onClick={() => alert("1337")} />
 ```
 
+### `allowFunctions`
+
+When `true` the following is not considered a warning:
+
+```jsx
+<div onClick={function () { alert("1337") }} />
+```
+
 ### `allowBind`
 
 When `true` the following is **not** considered a warning:

lib/rules/jsx-no-bind.js

@@ -1,7 +1,8 @@
 /**
  * @fileoverview Prevents usage of Function.prototype.bind and arrow functions
- *               in React component definition.
+ *               in React component props.
  * @author Daniel Lo Nigro <dan.cx>
+ * @author Jacky Ho
  */
 'use strict';
 
@@ -12,10 +13,17 @@ const propName = require('jsx-ast-utils/propName');
 // Rule Definition
 // -----------------------------------------------------------------------------
 
+const violationMessageStore = {
+  bindCall: 'JSX props should not use .bind()',
+  arrowFunc: 'JSX props should not use arrow functions',
+  bindExpression: 'JSX props should not use ::',
+  func: 'JSX props should not use functions'
+};
+
 module.exports = {
   meta: {
     docs: {
-      description: 'Prevents usage of Function.prototype.bind and arrow functions in React component definition',
+      description: 'Prevents usage of Function.prototype.bind and arrow functions in React component props',
       category: 'Best Practices',
       recommended: false
     },
@@ -31,6 +39,10 @@ module.exports = {
           default: false,
           type: 'boolean'
         },
+        allowFunctions: {
+          default: false,
+          type: 'boolean'
+        },
         ignoreRefs: {
           default: false,
           type: 'boolean'
@@ -40,33 +52,100 @@ module.exports = {
     }]
   },
 
-  create: Components.detect((context, components, utils) => {
+  create: Components.detect(context => {
     const configuration = context.options[0] || {};
 
+    // Keep track of all the variable names pointing to a bind call,
+    // bind expression or an arrow function in different block statements
+    const blockVariableNameSets = {};
+
+    function setBlockVariableNameSet(blockStart) {
+      blockVariableNameSets[blockStart] = {
+        arrowFunc: new Set(),
+        bindCall: new Set(),
+        bindExpression: new Set(),
+        func: new Set()
+      };
+    }
+
+    function getNodeViolationType(node) {
+      const nodeType = node.type;
+
+      if (
+        !configuration.allowBind &&
+        nodeType === 'CallExpression' &&
+        node.callee.type === 'MemberExpression' &&
+        node.callee.property.type === 'Identifier' &&
+        node.callee.property.name === 'bind'
+      ) {
+        return 'bindCall';
+      } else if (
+        !configuration.allowArrowFunctions &&
+        nodeType === 'ArrowFunctionExpression'
+      ) {
+        return 'arrowFunc';
+      } else if (
+        !configuration.allowFunctions &&
+        nodeType === 'FunctionExpression'
+      ) {
+        return 'func';
+      } else if (
+        !configuration.allowBind &&
+        nodeType === 'BindExpression'
+      ) {
+        return 'bindExpression';
+      }
+
+      return null;
+    }
+
+    function addVariableNameToSet(violationType, variableName, blockStart) {
+      blockVariableNameSets[blockStart][violationType].add(variableName);
+    }
+
+    function getBlockStatementAncestors(node) {
+      return context.getAncestors(node).reverse().filter(
+        ancestor => ancestor.type === 'BlockStatement'
+      );
+    }
+
+    function reportVariableViolation(node, name, blockStart) {
+      const blockSets = blockVariableNameSets[blockStart];
+      const violationTypes = Object.keys(blockSets);
+
+      return violationTypes.find(type => {
+        if (blockSets[type].has(name)) {
+          context.report({node: node, message: violationMessageStore[type]});
+          return true;
+        }
+
+        return false;
+      });
+    }
+
+    function findVariableViolation(node, name) {
+      getBlockStatementAncestors(node).find(
+        block => reportVariableViolation(node, name, block.start)
+      );
+    }
+
     return {
-      CallExpression: function(node) {
-        const callee = node.callee;
+      BlockStatement(node) {
+        setBlockVariableNameSet(node.start);
+      },
+
+      VariableDeclarator(node) {
+        const blockAncestors = getBlockStatementAncestors(node);
+        const variableViolationType = getNodeViolationType(node.init);
+
         if (
-          !configuration.allowBind &&
-          (callee.type !== 'MemberExpression' || callee.property.name !== 'bind')
+          blockAncestors.length > 0 &&
+          variableViolationType &&
+          node.parent.kind === 'const' // only support const right now
         ) {
-          return;
-        }
-        const ancestors = context.getAncestors(callee).reverse();
-        for (let i = 0, j = ancestors.length; i < j; i++) {
-          if (
-            !configuration.allowBind &&
-            (ancestors[i].type === 'MethodDefinition' && ancestors[i].key.name === 'render') ||
-            (ancestors[i].type === 'Property' && ancestors[i].key.name === 'render')
-          ) {
-            if (utils.isReturningJSX(ancestors[i])) {
-              context.report({
-                node: callee,
-                message: 'JSX props should not use .bind()'
-              });
-            }
-            break;
-          }
+          addVariableNameToSet(
+            variableViolationType, node.id.name, blockAncestors[0].start
+          );
         }
       },
 
@@ -76,31 +155,14 @@ module.exports = {
           return;
         }
         const valueNode = node.value.expression;
-        if (
-          !configuration.allowBind &&
-          valueNode.type === 'CallExpression' &&
-          valueNode.callee.type === 'MemberExpression' &&
-          valueNode.callee.property.name === 'bind'
-        ) {
-          context.report({
-            node: node,
-            message: 'JSX props should not use .bind()'
-          });
-        } else if (
-          !configuration.allowArrowFunctions &&
-          valueNode.type === 'ArrowFunctionExpression'
-        ) {
-          context.report({
-            node: node,
-            message: 'JSX props should not use arrow functions'
-          });
-        } else if (
-          !configuration.allowBind &&
-          valueNode.type === 'BindExpression'
-        ) {
+        const valueNodeType = valueNode.type;
+        const nodeViolationType = getNodeViolationType(valueNode);
+
+        if (valueNodeType === 'Identifier') {
+          findVariableViolation(node, valueNode.name);
+        } else if (nodeViolationType) {
           context.report({
-            node: node,
-            message: 'JSX props should not use ::'
+            node: node, message: violationMessageStore[nodeViolationType]
           });
         }
       }