Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure that jsdoc is always in range with parameter declaration #58538

Merged
merged 1 commit into from
May 15, 2024

Conversation

sheetalkamat
Copy link
Member

Fixes #56677

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels May 14, 2024
@sheetalkamat sheetalkamat marked this pull request as ready for review May 14, 2024 21:56
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funky. I'm not sure why we even parse a comment range on the incremental reparse in this case, since we deleted the leading /. The comment range scanner function pretty clearly only matches comments on seeing a // or /* sequence. The single / left after the edit should be interpreted as a division operator or opening to a regex literal, I'd think. I get that the jsdoc nodes we make from these spans, and adjusting them in the incremental parser, is ultimately what triggers the assertion (which is why excluding them fixes it), just feels weird that we have a jsdoc node at all after the edit - I'd assume we'd have rescanned from in front of the jsdoc host node and thrown it out.

@jakebailey
Copy link
Member

jakebailey commented May 14, 2024

I guess I would have thought this were a bug somewhere in updateTokenPositionsAndMarkElements in the incremental parser, since we should be marking nodes as need to be fully reparsed...

@sheetalkamat
Copy link
Member Author

The issue was that when first source File was created with function bug(\r\n test /** */ true = test test 123\r\n) {} it created a parameter declaration of 0 width just before the jsdoc and jsdoc was attached to that parameter and hence the assert in incremental parsing was failing where it adjusts the spans for nodes in the change range and keeps it around so if parser can reuse node, it will. ( in this case it obviously doesnt as removing that leading / makes it expression)

@sheetalkamat sheetalkamat merged commit 84ed6a3 into main May 15, 2024
28 checks passed
@sheetalkamat sheetalkamat deleted the incrementalParsing branch May 15, 2024 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Diagnostics and semantic tokens get incorrectly highlighted after doing some specific actions in a line
4 participants