From 0fbfe64ebfb7fac901c30f0428a643381fb09ef0 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Mon, 13 Feb 2023 18:03:22 +0100 Subject: [PATCH 1/4] fix: `getFirstToken`/`getLastToken` on comment-only node --- lib/source-code/token-store/utils.js | 4 +- tests/lib/source-code/token-store.js | 80 +++++++++++++++++++++++++--- 2 files changed, 74 insertions(+), 10 deletions(-) diff --git a/lib/source-code/token-store/utils.js b/lib/source-code/token-store/utils.js index a2bd77de71a..8ee866298b1 100644 --- a/lib/source-code/token-store/utils.js +++ b/lib/source-code/token-store/utils.js @@ -55,7 +55,7 @@ exports.getFirstIndex = function getFirstIndex(tokens, indexMap, startLoc) { * For the map of "comment's location -> token's index", it points the next token of a comment. * In that case, +1 is unnecessary. */ - if (token && token.range[0] >= startLoc) { + if (!token || token.range[0] >= startLoc) { return index; } return index + 1; @@ -83,7 +83,7 @@ exports.getLastIndex = function getLastIndex(tokens, indexMap, endLoc) { * For the map of "comment's location -> token's index", it points the next token of a comment. * In that case, -1 is necessary. */ - if (token && token.range[1] > endLoc) { + if (!token || token.range[1] > endLoc) { return index - 1; } return index; diff --git a/tests/lib/source-code/token-store.js b/tests/lib/source-code/token-store.js index ff54a6a838a..0c1937692c4 100644 --- a/tests/lib/source-code/token-store.js +++ b/tests/lib/source-code/token-store.js @@ -627,8 +627,8 @@ describe("TokenStore", () => { const tokenStore = new TokenStore(ast.tokens, ast.comments); /* - * Actually, the first of nodes is always tokens, not comments. - * But I think this test case is needed for completeness. + * A node must not start with a token: it can start with a comment or be empty. + * This test case is needed for completeness. */ const token = tokenStore.getFirstToken( { range: [ast.comments[0].range[0], ast.tokens[5].range[1]] }, @@ -644,8 +644,8 @@ describe("TokenStore", () => { const tokenStore = new TokenStore(ast.tokens, ast.comments); /* - * Actually, the first of nodes is always tokens, not comments. - * But I think this test case is needed for completeness. + * A node must not start with a token: it can start with a comment or be empty. + * This test case is needed for completeness. */ const token = tokenStore.getFirstToken( { range: [ast.comments[0].range[0], ast.tokens[5].range[1]] } @@ -654,6 +654,38 @@ describe("TokenStore", () => { assert.strictEqual(token.value, "c"); }); + it("should return null if the source contains only comments", () => { + const code = "// comment"; + const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const token = tokenStore.getFirstToken(ast, { + filter() { + assert.fail("Unexpected call to filter callback"); + } + }); + + assert.strictEqual(token, null); + }); + + it("should return null if the source is empty", () => { + const code = ""; + const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const token = tokenStore.getFirstToken(ast); + + assert.strictEqual(token, null); + }); + + it("should retrieve a parent node's token if a node has no tokens", () => { + const code = "foo``"; + const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true, ecmaVersion: 6 }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const emptyTemplateElementNode = ast.body[0].expression.quasi.quasis[0]; + const token = tokenStore.getFirstToken(emptyTemplateElementNode); + + assert.strictEqual(token.value, "``"); + }); + }); describe("when calling getLastTokens", () => { @@ -814,8 +846,8 @@ describe("TokenStore", () => { const tokenStore = new TokenStore(ast.tokens, ast.comments); /* - * Actually, the last of nodes is always tokens, not comments. - * But I think this test case is needed for completeness. + * A node must not end with a token: it can end with a comment or be empty. + * This test case is needed for completeness. */ const token = tokenStore.getLastToken( { range: [ast.tokens[0].range[0], ast.comments[0].range[1]] }, @@ -831,8 +863,8 @@ describe("TokenStore", () => { const tokenStore = new TokenStore(ast.tokens, ast.comments); /* - * Actually, the last of nodes is always tokens, not comments. - * But I think this test case is needed for completeness. + * A node must not end with a token: it can end with a comment or be empty. + * This test case is needed for completeness. */ const token = tokenStore.getLastToken( { range: [ast.tokens[0].range[0], ast.comments[0].range[1]] } @@ -841,6 +873,38 @@ describe("TokenStore", () => { assert.strictEqual(token.value, "b"); }); + it("should return null if the source contains only comments", () => { + const code = "// comment"; + const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const token = tokenStore.getLastToken(ast, { + filter() { + assert.fail("Unexpected call to filter callback"); + } + }); + + assert.strictEqual(token, null); + }); + + it("should return null if the source is empty", () => { + const code = ""; + const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const token = tokenStore.getLastToken(ast); + + assert.strictEqual(token, null); + }); + + it("should retrieve a parent node's token if a node has no tokens", () => { + const code = "foo``"; + const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true, ecmaVersion: 6 }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const emptyTemplateElementNode = ast.body[0].expression.quasi.quasis[0]; + const token = tokenStore.getLastToken(emptyTemplateElementNode); + + assert.strictEqual(token.value, "``"); + }); + }); describe("when calling getFirstTokensBetween", () => { From 9c7b97868d676650a01c2c6fe632f9078a42cf73 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Wed, 1 Mar 2023 13:57:34 +0100 Subject: [PATCH 2/4] Remove unnecessary array bound checks --- lib/source-code/token-store/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/source-code/token-store/utils.js b/lib/source-code/token-store/utils.js index 8ee866298b1..15a4f2e5359 100644 --- a/lib/source-code/token-store/utils.js +++ b/lib/source-code/token-store/utils.js @@ -49,7 +49,7 @@ exports.getFirstIndex = function getFirstIndex(tokens, indexMap, startLoc) { } if ((startLoc - 1) in indexMap) { const index = indexMap[startLoc - 1]; - const token = (index >= 0 && index < tokens.length) ? tokens[index] : null; + const token = tokens[index]; /* * For the map of "comment's location -> token's index", it points the next token of a comment. @@ -77,7 +77,7 @@ exports.getLastIndex = function getLastIndex(tokens, indexMap, endLoc) { } if ((endLoc - 1) in indexMap) { const index = indexMap[endLoc - 1]; - const token = (index >= 0 && index < tokens.length) ? tokens[index] : null; + const token = tokens[index]; /* * For the map of "comment's location -> token's index", it points the next token of a comment. From 551ad3b511e80cebda322d3d084c9431f4ebb226 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Thu, 9 Mar 2023 17:19:36 +0100 Subject: [PATCH 3/4] Split `if`-statement for clarity, remove inaccurate unit test --- lib/source-code/token-store/utils.js | 14 ++++++++++++-- tests/lib/source-code/token-store.js | 20 -------------------- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/lib/source-code/token-store/utils.js b/lib/source-code/token-store/utils.js index 15a4f2e5359..4a0170992ea 100644 --- a/lib/source-code/token-store/utils.js +++ b/lib/source-code/token-store/utils.js @@ -51,11 +51,16 @@ exports.getFirstIndex = function getFirstIndex(tokens, indexMap, startLoc) { const index = indexMap[startLoc - 1]; const token = tokens[index]; + // If the mapped index is out of bounds, the returned cursor index will point after the end of the tokens array. + if (!token) { + return tokens.length; + } + /* * For the map of "comment's location -> token's index", it points the next token of a comment. * In that case, +1 is unnecessary. */ - if (!token || token.range[0] >= startLoc) { + if (token.range[0] >= startLoc) { return index; } return index + 1; @@ -79,11 +84,16 @@ exports.getLastIndex = function getLastIndex(tokens, indexMap, endLoc) { const index = indexMap[endLoc - 1]; const token = tokens[index]; + // If the mapped index is out of bounds, the returned cursor index will point before the start of the tokens array. + if (!token) { + return -1; + } + /* * For the map of "comment's location -> token's index", it points the next token of a comment. * In that case, -1 is necessary. */ - if (!token || token.range[1] > endLoc) { + if (token.range[1] > endLoc) { return index - 1; } return index; diff --git a/tests/lib/source-code/token-store.js b/tests/lib/source-code/token-store.js index 0c1937692c4..8c10a092f50 100644 --- a/tests/lib/source-code/token-store.js +++ b/tests/lib/source-code/token-store.js @@ -676,16 +676,6 @@ describe("TokenStore", () => { assert.strictEqual(token, null); }); - it("should retrieve a parent node's token if a node has no tokens", () => { - const code = "foo``"; - const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true, ecmaVersion: 6 }); - const tokenStore = new TokenStore(ast.tokens, ast.comments); - const emptyTemplateElementNode = ast.body[0].expression.quasi.quasis[0]; - const token = tokenStore.getFirstToken(emptyTemplateElementNode); - - assert.strictEqual(token.value, "``"); - }); - }); describe("when calling getLastTokens", () => { @@ -895,16 +885,6 @@ describe("TokenStore", () => { assert.strictEqual(token, null); }); - it("should retrieve a parent node's token if a node has no tokens", () => { - const code = "foo``"; - const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true, ecmaVersion: 6 }); - const tokenStore = new TokenStore(ast.tokens, ast.comments); - const emptyTemplateElementNode = ast.body[0].expression.quasi.quasis[0]; - const token = tokenStore.getLastToken(emptyTemplateElementNode); - - assert.strictEqual(token.value, "``"); - }); - }); describe("when calling getFirstTokensBetween", () => { From 449e8942600804757ed871dbd41ac7ca0ceb3ffa Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Mon, 20 Mar 2023 19:47:55 +0100 Subject: [PATCH 4/4] Fix for trailing comments in root node --- lib/source-code/token-store/utils.js | 4 +-- tests/fixtures/parsers/all-comments-parser.js | 28 +++++++++++++++++++ tests/lib/source-code/token-store.js | 20 +++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/parsers/all-comments-parser.js diff --git a/lib/source-code/token-store/utils.js b/lib/source-code/token-store/utils.js index 4a0170992ea..859831916ea 100644 --- a/lib/source-code/token-store/utils.js +++ b/lib/source-code/token-store/utils.js @@ -84,9 +84,9 @@ exports.getLastIndex = function getLastIndex(tokens, indexMap, endLoc) { const index = indexMap[endLoc - 1]; const token = tokens[index]; - // If the mapped index is out of bounds, the returned cursor index will point before the start of the tokens array. + // If the mapped index is out of bounds, the returned cursor index will point before the end of the tokens array. if (!token) { - return -1; + return tokens.length - 1; } /* diff --git a/tests/fixtures/parsers/all-comments-parser.js b/tests/fixtures/parsers/all-comments-parser.js new file mode 100644 index 00000000000..595392a0970 --- /dev/null +++ b/tests/fixtures/parsers/all-comments-parser.js @@ -0,0 +1,28 @@ +// Similar to the default parser, but considers leading and trailing comments to be part of the root node. +// Some custom parsers like @typescript-eslint/parser behave in this way. + +const espree = require("espree"); +exports.parse = function(code, options) { + const ast = espree.parse(code, options); + + if (ast.range && ast.comments && ast.comments.length > 0) { + const firstComment = ast.comments[0]; + const lastComment = ast.comments[ast.comments.length - 1]; + + if (ast.range[0] > firstComment.range[0]) { + ast.range[0] = firstComment.range[0]; + ast.start = firstComment.start; + if (ast.loc) { + ast.loc.start = firstComment.loc.start; + } + } + if (ast.range[1] < lastComment.range[1]) { + ast.range[1] = lastComment.range[1]; + ast.end = lastComment.end; + if (ast.loc) { + ast.loc.end = lastComment.loc.end; + } + } + } + return ast; +}; diff --git a/tests/lib/source-code/token-store.js b/tests/lib/source-code/token-store.js index 8c10a092f50..3b9db7cb95f 100644 --- a/tests/lib/source-code/token-store.js +++ b/tests/lib/source-code/token-store.js @@ -654,6 +654,16 @@ describe("TokenStore", () => { assert.strictEqual(token.value, "c"); }); + it("should retrieve the first token if the root node contains a trailing comment", () => { + const parser = require("../../fixtures/parsers/all-comments-parser"); + const code = "foo // comment"; + const ast = parser.parse(code, { loc: true, range: true, tokens: true, comment: true }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const token = tokenStore.getFirstToken(ast); + + assert.strictEqual(token, ast.tokens[0]); + }); + it("should return null if the source contains only comments", () => { const code = "// comment"; const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true }); @@ -863,6 +873,16 @@ describe("TokenStore", () => { assert.strictEqual(token.value, "b"); }); + it("should retrieve the last token if the root node contains a trailing comment", () => { + const parser = require("../../fixtures/parsers/all-comments-parser"); + const code = "foo // comment"; + const ast = parser.parse(code, { loc: true, range: true, tokens: true, comment: true }); + const tokenStore = new TokenStore(ast.tokens, ast.comments); + const token = tokenStore.getLastToken(ast); + + assert.strictEqual(token, ast.tokens[0]); + }); + it("should return null if the source contains only comments", () => { const code = "// comment"; const ast = espree.parse(code, { loc: true, range: true, tokens: true, comment: true });