Skip to content

Support completions for JSDoc @param tag names #16299

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
3 commits merged into from
Jun 7, 2017
Merged

Support completions for JSDoc @param tag names #16299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
117926a
Support completions for JSDoc @param tag names
May 26, 2017
cace406
Undo change to finishNode
Jun 6, 2017
1e0236b
Don't include trailing whitespace in @param range; instead, specializ…
Jun 6, 2017
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
4 changes: 2 additions & 2 deletions src/compiler/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,8 +521,8 @@ namespace ts {
return result || array;
}

export function mapDefined<T>(array: ReadonlyArray<T>, mapFn: (x: T, i: number) => T | undefined): ReadonlyArray<T> {
const result: T[] = [];
export function mapDefined<T, U>(array: ReadonlyArray<T>, mapFn: (x: T, i: number) => U | undefined): U[] {

@amcasey amcasey Jun 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not ReadonlyArray<U>?

@ghost ghost Jun 6, 2017 •
edited by ghost
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We always create a fresh array. If the caller wants a readonly array they can type it that way.
Can't just change this one to ReadonlyArray because services/types.ts currently uses mutable arrays. We should change that separately. #16312

const result: U[] = [];
for (let i = 0; i < array.length; i++) {
const item = array[i];
const mapped = mapFn(item, i);
Expand Down
34 changes: 15 additions & 19 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6659,14 +6659,12 @@ namespace ts {
});
}

function parseBracketNameInPropertyAndParamTag() {
let name: Identifier;
let isBracketed: boolean;
function parseBracketNameInPropertyAndParamTag(): { name: Identifier, isBracketed: boolean } {
// Looking for something like '[foo]' or 'foo'
if (parseOptionalToken(SyntaxKind.OpenBracketToken)) {
name = parseJSDocIdentifierName();
const isBracketed = parseOptional(SyntaxKind.OpenBracketToken);
const name = parseJSDocIdentifierName(/*createIfMissing*/ true);
if (isBracketed) {
skipWhitespace();
isBracketed = true;

// May have an optional default, e.g. '[foo = 42]'
if (parseOptionalToken(SyntaxKind.EqualsToken)) {
Expand All @@ -6675,9 +6673,7 @@ namespace ts {

parseExpected(SyntaxKind.CloseBracketToken);
}
else if (tokenIsIdentifierOrKeyword(token())) {
name = parseJSDocIdentifierName();
}

return { name, isBracketed };
}

Expand All @@ -6688,11 +6684,6 @@ namespace ts {
const { name, isBracketed } = parseBracketNameInPropertyAndParamTag();
skipWhitespace();

if (!name) {
parseErrorAtPosition(scanner.getStartPos(), 0, Diagnostics.Identifier_expected);
return undefined;
}

let preName: Identifier, postName: Identifier;
if (typeExpression) {
postName = name;
Expand Down Expand Up @@ -6936,14 +6927,19 @@ namespace ts {
return currentToken = scanner.scanJSDocToken();
}

function parseJSDocIdentifierName(): Identifier {
return createJSDocIdentifier(tokenIsIdentifierOrKeyword(token()));
function parseJSDocIdentifierName(createIfMissing = false): Identifier {
return createJSDocIdentifier(tokenIsIdentifierOrKeyword(token()), createIfMissing);
}

function createJSDocIdentifier(isIdentifier: boolean): Identifier {
function createJSDocIdentifier(isIdentifier: boolean, createIfMissing: boolean): Identifier {
if (!isIdentifier) {
parseErrorAtCurrentToken(Diagnostics.Identifier_expected);
return undefined;
if (createIfMissing) {
return <Identifier>createMissingNode(SyntaxKind.Identifier, /*reportAtCurrentPosition*/ true, Diagnostics.Identifier_expected);
}
else {
parseErrorAtCurrentToken(Diagnostics.Identifier_expected);
return undefined;
}
}

const pos = scanner.getTokenPos();
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ namespace ts {
FirstNode = QualifiedName,
FirstJSDocNode = JSDocTypeExpression,
LastJSDocNode = JSDocLiteralType,
FirstJSDocTagNode = JSDocComment,
FirstJSDocTagNode = JSDocTag,
LastJSDocTagNode = JSDocLiteralType
}

Expand Down
21 changes: 13 additions & 8 deletions src/harness/fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1595,16 +1595,19 @@ namespace FourSlash {
}

private printMembersOrCompletions(info: ts.CompletionInfo) {
if (info === undefined) { return "No completion info."; }
const { entries } = info;

function pad(s: string, length: number) {
return s + new Array(length - s.length + 1).join(" ");
}
function max<T>(arr: T[], selector: (x: T) => number): number {
return arr.reduce((prev, x) => Math.max(prev, selector(x)), 0);
}
const longestNameLength = max(info.entries, m => m.name.length);
const longestKindLength = max(info.entries, m => m.kind.length);
info.entries.sort((m, n) => m.sortText > n.sortText ? 1 : m.sortText < n.sortText ? -1 : m.name > n.name ? 1 : m.name < n.name ? -1 : 0);
const membersString = info.entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers}`).join("\n");
const longestNameLength = max(entries, m => m.name.length);
const longestKindLength = max(entries, m => m.kind.length);
entries.sort((m, n) => m.sortText > n.sortText ? 1 : m.sortText < n.sortText ? -1 : m.name > n.name ? 1 : m.name < n.name ? -1 : 0);
const membersString = entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers}`).join("\n");
Harness.IO.log(membersString);
}

Expand Down Expand Up @@ -2156,7 +2159,7 @@ namespace FourSlash {
Harness.IO.log(this.spanInfoToString(this.getNameOrDottedNameSpan(pos), "**"));
}

private verifyClassifications(expected: { classificationType: string; text: string; textSpan?: TextSpan }[], actual: ts.ClassifiedSpan[]) {
private verifyClassifications(expected: { classificationType: string; text: string; textSpan?: TextSpan }[], actual: ts.ClassifiedSpan[], sourceFileText: string) {
if (actual.length !== expected.length) {
this.raiseError("verifyClassifications failed - expected total classifications to be " + expected.length +
", but was " + actual.length +
Expand Down Expand Up @@ -2196,9 +2199,11 @@ namespace FourSlash {
});

function jsonMismatchString() {
const showActual = actual.map(({ classificationType, textSpan }) =>
({ classificationType, text: sourceFileText.slice(textSpan.start, textSpan.start + textSpan.length) }));
return Harness.IO.newLine() +
"expected: '" + Harness.IO.newLine() + stringify(expected) + "'" + Harness.IO.newLine() +
"actual: '" + Harness.IO.newLine() + stringify(actual) + "'";
"actual: '" + Harness.IO.newLine() + stringify(showActual) + "'";
}
}

Expand All @@ -2221,14 +2226,14 @@ namespace FourSlash {
const actual = this.languageService.getSemanticClassifications(this.activeFile.fileName,
ts.createTextSpan(0, this.activeFile.content.length));

this.verifyClassifications(expected, actual);
this.verifyClassifications(expected, actual, this.activeFile.content);
}

public verifySyntacticClassifications(expected: { classificationType: string; text: string }[]) {
const actual = this.languageService.getSyntacticClassifications(this.activeFile.fileName,
ts.createTextSpan(0, this.activeFile.content.length));

this.verifyClassifications(expected, actual);
this.verifyClassifications(expected, actual, this.activeFile.content);
}

public verifyOutliningSpans(spans: TextSpan[]) {
Expand Down
2 changes: 1 addition & 1 deletion src/services/classifier.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,7 +814,7 @@ namespace ts {
* False will mean that node is not classified and traverse routine should recurse into node contents.
*/
function tryClassifyNode(node: Node): boolean {
if (isJSDocTag(node)) {
if (isJSDocNode(node)) {
return true;
}

Expand Down
101 changes: 74 additions & 27 deletions src/services/completions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace ts.Completions {
return undefined;
}

const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords } = completionData;
const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, hasFilteredClassMemberKeywords } = completionData;

if (sourceFile.languageVariant === LanguageVariant.JSX &&
location && location.parent && location.parent.kind === SyntaxKind.JsxClosingElement) {
Expand All @@ -36,14 +36,15 @@ namespace ts.Completions {
}]};
}

if (requestJsDocTagName) {
// If the current position is a jsDoc tag name, only tag names should be provided for completion
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries: JsDoc.getJSDocTagNameCompletions() };
}

if (requestJsDocTag) {
// If the current position is a jsDoc tag, only tags should be provided for completion
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries: JsDoc.getJSDocTagCompletions() };
if (request) {
const entries = request.kind === "JsDocTagName"
// If the current position is a jsDoc tag name, only tag names should be provided for completion
? JsDoc.getJSDocTagNameCompletions()
: request.kind === "JsDocTag"
// If the current position is a jsDoc tag, only tags should be provided for completion
? JsDoc.getJSDocTagCompletions()
: JsDoc.getJSDocParameterNameCompletions(request.tag);
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries };
}

const entries: CompletionEntry[] = [];
Expand All @@ -66,7 +67,7 @@ namespace ts.Completions {
addRange(entries, classMemberKeywordCompletions);
}
// Add keywords if this is not a member completion list
else if (!isMemberCompletion && !requestJsDocTag && !requestJsDocTagName) {
else if (!isMemberCompletion) {
addRange(entries, keywordCompletions);
}

Expand Down Expand Up @@ -347,16 +348,27 @@ namespace ts.Completions {
return undefined;
}

function getCompletionData(typeChecker: TypeChecker, log: (message: string) => void, sourceFile: SourceFile, position: number) {
interface CompletionData {
symbols: Symbol[];
isGlobalCompletion: boolean;
isMemberCompletion: boolean;
isNewIdentifierLocation: boolean;
location: Node;
isRightOfDot: boolean;
request?: Request;
hasFilteredClassMemberKeywords: boolean;
}
type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag };

function getCompletionData(typeChecker: TypeChecker, log: (message: string) => void, sourceFile: SourceFile, position: number): CompletionData {
const isJavaScriptFile = isSourceFileJavaScript(sourceFile);

// JsDoc tag-name is just the name of the JSDoc tagname (exclude "@")
let requestJsDocTagName = false;
// JsDoc tag includes both "@" and tag-name
let requestJsDocTag = false;
let request: Request | undefined;

let start = timestamp();
const currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); // TODO: GH#15853
const currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false);
// We will check for jsdoc comments with insideComment and getJsDocTagAtPosition. (TODO: that seems rather inefficient to check the same thing so many times.)

log("getCompletionData: Get current token: " + (timestamp() - start));

start = timestamp();
Expand All @@ -366,10 +378,10 @@ namespace ts.Completions {

if (insideComment) {
if (hasDocComment(sourceFile, position)) {
// The current position is next to the '@' sign, when no tag name being provided yet.
// Provide a full list of tag names
if (sourceFile.text.charCodeAt(position - 1) === CharacterCodes.at) {
requestJsDocTagName = true;
// The current position is next to the '@' sign, when no tag name being provided yet.
// Provide a full list of tag names
request = { kind: "JsDocTagName" };
}
else {
// When completion is requested without "@", we will have check to make sure that
Expand All @@ -389,34 +401,39 @@ namespace ts.Completions {
// * |c|
// */
const lineStart = getLineStartPositionForPosition(position, sourceFile);
requestJsDocTag = !(sourceFile.text.substring(lineStart, position).match(/[^\*|\s|(/\*\*)]/));
if (!(sourceFile.text.substring(lineStart, position).match(/[^\*|\s|(/\*\*)]/))) {
request = { kind: "JsDocTag" };
}
}
}

// Completion should work inside certain JsDoc tags. For example:
// /** @type {number | string} */
// Completion should work in the brackets
let insideJsDocTagExpression = false;
const tag = getJsDocTagAtPosition(sourceFile, position);
const tag = getJsDocTagAtPosition(currentToken, position);
if (tag) {
if (tag.tagName.pos <= position && position <= tag.tagName.end) {
requestJsDocTagName = true;
request = { kind: "JsDocTagName" };
}

switch (tag.kind) {
case SyntaxKind.JSDocTypeTag:
case SyntaxKind.JSDocParameterTag:
case SyntaxKind.JSDocReturnTag:
const tagWithExpression = <JSDocTypeTag | JSDocParameterTag | JSDocReturnTag>tag;
if (tagWithExpression.typeExpression) {
insideJsDocTagExpression = tagWithExpression.typeExpression.pos < position && position < tagWithExpression.typeExpression.end;
if (tagWithExpression.typeExpression && tagWithExpression.typeExpression.pos < position && position < tagWithExpression.typeExpression.end) {
insideJsDocTagExpression = true;
}
else if (isJSDocParameterTag(tag) && (nodeIsMissing(tag.name) || tag.name.pos <= position && position <= tag.name.end)) {
request = { kind: "JsDocParameterName", tag };
}
break;
}
}

if (requestJsDocTagName || requestJsDocTag) {
return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords: false };
if (request) {
return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, hasFilteredClassMemberKeywords: false };
}

if (!insideJsDocTagExpression) {
Expand Down Expand Up @@ -553,7 +570,7 @@ namespace ts.Completions {

log("getCompletionData: Semantic work: " + (timestamp() - semanticStart));

return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords };
return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, hasFilteredClassMemberKeywords };

function getTypeScriptMemberSymbols(): void {
// Right of dot member completion list
Expand Down Expand Up @@ -1518,4 +1535,34 @@ namespace ts.Completions {
kind === SyntaxKind.EqualsEqualsEqualsToken ||
kind === SyntaxKind.ExclamationEqualsEqualsToken;
}

/** Get the corresponding JSDocTag node if the position is in a jsDoc comment */
function getJsDocTagAtPosition(node: Node, position: number): JSDocTag | undefined {
const { jsDoc } = getJsDocHavingNode(node);
if (!jsDoc) return undefined;

for (const { pos, end, tags } of jsDoc) {
if (!tags || position < pos || position > end) continue;
for (let i = tags.length - 1; i >= 0; i--) {
const tag = tags[i];
if (position >= tag.pos) {
return tag;
}
}
}
}

function getJsDocHavingNode(node: Node): Node {
if (!isToken(node)) return node;

switch (node.kind) {
case SyntaxKind.VarKeyword:
case SyntaxKind.LetKeyword:
case SyntaxKind.ConstKeyword:
// if the current token is var, let or const, skip the VariableDeclarationList
return node.parent.parent;
default:
return node.parent;
}
}
}
18 changes: 18 additions & 0 deletions src/services/jsDoc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,24 @@ namespace ts.JsDoc {
}));
}

export function getJSDocParameterNameCompletions(tag: JSDocParameterTag): CompletionEntry[] {
const nameThusFar = tag.name.text;
const jsdoc = tag.parent;
const fn = jsdoc.parent;
if (!ts.isFunctionLike(fn)) return [];

return mapDefined(fn.parameters, param => {
if (!isIdentifier(param.name)) return undefined;

const name = param.name.text;
if (jsdoc.tags.some(t => t !== tag && isJSDocParameterTag(t) && t.name.text === name)
|| nameThusFar !== undefined && !startsWith(name, nameThusFar))
return undefined;

return { name, kind: ScriptElementKind.parameterElement, kindModifiers: "", sortText: "0" };
});
}

/**
* Checks if position points to a valid position to add JSDoc comments, and if so,
* returns the appropriate template. Otherwise returns an empty string.
Expand Down
6 changes: 3 additions & 3 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ namespace ts {
}

private createChildren(sourceFile?: SourceFileLike) {
if (isJSDocTag(this)) {
if (this.kind === SyntaxKind.JSDocComment || isJSDocTag(this)) {
/** Don't add trivia for "tokens" since this is in a comment. */
const children: Node[] = [];
this.forEachChild(child => { children.push(child); });
Expand All @@ -146,9 +146,9 @@ namespace ts {
const children: Node[] = [];
scanner.setText((sourceFile || this.getSourceFile()).text);
let pos = this.pos;
const useJSDocScanner = this.kind >= SyntaxKind.FirstJSDocTagNode && this.kind <= SyntaxKind.LastJSDocTagNode;
const useJSDocScanner = isJSDocNode(this);
const processNode = (node: Node) => {
const isJSDocTagNode = isJSDocTag(node);
const isJSDocTagNode = isJSDocNode(node);
if (!isJSDocTagNode && pos < node.pos) {
pos = this.addSyntheticNodes(children, pos, node.pos, useJSDocScanner);
}
Expand Down
Loading