Skip to content

Commit

Permalink
Fix: astUtils.getNextLocation returns invalid location after CRLF (#1…
Browse files Browse the repository at this point in the history 
…3275)
  • Loading branch information
@mdjermanovic
mdjermanovic committed May 23, 2020
1 parent df01af1 commit a93083a
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 36 deletions.
57 changes: 51 additions & 6 deletions lib/rules/utils/ast-utils.js
View file
Expand Up @@ -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;
},

/**
Expand Down
15 changes: 15 additions & 0 deletions tests/lib/rules/comma-dangle.js
View file
Expand Up @@ -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', });",
Expand Down
22 changes: 22 additions & 0 deletions tests/lib/rules/semi.js
View file
Expand Up @@ -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();",
Expand All @@ -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; ",
Expand Down
71 changes: 41 additions & 30 deletions tests/lib/rules/utils/ast-utils.js
View file
Expand Up @@ -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
);
}
});
});
});

Expand Down

0 comments on commit a93083a

Please sign in to comment.