From a93083af89c6f9714dcdd4a7f27c8655a0b0dba6 Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Sat, 23 May 2020 02:35:08 +0200 Subject: [PATCH] Fix: astUtils.getNextLocation returns invalid location after CRLF (#13275) --- lib/rules/utils/ast-utils.js | 57 +++++++++++++++++++++--- tests/lib/rules/comma-dangle.js | 15 +++++++ tests/lib/rules/semi.js | 22 +++++++++ tests/lib/rules/utils/ast-utils.js | 71 +++++++++++++++++------------- 4 files changed, 129 insertions(+), 36 deletions(-) diff --git a/lib/rules/utils/ast-utils.js b/lib/rules/utils/ast-utils.js index e6a3cb4cac5..83ecf69434d 100644 --- a/lib/rules/utils/ast-utils.js +++ b/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 + * locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> (2, 0) -> null + * + * code: foo + * locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> (2, 0) -> null + * + * code: ab + * locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> null + * + * code: ab + * locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> (3, 0) -> null + * + * code: ab + * locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> (3, 0) -> null + * + * code: a + * locations: (1, 0) -> (1, 1) -> (2, 0) -> (3, 0) -> null + * + * code: + * 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; }, /** diff --git a/tests/lib/rules/comma-dangle.js b/tests/lib/rules/comma-dangle.js index 3a4f0aa735a..2ef7a008e2e 100644 --- a/tests/lib/rules/comma-dangle.js +++ b/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', });", diff --git a/tests/lib/rules/semi.js b/tests/lib/rules/semi.js index d38fc89284b..ce6310e1be0 100644 --- a/tests/lib/rules/semi.js +++ b/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; ", diff --git a/tests/lib/rules/utils/ast-utils.js b/tests/lib/rules/utils/ast-utils.js index dfa494fb6dc..683e5ed2a6e 100644 --- a/tests/lib/rules/utils/ast-utils.js +++ b/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 + ); + } + }); }); });