Jsdoc property description - #50269
Conversation
…99/TypeScript into jsdocPropertyDescription
TypeScript Bot (typescript-bot)
added
the
For Milestone Bug
| const expressionType = checkExpressionCached(name.expression); | ||
| const infos = getApplicableIndexInfos(expressionType, getLiteralTypeFromPropertyName(name.name)); | ||
| if (length(infos) && infos[0].declaration && infos[0].declaration?.symbol.flags & SymbolFlags.Signature && infos[0].declaration?.jsDoc) { | ||
| const copy = createSymbol(SymbolFlags.Signature, InternalSymbolName.Index); |
There was a problem hiding this comment.
Why are we creating a new symbol here? Given
interface Foo { [key: string]: any }
declare let foo: Foo;
foo.bar;
foo.bar;
foo.bar;
foo.bar;every mention of foo.bar would have its own copy of the index symbol, which seems wrong.
There was a problem hiding this comment.
I suggested it to handle filtering the symbol's declarations to only those matching the use site in order to handle when multiple index signatures exist (since all index signatures usually get tossed into the same __index symbol, rather than each having their own). Certainly, if the filtered list of applicable index signature declarations is identical to the full list, reusing the original symbol should be OK, and we could probably cache and reuse symbols when they're the same declaration subset across usages.
There was a problem hiding this comment.
New approach, as explained by Wesley Wigham (@weswigham)
- If infos is the same as all the infos available in the type, just return the InternalSymbolName.Index member of the type directly.
- If you do have to make a filtered one - cache it for each set of declarations you use. Add a member to SymbolLinks and store the cache on the original Index symbol, keyed by the list of (ids of) declarations in the filtered symbol.
| if (flags & SymbolFlags.SetAccessor) return ScriptElementKind.memberSetAccessorElement; | ||
| if (flags & SymbolFlags.Method) return ScriptElementKind.memberFunctionElement; | ||
| if (flags & SymbolFlags.Constructor) return ScriptElementKind.constructorImplementationElement; | ||
| if (flags & SymbolFlags.Signature) return ScriptElementKind.indexSignatureElement; |
There was a problem hiding this comment.
SymbolFlags.Signature is also used for call and construct signatures. If tests are passing this might be fine, but I would want to understand why you don’t have to worry about call and construct signatures here.
There was a problem hiding this comment.
I am actually not sure about this. The function returns ScriptElementKind.memberVariableElement ('property') for SymbolFlags.Property which is the other example I was looking at, I want to return ScriptElementKind.indexSignatureElement ('index') for SymbolFlags.Signature which is why I added this here, but I wasn't aware of SymbolFlags.Signature also being used for call and construct signatures.
Co-authored-by: Andrew Branch <andrewbranch@users.noreply.github.com>
…99/TypeScript into jsdocPropertyDescription
Andrew Branch (andrewbranch)
left a comment
Andrew Branch (andrewbranch)
left a comment
There was a problem hiding this comment.
Looking much better, almost there 👍
5 participants
Fixes #47933