Add jsdoc member names: Class#method #44150
Conversation
A couple of mixed, nested references don't work yet. The scanner+parser interaction is wrong, the parser consumes one too many spaces, and the checker+services code needs a little cleanup.
1. I decided that correctly parsing a#b.c, an entity name containing an instance reference, is not worth the work. 2. I made the scanner, at least the jsdoc part, emit a # token, and provided a reScanPrivateIdentifier in order to convert #a to # a. 3. I cleaned up the code in the checker. 2. Unrelated: I added a missing space in linkPart display.
typescript-bot
added
Author: Team
For Milestone Bug
sandersn
requested review from
andrewbranch,
elibarzilay,
amcasey and
DanielRosenwasser
|
@DanielRosenwasser @amcasey I added you because you've had good opinions about usability in the past. Feel free to skip the implementation. @andrewbranch @elibarzilay I heard you like parsing.... |
| const entityName = parseEntityName(/* allowReservedWords*/ false); | ||
| const p2 = getNodePos(); | ||
| let entityName: EntityName | JSDocInstanceReference = parseEntityName(/* allowReservedWords*/ false); | ||
| while (token() === SyntaxKind.PrivateIdentifier) { |
There was a problem hiding this comment.
while?
What does Foo#Bar#Baz parse as?
There was a problem hiding this comment.
A left-deep JSDocInstanceReference tree: ((Foo#Bar)#Baz)
src/compiler/parser.ts
Outdated
| ? parseEntityName(/*allowReservedWords*/ true) | ||
| : undefined; | ||
| if (name) { | ||
| while(token() === SyntaxKind.PrivateIdentifier) { |
There was a problem hiding this comment.
Huh, the linter really doesn’t make you do this?
| while(token() === SyntaxKind.PrivateIdentifier) { | |
| while (token() === SyntaxKind.PrivateIdentifier) { |
There was a problem hiding this comment.
Well, it used to. I'm not sure why it didn't complain this time.
src/compiler/scanner.ts
Outdated
| @@ -2251,6 +2254,14 @@ namespace ts { | |||
| return token; | |||
| } | |||
|
|
|||
| function reScanPrivateIdentifier(): SyntaxKind { | |||
There was a problem hiding this comment.
I know the various reScan functions don’t all have perfectly comparable behavior, but it seems like the general naming convention is naming it for what you want, not what you currently have: “I currently have token X, but have reason to believe that could be interpreted as token Y, so to re-scan X as Y, I call reScanY.”
The naming convention is clearly bad when read like that, but diverging from it is probably worse. I would either conform, naming this reScanHashToken, or be totally explicit, naming it reScanPrivateIdentifierAsHashToken.
There was a problem hiding this comment.
Nope, I just confused myself. I will reverse it.
sandersn
changed the title
This PR adds support for jsdoc instance references of the form
Class#method. Right now, these are parsed only inside of@seeand@linktags, but they could probably be allowed other places fairly simply.Additional fixes
@see {A.B.c}, onlycworked. Now all 3 do.@linkquick info for links with unresolved entities. I think I was going to follow up with a general fix that proved too ambitious and forgot the ad-hoc fix until now.Implementation notes
rescanPrivateIdentifier, which breaks up#idinto two tokens,# id. This is the main way to get the newHashTokenout of the scanner, butscanJsDocTokennow also returnsHashTokeninstead ofUnknown. My first attempt just reused PrivateIdentifier, but this gave bad spans for find-all-refs. I also think that always scanning#as part of a private identifier ties that token too tightly to private identifiers, so when#gets used for other things, it'll probably make sense to invert the scanner's behaviour and provide arescanHashfunction when private identifiers are desired.resolveJSDocInstanceReferenceis recursive, then delegates toresolveEntityNamewhenever it encounters a normal EntityName likeK.xin, for example,K.x#m. However, whenK.xdoesn't resolve, it also triesK.prototype.x. That's because bothK#xandK.xare supported inside@see/@linkas synonyms ofK.prototype.x. This fallback is inefficient, but only happens in services when find-all-refs is requested for@see/@link, and only performs redundant work for chained entity names likeA.B.cwhich should actually beA#B#c.Fixes #43594