From 3b468525aafb83896e33c3f5142d63e357367550 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 17 Jul 2024 10:56:08 +0200 Subject: [PATCH 1/3] fix: properly assign trailing comments Trailing comments were added even if they started before the node end, which violates the definition of trailing comments. This fixes that, with the consequence that some comments no longer show up in the AST at all. Previously they were stuffed _somewhere_, but arguably at the wrong positions. For example the last comment in a function body got assigned as a trailing comma for the body, which is wrong, because the body ends _after_ the comma. The special case is comments at the end of an expression tag - they are added even if there are multiple, and they are added regardless of whether they are separated by newlines or not. This ensures the expression tag end is calculated correctly. Fixes #12466 --- .changeset/fast-toes-act.md | 5 + .../src/compiler/phases/1-parse/acorn.js | 9 +- .../samples/javascript-comments/input.svelte | 6 +- .../samples/javascript-comments/output.json | 188 +++++++++++------- .../samples/script-comment-only/output.json | 10 +- 5 files changed, 131 insertions(+), 87 deletions(-) create mode 100644 .changeset/fast-toes-act.md diff --git a/.changeset/fast-toes-act.md b/.changeset/fast-toes-act.md new file mode 100644 index 000000000000..121d5d4a7109 --- /dev/null +++ b/.changeset/fast-toes-act.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: properly assign trailing comments diff --git a/packages/svelte/src/compiler/phases/1-parse/acorn.js b/packages/svelte/src/compiler/phases/1-parse/acorn.js index 071e39421f2e..1a75f214a894 100644 --- a/packages/svelte/src/compiler/phases/1-parse/acorn.js +++ b/packages/svelte/src/compiler/phases/1-parse/acorn.js @@ -105,17 +105,22 @@ function get_comment_handlers(source) { if (comments[0]) { const parent = path.at(-1); + if (parent === undefined || node.end !== parent.end) { const slice = source.slice(node.end, comments[0].start); - if (/^[,) \t]*$/.test(slice)) { + if (node.end <= comments[0].start && /^[,) \t]*$/.test(slice)) { node.trailingComments = [/** @type {CommentWithLocation} */ (comments.shift())]; } } } } }); - if (comments.length > 0) { + + // Special case: Trailing comments after the root node (which can only happen for expression tags) get added + // regardless of line breaks between the root node and the comments. This ensures that we can later detect + // the end of the expression tag correctly. + if (comments.length > 0 && comments[0].start >= ast.end) { (ast.trailingComments ||= []).push(...comments.splice(0)); } } diff --git a/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte b/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte index 5300667984c3..33ce9d1942ed 100644 --- a/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte +++ b/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte @@ -5,7 +5,8 @@ /** a comment */ function asd() { - + foo; + /* comment belonging to noone */ } @@ -13,7 +14,8 @@ on:click={// comment () => { /* another comment */ - fn(); + fn(); // a trailing comment + /* comment belonging to noone */ }} > {/* leading block comment */ a} diff --git a/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json b/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json index 864306ca04ce..9fe306f1342e 100644 --- a/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json +++ b/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json @@ -1,39 +1,39 @@ { "html": { "type": "Fragment", - "start": 133, - "end": 341, + "start": 174, + "end": 439, "children": [ { "type": "Text", - "start": 131, - "end": 133, + "start": 172, + "end": 174, "raw": "\n\n", "data": "\n\n" }, { "type": "Element", - "start": 133, - "end": 251, + "start": 174, + "end": 349, "name": "button", "attributes": [ { - "start": 142, - "end": 207, + "start": 183, + "end": 305, "type": "EventHandler", "name": "click", "modifiers": [], "expression": { "type": "ArrowFunctionExpression", - "start": 164, - "end": 206, + "start": 205, + "end": 304, "loc": { "start": { - "line": 14, + "line": 15, "column": 1 }, "end": { - "line": 17, + "line": 19, "column": 2 } }, @@ -44,58 +44,58 @@ "params": [], "body": { "type": "BlockStatement", - "start": 170, - "end": 206, + "start": 211, + "end": 304, "loc": { "start": { - "line": 14, + "line": 15, "column": 7 }, "end": { - "line": 17, + "line": 19, "column": 2 } }, "body": [ { "type": "ExpressionStatement", - "start": 198, - "end": 203, + "start": 239, + "end": 244, "loc": { "start": { - "line": 16, + "line": 17, "column": 2 }, "end": { - "line": 16, + "line": 17, "column": 7 } }, "expression": { "type": "CallExpression", - "start": 198, - "end": 202, + "start": 239, + "end": 243, "loc": { "start": { - "line": 16, + "line": 17, "column": 2 }, "end": { - "line": 16, + "line": 17, "column": 6 } }, "callee": { "type": "Identifier", - "start": 198, - "end": 200, + "start": 239, + "end": 241, "loc": { "start": { - "line": 16, + "line": 17, "column": 2 }, "end": { - "line": 16, + "line": 17, "column": 4 } }, @@ -108,8 +108,16 @@ { "type": "Block", "value": " another comment ", - "start": 174, - "end": 195 + "start": 215, + "end": 236 + } + ], + "trailingComments": [ + { + "type": "Line", + "value": " a trailing comment", + "start": 245, + "end": 266 } ] } @@ -119,8 +127,8 @@ { "type": "Line", "value": " comment", - "start": 152, - "end": 162 + "start": 193, + "end": 203 } ] } @@ -129,26 +137,26 @@ "children": [ { "type": "Text", - "start": 209, - "end": 210, + "start": 307, + "end": 308, "raw": "\n", "data": "\n" }, { "type": "MustacheTag", - "start": 210, - "end": 241, + "start": 308, + "end": 339, "expression": { "type": "Identifier", - "start": 239, - "end": 240, + "start": 337, + "end": 338, "loc": { "start": { - "line": 19, + "line": 21, "column": 29 }, "end": { - "line": 19, + "line": 21, "column": 30 } }, @@ -157,16 +165,16 @@ { "type": "Block", "value": " leading block comment ", - "start": 211, - "end": 238 + "start": 309, + "end": 336 } ] } }, { "type": "Text", - "start": 241, - "end": 242, + "start": 339, + "end": 340, "raw": "\n", "data": "\n" } @@ -174,40 +182,40 @@ }, { "type": "Text", - "start": 251, - "end": 252, + "start": 349, + "end": 350, "raw": "\n", "data": "\n" }, { "type": "MustacheTag", - "start": 252, - "end": 341, + "start": 350, + "end": 439, "expression": { "type": "BinaryExpression", - "start": 279, - "end": 284, + "start": 377, + "end": 382, "loc": { "start": { - "line": 22, + "line": 24, "column": 1 }, "end": { - "line": 22, + "line": 24, "column": 6 } }, "left": { "type": "Identifier", - "start": 279, - "end": 280, + "start": 377, + "end": 378, "loc": { "start": { - "line": 22, + "line": 24, "column": 1 }, "end": { - "line": 22, + "line": 24, "column": 2 } }, @@ -216,15 +224,15 @@ "operator": "+", "right": { "type": "Identifier", - "start": 283, - "end": 284, + "start": 381, + "end": 382, "loc": { "start": { - "line": 22, + "line": 24, "column": 5 }, "end": { - "line": 22, + "line": 24, "column": 6 } }, @@ -234,22 +242,22 @@ { "type": "Line", "value": " leading line comment", - "start": 254, - "end": 277 + "start": 352, + "end": 375 } ], "trailingComments": [ { "type": "Line", "value": " trailing line comment", - "start": 285, - "end": 309 + "start": 383, + "end": 407 }, { "type": "Block", "value": " trailing block comment ", - "start": 311, - "end": 339 + "start": 409, + "end": 437 } ] } @@ -259,19 +267,19 @@ "instance": { "type": "Script", "start": 0, - "end": 131, + "end": 172, "context": "default", "content": { "type": "Program", "start": 8, - "end": 122, + "end": 163, "loc": { "start": { "line": 1, "column": 0 }, "end": { - "line": 10, + "line": 11, "column": 0 } }, @@ -427,14 +435,14 @@ { "type": "FunctionDeclaration", "start": 101, - "end": 121, + "end": 162, "loc": { "start": { "line": 7, "column": 1 }, "end": { - "line": 9, + "line": 10, "column": 2 } }, @@ -461,18 +469,50 @@ "body": { "type": "BlockStatement", "start": 116, - "end": 121, + "end": 162, "loc": { "start": { "line": 7, "column": 16 }, "end": { - "line": 9, + "line": 10, "column": 2 } }, - "body": [] + "body": [ + { + "type": "ExpressionStatement", + "start": 120, + "end": 124, + "loc": { + "start": { + "line": 8, + "column": 2 + }, + "end": { + "line": 8, + "column": 6 + } + }, + "expression": { + "type": "Identifier", + "start": 120, + "end": 123, + "loc": { + "start": { + "line": 8, + "column": 2 + }, + "end": { + "line": 8, + "column": 5 + } + }, + "name": "foo" + } + } + ] }, "leadingComments": [ { diff --git a/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json b/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json index b04a823e8db6..ed0c3fde2def 100644 --- a/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json +++ b/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json @@ -41,15 +41,7 @@ } }, "body": [], - "sourceType": "module", - "trailingComments": [ - { - "type": "Line", - "value": " TODO write some code", - "start": 10, - "end": 33 - } - ] + "sourceType": "module" } } } From b6ec803c3fed94a1d8747671e47c0f86830a642e Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 17 Jul 2024 11:37:20 +0200 Subject: [PATCH 2/3] support multiple trailing comments after last statement in a body --- .../src/compiler/phases/1-parse/acorn.js | 34 ++- .../samples/javascript-comments/input.svelte | 12 +- .../samples/javascript-comments/output.json | 237 ++++++++++++------ .../samples/script-comment-only/output.json | 10 +- 4 files changed, 206 insertions(+), 87 deletions(-) diff --git a/packages/svelte/src/compiler/phases/1-parse/acorn.js b/packages/svelte/src/compiler/phases/1-parse/acorn.js index 1a75f214a894..94072564465f 100644 --- a/packages/svelte/src/compiler/phases/1-parse/acorn.js +++ b/packages/svelte/src/compiler/phases/1-parse/acorn.js @@ -104,12 +104,33 @@ function get_comment_handlers(source) { next(); if (comments[0]) { - const parent = path.at(-1); + const parent = /** @type {any} */ (path.at(-1)); if (parent === undefined || node.end !== parent.end) { const slice = source.slice(node.end, comments[0].start); - - if (node.end <= comments[0].start && /^[,) \t]*$/.test(slice)) { + const is_last_in_body = + // BlockStatement and Program nodes have a body property + parent?.body?.length && + Array.isArray(parent?.body) && + parent.body.indexOf(node) === parent.body.length - 1; + + if (is_last_in_body) { + // Special case: There can be multiple trailing comments after the last node in a block, + // and they can be separated by newlines + let end = node.end; + + while (comments.length) { + const comment = comments[0]; + if (parent && comment.start > parent.end) break; + + const slice = source.slice(end, comment.start); + if (node.end === end ? !/^[,)\s]*$/.test(slice) : slice.trim() !== '') break; + + (node.trailingComments ||= []).push(comment); + comments.shift(); + end = comment.end; + } + } else if (node.end <= comments[0].start && /^[,) \t]*$/.test(slice)) { node.trailingComments = [/** @type {CommentWithLocation} */ (comments.shift())]; } } @@ -117,10 +138,9 @@ function get_comment_handlers(source) { } }); - // Special case: Trailing comments after the root node (which can only happen for expression tags) get added - // regardless of line breaks between the root node and the comments. This ensures that we can later detect - // the end of the expression tag correctly. - if (comments.length > 0 && comments[0].start >= ast.end) { + // Special case: Trailing comments after the root node (which can only happen for expression tags or for Program nodes). + // Adding them ensures that we can later detect the end of the expression tag correctly. + if (comments.length > 0 && (comments[0].start >= ast.end || ast.type === 'Program')) { (ast.trailingComments ||= []).push(...comments.splice(0)); } } diff --git a/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte b/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte index 33ce9d1942ed..9c0eced21916 100644 --- a/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte +++ b/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte @@ -5,8 +5,14 @@ /** a comment */ function asd() { - foo; - /* comment belonging to noone */ + foo; // trailing + /* leading comment 1 */ + /* leading comment 2 */ + /* leading comment 3 */ + bar; + /* trailing comment 1 */ + /* trailing comment 2 */ + /* trailing comment 3 */ } @@ -15,7 +21,7 @@ () => { /* another comment */ fn(); // a trailing comment - /* comment belonging to noone */ + /* trailing block comment */ }} > {/* leading block comment */ a} diff --git a/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json b/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json index 9fe306f1342e..45ec084f22d6 100644 --- a/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json +++ b/packages/svelte/tests/parser-legacy/samples/javascript-comments/output.json @@ -1,39 +1,39 @@ { "html": { "type": "Fragment", - "start": 174, - "end": 439, + "start": 317, + "end": 578, "children": [ { "type": "Text", - "start": 172, - "end": 174, + "start": 315, + "end": 317, "raw": "\n\n", "data": "\n\n" }, { "type": "Element", - "start": 174, - "end": 349, + "start": 317, + "end": 488, "name": "button", "attributes": [ { - "start": 183, - "end": 305, + "start": 326, + "end": 444, "type": "EventHandler", "name": "click", "modifiers": [], "expression": { "type": "ArrowFunctionExpression", - "start": 205, - "end": 304, + "start": 348, + "end": 443, "loc": { "start": { - "line": 15, + "line": 21, "column": 1 }, "end": { - "line": 19, + "line": 25, "column": 2 } }, @@ -44,58 +44,58 @@ "params": [], "body": { "type": "BlockStatement", - "start": 211, - "end": 304, + "start": 354, + "end": 443, "loc": { "start": { - "line": 15, + "line": 21, "column": 7 }, "end": { - "line": 19, + "line": 25, "column": 2 } }, "body": [ { "type": "ExpressionStatement", - "start": 239, - "end": 244, + "start": 382, + "end": 387, "loc": { "start": { - "line": 17, + "line": 23, "column": 2 }, "end": { - "line": 17, + "line": 23, "column": 7 } }, "expression": { "type": "CallExpression", - "start": 239, - "end": 243, + "start": 382, + "end": 386, "loc": { "start": { - "line": 17, + "line": 23, "column": 2 }, "end": { - "line": 17, + "line": 23, "column": 6 } }, "callee": { "type": "Identifier", - "start": 239, - "end": 241, + "start": 382, + "end": 384, "loc": { "start": { - "line": 17, + "line": 23, "column": 2 }, "end": { - "line": 17, + "line": 23, "column": 4 } }, @@ -108,16 +108,22 @@ { "type": "Block", "value": " another comment ", - "start": 215, - "end": 236 + "start": 358, + "end": 379 } ], "trailingComments": [ { "type": "Line", "value": " a trailing comment", - "start": 245, - "end": 266 + "start": 388, + "end": 409 + }, + { + "type": "Block", + "value": " trailing block comment ", + "start": 412, + "end": 440 } ] } @@ -127,8 +133,8 @@ { "type": "Line", "value": " comment", - "start": 193, - "end": 203 + "start": 336, + "end": 346 } ] } @@ -137,26 +143,26 @@ "children": [ { "type": "Text", - "start": 307, - "end": 308, + "start": 446, + "end": 447, "raw": "\n", "data": "\n" }, { "type": "MustacheTag", - "start": 308, - "end": 339, + "start": 447, + "end": 478, "expression": { "type": "Identifier", - "start": 337, - "end": 338, + "start": 476, + "end": 477, "loc": { "start": { - "line": 21, + "line": 27, "column": 29 }, "end": { - "line": 21, + "line": 27, "column": 30 } }, @@ -165,16 +171,16 @@ { "type": "Block", "value": " leading block comment ", - "start": 309, - "end": 336 + "start": 448, + "end": 475 } ] } }, { "type": "Text", - "start": 339, - "end": 340, + "start": 478, + "end": 479, "raw": "\n", "data": "\n" } @@ -182,40 +188,40 @@ }, { "type": "Text", - "start": 349, - "end": 350, + "start": 488, + "end": 489, "raw": "\n", "data": "\n" }, { "type": "MustacheTag", - "start": 350, - "end": 439, + "start": 489, + "end": 578, "expression": { "type": "BinaryExpression", - "start": 377, - "end": 382, + "start": 516, + "end": 521, "loc": { "start": { - "line": 24, + "line": 30, "column": 1 }, "end": { - "line": 24, + "line": 30, "column": 6 } }, "left": { "type": "Identifier", - "start": 377, - "end": 378, + "start": 516, + "end": 517, "loc": { "start": { - "line": 24, + "line": 30, "column": 1 }, "end": { - "line": 24, + "line": 30, "column": 2 } }, @@ -224,15 +230,15 @@ "operator": "+", "right": { "type": "Identifier", - "start": 381, - "end": 382, + "start": 520, + "end": 521, "loc": { "start": { - "line": 24, + "line": 30, "column": 5 }, "end": { - "line": 24, + "line": 30, "column": 6 } }, @@ -242,22 +248,22 @@ { "type": "Line", "value": " leading line comment", - "start": 352, - "end": 375 + "start": 491, + "end": 514 } ], "trailingComments": [ { "type": "Line", "value": " trailing line comment", - "start": 383, - "end": 407 + "start": 522, + "end": 546 }, { "type": "Block", "value": " trailing block comment ", - "start": 409, - "end": 437 + "start": 548, + "end": 576 } ] } @@ -267,19 +273,19 @@ "instance": { "type": "Script", "start": 0, - "end": 172, + "end": 315, "context": "default", "content": { "type": "Program", "start": 8, - "end": 163, + "end": 306, "loc": { "start": { "line": 1, "column": 0 }, "end": { - "line": 11, + "line": 17, "column": 0 } }, @@ -435,14 +441,14 @@ { "type": "FunctionDeclaration", "start": 101, - "end": 162, + "end": 305, "loc": { "start": { "line": 7, "column": 1 }, "end": { - "line": 10, + "line": 16, "column": 2 } }, @@ -469,14 +475,14 @@ "body": { "type": "BlockStatement", "start": 116, - "end": 162, + "end": 305, "loc": { "start": { "line": 7, "column": 16 }, "end": { - "line": 10, + "line": 16, "column": 2 } }, @@ -510,7 +516,86 @@ } }, "name": "foo" - } + }, + "trailingComments": [ + { + "type": "Line", + "value": " trailing", + "start": 125, + "end": 136 + } + ] + }, + { + "type": "ExpressionStatement", + "start": 217, + "end": 221, + "loc": { + "start": { + "line": 12, + "column": 2 + }, + "end": { + "line": 12, + "column": 6 + } + }, + "expression": { + "type": "Identifier", + "start": 217, + "end": 220, + "loc": { + "start": { + "line": 12, + "column": 2 + }, + "end": { + "line": 12, + "column": 5 + } + }, + "name": "bar" + }, + "leadingComments": [ + { + "type": "Block", + "value": " leading comment 1 ", + "start": 139, + "end": 162 + }, + { + "type": "Block", + "value": " leading comment 2 ", + "start": 165, + "end": 188 + }, + { + "type": "Block", + "value": " leading comment 3 ", + "start": 191, + "end": 214 + } + ], + "trailingComments": [ + { + "type": "Block", + "value": " trailing comment 1 ", + "start": 224, + "end": 248 + }, + { + "type": "Block", + "value": " trailing comment 2 ", + "start": 251, + "end": 275 + }, + { + "type": "Block", + "value": " trailing comment 3 ", + "start": 278, + "end": 302 + } + ] } ] }, diff --git a/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json b/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json index ed0c3fde2def..b04a823e8db6 100644 --- a/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json +++ b/packages/svelte/tests/parser-legacy/samples/script-comment-only/output.json @@ -41,7 +41,15 @@ } }, "body": [], - "sourceType": "module" + "sourceType": "module", + "trailingComments": [ + { + "type": "Line", + "value": " TODO write some code", + "start": 10, + "end": 33 + } + ] } } } From 1280a603f54f46069832280f2447f7d716e3659f Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 17 Jul 2024 12:36:16 +0200 Subject: [PATCH 3/3] simplify, handle object and array expressions --- .../src/compiler/phases/1-parse/acorn.js | 15 +- .../samples/javascript-comments/input.svelte | 14 + .../samples/javascript-comments/output.json | 403 ++++++++++++++---- 3 files changed, 351 insertions(+), 81 deletions(-) diff --git a/packages/svelte/src/compiler/phases/1-parse/acorn.js b/packages/svelte/src/compiler/phases/1-parse/acorn.js index 94072564465f..32be46401358 100644 --- a/packages/svelte/src/compiler/phases/1-parse/acorn.js +++ b/packages/svelte/src/compiler/phases/1-parse/acorn.js @@ -109,10 +109,12 @@ function get_comment_handlers(source) { if (parent === undefined || node.end !== parent.end) { const slice = source.slice(node.end, comments[0].start); const is_last_in_body = - // BlockStatement and Program nodes have a body property - parent?.body?.length && - Array.isArray(parent?.body) && - parent.body.indexOf(node) === parent.body.length - 1; + ((parent?.type === 'BlockStatement' || parent?.type === 'Program') && + parent.body.indexOf(node) === parent.body.length - 1) || + (parent?.type === 'ArrayExpression' && + parent.elements.indexOf(node) === parent.elements.length - 1) || + (parent?.type === 'ObjectExpression' && + parent.properties.indexOf(node) === parent.properties.length - 1); if (is_last_in_body) { // Special case: There can be multiple trailing comments after the last node in a block, @@ -121,10 +123,7 @@ function get_comment_handlers(source) { while (comments.length) { const comment = comments[0]; - if (parent && comment.start > parent.end) break; - - const slice = source.slice(end, comment.start); - if (node.end === end ? !/^[,)\s]*$/.test(slice) : slice.trim() !== '') break; + if (parent && comment.start >= parent.end) break; (node.trailingComments ||= []).push(comment); comments.shift(); diff --git a/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte b/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte index 9c0eced21916..35a9125cb987 100644 --- a/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte +++ b/packages/svelte/tests/parser-legacy/samples/javascript-comments/input.svelte @@ -14,6 +14,20 @@ /* trailing comment 2 */ /* trailing comment 3 */ } + + const array = [ + // leading comment 1 + // leading comment 2 + 1, // trailing comment 1 + /* trailing comment 2 */ + ]; + + const object = { + // leading comment 1 + // leading comment 2 + a: 1, // trailing comment 1 + /* trailing comment 2 */ + };