Repository: eslint/eslint (js)

Issue:
Eslint --fix is breaking code with ".bind(this, variable)".
**Tell us about your environment**
Windows 10
* **ESLint Version:**  v3.19.0
* **Node Version:** v6.10.2
* **npm Version:** 4.5.0

**What parser (default, Babel-ESLint, etc.) are you using?**
default
**Please show your full configuration:**
```js
module.exports = {
  "parser": "babel-eslint",
  "extends": [
    "airbnb"
  ],
  "plugins": [
    "babel",
    "react",
    "promise"
  ],
  "env": {
    "browser": true,
    "jest": true
  },
  "rules": {
    "arrow-parens": [1, "always"],
    "arrow-body-style": [1, "always"],
    "key-spacing": 0,
    "no-mixed-operators": 0,
    "no-param-reassign": 0,

    // Allow for-of loops. 
    "no-restricted-syntax": [
      'error',
      {
        selector: 'LabeledStatement',
        message: 'Labels are a form of GOTO; using them makes code confusing and hard to maintain and understand.',
      },
      {
        selector: 'WithStatement',
        message: '`with` is disallowed in strict mode because it makes code impossible to predict and optimize.',
      },
    ],

    // This rule requires that all class methods use this. 
    // Disabling this allows making class methods that do not reference "this", which is helpful for private helper functions. 
    "class-methods-use-this": 0,

    // This config disallows if(true) but allows while(true)
    "no-constant-condition": ["error", { "checkLoops": false }],
    "no-cond-assign": ["error", "except-parens"],
    "no-plusplus": [0, {
      "allowForLoopAfterthoughts": true
    }],

    // Prevents using variables before they are defined. 
    "no-use-before-define": 1,
    "jsx-quotes": [2, "prefer-single"],
    "max-len": [2, 300, 2],
    "object-shorthand": ["error", "never"],
    "object-curly-spacing": [2, "always"],
    "react/forbid-prop-types": 0,
    "react/no-danger": 1,
    "no-continue": 0,

    // This rule always triggers on Windows...
    "import/no-unresolved": Number(process.env.OS !== 'Windows_NT') * 2,
    "spaced-comment": 0,
    "react/prefer-stateless-function": [1],
    "react/jsx-filename-extension": [1, {
      "extensions": [".js", ".jsx"]
    }],
    "react/jsx-curly-spacing": [2, "always", {
      "spacing": {
        "objectLiterals": "never"
      }
    }],
    "no-console": "off"
  }
}

```


**What did you do? Please include the actual source code causing the issue.**

I was converting some ES5 code to ES6 and `eslint --fix` broke some code. Here is a small example:

<!-- Paste the source code below: -->
```js
let queue = require('d3-queue').queue
const attrName = 'hi';

let q = queue()

q.defer(function(attrName, callback) {

  // No mention of "this" anywhere in the body of the function

}.bind(this, attrName));

```

**What did you expect to happen?**
I expected the code to stay as valid Javascript. Here is one valid transformation:
```js
const queue = require('d3-queue').queue;
const attrName = 'hi';

const q = queue();

q.defer(((attrName, callback) => {

  // No mention of "this" anywhere in the body of the function

}).bind(this, attrName)); // This line is now valid because of the added parens. 

```

**What actually happened? Please include the actual, raw output from ESLint.**

```js
const queue = require('d3-queue').queue;
const attrName = 'hi';

const q = queue();

q.defer((attrName, callback) => {

  // No mention of "this" anywhere in the body of the function

}.bind(this, attrName)); // This line is now invalid because the function has been transformed into an arrow function. This now gives an Error: 10,2: Parsing error: Unexpected token, expected ,
```

