[AST] Incorrect parsing of comments inside the callback scope

[xeho91]

Describe the bug

Sorry in advance, but I might be bringing a PITA with comments, again.

I noticed while working on updating svelte-ast-print, I've had inline snapshots mismatch (thanks Vitest!). There was a bug which I've overlooked before and noticed that something was off. I was able to narrow that there's a parsing difference between version svelte@5.0.0-next.190 and svelte@5.0.0-next.191. Regardless, both versions parse it wrongly, including the latest one.

Possibly related PR: https://github.com/sveltejs/svelte/pull/12471

Piece of input inside the <script> tag that gets parsed wrong:

beforeUpdate(() => {
  // determine whether we should auto-scroll
  // once the DOM is updated...
});

afterUpdate(() => {
  // ...the DOM is now in sync with the data
});

`svelte@5.0.0-next.190`	`svelte@5.0.0-next.191`
```diff - beforeUpdate(() => { - // determine whether we should auto-scroll + beforeUpdate(() => {}); // determine whether we should auto-scroll - // once the DOM is updated... + // once the DOM is updated... - }); - - afterUpdate(() => { - // ...the DOM is now in sync with the data - }); + afterUpdate(() => {}); // ...the DOM is now in sync with the data ```	```diff - beforeUpdate(() => { + beforeUpdate(() => {}); - // determine whether we should auto-scroll + // determine whether we should auto-scroll - // once the DOM is updated... + // once the DOM is updated... - }); - - afterUpdate(() => { + afterUpdate(() => {}); + - // ...the DOM is now in sync with the data + // ...the DOM is now in sync with the data - }); - ```

Below I will provide a simple reproduction where you can see that the comment inside the callback scope of `beforeUpdate()` gets wrongly parsed as `trailingComments`. ### Reproduction https://svelte-5-preview.vercel.app/#H4sIAAAAAAAAE02OywrCMBBFf2WYTS0Uum-14D_oyriIyZQG8yIzFaT03yUI4vIezoG74ew8MQ63DaMOhAOec8YO5Z3r4Bd5IeyQ01pMJUc2xWWZVFTiQk5FYIMHzanQNVstBDvMJQVovm0zqljdf-VwaOE0wVa5kr6Hy-IYTAqBogAvafUWWPQbXGRnCYz2_qHNE9ikTDXb21HFY_87gx2GZN3syOIgZaX9vn8AcUwca90AAAA= ### Logs _No response_ ### System Info ```shell next ``` ### Severity annoyance

dummdidumm commented 3 months ago

Ah yeah, comments ... sigh. The referenced PR improved the situations, but as can be seen here it still has problems with comments inside empty blocks. The problem is that - to my knowledge - there's no real standard on where to put comments on the AST, and for cases like this the semi-standard of leading/trailingComments doesn't work, because this is neither. Eslint uses innerComments for this. To fix this I think we need to decide on a structure we want to go with and then implement that in esrap accordingly.

xeho91 commented 3 months ago

Aye, so this is very edge case 😅 (I feel sorry for bringing this up).

It can affect 'codemod' ecosystem scope. So, in case someone else is playing around with Svelte AST and will notice this issue... the possible and temporary workaround is very trivial. Don't write lone comments. The ones which cannot be assigned as leadingComment or trailingComment.

Example:

beforeUpdate(() => {
    // This comment should stay inside callback scope 
        console.log(); // 👈 Add any statement, so 👆 comment becomes a `trailingComment`
});

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[dummdidumm]

Ah yeah, comments ... sigh. The referenced PR improved the situations, but as can be seen here it still has problems with comments inside empty blocks. The problem is that - to my knowledge - there's no real standard on where to put comments on the AST, and for cases like this the semi-standard of leading/trailingComments doesn't work, because this is neither. Eslint uses innerComments for this. To fix this I think we need to decide on a structure we want to go with and then implement that in esrap accordingly.

[xeho91]

Aye, so this is very edge case 😅 (I feel sorry for bringing this up).

It can affect 'codemod' ecosystem scope. So, in case someone else is playing around with Svelte AST and will notice this issue... the possible and temporary workaround is very trivial. Don't write lone comments. The ones which cannot be assigned as leadingComment or trailingComment.

Example:

beforeUpdate(() => {
    // This comment should stay inside callback scope 
        console.log(); // 👈 Add any statement, so 👆 comment becomes a `trailingComment`
});