Fix node.getStart() for nodes spanning multiline JSDoc comments
#43854
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
|
|
||
| case CharacterCodes.asterisk: | ||
| if (canConsumeStar) { | ||
| pos++; |
There was a problem hiding this comment.
Dont you need to set canConsumeStar to false in the default case as well?
There was a problem hiding this comment.
I don’t think so—the default case is either whitespace (which doesn’t change whether we can consume a leading *) or it breaks and returns.
| @@ -478,7 +478,12 @@ namespace ts { | |||
| return getTokenPosOfNode((<SyntaxList>node)._children[0], sourceFile, includeJsDoc); | |||
| } | |||
|
|
|||
| return skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.pos); | |||
There was a problem hiding this comment.
this looks like an OK fix, but I don't think skipTrivia is called on the inside of jsdoc anywhere else. Maybe there should be a completely separate code path?
I haven't thought about it deeply, so it's likely that skipTrivia is 98% right and just needed this tweak to work.
There was a problem hiding this comment.
Well, it’s definitely true that I didn’t pass the parameter anywhere else. It occurred to me that I could accomplish the same thing by wrapping skipTrivia and calling it in a loop with stopAfterLineBreak, but I felt like it was a little less clear what was happening. But I can go back to that—I’m pretty ambivalent about it.
There was a problem hiding this comment.
Ummm, me too. Neither one of these functions are specific to jsdoc, so jsdoc-specific code is surprise in both.
Probably means that this is good enough.
Fixes #43442
skipTrivianeeds to skip a leading*inside JSDoc.