Pull request:
Fix: invalid syntax from prefer-arrow-callback autofixer (fixes #8541)
**What is the purpose of this pull request? (put an "X" next to item)**

[x] Bug fix (https://github.com/eslint/eslint/issues/8541)

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (http://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

Previously, the `prefer-arrow-callback` autofixer would simply replace a function expression with an arrow function. However, arrow functions have a different precedence than regular functions, so this was sometimes creating invalid syntax. This commit updates the fixer to parenthesize the arrow functions if necessary to avoid a syntax error.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular

Patch:
diff --git a/lib/rules/indent.js b/lib/rules/indent.js
index b7597e57b..0e6154d55 100644
--- a/lib/rules/indent.js
+++ b/lib/rules/indent.js
@@ -924,21 +924,49 @@ module.exports = {
 
             MemberExpression(node) {
                 const firstNonObjectToken = sourceCode.getFirstTokenBetween(node.object, node.property, astUtils.isNotClosingParenToken);
-                const tokensToIndent = node.computed ? [sourceCode.getTokenAfter(firstNonObjectToken)] : [firstNonObjectToken, sourceCode.getTokenAfter(firstNonObjectToken)];
+                const secondNonObjectToken = sourceCode.getTokenAfter(firstNonObjectToken);
+
+                const tokenBeforeObject = sourceCode.getTokenBefore(node.object, token => astUtils.isNotOpeningParenToken(token) || parameterParens.has(token));
+                const firstObjectToken = tokenBeforeObject ? sourceCode.getTokenAfter(tokenBeforeObject) : sourceCode.ast.tokens[0];
+                const lastObjectToken = sourceCode.getTokenBefore(firstNonObjectToken);
 
                 if (node.computed) {
-                    offsets.matchIndentOf(firstNonObjectToken, sourceCode.getLastToken(node));
-                }
 
-                if (typeof options.MemberExpression !== "number") {
-                    tokensToIndent.forEach(token => offsets.ignoreToken(token));
+                    // For computed MemberExpressions, match the closing bracket with the opening bracket.
+                    offsets.matchIndentOf(firstNonObjectToken, sourceCode.getLastToken(node));
                 }
 
-                const offsetBase = node.object.loc.end.line === node.property.loc.start.line
-                    ? sourceCode.getLastToken(node.object)
-                    : sourceCode.getFirstToken(node.object);
+                if (typeof options.MemberExpression === "number") {
+                    const firstPropertyToken = node.computed ? firstNonObjectToken : secondNonObjectToken;
+
+                    /*
+                     * If the object ends on the same line that the property starts, match against the last token
+                     * of the object, to ensure that the MemberExpression is not indented.
+                     *
+                     * Otherwise, match against the first token of the object, e.g.
+                     * foo
+                     *   .bar
+                     *   .baz // <-- offset by 1 from `foo`
+                     */
+                    const offsetBase = lastObjectToken.loc.end.line === firstPropertyToken.loc.start.line
+                        ? lastObjectToken
+                        : firstObjectToken;
+
+                    // Match the dot (for non-computed properties) or the opening bracket (for computed properties) against the object.
+                    offsets.setDesiredOffset(firstNonObjectToken, offsetBase, options.MemberExpression);
+
+                    /*
+                     * For computed MemberExpressions, match the first token of the property against the opening bracket.
+                     * Otherwise, match the first token of the property against the object.
+                     */
+                    offsets.setDesiredOffset(secondNonObjectToken, node.computed ? firstNonObjectToken : offsetBase, options.MemberExpression);
+                } else {
 
-                offsets.setDesiredOffsets(tokensToIndent, offsetBase, options.MemberExpression);
+                    // If the MemberExpression option is off, ignore the dot and the first token of the property.
+                    offsets.ignoreToken(firstNonObjectToken);
+                    offsets.ignoreToken(secondNonObjectToken);
+                    offsets.matchIndentOf(firstNonObjectToken, secondNonObjectToken);
+                }
             },
 
             NewExpression(node) {
diff --git a/lib/rules/no-unexpected-multiline.js b/lib/rules/no-unexpected-multiline.js
index 2c2ac2db3..9398b8a60 100644
--- a/lib/rules/no-unexpected-multiline.js
+++ b/lib/rules/no-unexpected-multiline.js
@@ -80,7 +80,7 @@ module.exports = {
                 checkForBreakAfter(node.callee, FUNCTION_MESSAGE);
             },
 
-            "BinaryExpression[operator='/'] > BinaryExpression[operator='/']"(node) {
+            "BinaryExpression[operator='/'] > BinaryExpression[operator='/'].left"(node) {
                 const secondSlash = sourceCode.getTokenAfter(node, token => token.value === "/");
                 const tokenAfterOperator = sourceCode.getTokenAfter(secondSlash);
 
diff --git a/lib/rules/prefer-arrow-callback.js b/lib/rules/prefer-arrow-callback.js
index a867a9390..43a8bb77d 100644
--- a/lib/rules/prefer-arrow-callback.js
+++ b/lib/rules/prefer-arrow-callback.js
@@ -277,15 +277,23 @@ module.exports = {
                             const paramsRightParen = sourceCode.getTokenBefore(node.body);
                             const asyncKeyword = node.async ? "async " : "";
                             const paramsFullText = sourceCode.text.slice(paramsLeftParen.range[0], paramsRightParen.range[1]);
-
-                            if (callbackInfo.isLexicalThis) {
-
-                                // If the callback function has `.bind(this)`, replace it with an arrow function and remove the binding.
-                                return fixer.replaceText(node.parent.parent, `${asyncKeyword}${paramsFullText} => ${sourceCode.getText(node.body)}`);
-                            }
-
-                            // Otherwise, only replace the `function` keyword and parameters with the arrow function parameters.
-                            return fixer.replaceTextRange([node.start, node.body.start], `${asyncKeyword}${paramsFullText} => `);
+                            const arrowFunctionText = `${asyncKeyword}${paramsFullText} => ${sourceCode.getText(node.body)}`;
+
+                            /*
+                             * If the callback function has `.bind(this)`, replace it with an arrow function and remove the binding.
+                             * Otherwise, just replace the arrow function itself.
+                             */
+                            const replacedNode = callbackInfo.isLexicalThis ? node.parent.parent : node;
+
+                            /*
+                             * If the replaced node is part of a BinaryExpression, LogicalExpression, or MemberExpression, then
+                             * the arrow function needs to be parenthesized, because `foo || () => {}` is invalid syntax even
+                             * though `foo || function() {}` is valid.
+                             */
+                            const needsParens = replacedNode.parent.type !== "CallExpression" && replacedNode.parent.type !== "ConditionalExpression";
+                            const replacementText = needsParens ? `(${arrowFunctionText})` : arrowFunctionText;
+
+                            return fixer.replaceText(replacedNode, replacementText);
                         }
                     });
                 }


Test patch:
diff --git a/lib/testers/rule-tester.js b/lib/testers/rule-tester.js
index 9f6bc2a53..644bf5bdf 100644
--- a/lib/testers/rule-tester.js
+++ b/lib/testers/rule-tester.js
@@ -304,11 +304,9 @@ class RuleTester {
                 filename = item.filename;
             }
 
-            if (item.options) {
-                const options = item.options.concat();
-
-                options.unshift(1);
-                config.rules[ruleName] = options;
+            if (Object.prototype.hasOwnProperty.call(item, "options")) {
+                assert(Array.isArray(item.options), "options must be an array");
+                config.rules[ruleName] = [1].concat(item.options);
             } else {
                 config.rules[ruleName] = 1;
             }
diff --git a/tests/fixtures/config-hierarchy/overwrite-ecmaFeatures/.eslintrc b/tests/fixtures/config-hierarchy/overwrite-ecmaFeatures/.eslintrc
index 4ac0a584e..f61c28002 100644
--- a/tests/fixtures/config-hierarchy/overwrite-ecmaFeatures/.eslintrc
+++ b/tests/fixtures/config-hierarchy/overwrite-ecmaFeatures/.eslintrc
@@ -1,5 +1,7 @@
 {
-    "ecmaFeatures": {
-        "globalReturn": false
+    "parserOptions": {
+        "ecmaFeatures": {
+            "globalReturn": false
+        }
     }
 }
diff --git a/tests/lib/config.js b/tests/lib/config.js
index 1ebd20c2f..4e2ac050e 100644
--- a/tests/lib/config.js
+++ b/tests/lib/config.js
@@ -773,13 +773,13 @@ describe("Config", () => {
         });
 
         describe("with env in a child configuration file", () => {
-            xit("should overwrite parserOptions of the parent with env of the child", () => {
+            it("should not overwrite parserOptions of the parent with env of the child", () => {
                 const config = new Config({ cwd: process.cwd() });
                 const targetPath = getFixturePath("overwrite-ecmaFeatures", "child", "foo.js");
                 const expected = {
                     rules: {},
                     env: { commonjs: true },
-                    parserOptions: { ecmaFeatures: { globalReturn: true } }
+                    parserOptions: { ecmaFeatures: { globalReturn: false } }
                 };
                 const actual = config.getConfig(targetPath);
 
diff --git a/tests/lib/rules/indent.js b/tests/lib/rules/indent.js
index 9d19c6b23..2d56b7297 100644
--- a/tests/lib/rules/indent.js
+++ b/tests/lib/rules/indent.js
@@ -3166,6 +3166,121 @@ ruleTester.run("indent", rule, {
                 }
             `,
             parserOptions: { ecmaVersion: 6 }
+        },
+        {
+            code: unIndent`
+                foo
+                    .bar(function() {
+                        baz
+                    })
+            `
+        },
+        {
+            code: unIndent`
+                foo
+                        .bar(function() {
+                            baz
+                        })
+            `,
+            options: [4, { MemberExpression: 2 }]
+        },
+        {
+            code: unIndent`
+                foo
+                    [bar](function() {
+                        baz
+                    })
+            `
+        },
+        {
+            code: unIndent`
+                foo.
+                    bar.
+                    baz
+            `
+        },
+        {
+            code: unIndent`
+                foo
+                    .bar(function() {
+                        baz
+                    })
+            `,
+            options: [4, { MemberExpression: "off" }]
+        },
+        {
+            code: unIndent`
+                foo
+                                .bar(function() {
+                                    baz
+                                })
+            `,
+            options: [4, { MemberExpression: "off" }]
+        },
+        {
+            code: unIndent`
+                foo
+                                [bar](function() {
+                                    baz
+                                })
+            `,
+            options: [4, { MemberExpression: "off" }]
+        },
+        {
+            code: unIndent`
+                  foo.
+                          bar.
+                                      baz
+            `,
+            options: [4, { MemberExpression: "off" }]
+        },
+        {
+            code: unIndent`
+                  foo
+                      [
+                          bar
+                      ]
+                      .baz(function() {
+                          quz();
+                      })
+            `
+        },
+        {
+            code: unIndent`
+                  [
+                      foo
+                  ][
+                      "map"](function() {
+                      qux();
+                  })
+            `
+        },
+        {
+            code: unIndent`
+                (
+                    a.b(function() {
+                        c;
+                    })
+                )
+            `
+        },
+        {
+            code: unIndent`
+                (
+                    foo
+                ).bar(function() {
+                    baz();
+                })
+            `
+        },
+        {
+            code: unIndent`
+                new Foo(
+                    bar
+                        .baz
+                        .qux
+                )
+            `
         }
     ],
 
@@ -6467,6 +6582,19 @@ ruleTester.run("indent", rule, {
                 );
             `,
             errors: expectedErrors([[4, 4, 8, "Identifier"], [5, 0, 4, "Punctuator"]])
+        },
+        {
+            code: unIndent`
+                foo.
+                  bar.
+                      baz
+            `,
+            output: unIndent`
+                foo.
+                    bar.
+                    baz
+            `,
+            errors: expectedErrors([[2, 4, 2, "Identifier"], [3, 4, 6, "Identifier"]])
         }
     ]
 });
diff --git a/tests/lib/rules/no-unexpected-multiline.js b/tests/lib/rules/no-unexpected-multiline.js
index a3b662b0d..0cc8c2bdd 100644
--- a/tests/lib/rules/no-unexpected-multiline.js
+++ b/tests/lib/rules/no-unexpected-multiline.js
@@ -78,6 +78,10 @@ ruleTester.run("no-unexpected-multiline", rule, {
         `
             foo
             / /abc/
+        `,
+        `
+            5 / (5
+            / 5)
         `
     ],
     invalid: [
diff --git a/tests/lib/rules/prefer-arrow-callback.js b/tests/lib/rules/prefer-arrow-callback.js
index c73cb825c..3480849a8 100644
--- a/tests/lib/rules/prefer-arrow-callback.js
+++ b/tests/lib/rules/prefer-arrow-callback.js
@@ -70,7 +70,7 @@ ruleTester.run("prefer-arrow-callback", rule, {
         {
             code: "foo(nativeCb || function() {});",
             errors,
-            output: "foo(nativeCb || () => {});"
+            output: "foo(nativeCb || (() => {}));"
         },
         {
             code: "foo(bar ? function() {} : function() {});",
@@ -87,6 +87,11 @@ ruleTester.run("prefer-arrow-callback", rule, {
             errors,
             output: "foo(() => { this; });"
         },
+        {
+            code: "foo(bar || function() { this; }.bind(this));",
+            errors,
+            output: "foo(bar || (() => { this; }));"
+        },
         {
             code: "foo(function() { (() => this); }.bind(this));",
             errors,
@@ -134,6 +139,11 @@ ruleTester.run("prefer-arrow-callback", rule, {
             errors,
             output: "qux((foo, bar, baz) => { return foo * this.qux; })"
         },
+        {
+            code: "foo(function() {}.bind(this, somethingElse))",
+            errors,
+            output: "foo((() => {}).bind(this, somethingElse))"
+        },
         {
             code: "qux(function(foo = 1, [bar = 2] = [], {qux: baz = 3} = {foo: 'bar'}) { return foo + bar; });",
             errors,
diff --git a/tests/lib/testers/rule-tester.js b/tests/lib/testers/rule-tester.js
index 7e953016d..36663f30b 100644
--- a/tests/lib/testers/rule-tester.js
+++ b/tests/lib/testers/rule-tester.js
@@ -557,6 +557,34 @@ describe("RuleTester", () => {
         });
     });
 
+    it("should throw an error if the options are an object", () => {
+        assert.throws(() => {
+            ruleTester.run("no-invalid-args", require("../../fixtures/testers/rule-tester/no-invalid-args"), {
+                valid: [
+                    {
+                        code: "foo",
+                        options: { ok: true }
+                    }
+                ],
+                invalid: []
+            });
+        }, /options must be an array/);
+    });
+
+    it("should throw an error if the options are a number", () => {
+        assert.throws(() => {
+            ruleTester.run("no-invalid-args", require("../../fixtures/testers/rule-tester/no-invalid-args"), {
+                valid: [
+                    {
+                        code: "foo",
+                        options: 0
+                    }
+                ],
+                invalid: []
+            });
+        }, /options must be an array/);
+    });
+
     it("should pass-through the parser to the rule", () => {
 
         assert.doesNotThrow(() => {