From 55f5c1c1d6e21fc89b727f88e63c77aa42c36760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Mon, 16 Sep 2019 14:43:49 +0200 Subject: [PATCH 1/2] Leave trailing comments aftre handling a possible trailing comma --- packages/babel-parser/src/parser/comments.js | 37 +- .../output.json | 20 +- .../comments/regression/10230/output.json | 20 +- .../comments/regression/10432/input.js | 5 + .../comments/regression/10432/output.json | 431 ++++++++++++++++++ 5 files changed, 503 insertions(+), 10 deletions(-) create mode 100644 packages/babel-parser/test/fixtures/comments/regression/10432/input.js create mode 100644 packages/babel-parser/test/fixtures/comments/regression/10432/output.json diff --git a/packages/babel-parser/src/parser/comments.js b/packages/babel-parser/src/parser/comments.js index 2f512ed904e9..f9b8e94e9c46 100644 --- a/packages/babel-parser/src/parser/comments.js +++ b/packages/babel-parser/src/parser/comments.js @@ -38,7 +38,19 @@ export default class CommentsParser extends BaseParser { this.state.leadingComments.push(comment); } - adjustCommentsAfterTrailingComma(node: Node, elements: Node[]) { + adjustCommentsAfterTrailingComma( + node: Node, + elements: Node[], + // When the current node is followed by a token which hasn't a respective AST node, we + // need to take all the trailing comments to prevent them from being attached to an + // unrelated node. e.g. in + // var { x } /* cmt */ = { y } + // we don't want /* cmt */ to be attached to { y }. + // On the other hand, in + // fn(x) [new line] /* cmt */ [new line] y + // /* cmt */ is both a trailing comment of fn(x) and a leading comment of y + takeAllComments?: boolean, + ) { if (this.state.leadingComments.length === 0) { return; } @@ -59,10 +71,12 @@ export default class CommentsParser extends BaseParser { } const newTrailingComments = []; - while (this.state.leadingComments.length) { - const leadingComment = this.state.leadingComments.shift(); + for (let i = 0; i < this.state.leadingComments.length; i++) { + const leadingComment = this.state.leadingComments[i]; if (leadingComment.end < node.end) { newTrailingComments.push(leadingComment); + this.state.leadingComments.splice(i, 1); + i--; } else { if (node.trailingComments === undefined) { node.trailingComments = []; @@ -70,6 +84,7 @@ export default class CommentsParser extends BaseParser { node.trailingComments.push(leadingComment); } } + if (takeAllComments) this.state.leadingComments.length = 0; if (newTrailingComments.length > 0) { lastElement.trailingComments = newTrailingComments; @@ -130,16 +145,20 @@ export default class CommentsParser extends BaseParser { if (firstChild) { switch (node.type) { case "ObjectExpression": - case "ObjectPattern": this.adjustCommentsAfterTrailingComma(node, node.properties); break; + case "ObjectPattern": + this.adjustCommentsAfterTrailingComma(node, node.properties, true); + break; case "CallExpression": this.adjustCommentsAfterTrailingComma(node, node.arguments); break; case "ArrayExpression": - case "ArrayPattern": this.adjustCommentsAfterTrailingComma(node, node.elements); break; + case "ArrayPattern": + this.adjustCommentsAfterTrailingComma(node, node.elements, true); + break; } } else if ( this.state.commentPreviousNode && @@ -148,9 +167,11 @@ export default class CommentsParser extends BaseParser { (this.state.commentPreviousNode.type === "ExportSpecifier" && node.type !== "ExportSpecifier")) ) { - this.adjustCommentsAfterTrailingComma(node, [ - this.state.commentPreviousNode, - ]); + this.adjustCommentsAfterTrailingComma( + node, + [this.state.commentPreviousNode], + true, + ); } if (lastChild) { diff --git a/packages/babel-parser/test/fixtures/comments/basic/switch-function-call-no-semicolon/output.json b/packages/babel-parser/test/fixtures/comments/basic/switch-function-call-no-semicolon/output.json index 4452426a990f..f732250d6daa 100644 --- a/packages/babel-parser/test/fixtures/comments/basic/switch-function-call-no-semicolon/output.json +++ b/packages/babel-parser/test/fixtures/comments/basic/switch-function-call-no-semicolon/output.json @@ -250,7 +250,25 @@ "label": null } ], - "test": null + "test": null, + "leadingComments": [ + { + "type": "CommentLine", + "value": " comment", + "start": 47, + "end": 57, + "loc": { + "start": { + "line": 4, + "column": 4 + }, + "end": { + "line": 4, + "column": 14 + } + } + } + ] } ] } diff --git a/packages/babel-parser/test/fixtures/comments/regression/10230/output.json b/packages/babel-parser/test/fixtures/comments/regression/10230/output.json index 9e185da12f9c..f081d1ee65fd 100644 --- a/packages/babel-parser/test/fixtures/comments/regression/10230/output.json +++ b/packages/babel-parser/test/fixtures/comments/regression/10230/output.json @@ -215,7 +215,25 @@ "identifierName": "B" }, "name": "B" - } + }, + "leadingComments": [ + { + "type": "CommentLine", + "value": " Two", + "start": 27, + "end": 33, + "loc": { + "start": { + "line": 6, + "column": 0 + }, + "end": { + "line": 6, + "column": 6 + } + } + } + ] } ], "directives": [] diff --git a/packages/babel-parser/test/fixtures/comments/regression/10432/input.js b/packages/babel-parser/test/fixtures/comments/regression/10432/input.js new file mode 100644 index 000000000000..645dc4cde990 --- /dev/null +++ b/packages/babel-parser/test/fixtures/comments/regression/10432/input.js @@ -0,0 +1,5 @@ +const socket = socketClient(address) +/* istanbul ignore next */ +socket.on('connect', function () { + debug('Connected to ' + address) +}) diff --git a/packages/babel-parser/test/fixtures/comments/regression/10432/output.json b/packages/babel-parser/test/fixtures/comments/regression/10432/output.json new file mode 100644 index 000000000000..075deef7bf64 --- /dev/null +++ b/packages/babel-parser/test/fixtures/comments/regression/10432/output.json @@ -0,0 +1,431 @@ +{ + "type": "File", + "start": 0, + "end": 136, + "loc": { + "start": { + "line": 1, + "column": 0 + }, + "end": { + "line": 5, + "column": 2 + } + }, + "program": { + "type": "Program", + "start": 0, + "end": 136, + "loc": { + "start": { + "line": 1, + "column": 0 + }, + "end": { + "line": 5, + "column": 2 + } + }, + "sourceType": "script", + "interpreter": null, + "body": [ + { + "type": "VariableDeclaration", + "start": 0, + "end": 36, + "loc": { + "start": { + "line": 1, + "column": 0 + }, + "end": { + "line": 1, + "column": 36 + } + }, + "declarations": [ + { + "type": "VariableDeclarator", + "start": 6, + "end": 36, + "loc": { + "start": { + "line": 1, + "column": 6 + }, + "end": { + "line": 1, + "column": 36 + } + }, + "id": { + "type": "Identifier", + "start": 6, + "end": 12, + "loc": { + "start": { + "line": 1, + "column": 6 + }, + "end": { + "line": 1, + "column": 12 + }, + "identifierName": "socket" + }, + "name": "socket" + }, + "init": { + "type": "CallExpression", + "start": 15, + "end": 36, + "loc": { + "start": { + "line": 1, + "column": 15 + }, + "end": { + "line": 1, + "column": 36 + } + }, + "callee": { + "type": "Identifier", + "start": 15, + "end": 27, + "loc": { + "start": { + "line": 1, + "column": 15 + }, + "end": { + "line": 1, + "column": 27 + }, + "identifierName": "socketClient" + }, + "name": "socketClient" + }, + "arguments": [ + { + "type": "Identifier", + "start": 28, + "end": 35, + "loc": { + "start": { + "line": 1, + "column": 28 + }, + "end": { + "line": 1, + "column": 35 + }, + "identifierName": "address" + }, + "name": "address" + } + ] + } + } + ], + "kind": "const", + "trailingComments": [ + { + "type": "CommentBlock", + "value": " istanbul ignore next ", + "start": 37, + "end": 63, + "loc": { + "start": { + "line": 2, + "column": 0 + }, + "end": { + "line": 2, + "column": 26 + } + } + } + ] + }, + { + "type": "ExpressionStatement", + "start": 64, + "end": 136, + "loc": { + "start": { + "line": 3, + "column": 0 + }, + "end": { + "line": 5, + "column": 2 + } + }, + "expression": { + "type": "CallExpression", + "start": 64, + "end": 136, + "loc": { + "start": { + "line": 3, + "column": 0 + }, + "end": { + "line": 5, + "column": 2 + } + }, + "callee": { + "type": "MemberExpression", + "start": 64, + "end": 73, + "loc": { + "start": { + "line": 3, + "column": 0 + }, + "end": { + "line": 3, + "column": 9 + } + }, + "object": { + "type": "Identifier", + "start": 64, + "end": 70, + "loc": { + "start": { + "line": 3, + "column": 0 + }, + "end": { + "line": 3, + "column": 6 + }, + "identifierName": "socket" + }, + "name": "socket" + }, + "property": { + "type": "Identifier", + "start": 71, + "end": 73, + "loc": { + "start": { + "line": 3, + "column": 7 + }, + "end": { + "line": 3, + "column": 9 + }, + "identifierName": "on" + }, + "name": "on" + }, + "computed": false + }, + "arguments": [ + { + "type": "StringLiteral", + "start": 74, + "end": 83, + "loc": { + "start": { + "line": 3, + "column": 10 + }, + "end": { + "line": 3, + "column": 19 + } + }, + "extra": { + "rawValue": "connect", + "raw": "'connect'" + }, + "value": "connect" + }, + { + "type": "FunctionExpression", + "start": 85, + "end": 135, + "loc": { + "start": { + "line": 3, + "column": 21 + }, + "end": { + "line": 5, + "column": 1 + } + }, + "id": null, + "generator": false, + "async": false, + "params": [], + "body": { + "type": "BlockStatement", + "start": 97, + "end": 135, + "loc": { + "start": { + "line": 3, + "column": 33 + }, + "end": { + "line": 5, + "column": 1 + } + }, + "body": [ + { + "type": "ExpressionStatement", + "start": 101, + "end": 133, + "loc": { + "start": { + "line": 4, + "column": 2 + }, + "end": { + "line": 4, + "column": 34 + } + }, + "expression": { + "type": "CallExpression", + "start": 101, + "end": 133, + "loc": { + "start": { + "line": 4, + "column": 2 + }, + "end": { + "line": 4, + "column": 34 + } + }, + "callee": { + "type": "Identifier", + "start": 101, + "end": 106, + "loc": { + "start": { + "line": 4, + "column": 2 + }, + "end": { + "line": 4, + "column": 7 + }, + "identifierName": "debug" + }, + "name": "debug" + }, + "arguments": [ + { + "type": "BinaryExpression", + "start": 107, + "end": 132, + "loc": { + "start": { + "line": 4, + "column": 8 + }, + "end": { + "line": 4, + "column": 33 + } + }, + "left": { + "type": "StringLiteral", + "start": 107, + "end": 122, + "loc": { + "start": { + "line": 4, + "column": 8 + }, + "end": { + "line": 4, + "column": 23 + } + }, + "extra": { + "rawValue": "Connected to ", + "raw": "'Connected to '" + }, + "value": "Connected to " + }, + "operator": "+", + "right": { + "type": "Identifier", + "start": 125, + "end": 132, + "loc": { + "start": { + "line": 4, + "column": 26 + }, + "end": { + "line": 4, + "column": 33 + }, + "identifierName": "address" + }, + "name": "address" + } + } + ] + } + } + ], + "directives": [] + } + } + ] + }, + "leadingComments": [ + { + "type": "CommentBlock", + "value": " istanbul ignore next ", + "start": 37, + "end": 63, + "loc": { + "start": { + "line": 2, + "column": 0 + }, + "end": { + "line": 2, + "column": 26 + } + } + } + ] + } + ], + "directives": [] + }, + "comments": [ + { + "type": "CommentBlock", + "value": " istanbul ignore next ", + "start": 37, + "end": 63, + "loc": { + "start": { + "line": 2, + "column": 0 + }, + "end": { + "line": 2, + "column": 26 + } + } + } + ] +} \ No newline at end of file From 4729994c0ed351ec5afead50218481663bda4ed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= Date: Mon, 16 Sep 2019 22:41:00 +0200 Subject: [PATCH 2/2] perf --- packages/babel-parser/src/parser/comments.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/babel-parser/src/parser/comments.js b/packages/babel-parser/src/parser/comments.js index f9b8e94e9c46..8554ffaf52cc 100644 --- a/packages/babel-parser/src/parser/comments.js +++ b/packages/babel-parser/src/parser/comments.js @@ -75,8 +75,12 @@ export default class CommentsParser extends BaseParser { const leadingComment = this.state.leadingComments[i]; if (leadingComment.end < node.end) { newTrailingComments.push(leadingComment); - this.state.leadingComments.splice(i, 1); - i--; + + // Perf: we don't need to splice if we are going to reset the array anyway + if (!takeAllComments) { + this.state.leadingComments.splice(i, 1); + i--; + } } else { if (node.trailingComments === undefined) { node.trailingComments = []; @@ -84,7 +88,7 @@ export default class CommentsParser extends BaseParser { node.trailingComments.push(leadingComment); } } - if (takeAllComments) this.state.leadingComments.length = 0; + if (takeAllComments) this.state.leadingComments = []; if (newTrailingComments.length > 0) { lastElement.trailingComments = newTrailingComments;