Fix: no-extra-bind No autofix if arg may have side effect (fixes #10846…

…) (#10918)
eslint · Oct 12, 2018 · d12be69 · d12be69
1 parent 847372f
commit d12be69
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 0 deletions.
diff --git a/lib/rules/no-extra-bind.js b/lib/rules/no-extra-bind.js
@@ -10,6 +10,12 @@
 
 const astUtils = require("../util/ast-utils");
 
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+const SIDE_EFFECT_FREE_NODE_TYPES = new Set(["Literal", "Identifier", "ThisExpression", "FunctionExpression"]);
+
 //------------------------------------------------------------------------------
 // Rule Definition
 //------------------------------------------------------------------------------
@@ -35,6 +41,18 @@ module.exports = {
     create(context) {
         let scopeInfo = null;
 
+        /**
+         * Checks if a node is free of side effects.
+         *
+         * This check is stricter than it needs to be, in order to keep the implementation simple.
+         *
+         * @param {ASTNode} node A node to check.
+         * @returns {boolean} True if the node is known to be side-effect free, false otherwise.
+         */
+        function isSideEffectFree(node) {
+            return SIDE_EFFECT_FREE_NODE_TYPES.has(node.type);
+        }
+
         /**
          * Reports a given function node.
          *
@@ -48,6 +66,10 @@ module.exports = {
                 messageId: "unexpected",
                 loc: node.parent.property.loc.start,
                 fix(fixer) {
+                    if (node.parent.parent.arguments.length && !isSideEffectFree(node.parent.parent.arguments[0])) {
+                        return null;
+                    }
+
                     const firstTokenToRemove = context.getSourceCode()
                         .getFirstTokenBetween(node.parent.object, node.parent.property, astUtils.isNotClosingParenToken);
 

diff --git a/tests/lib/rules/no-extra-bind.js b/tests/lib/rules/no-extra-bind.js
@@ -70,10 +70,32 @@ ruleTester.run("no-extra-bind", rule, {
             output: "var a = function() { function c(){ this.d } }",
             errors
         },
+        {
+            code: "var a = function() { return 1; }.bind(this)",
+            output: "var a = function() { return 1; }",
+            errors
+        },
         {
             code: "var a = function() { (function(){ (function(){ this.d }.bind(c)) }) }.bind(b)",
             output: "var a = function() { (function(){ (function(){ this.d }.bind(c)) }) }",
             errors: [{ messageId: "unexpected", type: "CallExpression", column: 71 }]
+        },
+
+        // Should not autofix if bind expression args have side effects
+        {
+            code: "var a = function() {}.bind(b++)",
+            output: null,
+            errors
+        },
+        {
+            code: "var a = function() {}.bind(b())",
+            output: null,
+            errors
+        },
+        {
+            code: "var a = function() {}.bind(b.c)",
+            output: null,
+            errors
         }
     ]
 });