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.

Sign up for GitHub

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

Jump to bottom

Jsdoc property description #50269

Merged
merged 16 commits into from Aug 25, 2022
Merged

Jsdoc property description #50269

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
0c51f47
jsdocPropertyDescription
Jun 9, 2022
3add1f9
jsdocPropertyDescription
Jun 10, 2022
c74367d
jsdocPropertyDescription
Jun 14, 2022
84e4611
Fixes #47933
danay1999 Jul 6, 2022
8ba9d2c
added additional test
danay1999 Jul 7, 2022
6af103b
added additional example
danay1999 Jul 8, 2022
f404c80
fixed bug
danay1999 Jul 9, 2022
50372d9
changed function to only grab the literal type
danay1999 Jul 11, 2022
868ef27
added additional condition for literals and symbols
danay1999 Jul 12, 2022
ecfbf8b
added additional test cases
danay1999 Aug 11, 2022
cca8b27
Merge branch 'jsdocPropertyDescription' of https://github.com/danay19…
danay1999 Aug 11, 2022
b7011f9
Merge branch 'main' into jsdocPropertyDescription
danay1999 Aug 18, 2022
bef80c2
Update src/services/symbolDisplay.ts
danay1999 Aug 19, 2022
38e27cc
addressed PR review
danay1999 Aug 23, 2022
c42189b
Merge branch 'jsdocPropertyDescription' of https://github.com/danay19…
danay1999 Aug 23, 2022
6440898
addressed new PR review
danay1999 Aug 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/compiler/checker.ts
View file
Open in desktop
Expand Up @@ -439,6 +439,7 @@ namespace ts {
getTypeOfPropertyOfType: (type, name) => getTypeOfPropertyOfType(type, escapeLeadingUnderscores(name)),
getIndexInfoOfType: (type, kind) => getIndexInfoOfType(type, kind === IndexKind.String ? stringType : numberType),
getIndexInfosOfType,
getIndexInfosOfIndexSymbol,
getSignaturesOfType,
getIndexTypeOfType: (type, kind) => getIndexTypeOfType(type, kind === IndexKind.String ? stringType : numberType),
getIndexType: type => getIndexType(type),
Expand Down Expand Up @@ -42580,6 +42581,17 @@ namespace ts {

if (name.kind === SyntaxKind.PropertyAccessExpression) {
checkPropertyAccessExpression(name, CheckMode.Normal);
if (!links.resolvedSymbol) {
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) {
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
const copy = createSymbol(SymbolFlags.Signature, InternalSymbolName.Index);
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

@weswigham weswigham Aug 18, 2022 •
edited

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New approach, as explained by @weswigham

  1. If infos is the same as all the infos available in the type, just return the InternalSymbolName.Index member of the type directly.
  2. 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.

copy.declarations = mapDefined(infos, i => i.declaration);
copy.parent = expressionType.aliasSymbol ? expressionType.aliasSymbol : expressionType.symbol ? expressionType.symbol : getSymbolAtLocation(copy.declarations[0].parent);
links.resolvedSymbol = copy;
return copy;
}
}
}
else {
checkQualifiedName(name, CheckMode.Normal);
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
View file
Open in desktop
Expand Up @@ -4369,6 +4369,7 @@ namespace ts {
/* @internal */ getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined;
getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined;
getIndexInfosOfType(type: Type): readonly IndexInfo[];
getIndexInfosOfIndexSymbol: (indexSymbol: Symbol) => IndexInfo[];
getSignaturesOfType(type: Type, kind: SignatureKind): readonly Signature[];
getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined;
/* @internal */ getIndexType(type: Type): Type;
Expand Down
131 changes: 82 additions & 49 deletions src/services/symbolDisplay.ts
View file
Open in desktop
Expand Up @@ -64,6 +64,7 @@ namespace ts.SymbolDisplay {
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;
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


if (flags & SymbolFlags.Property) {
if (flags & SymbolFlags.Transient && (symbol as TransientSymbol).checkFlags & CheckFlags.Synthetic) {
Expand Down Expand Up @@ -498,58 +499,67 @@ namespace ts.SymbolDisplay {
}
if (!hasAddedSymbolInfo) {
if (symbolKind !== ScriptElementKind.unknown) {
if (type) {
if (isThisExpression) {
prefixNextMeaning();
displayParts.push(keywordPart(SyntaxKind.ThisKeyword));
}
else {
addPrefixForAnyFunctionOrVar(symbol, symbolKind);
}

// For properties, variables and local vars: show the type
if (symbolKind === ScriptElementKind.memberVariableElement ||
symbolKind === ScriptElementKind.memberGetAccessorElement ||
symbolKind === ScriptElementKind.memberSetAccessorElement ||
symbolKind === ScriptElementKind.jsxAttribute ||
symbolFlags & SymbolFlags.Variable ||
symbolKind === ScriptElementKind.localVariableElement ||
isThisExpression) {
displayParts.push(punctuationPart(SyntaxKind.ColonToken));
displayParts.push(spacePart());
// If the type is type parameter, format it specially
if (type.symbol && type.symbol.flags & SymbolFlags.TypeParameter) {
const typeParameterParts = mapToDisplayParts(writer => {
const param = typeChecker.typeParameterToDeclaration(type as TypeParameter, enclosingDeclaration, symbolDisplayNodeBuilderFlags)!;
getPrinter().writeNode(EmitHint.Unspecified, param, getSourceFileOfNode(getParseTreeNode(enclosingDeclaration)), writer);
});
addRange(displayParts, typeParameterParts);
let indexInfos;
if(symbolKind === ScriptElementKind.indexSignatureElement) indexInfos = typeChecker.getIndexInfosOfIndexSymbol(symbol);
if (type) {
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
if (isThisExpression) {
prefixNextMeaning();
displayParts.push(keywordPart(SyntaxKind.ThisKeyword));
}
else {
addRange(displayParts, typeToDisplayParts(typeChecker, type, enclosingDeclaration));
addPrefixForAnyFunctionOrVar(symbol, symbolKind, indexInfos);
}
if ((symbol as TransientSymbol).target && ((symbol as TransientSymbol).target as TransientSymbol).tupleLabelDeclaration) {
const labelDecl = ((symbol as TransientSymbol).target as TransientSymbol).tupleLabelDeclaration!;
Debug.assertNode(labelDecl.name, isIdentifier);
// For properties, variables and local vars: show the type
if (symbolKind === ScriptElementKind.memberVariableElement ||
symbolKind === ScriptElementKind.memberGetAccessorElement ||
symbolKind === ScriptElementKind.memberSetAccessorElement ||
symbolKind === ScriptElementKind.jsxAttribute ||
symbolFlags & SymbolFlags.Variable ||
symbolKind === ScriptElementKind.localVariableElement ||
symbolKind === ScriptElementKind.indexSignatureElement ||
isThisExpression) {
displayParts.push(punctuationPart(SyntaxKind.ColonToken));
displayParts.push(spacePart());
displayParts.push(punctuationPart(SyntaxKind.OpenParenToken));
displayParts.push(textPart(idText(labelDecl.name)));
displayParts.push(punctuationPart(SyntaxKind.CloseParenToken));
// If the type is type parameter, format it specially
if (type.symbol && type.symbol.flags & SymbolFlags.TypeParameter && symbolKind !== ScriptElementKind.indexSignatureElement) {
const typeParameterParts = mapToDisplayParts(writer => {
const param = typeChecker.typeParameterToDeclaration(type as TypeParameter, enclosingDeclaration, symbolDisplayNodeBuilderFlags)!;
getPrinter().writeNode(EmitHint.Unspecified, param, getSourceFileOfNode(getParseTreeNode(enclosingDeclaration)), writer);
});
addRange(displayParts, typeParameterParts);
}
//If the type is index signature, format it specially as well
else if(symbolKind === ScriptElementKind.indexSignatureElement){
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
if(indexInfos){
const indexTypeParts = typeToDisplayParts(typeChecker, indexInfos[0].type);
addRange(displayParts, indexTypeParts);
}
}
else {
addRange(displayParts, typeToDisplayParts(typeChecker, type, enclosingDeclaration));
}
if ((symbol as TransientSymbol).target && ((symbol as TransientSymbol).target as TransientSymbol).tupleLabelDeclaration) {
const labelDecl = ((symbol as TransientSymbol).target as TransientSymbol).tupleLabelDeclaration!;
Debug.assertNode(labelDecl.name, isIdentifier);
displayParts.push(spacePart());
displayParts.push(punctuationPart(SyntaxKind.OpenParenToken));
displayParts.push(textPart(idText(labelDecl.name)));
displayParts.push(punctuationPart(SyntaxKind.CloseParenToken));
}
}
}
else if (symbolFlags & SymbolFlags.Function ||
symbolFlags & SymbolFlags.Method ||
symbolFlags & SymbolFlags.Constructor ||
symbolFlags & SymbolFlags.Signature ||
symbolFlags & SymbolFlags.Accessor ||
symbolKind === ScriptElementKind.memberFunctionElement) {
const allSignatures = type.getNonNullableType().getCallSignatures();
if (allSignatures.length) {
addSignatureDisplayParts(allSignatures[0], allSignatures);
hasMultipleSignatures = allSignatures.length > 1;
else if (symbolFlags & SymbolFlags.Function ||
symbolFlags & SymbolFlags.Method ||
symbolFlags & SymbolFlags.Constructor ||
symbolFlags & SymbolFlags.Signature ||
symbolFlags & SymbolFlags.Accessor ||
symbolKind === ScriptElementKind.memberFunctionElement) {
const allSignatures = type.getNonNullableType().getCallSignatures();
if (allSignatures.length) {
addSignatureDisplayParts(allSignatures[0], allSignatures);
hasMultipleSignatures = allSignatures.length > 1;
}
}
}
}
}
else {
symbolKind = getSymbolKind(typeChecker, symbol, location);
Expand Down Expand Up @@ -638,26 +648,49 @@ namespace ts.SymbolDisplay {
displayParts.push(spacePart());
}

function addFullSymbolName(symbolToDisplay: Symbol, enclosingDeclaration?: Node) {
function addFullSymbolName(symbolToDisplay: Symbol, enclosingDeclaration?: Node, indexInfos?: IndexInfo[]) {
if (alias && symbolToDisplay === symbol) {
symbolToDisplay = alias;
}
const fullSymbolDisplayParts = symbolToDisplayParts(typeChecker, symbolToDisplay, enclosingDeclaration || sourceFile, /*meaning*/ undefined,
SymbolFormatFlags.WriteTypeParametersOrArguments | SymbolFormatFlags.UseOnlyExternalAliasing | SymbolFormatFlags.AllowAnyNodeKind);
if (symbolToDisplay.flags & SymbolFlags.Signature) {
if (indexInfos) {
let index = 1;
fullSymbolDisplayParts[index++] = punctuationPart(SyntaxKind.OpenBracketToken);
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
if (length(indexInfos)) {
//Needed to handle more than one type of index
for (let info = 0; info < indexInfos.length; info++) {
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
const indexTypeDisplayParts = typeToDisplayParts(typeChecker, indexInfos[info].keyType);
//Needed to handle template literals
for (const part of indexTypeDisplayParts) fullSymbolDisplayParts[index++] = part;
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
if (info !== indexInfos.length - 1) {
fullSymbolDisplayParts[index++] = spacePart();
fullSymbolDisplayParts[index++] = punctuationPart(SyntaxKind.BarToken);
fullSymbolDisplayParts[index++] = spacePart();
}
}
fullSymbolDisplayParts[index] = punctuationPart(SyntaxKind.CloseBracketToken);
}
}
else {
fullSymbolDisplayParts[2] = textOrKeywordPart("Any"); //This is the fallback in case else fails
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
}
}
addRange(displayParts, fullSymbolDisplayParts);

if (symbol.flags & SymbolFlags.Optional) {
displayParts.push(punctuationPart(SyntaxKind.QuestionToken));
}
}

function addPrefixForAnyFunctionOrVar(symbol: Symbol, symbolKind: string) {
function addPrefixForAnyFunctionOrVar(symbol: Symbol, symbolKind: string, indexInfos?: IndexInfo[]) {
prefixNextMeaning();
if (symbolKind) {
pushSymbolKind(symbolKind);
if (symbol && !some(symbol.declarations, d => isArrowFunction(d) || (isFunctionExpression(d) || isClassExpression(d)) && !d.name)) {
displayParts.push(spacePart());
addFullSymbolName(symbol);
const enclosingDeclaration = undefined;
addFullSymbolName(symbol, enclosingDeclaration, indexInfos);
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Expand Up @@ -2316,6 +2316,7 @@ declare namespace ts {
getPrivateIdentifierPropertyOfType(leftType: Type, name: string, location: Node): Symbol | undefined;
getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined;
getIndexInfosOfType(type: Type): readonly IndexInfo[];
getIndexInfosOfIndexSymbol: (indexSymbol: Symbol) => IndexInfo[];
getSignaturesOfType(type: Type, kind: SignatureKind): readonly Signature[];
getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined;
getBaseTypes(type: InterfaceType): BaseType[];
Expand Down
1 change: 1 addition & 0 deletions tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Expand Up @@ -2316,6 +2316,7 @@ declare namespace ts {
getPrivateIdentifierPropertyOfType(leftType: Type, name: string, location: Node): Symbol | undefined;
getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined;
getIndexInfosOfType(type: Type): readonly IndexInfo[];
getIndexInfosOfIndexSymbol: (indexSymbol: Symbol) => IndexInfo[];
getSignaturesOfType(type: Type, kind: SignatureKind): readonly Signature[];
getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined;
getBaseTypes(type: InterfaceType): BaseType[];
Expand Down
15 changes: 15 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription1.ts
View file
Open in desktop
@@ -0,0 +1,15 @@
///<reference path="fourslash.ts" />

//// interface StringExample {
//// /** Something generic */
//// [p: string]: any;
//// /** Something specific */
//// property: number;
//// }
//// function stringExample(e: StringExample) {
//// console.log(e./*property*/property);
//// console.log(e./*string*/anything);
//// }

verify.quickInfoAt("property", "(property) StringExample.property: number", 'Something specific');
verify.quickInfoAt("string", "(index) StringExample[string]: any", "Something generic");
11 changes: 11 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription10.ts
View file
Open in desktop
@@ -0,0 +1,11 @@
///<reference path="fourslash.ts" />

//// class MultipleClass {
//// /** Something generic */
//// [key: number | symbol | `data-${string}` | `data-${number}`]: string;
//// }
//// function multipleClass(e: typeof MultipleClass) {
//// console.log(e./*multipleClass*/anything);
//// }

verify.quickInfoAt("multipleClass", "any");
danay1999 marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 12 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription11.ts
View file
Open in desktop
@@ -0,0 +1,12 @@
///<reference path="fourslash.ts" />

//// type AliasExample = {
//// /** Something generic */
//// [p: string]: string;
//// [key: `any${string}`]: string; //TODOFIX add JSDoc here
//// }
//// function aliasExample(e: AliasExample) {
//// console.log(e./*alias*/anything);
//// }

verify.quickInfoAt("alias", "(index) AliasExample[string | `any${string}`]: string", "Something generic");
11 changes: 11 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription12.ts
View file
Open in desktop
@@ -0,0 +1,11 @@
///<reference path="fourslash.ts" />

//// type SymbolAlias = {
//// /** Something generic */
//// [p: symbol]: string;
//// }
//// function symbolAlias(e: SymbolAlias) {
//// console.log(e./*symbolAlias*/anything);
//// }

verify.quickInfoAt("symbolAlias", "any");
11 changes: 11 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription2.ts
View file
Open in desktop
@@ -0,0 +1,11 @@
///<reference path="fourslash.ts" />

//// interface SymbolExample {
//// /** Something generic */
//// [key: symbol]: string;
//// }
//// function symbolExample(e: SymbolExample) {
//// console.log(e./*symbol*/anything);
//// }

verify.quickInfoAt("symbol", "any")
13 changes: 13 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription3.ts
View file
Open in desktop
@@ -0,0 +1,13 @@
///<reference path="fourslash.ts" />

//// interface LiteralExample {
//// /** Something generic */
//// [key: `data-${string}`]: string;
//// /** Something else */
//// [key: `prefix${number}`]: number;
//// }
//// function literalExample(e: LiteralExample) {
//// console.log(e./*literal*/anything);
//// }

verify.quickInfoAt("literal", "any");
11 changes: 11 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription4.ts
View file
Open in desktop
@@ -0,0 +1,11 @@
///<reference path="fourslash.ts" />

//// interface MultipleExample {
//// /** Something generic */
//// [key: string | number | symbol]: string;
//// }
//// function multipleExample(e: MultipleExample) {
//// console.log(e./*multiple*/anything);
//// }

verify.quickInfoAt("multiple", "(index) MultipleExample[string | number | symbol]: string", "Something generic");
11 changes: 11 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription5.ts
View file
Open in desktop
@@ -0,0 +1,11 @@
///<reference path="fourslash.ts" />

//// interface Multiple1Example {
//// /** Something generic */
//// [key: number | symbol | `data-${string}` | `data-${number}`]: string;
//// }
//// function multiple1Example(e: Multiple1Example) {
//// console.log(e./*multiple1*/anything);
//// }

verify.quickInfoAt("multiple1", "any");
17 changes: 17 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription6.ts
View file
Open in desktop
@@ -0,0 +1,17 @@
///<reference path="fourslash.ts" />

//// interface Literal1Example {
//// /** Something generic */
//// [key: `prefix${string}`]: number | string;
//// /** Something else */
//// [key: `prefix${number}`]: number;
//// }
//// function literal1Example(e: Literal1Example) {
//// console.log(e./*literal1*/prefixMember);
//// console.log(e./*literal2*/anything);
//// console.log(e./*literal3*/prefix0);
//// }

verify.quickInfoAt("literal1", "(index) Literal1Example[`prefix${string}`]: string | number", "Something generic");
verify.quickInfoAt("literal2", "any");
verify.quickInfoAt("literal3", "(index) Literal1Example[`prefix${string}` | `prefix${number}`]: string | number", "Something generic\nSomething else");
11 changes: 11 additions & 0 deletions tests/cases/fourslash/jsDocPropertyDescription7.ts
View file
Open in desktop
@@ -0,0 +1,11 @@
///<reference path="fourslash.ts" />

//// class StringClass {
//// /** Something generic */
//// static [p: string]: any;
//// }
//// function stringClass(e: typeof StringClass) {
//// console.log(e./*stringClass*/anything);
//// }

verify.quickInfoAt("stringClass", "(index) StringClass[string]: any", "Something generic");