Skip to content

Commit a93083a

Browse files
mdjermanovicmdjermanovic
authored
Fix: astUtils.getNextLocation returns invalid location after CRLF (#13275)
1 parent df01af1 commit a93083a

4 files changed

Lines changed: 129 additions & 36 deletions

File tree

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

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1243,19 +1243,64 @@ module.exports = {
12431243

12441244
/**
12451245
* Gets next location when the result is not out of bound, otherwise returns null.
1246+
*
1247+
* Assumptions:
1248+
*
1249+
* - The given location represents a valid location in the given source code.
1250+
* - Columns are 0-based.
1251+
* - Lines are 1-based.
1252+
* - Column immediately after the last character in a line (not incl. linebreaks) is considered to be a valid location.
1253+
* - If the source code ends with a linebreak, `sourceCode.lines` array will have an extra element (empty string) at the end.
1254+
* The start (column 0) of that extra line is considered to be a valid location.
1255+
*
1256+
* Examples of successive locations (line, column):
1257+
*
1258+
* code: foo
1259+
* locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> null
1260+
*
1261+
* code: foo<LF>
1262+
* locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> (2, 0) -> null
1263+
*
1264+
* code: foo<CR><LF>
1265+
* locations: (1, 0) -> (1, 1) -> (1, 2) -> (1, 3) -> (2, 0) -> null
1266+
*
1267+
* code: a<LF>b
1268+
* locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> null
1269+
*
1270+
* code: a<LF>b<LF>
1271+
* locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> (3, 0) -> null
1272+
*
1273+
* code: a<CR><LF>b<CR><LF>
1274+
* locations: (1, 0) -> (1, 1) -> (2, 0) -> (2, 1) -> (3, 0) -> null
1275+
*
1276+
* code: a<LF><LF>
1277+
* locations: (1, 0) -> (1, 1) -> (2, 0) -> (3, 0) -> null
1278+
*
1279+
* code: <LF>
1280+
* locations: (1, 0) -> (2, 0) -> null
1281+
*
1282+
* code:
1283+
* locations: (1, 0) -> null
12461284
* @param {SourceCode} sourceCode The sourceCode
12471285
* @param {{line: number, column: number}} location The location
12481286
* @returns {{line: number, column: number} | null} Next location
12491287
*/
1250-
getNextLocation(sourceCode, location) {
1251-
const index = sourceCode.getIndexFromLoc(location);
1288+
getNextLocation(sourceCode, { line, column }) {
1289+
if (column < sourceCode.lines[line - 1].length) {
1290+
return {
1291+
line,
1292+
column: column + 1
1293+
};
1294+
}
12521295

1253-
// Avoid out of bound location
1254-
if (index + 1 > sourceCode.text.length) {
1255-
return null;
1296+
if (line < sourceCode.lines.length) {
1297+
return {
1298+
line: line + 1,
1299+
column: 0
1300+
};
12561301
}
12571302

1258-
return sourceCode.getLocFromIndex(index + 1);
1303+
return null;
12591304
},
12601305

12611306
/**

‎tests/lib/rules/comma-dangle.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,21 @@ ruleTester.run("comma-dangle", rule, {
672672
}
673673
]
674674
},
675+
{
676+
code: "var foo = {\nbar: 'baz'\r\n}",
677+
output: "var foo = {\nbar: 'baz',\r\n}",
678+
options: ["always"],
679+
errors: [
680+
{
681+
messageId: "missing",
682+
type: "Property",
683+
line: 2,
684+
column: 11,
685+
endLine: 3,
686+
endColumn: 1
687+
}
688+
]
689+
},
675690
{
676691
code: "foo({ bar: 'baz', qux: 'quux' });",
677692
output: "foo({ bar: 'baz', qux: 'quux', });",

‎tests/lib/rules/semi.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,17 @@ ruleTester.run("semi", rule, {
359359
endColumn: 1
360360
}]
361361
},
362+
{
363+
code: "foo()\r\n",
364+
output: "foo();\r\n",
365+
errors: [{
366+
messageId: "missingSemi",
367+
type: "ExpressionStatement",
368+
column: 6,
369+
endLine: 2,
370+
endColumn: 1
371+
}]
372+
},
362373
{
363374
code: "foo()\nbar();",
364375
output: "foo();\nbar();",
@@ -370,6 +381,17 @@ ruleTester.run("semi", rule, {
370381
endColumn: 1
371382
}]
372383
},
384+
{
385+
code: "foo()\r\nbar();",
386+
output: "foo();\r\nbar();",
387+
errors: [{
388+
messageId: "missingSemi",
389+
type: "ExpressionStatement",
390+
column: 6,
391+
endLine: 2,
392+
endColumn: 1
393+
}]
394+
},
373395
{
374396
code: "for (var a in b) var i ",
375397
output: "for (var a in b) var i; ",

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

Lines changed: 41 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -914,38 +914,49 @@ describe("ast-utils", () => {
914914
});
915915

916916
describe("getNextLocation", () => {
917-
const code = "foo;\n";
918-
const ast = espree.parse(code, ESPREE_CONFIG);
919-
const sourceCode = new SourceCode(code, ast);
920-
921-
it("should handle normal case", () => {
922-
assert.deepStrictEqual(
923-
astUtils.getNextLocation(
924-
sourceCode,
925-
{ line: 1, column: 0 }
926-
),
927-
{ line: 1, column: 1 }
928-
);
929-
});
930917

931-
it("should handle linebreaks", () => {
932-
assert.deepStrictEqual(
933-
astUtils.getNextLocation(
934-
sourceCode,
935-
{ line: 1, column: 4 }
936-
),
937-
{ line: 2, column: 0 }
938-
);
939-
});
918+
/* eslint-disable quote-props */
919+
const expectedResults = {
920+
"": [[1, 0], null],
921+
"\n": [[1, 0], [2, 0], null],
922+
"\r\n": [[1, 0], [2, 0], null],
923+
"foo": [[1, 0], [1, 1], [1, 2], [1, 3], null],
924+
"foo\n": [[1, 0], [1, 1], [1, 2], [1, 3], [2, 0], null],
925+
"foo\r\n": [[1, 0], [1, 1], [1, 2], [1, 3], [2, 0], null],
926+
"foo;\n": [[1, 0], [1, 1], [1, 2], [1, 3], [1, 4], [2, 0], null],
927+
"a\nb": [[1, 0], [1, 1], [2, 0], [2, 1], null],
928+
"a\nb\n": [[1, 0], [1, 1], [2, 0], [2, 1], [3, 0], null],
929+
"a\r\nb\r\n": [[1, 0], [1, 1], [2, 0], [2, 1], [3, 0], null],
930+
"a\nb\r\n": [[1, 0], [1, 1], [2, 0], [2, 1], [3, 0], null],
931+
"a\n\n": [[1, 0], [1, 1], [2, 0], [3, 0], null],
932+
"a\r\n\r\n": [[1, 0], [1, 1], [2, 0], [3, 0], null],
933+
"\n\r\n\n\r\n": [[1, 0], [2, 0], [3, 0], [4, 0], [5, 0], null],
934+
"ab\u2029c": [[1, 0], [1, 1], [1, 2], [2, 0], [2, 1], null],
935+
"ab\ncde\n": [[1, 0], [1, 1], [1, 2], [2, 0], [2, 1], [2, 2], [2, 3], [3, 0], null],
936+
"a ": [[1, 0], [1, 1], [1, 2], null],
937+
"a\t": [[1, 0], [1, 1], [1, 2], null],
938+
"a \n": [[1, 0], [1, 1], [1, 2], [2, 0], null]
939+
};
940+
/* eslint-enable quote-props */
940941

941-
it("should return null when result is out of bound", () => {
942-
assert.strictEqual(
943-
astUtils.getNextLocation(
944-
sourceCode,
945-
{ line: 2, column: 0 }
946-
),
947-
null
948-
);
942+
Object.keys(expectedResults).forEach(code => {
943+
it(`should return expected locations for "${code}".`, () => {
944+
const ast = espree.parse(code, ESPREE_CONFIG);
945+
const sourceCode = new SourceCode(code, ast);
946+
const locations = expectedResults[code];
947+
948+
for (let i = 0; i < locations.length - 1; i++) {
949+
const location = { line: locations[i][0], column: locations[i][1] };
950+
const expectedNextLocation = locations[i + 1]
951+
? { line: locations[i + 1][0], column: locations[i + 1][1] }
952+
: null;
953+
954+
assert.deepStrictEqual(
955+
astUtils.getNextLocation(sourceCode, location),
956+
expectedNextLocation
957+
);
958+
}
959+
});
949960
});
950961
});
951962

0 commit comments

Comments
 (0)