Fixed a class member completion crash after JSDoc comment with degenerate JSDoc tag in it#2888
Conversation
…rate JSDoc tag in it
| result := find(rightmostValidNode, endPos) | ||
| if result == nil && n != containingNode { | ||
| return n | ||
| } |
There was a problem hiding this comment.
In this case, the JSDoc comment contains a degenerate JSDocTag with 0-width (representing "@-rule") - given @tag requires an identifier and - isn't a valid identifier character.
This matches how Strada parses the same case. But it creates an issue because the compiler fails to find contextToken. A nil from here just propagates up and later on it crashes in the completion code.
There was a problem hiding this comment.
This looks ok if we don't want to change the parsing, but I do wonder if maybe that's what we should do (even though we had this problem in Strada, too).
There was a problem hiding this comment.
It crossed my mind to attempt a change there but I decided to err on the side of Strada compatibility for now. If you think the parsing should be changed instead, then I could start exploring that option.
There was a problem hiding this comment.
I think it's worth trying to fix it during parsing, yes. Given this would be a change in JSDoc invalid tag parsing, I don't think it's likely it would break people.
| Items: &fourslash.CompletionsExpectedItems{ | ||
| Includes: []fourslash.CompletionsExpectedItem{}, // no crash check | ||
| }, | ||
| Items: &fourslash.CompletionsExpectedItems{}, |
There was a problem hiding this comment.
this is a driveby change, I just corrected here one of the recent tests I added to use the more common codebase pattern
gabritto
left a comment
gabritto
left a comment
There was a problem hiding this comment.
(see comment on fixing the tag parsing)
2 participants
fixes the crash reported here: #2871 (comment)
it also crashes in Strada