Make jsx-no-bind only warn for props

jsx-eslint · Oct 2, 2017 · a1448aa · a1448aa
1 parent 1f14fad
commit a1448aa
Show file tree

Hide file tree

Showing 2 changed files with 262 additions and 49 deletions.
diff --git a/lib/rules/jsx-no-bind.js b/lib/rules/jsx-no-bind.js
@@ -1,7 +1,7 @@
 /**
  * @fileoverview Prevents usage of Function.prototype.bind and arrow functions
- *               in React component definition.
- * @author Daniel Lo Nigro <dan.cx>
+ *               in React component props.
+ * @author Daniel Lo Nigro <dan.cx>, Jacky Ho
  */
 'use strict';
 
@@ -12,10 +12,14 @@ const propName = require('jsx-ast-utils/propName');
 // Rule Definition
 // -----------------------------------------------------------------------------
 
+const bindViolationMessage = 'JSX props should not use .bind()';
+const arrowViolationMessage = 'JSX props should not use arrow functions';
+const BindExpressionViolationMessage = 'JSX props should not use ::';
+
 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
     },
@@ -40,33 +44,72 @@ module.exports = {
     }]
   },
 
-  create: Components.detect((context, components, utils) => {
+  create: Components.detect(context => {
     const configuration = context.options[0] || {};
 
+    // This contains the variable names that are defined in the current class
+    // method and reference a `bind` call expression or an arrow function. This
+    // needs to be set to an empty Set after each class method defintion.
+    let currentArrowViolationVariableNames = new Set();
+    let currentBindViolationVariableNames = new Set();
+    let currentBindExpressionViolationVariableNames = new Set();
+
+    function resetVariableNameSets() {
+      currentArrowViolationVariableNames = new Set();
+      currentBindViolationVariableNames = new Set();
+      currentBindExpressionViolationVariableNames = new Set();
+    }
+
+    function reportVariableViolation(node, name) {
+      if (currentBindViolationVariableNames.has(name)) {
+        context.report({node: node, message: bindViolationMessage});
+      } else if (currentArrowViolationVariableNames.has(name)) {
+        context.report({node: node, message: arrowViolationMessage});
+      } else if (
+        currentBindExpressionViolationVariableNames.has(name)
+      ) {
+        context.report({node: node, message: BindExpressionViolationMessage});
+      }
+    }
+
     return {
-      CallExpression: function(node) {
-        const callee = node.callee;
+      'MethodDefinition:exit'() {
+        resetVariableNameSets();
+      },
+
+      'ClassProperty:exit'(node) {
         if (
-          !configuration.allowBind &&
-          (callee.type !== 'MemberExpression' || callee.property.name !== 'bind')
+          !node.static &&
+          node.value &&
+          node.value.type === 'ArrowFunctionExpression'
         ) {
-          return;
+          resetVariableNameSets();
         }
-        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;
-          }
+      },
+
+      VariableDeclarator(node) {
+        const name = node.id.name;
+        const init = node.init;
+        const initType = init.type;
+
+        if (
+          !configuration.allowBind &&
+          initType === 'CallExpression' &&
+          init.callee.type === 'MemberExpression' &&
+          init.callee.property.type === 'Identifier' &&
+          init.callee.property.name === 'bind'
+        ) {
+          currentBindViolationVariableNames.add(name);
+        } else if (
+          !configuration.allowArrowFunctions &&
+          initType === 'ArrowFunctionExpression'
+        ) {
+          currentArrowViolationVariableNames.add(name);
+        } else if (
+          !configuration.allowBind &&
+          initType === 'BindExpression'
+        ) {
+          currentBindExpressionViolationVariableNames.add(name);
         }
       },
 
@@ -76,32 +119,27 @@ module.exports = {
           return;
         }
         const valueNode = node.value.expression;
-        if (
+        const valueNodeType = valueNode.type;
+
+        if (valueNodeType === 'Identifier') {
+          reportVariableViolation(node, valueNode.name);
+        } else if (
           !configuration.allowBind &&
-          valueNode.type === 'CallExpression' &&
+          valueNodeType === 'CallExpression' &&
           valueNode.callee.type === 'MemberExpression' &&
           valueNode.callee.property.name === 'bind'
         ) {
-          context.report({
-            node: node,
-            message: 'JSX props should not use .bind()'
-          });
+          context.report({node: node, message: bindViolationMessage});
         } else if (
           !configuration.allowArrowFunctions &&
-          valueNode.type === 'ArrowFunctionExpression'
+          valueNodeType === 'ArrowFunctionExpression'
         ) {
-          context.report({
-            node: node,
-            message: 'JSX props should not use arrow functions'
-          });
+          context.report({node: node, message: arrowViolationMessage});
         } else if (
           !configuration.allowBind &&
-          valueNode.type === 'BindExpression'
+          valueNodeType === 'BindExpression'
         ) {
-          context.report({
-            node: node,
-            message: 'JSX props should not use ::'
-          });
+          context.report({node: node, message: BindExpressionViolationMessage});
         }
       }
     };

diff --git a/tests/lib/rules/jsx-no-bind.js b/tests/lib/rules/jsx-no-bind.js
@@ -82,6 +82,7 @@ ruleTester.run('jsx-no-bind', rule, {
       options: [{allowBind: true}],
       parser: 'babel-eslint'
     },
+
     // Backbone view with a bind
     {
       code: [
@@ -115,6 +116,110 @@ ruleTester.run('jsx-no-bind', rule, {
         '};'
       ].join('\n'),
       parser: 'babel-eslint'
+    },
+
+    {
+      code: [
+        'class Hello extends Component {',
+        '  render() {',
+        '    const click = this.onTap.bind(this);',
+        '    return <div onClick={onClick}>Hello</div>;',
+        '  }',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello extends Component {',
+        '  render() {',
+        '    return (<div>{',
+        '      this.props.list.map(this.wrap.bind(this, "span"))',
+        '    }</div>);',
+        '  }',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello extends Component {',
+        '  render() {',
+        '    const click = () => true;',
+        '    return <div onClick={onClick}>Hello</div>;',
+        '  }',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello extends Component {',
+        '  render() {',
+        '    return (<div>{',
+        '      this.props.list.map(item => <item hello="true"/>)',
+        '    }</div>);',
+        '  }',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello extends Component {',
+        '  render() {',
+        '    const click = this.bar::baz',
+        '    return <div onClick={onClick}>Hello</div>;',
+        '  }',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello extends Component {',
+        '  render() {',
+        '    return (<div>{',
+        '      this.props.list.map(this.bar::baz)',
+        '    }</div>);',
+        '  }',
+        '};'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'var Hello = React.createClass({',
+        '  render: function() { ',
+        '    return (<div>{',
+        '      this.props.list.map(this.wrap.bind(this, "span"))',
+        '    }</div>);',
+        '  }',
+        '});'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'var Hello = React.createClass({',
+        '  render: function() { ',
+        '    const click = this.bar::baz',
+        '    return <div onClick={onClick}>Hello</div>;',
+        '  }',
+        '});'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'var Hello = React.createClass({',
+        '  render: function() { ',
+        '    const click = () => true',
+        '    return <div onClick={onClick}>Hello</div>;',
+        '  }',
+        '});'
+      ].join('\n'),
+      parser: 'babel-eslint'
     }
   ],
 
@@ -166,10 +271,10 @@ ruleTester.run('jsx-no-bind', rule, {
     },
     {
       code: [
-        'const foo = {',
-        '  render: function() {',
-        '    const click = this.onTap.bind(this);',
-        '    return <div onClick={onClick}>Hello</div>;',
+        'class Hello23 extends React.Component {',
+        '  renderDiv() {',
+        '    const click = this.doSomething.bind(this, "no")',
+        '    return <div onClick={click}>Hello</div>;',
         '  }',
         '};'
       ].join('\n'),
@@ -189,12 +294,23 @@ ruleTester.run('jsx-no-bind', rule, {
     },
     {
       code: [
-        'const foo = {',
-        '  render() {',
-        '    const click = this.onTap.bind(this);',
-        '    return <div onClick={onClick}>Hello</div>;',
+        'var Hello = React.createClass({',
+        '  render: function() { ',
+        '   return <div onClick={this.doSomething.bind(this, "hey")} />',
         '  }',
-        '};'
+        '});'
+      ].join('\n'),
+      errors: [{message: 'JSX props should not use .bind()'}],
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'var Hello = React.createClass({',
+        '  render: function() { ',
+        '    const doThing = this.doSomething.bind(this, "hey")',
+        '    return <div onClick={doThing} />',
+        '  }',
+        '});'
       ].join('\n'),
       errors: [{message: 'JSX props should not use .bind()'}],
       parser: 'babel-eslint'
@@ -221,6 +337,41 @@ ruleTester.run('jsx-no-bind', rule, {
       errors: [{message: 'JSX props should not use arrow functions'}],
       parser: 'babel-eslint'
     },
+    {
+      code: [
+        'class Hello23 extends React.Component {',
+        '  renderDiv = () => {',
+        '    const click = () => true',
+        '    return <div onClick={click}>Hello</div>;',
+        '  }',
+        '};'
+      ].join('\n'),
+      errors: [{message: 'JSX props should not use arrow functions'}],
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'var Hello = React.createClass({',
+        '  render: function() { ',
+        '   return <div onClick={() => true} />',
+        '  }',
+        '});'
+      ].join('\n'),
+      errors: [{message: 'JSX props should not use arrow functions'}],
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'var Hello = React.createClass({',
+        '  render: function() { ',
+        '    const doThing = () => true',
+        '    return <div onClick={doThing} />',
+        '  }',
+        '});'
+      ].join('\n'),
+      errors: [{message: 'JSX props should not use arrow functions'}],
+      parser: 'babel-eslint'
+    },
 
     // Bind expression
     {
@@ -237,6 +388,30 @@ ruleTester.run('jsx-no-bind', rule, {
       code: '<div foo={foo::bar} />',
       errors: [{message: 'JSX props should not use ::'}],
       parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello23 extends React.Component {',
+        '  renderDiv() {',
+        '    const click = ::this.onChange',
+        '    return <div onClick={click}>Hello</div>;',
+        '  }',
+        '};'
+      ].join('\n'),
+      errors: [{message: 'JSX props should not use ::'}],
+      parser: 'babel-eslint'
+    },
+    {
+      code: [
+        'class Hello23 extends React.Component {',
+        '  renderDiv() {',
+        '    const click = this.bar::baz',
+        '    return <div onClick={click}>Hello</div>;',
+        '  }',
+        '};'
+      ].join('\n'),
+      errors: [{message: 'JSX props should not use ::'}],
+      parser: 'babel-eslint'
     }
   ]
 });