Skip to content

Expose JSDoc tags through the language service #12856

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

Merged
merged 5 commits into from
Dec 12, 2016
Merged

Expose JSDoc tags through the language service #12856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd0893a
Add JSDoc tag support for QuickInfo
minestarks Dec 7, 2016
cf2bfc0
Add JSDoc tag support for Signature Help
minestarks Dec 8, 2016
5130e95
Add JSDoc tag support for completion entry details
minestarks Dec 8, 2016
d7c4001
Clean up per CR comments
minestarks Dec 12, 2016
2875d5b
Fix whitespace
minestarks Dec 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6468,7 +6468,7 @@ namespace ts {

function parseTagComments(indent: number) {
const comments: string[] = [];
let state = JSDocState.SawAsterisk;
let state = JSDocState.BeginningOfLine;
Copy link
Member

Choose a reason for hiding this comment

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

Is this just a drive-by bug fix? I didn't see any real difference in parsing functionality.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes I should clarify. This isn't irrelevant. It fixes a bug I discovered while running the new quickinfo test I added. Consider this comment:

/**
  * @mytag1 
  * here all the comments are on a new line
  * @mytag2
  */

When the comment parser starts out at "SawAsterisk" state I get this tag comment for mytag1: "* here all the comments are on a new line"

With my change it's fixed: "here all the comments are on a new line"

This is because we skip all the whitespace (including newlines) before we begin parsing so we start right at the *. This would have been handled in the AsteriskToken case below, but only if the last state had been BeginningOfLine.

Going over the whole logic I believe it just makes more sense this way. Starting at BeginningOfLine is more logical since we didn't actually see an asterisk so why start at SawAsterisk...

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. There was a similar bug in the start state for the main jsdoc parsing, so it makes sense that you ran into it here too.

let margin: number | undefined;
function pushComment(text: string) {
if (!margin) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1442,7 +1442,7 @@ namespace ts {
return node && firstOrUndefined(getJSDocTags(node, kind));
}

function getJSDocs(node: Node): (JSDoc | JSDocTag)[] {
export function getJSDocs(node: Node): (JSDoc | JSDocTag)[] {
let cache: (JSDoc | JSDocTag)[] = node.jsDocCache;
if (!cache) {
getJSDocsWorker(node);
Expand Down
41 changes: 35 additions & 6 deletions src/harness/fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ namespace FourSlash {
}
}

public verifyCompletionEntryDetails(entryName: string, expectedText: string, expectedDocumentation?: string, kind?: string) {
public verifyCompletionEntryDetails(entryName: string, expectedText: string, expectedDocumentation?: string, kind?: string, tags?: ts.JSDocTagInfo[]) {
const details = this.getCompletionEntryDetails(entryName);

assert(details, "no completion entry available");
Expand All @@ -815,6 +815,14 @@ namespace FourSlash {
if (kind !== undefined) {
assert.equal(details.kind, kind, this.assertionMessageAtLastKnownMarker("completion entry kind"));
}

if (tags !== undefined) {
assert.equal(details.tags.length, tags.length, this.messageAtLastKnownMarker("QuickInfo tags"));
ts.zipWith(tags, details.tags, (expectedTag, actualTag) => {
assert.equal(expectedTag.name, actualTag.name);
assert.equal(expectedTag.text, actualTag.text, this.messageAtLastKnownMarker("QuickInfo tag " + actualTag.name));
});
}
}

public verifyReferencesAre(expectedReferences: Range[]) {
Expand Down Expand Up @@ -961,14 +969,21 @@ namespace FourSlash {

public verifyQuickInfoDisplayParts(kind: string, kindModifiers: string, textSpan: { start: number; length: number; },
displayParts: ts.SymbolDisplayPart[],
documentation: ts.SymbolDisplayPart[]) {
documentation: ts.SymbolDisplayPart[],
tags: ts.JSDocTagInfo[]
) {

const actualQuickInfo = this.languageService.getQuickInfoAtPosition(this.activeFile.fileName, this.currentCaretPosition);
assert.equal(actualQuickInfo.kind, kind, this.messageAtLastKnownMarker("QuickInfo kind"));
assert.equal(actualQuickInfo.kindModifiers, kindModifiers, this.messageAtLastKnownMarker("QuickInfo kindModifiers"));
assert.equal(JSON.stringify(actualQuickInfo.textSpan), JSON.stringify(textSpan), this.messageAtLastKnownMarker("QuickInfo textSpan"));
assert.equal(TestState.getDisplayPartsJson(actualQuickInfo.displayParts), TestState.getDisplayPartsJson(displayParts), this.messageAtLastKnownMarker("QuickInfo displayParts"));
assert.equal(TestState.getDisplayPartsJson(actualQuickInfo.documentation), TestState.getDisplayPartsJson(documentation), this.messageAtLastKnownMarker("QuickInfo documentation"));
assert.equal(actualQuickInfo.tags.length, tags.length, this.messageAtLastKnownMarker("QuickInfo tags"));
ts.zipWith(tags, actualQuickInfo.tags, (expectedTag, actualTag) => {
assert.equal(expectedTag.name, actualTag.name);
assert.equal(expectedTag.text, actualTag.text, this.messageAtLastKnownMarker("QuickInfo tag " + actualTag.name));
});
}

public verifyRenameLocations(findInStrings: boolean, findInComments: boolean, ranges?: Range[]) {
Expand Down Expand Up @@ -1062,6 +1077,16 @@ namespace FourSlash {
assert.equal(ts.displayPartsToString(actualDocComment), docComment, this.assertionMessageAtLastKnownMarker("current signature help doc comment"));
}

public verifyCurrentSignatureHelpTags(tags: ts.JSDocTagInfo[]) {
const actualTags = this.getActiveSignatureHelpItem().tags;

assert.equal(actualTags.length, tags.length, this.assertionMessageAtLastKnownMarker("signature help tags"));
ts.zipWith(tags, actualTags, (expectedTag, actualTag) => {
assert.equal(expectedTag.name, actualTag.name);
assert.equal(expectedTag.text, actualTag.text, this.assertionMessageAtLastKnownMarker("signature help tag " + actualTag.name));
});
}

public verifySignatureHelpCount(expected: number) {
const help = this.languageService.getSignatureHelpItems(this.activeFile.fileName, this.currentCaretPosition);
const actual = help && help.items ? help.items.length : 0;
Expand Down Expand Up @@ -3263,6 +3288,10 @@ namespace FourSlashInterface {
this.state.verifyCurrentSignatureHelpDocComment(docComment);
}

public currentSignatureHelpTagsAre(tags: ts.JSDocTagInfo[]) {
this.state.verifyCurrentSignatureHelpTags(tags);
}

public signatureHelpCountIs(expected: number) {
this.state.verifySignatureHelpCount(expected);
}
Expand Down Expand Up @@ -3383,8 +3412,8 @@ namespace FourSlashInterface {
this.state.verifyDocumentHighlightsAtPositionListCount(expectedCount, fileNamesToSearch);
}

public completionEntryDetailIs(entryName: string, text: string, documentation?: string, kind?: string) {
this.state.verifyCompletionEntryDetails(entryName, text, documentation, kind);
public completionEntryDetailIs(entryName: string, text: string, documentation?: string, kind?: string, tags?: ts.JSDocTagInfo[]) {
this.state.verifyCompletionEntryDetails(entryName, text, documentation, kind, tags);
}

/**
Expand Down Expand Up @@ -3414,8 +3443,8 @@ namespace FourSlashInterface {
}

public verifyQuickInfoDisplayParts(kind: string, kindModifiers: string, textSpan: { start: number; length: number; },
displayParts: ts.SymbolDisplayPart[], documentation: ts.SymbolDisplayPart[]) {
this.state.verifyQuickInfoDisplayParts(kind, kindModifiers, textSpan, displayParts, documentation);
displayParts: ts.SymbolDisplayPart[], documentation: ts.SymbolDisplayPart[], tags: ts.JSDocTagInfo[]) {
this.state.verifyQuickInfoDisplayParts(kind, kindModifiers, textSpan, displayParts, documentation, tags);
}

public getSyntacticDiagnostics(expected: string) {
Expand Down
3 changes: 2 additions & 1 deletion src/server/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,8 @@ namespace ts.server {
kindModifiers: response.body.kindModifiers,
textSpan: ts.createTextSpanFromBounds(start, end),
displayParts: [{ kind: "text", text: response.body.displayString }],
documentation: [{ kind: "text", text: response.body.documentation }]
documentation: [{ kind: "text", text: response.body.documentation }],
tags: response.body.tags
};
}

Expand Down
15 changes: 15 additions & 0 deletions src/server/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,11 @@ namespace ts.server.protocol {
* Documentation associated with symbol.
*/
documentation: string;

/**
* JSDoc tags associated with symbol.
*/
tags: JSDocTagInfo[]
}

/**
Expand Down Expand Up @@ -1525,6 +1530,11 @@ namespace ts.server.protocol {
* Documentation strings for the symbol.
*/
documentation: SymbolDisplayPart[];

/**
* JSDoc tags for the symbol.
*/
tags: JSDocTagInfo[];
}

export interface CompletionsResponse extends Response {
Expand Down Expand Up @@ -1595,6 +1605,11 @@ namespace ts.server.protocol {
* The signature's documentation
*/
documentation: SymbolDisplayPart[];

/**
* The signature's JSDoc tags
*/
tags: JSDocTagInfo[];
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/server/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,13 +853,15 @@ namespace ts.server {
if (simplifiedResult) {
const displayString = ts.displayPartsToString(quickInfo.displayParts);
const docString = ts.displayPartsToString(quickInfo.documentation);

return {
kind: quickInfo.kind,
kindModifiers: quickInfo.kindModifiers,
start: scriptInfo.positionToLineOffset(quickInfo.textSpan.start),
end: scriptInfo.positionToLineOffset(ts.textSpanEnd(quickInfo.textSpan)),
displayString: displayString,
documentation: docString,
tags: quickInfo.tags || []
};
}
else {
Expand Down
8 changes: 5 additions & 3 deletions src/services/completions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -759,13 +759,14 @@ namespace ts.Completions {
const symbol = forEach(symbols, s => getCompletionEntryDisplayNameForSymbol(typeChecker, s, compilerOptions.target, /*performCharacterChecks*/ false, location) === entryName ? s : undefined);

if (symbol) {
const { displayParts, documentation, symbolKind } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All);
const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All);
return {
name: entryName,
kindModifiers: SymbolDisplay.getSymbolModifiers(symbol),
kind: symbolKind,
displayParts,
documentation
documentation,
tags
};
}
}
Expand All @@ -778,7 +779,8 @@ namespace ts.Completions {
kind: ScriptElementKind.keyword,
kindModifiers: ScriptElementKindModifier.none,
displayParts: [displayPart(entryName, SymbolDisplayPartKind.keyword)],
documentation: undefined
documentation: undefined,
tags: undefined
};
}

Expand Down
22 changes: 22 additions & 0 deletions src/services/jsDoc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,28 @@ namespace ts.JsDoc {
return documentationComment;
}

export function getJsDocTagsFromDeclarations(declarations: Declaration[]) {
// Only collect doc comments from duplicate declarations once.
const tags: JSDocTagInfo[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too the array may be lazy-allocated when first tag is encountered -- meaning no allocation for the default case.

forEachUnique(declarations, declaration => {
const jsDocs = getJSDocs(declaration);
if (!jsDocs) {
return;
}
for (const doc of jsDocs) {
const tagsForDoc = (doc as JSDoc).tags;
Copy link
Contributor

Choose a reason for hiding this comment

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

is type assertion necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess not. I got confused by the 2.0 language service giving me an implicit any warning...

if (tagsForDoc) {
tags.push(...tagsForDoc.filter(tag => tag.kind === SyntaxKind.JSDocTag).map(jsDocTag => {
return {
name: jsDocTag.tagName.text,
text: jsDocTag.comment
} }));
}
}
});
return tags;
}

/**
* Iterates through 'array' by index and performs the callback on each element of array until the callback
* returns a truthy value, then returns that value.
Expand Down
30 changes: 28 additions & 2 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,10 @@ namespace ts {
// symbol has no doc comment, then the empty string will be returned.
documentationComment: SymbolDisplayPart[];

// Undefined is used to indicate the value has not been computed. If, after computing, the
// symbol has no JSDoc tags, then the empty array will be returned.
tags: JSDocTagInfo[];

constructor(flags: SymbolFlags, name: string) {
this.flags = flags;
this.name = name;
Expand All @@ -316,6 +320,14 @@ namespace ts {

return this.documentationComment;
}

getJsDocTags(): JSDocTagInfo[] {
if (this.tags === undefined) {
this.tags = JsDoc.getJsDocTagsFromDeclarations(this.declarations);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too the check would need to change to avoid empty array allocation for the default no-JSDoc case.
(to mark the difference between un-computed value and empty value in this.tags -- undefined versus null can be used)


return this.tags;
}
}

class TokenObject<TKind extends SyntaxKind> extends TokenOrIdentifierObject implements Token<TKind> {
Expand Down Expand Up @@ -404,6 +416,10 @@ namespace ts {
// symbol has no doc comment, then the empty string will be returned.
documentationComment: SymbolDisplayPart[];

// Undefined is used to indicate the value has not been computed. If, after computing, the
// symbol has no doc comment, then the empty array will be returned.
jsDocTags: JSDocTagInfo[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Empty array is expensive here.

Most of SignatureObject instances will have no JSDoc comments. Allocating empty arrays will add bunch of fluff to GC. Would be best to avoid that.


constructor(checker: TypeChecker) {
this.checker = checker;
}
Expand All @@ -427,6 +443,14 @@ namespace ts {

return this.documentationComment;
}

getJsDocTags(): JSDocTagInfo[] {
if (this.jsDocTags === undefined) {
this.jsDocTags = this.declaration ? JsDoc.getJsDocTagsFromDeclarations([this.declaration]) : [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

How about using undefined for storing un-computed state and null for computed and empty?


return this.jsDocTags;
}
}

class SourceFileObject extends NodeObject implements SourceFile {
Expand Down Expand Up @@ -1315,7 +1339,8 @@ namespace ts {
kindModifiers: ScriptElementKindModifier.none,
textSpan: createTextSpan(node.getStart(), node.getWidth()),
displayParts: typeToDisplayParts(typeChecker, type, getContainerNode(node)),
documentation: type.symbol ? type.symbol.getDocumentationComment() : undefined
documentation: type.symbol ? type.symbol.getDocumentationComment() : undefined,
tags: type.symbol ? type.symbol.getJsDocTags() : undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that the API may return undefined here, so returning null or undefined instead of an empty array shouldn't break expectations.

};
}
}
Expand All @@ -1329,7 +1354,8 @@ namespace ts {
kindModifiers: SymbolDisplay.getSymbolModifiers(symbol),
textSpan: createTextSpan(node.getStart(), node.getWidth()),
displayParts: displayPartsDocumentationsAndKind.displayParts,
documentation: displayPartsDocumentationsAndKind.documentation
documentation: displayPartsDocumentationsAndKind.documentation,
tags: displayPartsDocumentationsAndKind.tags
};
}

Expand Down
3 changes: 2 additions & 1 deletion src/services/signatureHelp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,8 @@ namespace ts.SignatureHelp {
suffixDisplayParts,
separatorDisplayParts: [punctuationPart(SyntaxKind.CommaToken), spacePart()],
parameters: signatureHelpParameters,
documentation: candidateSignature.getDocumentationComment()
documentation: candidateSignature.getDocumentationComment(),
tags: candidateSignature.getJsDocTags()
};
});

Expand Down
6 changes: 5 additions & 1 deletion src/services/symbolDisplay.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ namespace ts.SymbolDisplay {

const displayParts: SymbolDisplayPart[] = [];
let documentation: SymbolDisplayPart[];
let tags: JSDocTagInfo[];
const symbolFlags = symbol.flags;
let symbolKind = getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(typeChecker, symbol, symbolFlags, location);
let hasAddedSymbolInfo: boolean;
Expand Down Expand Up @@ -407,6 +408,7 @@ namespace ts.SymbolDisplay {

if (!documentation) {
documentation = symbol.getDocumentationComment();
tags = symbol.getJsDocTags();
if (documentation.length === 0 && symbol.flags & SymbolFlags.Property) {
// For some special property access expressions like `experts.foo = foo` or `module.exports.foo = foo`
// there documentation comments might be attached to the right hand side symbol of their declarations.
Expand All @@ -423,6 +425,7 @@ namespace ts.SymbolDisplay {
}

documentation = rhsSymbol.getDocumentationComment();
tags = rhsSymbol.getJsDocTags();
if (documentation.length > 0) {
break;
}
Expand All @@ -431,7 +434,7 @@ namespace ts.SymbolDisplay {
}
}

return { displayParts, documentation, symbolKind };
return { displayParts, documentation, symbolKind, tags };

function addNewLineIfDisplayPartsExist() {
if (displayParts.length) {
Expand Down Expand Up @@ -489,6 +492,7 @@ namespace ts.SymbolDisplay {
displayParts.push(punctuationPart(SyntaxKind.CloseParenToken));
}
documentation = signature.getDocumentationComment();
tags = signature.getJsDocTags();
}

function writeTypeParametersOfSymbol(symbol: Symbol, enclosingDeclaration: Node) {
Expand Down
Loading