Skip to content

Commit 1fbf118

Browse files
fasttimefasttime
authored
fix: getFirstToken/getLastToken on comment-only node (#16889)
* fix: `getFirstToken`/`getLastToken` on comment-only node * Remove unnecessary array bound checks * Split `if`-statement for clarity, remove inaccurate unit test * Fix for trailing comments in root node
1 parent 129e252 commit 1fbf118

3 files changed

Lines changed: 114 additions & 12 deletions

File tree

‎lib/source-code/token-store/utils.js‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,18 @@ exports.getFirstIndex = function getFirstIndex(tokens, indexMap, startLoc) {
4949
}
5050
if ((startLoc - 1) in indexMap) {
5151
const index = indexMap[startLoc - 1];
52-
const token = (index >= 0 && index < tokens.length) ? tokens[index] : null;
52+
const token = tokens[index];
53+
54+
// If the mapped index is out of bounds, the returned cursor index will point after the end of the tokens array.
55+
if (!token) {
56+
return tokens.length;
57+
}
5358

5459
/*
5560
* For the map of "comment's location -> token's index", it points the next token of a comment.
5661
* In that case, +1 is unnecessary.
5762
*/
58-
if (token && token.range[0] >= startLoc) {
63+
if (token.range[0] >= startLoc) {
5964
return index;
6065
}
6166
return index + 1;
@@ -77,13 +82,18 @@ exports.getLastIndex = function getLastIndex(tokens, indexMap, endLoc) {
7782
}
7883
if ((endLoc - 1) in indexMap) {
7984
const index = indexMap[endLoc - 1];
80-
const token = (index >= 0 && index < tokens.length) ? tokens[index] : null;
85+
const token = tokens[index];
86+
87+
// If the mapped index is out of bounds, the returned cursor index will point before the end of the tokens array.
88+
if (!token) {
89+
return tokens.length - 1;
90+
}
8191

8292
/*
8393
* For the map of "comment's location -> token's index", it points the next token of a comment.
8494
* In that case, -1 is necessary.
8595
*/
86-
if (token && token.range[1] > endLoc) {
96+
if (token.range[1] > endLoc) {
8797
return index - 1;
8898
}
8999
return index;

‎tests/fixtures/parsers/all-comments-parser.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Similar to the default parser, but considers leading and trailing comments to be part of the root node.
2+
// Some custom parsers like @typescript-eslint/parser behave in this way.
3+
4+
const espree = require("espree");
5+
exports.parse = function(code, options) {
6+
const ast = espree.parse(code, options);
7+
8+
if (ast.range && ast.comments && ast.comments.length > 0) {
9+
const firstComment = ast.comments[0];
10+
const lastComment = ast.comments[ast.comments.length - 1];
11+
12+
if (ast.range[0] > firstComment.range[0]) {
13+
ast.range[0] = firstComment.range[0];
14+
ast.start = firstComment.start;
15+
if (ast.loc) {
16+
ast.loc.start = firstComment.loc.start;
17+
}
18+
}
19+
if (ast.range[1] < lastComment.range[1]) {
20+
ast.range[1] = lastComment.range[1];
21+
ast.end = lastComment.end;
22+
if (ast.loc) {
23+
ast.loc.end = lastComment.loc.end;
24+
}
25+
}
26+
}
27+
return ast;
28+
};

‎tests/lib/source-code/token-store.js‎

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -627,8 +627,8 @@ describe("TokenStore", () => {
627627
const tokenStore = new TokenStore(ast.tokens, ast.comments);
628628

629629
/*
630-
* Actually, the first of nodes is always tokens, not comments.
631-
* But I think this test case is needed for completeness.
630+
* A node must not start with a token: it can start with a comment or be empty.
631+
* This test case is needed for completeness.
632632
*/
633633
const token = tokenStore.getFirstToken(
634634
{ range: [ast.comments[0].range[0], ast.tokens[5].range[1]] },
@@ -644,8 +644,8 @@ describe("TokenStore", () => {
644644
const tokenStore = new TokenStore(ast.tokens, ast.comments);
645645

646646
/*
647-
* Actually, the first of nodes is always tokens, not comments.
648-
* But I think this test case is needed for completeness.
647+
* A node must not start with a token: it can start with a comment or be empty.
648+
* This test case is needed for completeness.
649649
*/
650650
const token = tokenStore.getFirstToken(
651651
{ range: [ast.comments[0].range[0], ast.tokens[5].range[1]] }
@@ -654,6 +654,38 @@ describe("TokenStore", () => {
654654
assert.strictEqual(token.value, "c");
655655
});
656656

657+
it("should retrieve the first token if the root node contains a trailing comment", () => {
658+
const parser = require("../../fixtures/parsers/all-comments-parser");
659+
const code = "foo // comment";
660+
const ast = parser.parse(code, { loc: true, range: true, tokens: true, comment: true });
661+
const tokenStore = new TokenStore(ast.tokens, ast.comments);
662+
const token = tokenStore.getFirstToken(ast);
663+
664+
assert.strictEqual(token, ast.tokens[0]);
665+
});
666+
667+
it("should return null if the source contains only comments", () => {
668+
const code = "// comment";
669+
const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true });
670+
const tokenStore = new TokenStore(ast.tokens, ast.comments);
671+
const token = tokenStore.getFirstToken(ast, {
672+
filter() {
673+
assert.fail("Unexpected call to filter callback");
674+
}
675+
});
676+
677+
assert.strictEqual(token, null);
678+
});
679+
680+
it("should return null if the source is empty", () => {
681+
const code = "";
682+
const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true });
683+
const tokenStore = new TokenStore(ast.tokens, ast.comments);
684+
const token = tokenStore.getFirstToken(ast);
685+
686+
assert.strictEqual(token, null);
687+
});
688+
657689
});
658690

659691
describe("when calling getLastTokens", () => {
@@ -814,8 +846,8 @@ describe("TokenStore", () => {
814846
const tokenStore = new TokenStore(ast.tokens, ast.comments);
815847

816848
/*
817-
* Actually, the last of nodes is always tokens, not comments.
818-
* But I think this test case is needed for completeness.
849+
* A node must not end with a token: it can end with a comment or be empty.
850+
* This test case is needed for completeness.
819851
*/
820852
const token = tokenStore.getLastToken(
821853
{ range: [ast.tokens[0].range[0], ast.comments[0].range[1]] },
@@ -831,8 +863,8 @@ describe("TokenStore", () => {
831863
const tokenStore = new TokenStore(ast.tokens, ast.comments);
832864

833865
/*
834-
* Actually, the last of nodes is always tokens, not comments.
835-
* But I think this test case is needed for completeness.
866+
* A node must not end with a token: it can end with a comment or be empty.
867+
* This test case is needed for completeness.
836868
*/
837869
const token = tokenStore.getLastToken(
838870
{ range: [ast.tokens[0].range[0], ast.comments[0].range[1]] }
@@ -841,6 +873,38 @@ describe("TokenStore", () => {
841873
assert.strictEqual(token.value, "b");
842874
});
843875

876+
it("should retrieve the last token if the root node contains a trailing comment", () => {
877+
const parser = require("../../fixtures/parsers/all-comments-parser");
878+
const code = "foo // comment";
879+
const ast = parser.parse(code, { loc: true, range: true, tokens: true, comment: true });
880+
const tokenStore = new TokenStore(ast.tokens, ast.comments);
881+
const token = tokenStore.getLastToken(ast);
882+
883+
assert.strictEqual(token, ast.tokens[0]);
884+
});
885+
886+
it("should return null if the source contains only comments", () => {
887+
const code = "// comment";
888+
const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true });
889+
const tokenStore = new TokenStore(ast.tokens, ast.comments);
890+
const token = tokenStore.getLastToken(ast, {
891+
filter() {
892+
assert.fail("Unexpected call to filter callback");
893+
}
894+
});
895+
896+
assert.strictEqual(token, null);
897+
});
898+
899+
it("should return null if the source is empty", () => {
900+
const code = "";
901+
const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true });
902+
const tokenStore = new TokenStore(ast.tokens, ast.comments);
903+
const token = tokenStore.getLastToken(ast);
904+
905+
assert.strictEqual(token, null);
906+
});
907+
844908
});
845909

846910
describe("when calling getFirstTokensBetween", () => {

0 commit comments

Comments
 (0)