Repository: eslint/eslint (js)

Issue:
3.16.0 regression: padded-blocks autofix can generate incorrect code
**Tell us about your environment**

* **ESLint Version:** 3.16.0
* **Node Version:** 6.9.1
* **npm Version:** 3.10.8

**What parser (default, Babel-ESLint, etc.) are you using?**
default

**Please show your full configuration:**

```
module.exports = {
  rules: {
    "padded-blocks": ['error', 'never'],
  }
};
```

(This is the value specified by eslint-config-airbnb.)

**What did you do? Please include the actual source code causing the issue.**

I ran `eslint --fix` on this source code:

```js
function f() { // a

  b;
}
```

**What did you expect to happen?**

I expected it to fix the formatting and generate correct code.

**What actually happened? Please include the actual, raw output from ESLint.**

eslint generated this output:
```
➜  eslint_test_3 git:(master) ./node_modules/.bin/eslint --fix test.js

/Users/alanpierce/code/eslint_test_3/test.js
  6:1  error  Parsing error: Unexpected token

✖ 1 problem (1 error, 0 warnings)
```

And the JS file was changed to have these contents:
```
function f() {
function f() { // a

  b;
}
```

This is a syntax error, so the file fails to parse after this change.

When I try this on eslint 3.15.0, it doesn't have any problems.

