Fix: astUtils.getNextLocation returns invalid location after CRLF

lib/rules/utils/ast-utils.js

@@ -1243,19 +1243,64 @@ module.exports = {
 
     /**
      * Gets next location when the result is not out of bound, otherwise returns null.
+     *
+     * Assumptions:
+     *
+     * - The given location represents a valid location in the given source code.
+     * - Columns are 0-based.
+     * - Lines are 1-based.
+     * - Column immediately after the last character in a line (not incl. linebreaks) is considered to be a valid location.
+     * - If the source code ends with a linebreak, `sourceCode.lines` array will have an extra element (empty string) at the end.
+     *   The start (column 0) of that extra line is considered to be a valid location.
+     *
+     * Examples of successive locations (line, column):
+     *
+     * code: foo
+     * locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> null
+     *
+     * code: foo<LF>
+     * locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> (2, 0) -> null
+     *
+     * code: foo<CR><LF>
+     * locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> (2, 0) -> null
+     *
+     * code: a<LF>b
+     * locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> null
+     *
+     * code: a<LF>b<LF>
+     * locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> (3, 0) -> null
+     *
+     * code: a<CR><LF>b<CR><LF>
+     * locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> (3, 0) -> null
+     *
+     * code: a<LF><LF>
+     * locations: (1, 0) -> (1, 1) -> (2, 0) -> (3, 0) -> null
+     *
+     * code: <LF>
+     * locations: (1, 0) -> (2, 0) -> null
+     *
+     * code:
+     * locations: (1, 0) -> null
      * @param {SourceCode} sourceCode The sourceCode
      * @param {{line: number, column: number}} location The location
      * @returns {{line: number, column: number} | null} Next location
      */
-    getNextLocation(sourceCode, location) {
-        const index = sourceCode.getIndexFromLoc(location);
+    getNextLocation(sourceCode, { line, column }) {
+        if (column < sourceCode.lines[line - 1].length) {
+            return {
+                line,
+                column: column + 1
+            };
+        }
 
-        // Avoid out of bound location
-        if (index + 1 > sourceCode.text.length) {
-            return null;
+        if (line < sourceCode.lines.length) {
+            return {
+                line: line + 1,
+                column: 0
+            };
         }
 
-        return sourceCode.getLocFromIndex(index + 1);
+        return null;
     },
 
     /**

tests/lib/rules/comma-dangle.js

@@ -672,6 +672,21 @@ ruleTester.run("comma-dangle", rule, {
                 }
             ]
         },
+        {
+            code: "var foo = {\nbar: 'baz'\r\n}",
+            output: "var foo = {\nbar: 'baz',\r\n}",
+            options: ["always"],
+            errors: [
+                {
+                    messageId: "missing",
+                    type: "Property",
+                    line: 2,
+                    column: 11,
+                    endLine: 3,
+                    endColumn: 1
+                }
+            ]
+        },
         {
             code: "foo({ bar: 'baz', qux: 'quux' });",
             output: "foo({ bar: 'baz', qux: 'quux', });",

tests/lib/rules/semi.js

@@ -359,6 +359,17 @@ ruleTester.run("semi", rule, {
                 endColumn: 1
             }]
         },
+        {
+            code: "foo()\r\n",
+            output: "foo();\r\n",
+            errors: [{
+                messageId: "missingSemi",
+                type: "ExpressionStatement",
+                column: 6,
+                endLine: 2,
+                endColumn: 1
+            }]
+        },
         {
             code: "foo()\nbar();",
             output: "foo();\nbar();",
@@ -370,6 +381,17 @@ ruleTester.run("semi", rule, {
                 endColumn: 1
             }]
         },
+        {
+            code: "foo()\r\nbar();",
+            output: "foo();\r\nbar();",
+            errors: [{
+                messageId: "missingSemi",
+                type: "ExpressionStatement",
+                column: 6,
+                endLine: 2,
+                endColumn: 1
+            }]
+        },
         {
             code: "for (var a in b) var i ",
             output: "for (var a in b) var i; ",

tests/lib/rules/utils/ast-utils.js

@@ -914,38 +914,49 @@ describe("ast-utils", () => {
     });
 
     describe("getNextLocation", () => {
-        const code = "foo;\n";
-        const ast = espree.parse(code, ESPREE_CONFIG);
-        const sourceCode = new SourceCode(code, ast);
-
-        it("should handle normal case", () => {
-            assert.deepStrictEqual(
-                astUtils.getNextLocation(
-                    sourceCode,
-                    { line: 1, column: 0 }
-                ),
-                { line: 1, column: 1 }
-            );
-        });
 
-        it("should handle linebreaks", () => {
-            assert.deepStrictEqual(
-                astUtils.getNextLocation(
-                    sourceCode,
-                    { line: 1, column: 4 }
-                ),
-                { line: 2, column: 0 }
-            );
-        });
+        /* eslint-disable quote-props */
+        const expectedResults = {
+            "": [[1, 0], null],
+            "\n": [[1, 0], [2, 0], null],
+            "\r\n": [[1, 0], [2, 0], null],
+            "foo": [[1, 0], [1, 1], [1, 2], [1, 3], null],
+            "foo\n": [[1, 0], [1, 1], [1, 2], [1, 3], [2, 0], null],
+            "foo\r\n": [[1, 0], [1, 1], [1, 2], [1, 3], [2, 0], null],
+            "foo;\n": [[1, 0], [1, 1], [1, 2], [1, 3], [1, 4], [2, 0], null],
+            "a\nb": [[1, 0], [1, 1], [2, 0], [2, 1], null],
+            "a\nb\n": [[1, 0], [1, 1], [2, 0], [2, 1], [3, 0], null],
+            "a\r\nb\r\n": [[1, 0], [1, 1], [2, 0], [2, 1], [3, 0], null],
+            "a\nb\r\n": [[1, 0], [1, 1], [2, 0], [2, 1], [3, 0], null],
+            "a\n\n": [[1, 0], [1, 1], [2, 0], [3, 0], null],
+            "a\r\n\r\n": [[1, 0], [1, 1], [2, 0], [3, 0], null],
+            "\n\r\n\n\r\n": [[1, 0], [2, 0], [3, 0], [4, 0], [5, 0], null],
+            "ab\u2029c": [[1, 0], [1, 1], [1, 2], [2, 0], [2, 1], null],
+            "ab\ncde\n": [[1, 0], [1, 1], [1, 2], [2, 0], [2, 1], [2, 2], [2, 3], [3, 0], null],
+            "a ": [[1, 0], [1, 1], [1, 2], null],
+            "a\t": [[1, 0], [1, 1], [1, 2], null],
+            "a \n": [[1, 0], [1, 1], [1, 2], [2, 0], null]
+        };
+        /* eslint-enable quote-props */
 
-        it("should return null when result is out of bound", () => {
-            assert.strictEqual(
-                astUtils.getNextLocation(
-                    sourceCode,
-                    { line: 2, column: 0 }
-                ),
-                null
-            );
+        Object.keys(expectedResults).forEach(code => {
+            it(`should return expected locations for "${code}".`, () => {
+                const ast = espree.parse(code, ESPREE_CONFIG);
+                const sourceCode = new SourceCode(code, ast);
+                const locations = expectedResults[code];
+
+                for (let i = 0; i < locations.length - 1; i++) {
+                    const location = { line: locations[i][0], column: locations[i][1] };
+                    const expectedNextLocation = locations[i + 1]
+                        ? { line: locations[i + 1][0], column: locations[i + 1][1] }
+                        : null;
+
+                    assert.deepStrictEqual(
+                        astUtils.getNextLocation(sourceCode, location),
+                        expectedNextLocation
+                    );
+                }
+            });
         });
     });