This is breaking the [coffeelint example build](https://github.com/decaffeinate/decaffeinate#status) for the decaffeinate project. The [lexical_linter.js](https://github.com/decaffeinate-examples/coffeelint/blob/decaffeinate/src/lexical_linter.js) file has two `static initClass` lines near the start, which causes a syntax error. (The example builds are a bit like eslint-canary for the decaffeinate project, and one of the conversion steps is to run `eslint --fix` on the resulting JS code.)

Pull request:
Fix: duplicated autofix output for inverted fix ranges (fixes #8116)
**What is the purpose of this pull request? (put an "X" next to item)**

[x] Bug fix (https://github.com/eslint/eslint/issues/8116)

**What changes did you make? (Give an overview)**

The padded-blocks rule sometimes produces a fix range where the start index of the range is larger than the end index. Due to the changes in fcc38db469d987e97b668675bf7822c7c0, when source-code-fixer encountered such a range, it would output the text between the end index and the start index twice. This commit ensures that source-code-fixer behaves the same way as it previously did when it encounters an inverted fix range (it should act as if the start and end indices were equal). In the future, we should also update padded-blocks to avoid producing inverted fix ranges.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular


Patch:
diff --git a/docs/rules/arrow-parens.md b/docs/rules/arrow-parens.md
index 075de1f0b..36e804481 100644
--- a/docs/rules/arrow-parens.md
+++ b/docs/rules/arrow-parens.md
@@ -58,7 +58,7 @@ String options are:
 
 Object properties for variants of the `"as-needed"` option:
 
-* `"requireForBlockBody": true` modifies the as-needed rule in order to require parens if the function body is in an intructions block (surrounded by braces).
+* `"requireForBlockBody": true` modifies the as-needed rule in order to require parens if the function body is in an instructions block (surrounded by braces).
 
 ### always
 
diff --git a/docs/rules/key-spacing.md b/docs/rules/key-spacing.md
index feb053375..e9284da3b 100644
--- a/docs/rules/key-spacing.md
+++ b/docs/rules/key-spacing.md
@@ -16,8 +16,8 @@ This rule has an object option:
 * `"beforeColon": true` requires at least one space between the key and the colon in object literals.
 * `"afterColon": true` (default) requires at least one space between the colon and the value in object literals.
 * `"afterColon": false` disallows spaces between the colon and the value in object literals.
-* `"mode": strict` (default) enforces exactly one space before or after colons in object literals.
-* `"mode": minimum` enforces one or more spaces before or after colons in object literals.
+* `"mode": "strict"` (default) enforces exactly one space before or after colons in object literals.
+* `"mode": "minimum"` enforces one or more spaces before or after colons in object literals.
 * `"align": "value"` enforces horizontal alignment of values in object literals.
 * `"align": "colon"` enforces horizontal alignment of both colons and values in object literals.
 * `"align"` with an object value allows for fine-grained spacing when values are being aligned in object literals.
diff --git a/lib/rules/padded-blocks.js b/lib/rules/padded-blocks.js
index 094ba0a71..27657360e 100644
--- a/lib/rules/padded-blocks.js
+++ b/lib/rules/padded-blocks.js
@@ -188,6 +188,9 @@ module.exports = {
                         node,
                         loc: { line: openBrace.loc.start.line, column: openBrace.loc.start.column },
                         fix(fixer) {
+
+                            // FIXME: The start of this range is sometimes larger than the end.
+                            // https://github.com/eslint/eslint/issues/8116
                             return fixer.replaceTextRange([openBrace.end, nextToken.start - nextToken.loc.start.column], "\n");
                         },
                         message: NEVER_MESSAGE
diff --git a/lib/util/source-code-fixer.js b/lib/util/source-code-fixer.js
index 8b8936204..ab42e9529 100644
--- a/lib/util/source-code-fixer.js
+++ b/lib/util/source-code-fixer.js
@@ -108,7 +108,13 @@ SourceCodeFixer.applyFixes = function(sourceCode, messages) {
             // Make output to this fix.
             output += text.slice(Math.max(0, lastPos), Math.max(0, start));
             output += fix.text;
-            lastPos = end;
+
+            /*
+             * If the start of the range is larger than the end for some reason, make sure
+             * the text between the end and the start doesn't get duplicated.
+             * https://github.com/eslint/eslint/issues/8116
+             */
+            lastPos = Math.max(start, end);
         }
         output += text.slice(Math.max(0, lastPos));
 


Test patch:
diff --git a/tests/lib/rules/comma-dangle.js b/tests/lib/rules/comma-dangle.js
index d73132ffd..5e9adb33f 100644
--- a/tests/lib/rules/comma-dangle.js
+++ b/tests/lib/rules/comma-dangle.js
@@ -760,7 +760,7 @@ ruleTester.run("comma-dangle", rule, {
         },
         {
             code: "var foo = { bar: 'baz', }",
-            ouput: "var foo = { bar: 'baz' }",
+            output: "var foo = { bar: 'baz' }",
             options: ["only-multiline"],
             errors: [
                 {
@@ -799,7 +799,7 @@ ruleTester.run("comma-dangle", rule, {
         },
         {
             code: "foo({ bar: 'baz', qux: 'quux', });",
-            ouput: "foo({ bar: 'baz', qux: 'quux' });",
+            output: "foo({ bar: 'baz', qux: 'quux' });",
             options: ["only-multiline"],
             errors: [
                 {
diff --git a/tests/lib/rules/computed-property-spacing.js b/tests/lib/rules/computed-property-spacing.js
index eb0fca8cf..98f34b376 100644
--- a/tests/lib/rules/computed-property-spacing.js
+++ b/tests/lib/rules/computed-property-spacing.js
@@ -156,7 +156,7 @@ ruleTester.run("computed-property-spacing", rule, {
         },
         {
             code: "obj[ foo ]",
-            outptu: "obj[foo]",
+            output: "obj[foo]",
             options: ["never"],
             errors: [
                 {
diff --git a/tests/lib/rules/indent.js b/tests/lib/rules/indent.js
index 11c8e3dc6..93378ae1b 100644
--- a/tests/lib/rules/indent.js
+++ b/tests/lib/rules/indent.js
@@ -900,16 +900,16 @@ ruleTester.run("indent", rule, {
             "        a: 1,\n" +
             "        b: 2\n" +
             "      };\n" +
-            "let abc = 5,\n" +
-            "  c = 2,\n" +
-            "  xyz = \n" +
+            "let abc2 = 5,\n" +
+            "  c2 = 2,\n" +
+            "  xyz2 = \n" +
             "  {\n" +
             "    a: 1,\n" +
             "    b: 2\n" +
             "  };\n" +
-            "var abc = 5,\n" +
-            "    c = 2,\n" +
-            "    xyz = \n" +
+            "var abc3 = 5,\n" +
+            "    c3 = 2,\n" +
+            "    xyz3 = \n" +
             "    {\n" +
             "      a: 1,\n" +
             "      b: 2\n" +
@@ -2806,19 +2806,19 @@ ruleTester.run("indent", rule, {
             "      a: 1\n" +
             "    }),\n" +
             "    b = 4;\n" +
-            "const a = new Test({\n" +
+            "const c = new Test({\n" +
             "      a: 1\n" +
             "    }),\n" +
-            "    b = 4;\n",
+            "    d = 4;\n",
             output:
             "var a = new Test({\n" +
             "      a: 1\n" +
             "    }),\n" +
             "    b = 4;\n" +
-            "const a = new Test({\n" +
+            "const c = new Test({\n" +
             "    a: 1\n" +
             "  }),\n" +
-            "  b = 4;\n",
+            "  d = 4;\n",
             options: [2, { VariableDeclarator: { var: 2 } }],
             parserOptions: { ecmaVersion: 6 },
             errors: expectedErrors([
diff --git a/tests/lib/rules/newline-after-var.js b/tests/lib/rules/newline-after-var.js
index 7763e856d..853cfe880 100644
--- a/tests/lib/rules/newline-after-var.js
+++ b/tests/lib/rules/newline-after-var.js
@@ -285,7 +285,7 @@ ruleTester.run("newline-after-var", rule, {
         { code: TWO_BLANKS, output: NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
         { code: THREE_BLANKS, output: NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
         { code: ONE_BLANK_WITH_TRAILING_WS, output: NO_BLANK_WITH_TRAILING_WS, options: ["never"], errors: [NEVER_ERROR] },
-        { code: ONE_BLANK_WITH_INLINE_COMMENT, NO_BLANK_WITH_INLINE_COMMENT, options: ["never"], errors: [NEVER_ERROR] },
+        { code: ONE_BLANK_WITH_INLINE_COMMENT, output: NO_BLANK_WITH_INLINE_COMMENT, options: ["never"], errors: [NEVER_ERROR] },
         { code: MULTI_VAR_ONE_BLANK, output: MULTI_VAR_NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
         { code: MULTI_DEC_ONE_BLANK, output: MULTI_DEC_NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
         { code: MULTI_LINE_ONE_BLANK, output: MULTI_LINE_NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
diff --git a/tests/lib/rules/no-redeclare.js b/tests/lib/rules/no-redeclare.js
index 3babc8397..72ab03ae6 100644
--- a/tests/lib/rules/no-redeclare.js
+++ b/tests/lib/rules/no-redeclare.js
@@ -39,7 +39,7 @@ ruleTester.run("no-redeclare", rule, {
     ],
     invalid: [
         { code: "var a = 3; var a = 10;", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "'a' is already defined.", type: "Identifier" }] },
-        { code: "switch(foo) { case a: let b = 3;\ncase b: let b = 4}", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "'b' is already defined.", type: "Identifier" }] },
+        { code: "switch(foo) { case a: var b = 3;\ncase b: var b = 4}", errors: [{ message: "'b' is already defined.", type: "Identifier" }] },
         { code: "var a = 3; var a = 10;", errors: [{ message: "'a' is already defined.", type: "Identifier" }] },
         { code: "var a = {}; var a = [];", errors: [{ message: "'a' is already defined.", type: "Identifier" }] },
         { code: "var a; function a() {}", errors: [{ message: "'a' is already defined.", type: "Identifier" }] },
diff --git a/tests/lib/rules/no-shadow.js b/tests/lib/rules/no-shadow.js
index fbc6d781d..c7bdb9bb4 100644
--- a/tests/lib/rules/no-shadow.js
+++ b/tests/lib/rules/no-shadow.js
@@ -27,7 +27,7 @@ ruleTester.run("no-shadow", rule, {
         { code: "class A {}", parserOptions: { ecmaVersion: 6 } },
         { code: "class A { constructor() { var a; } }", parserOptions: { ecmaVersion: 6 } },
         { code: "(function() { var A = class A {}; })()", parserOptions: { ecmaVersion: 6 } },
-        { code: "{ var a; } let a;", parserOptions: { ecmaVersion: 6 } }, // this case reports `no-redeclare`, not shadowing.
+        { code: "{ var a; } var a;", parserOptions: { ecmaVersion: 6 } }, // this case reports `no-redeclare`, not shadowing.
         { code: "{ let a; } let a;", options: [{ hoist: "never" }], parserOptions: { ecmaVersion: 6 } },
         { code: "{ let a; } var a;", options: [{ hoist: "never" }], parserOptions: { ecmaVersion: 6 } },
         { code: "{ let a; } function a() {}", options: [{ hoist: "never" }], parserOptions: { ecmaVersion: 6 } },
diff --git a/tests/lib/rules/padded-blocks.js b/tests/lib/rules/padded-blocks.js
index e38c7109c..561ec3afa 100644
--- a/tests/lib/rules/padded-blocks.js
+++ b/tests/lib/rules/padded-blocks.js
@@ -463,6 +463,12 @@ ruleTester.run("padded-blocks", rule, {
                     line: 9
                 }
             ]
+        },
+        {
+            code: "function foo() { // a\n\n  b;\n}",
+            output: "function foo() {\n // a\n\n  b;\n}",
+            options: ["never"],
+            errors: [NEVER_MESSAGE]
         }
     ]
 });
diff --git a/tests/lib/rules/prefer-const.js b/tests/lib/rules/prefer-const.js
index c38828ae3..495427667 100644
--- a/tests/lib/rules/prefer-const.js
+++ b/tests/lib/rules/prefer-const.js
@@ -92,13 +92,7 @@ ruleTester.run("prefer-const", rule, {
         {
             code: "let x; function foo() { bar(x); } x = 0;",
             options: [{ ignoreReadBeforeAssign: true }]
-        },
-
-        // https://github.com/eslint/eslint/issues/7712
-        // https://github.com/ternjs/acorn/issues/487
-        // This should be a SyntaxError, but espree parses it correctly. Don't throw an error if the variable has multiple declarations.
-        "let foo; const foo = 1;"
-
+        }
     ],
     invalid: [
         {
diff --git a/tests/lib/rules/sort-imports.js b/tests/lib/rules/sort-imports.js
index 92f6f64eb..4abe7a906 100644
--- a/tests/lib/rules/sort-imports.js
+++ b/tests/lib/rules/sort-imports.js
@@ -57,7 +57,7 @@ ruleTester.run("sort-imports", rule, {
         {
             code:
                 "import {a, b} from 'bar.js';\n" +
-                "import {b, c} from 'foo.js';"
+                "import {c, d} from 'foo.js';"
         },
         {
             code:
@@ -72,7 +72,7 @@ ruleTester.run("sort-imports", rule, {
         {
             code:
                 "import a, * as b from 'foo.js';\n" +
-                "import b from 'bar.js';"
+                "import c from 'bar.js';"
         },
         {
             code:
@@ -147,7 +147,7 @@ ruleTester.run("sort-imports", rule, {
         {
             code:
                 "import {b, c} from 'foo.js';\n" +
-                "import {a, b} from 'bar.js';",
+                "import {a, d} from 'bar.js';",
             errors: [expectedError]
         },
         {
diff --git a/tests/lib/util/source-code-fixer.js b/tests/lib/util/source-code-fixer.js
index dad6ece64..cb599c7b2 100644
--- a/tests/lib/util/source-code-fixer.js
+++ b/tests/lib/util/source-code-fixer.js
@@ -122,6 +122,13 @@ const INSERT_AT_END = {
         message: "nofix2",
         line: 1,
         column: 7
+    },
+    REVERSED_RANGE = {
+        message: "reversed range",
+        fix: {
+            range: [3, 0],
+            text: " "
+        }
     };
 
 //------------------------------------------------------------------------------
@@ -407,6 +414,12 @@ describe("SourceCodeFixer", () => {
                 assert.equal(result.messages.length, 0);
             });
 
+            it("should handle reversed ranges gracefully", () => {
+                const result = SourceCodeFixer.applyFixes(sourceCode, [REVERSED_RANGE]);
+
+                assert.equal(result.output, "\uFEFFvar  answer = 6 * 7;");
+            });
+
         });
 
         describe("Text Replacement", () => {