From d6085f75c4f47a034c0fb1e6eb91d65655344c0a Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Tue, 28 Feb 2017 15:41:35 -0800 Subject: [PATCH 1/7] Return completions for JsDoc tagname even when there are no "@' sign prefix --- src/services/completions.ts | 38 ++++++++++++++++++++++++++++--------- src/services/jsDoc.ts | 33 ++++++++++++++++++++++---------- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 1bf57d33fdc..4450be7cad6 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -16,11 +16,11 @@ namespace ts.Completions { return undefined; } - const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isJsDocTagName } = completionData; + const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isJsDocTagName, shouldAppendAtSignBeforeJsDocTagName } = completionData; if (isJsDocTagName) { // 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.getAllJsDocCompletionEntries() }; + return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries: JsDoc.getAllJsDocCompletionEntries(shouldAppendAtSignBeforeJsDocTagName) }; } const entries: CompletionEntry[] = []; @@ -815,6 +815,12 @@ namespace ts.Completions { const isJavaScriptFile = isSourceFileJavaScript(sourceFile); let isJsDocTagName = false; + // This is for the case when users request completion in JsDoc without "@" + // i.e. + // /** + // * |completion here| + // **/ + let shouldAppendAtSignBeforeJsDocTagName = false; let start = timestamp(); const currentToken = getTokenAtPosition(sourceFile, position); @@ -826,10 +832,24 @@ namespace ts.Completions { log("getCompletionData: Is inside comment: " + (timestamp() - start)); if (insideComment) { - // The current position is next to the '@' sign, when no tag name being provided yet. - // Provide a full list of tag names - if (hasDocComment(sourceFile, position) && sourceFile.text.charCodeAt(position - 1) === CharacterCodes.at) { - isJsDocTagName = true; + 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) { + isJsDocTagName = true; + } + else { + const lineStart = getLineStartPositionForPosition(position, sourceFile); + shouldAppendAtSignBeforeJsDocTagName = sourceFile.text.substr(lineStart, position).indexOf("@") === -1; + + // This is for the case + // /** + // * |completion here| + // **/ + if (shouldAppendAtSignBeforeJsDocTagName) { + isJsDocTagName = true; + } + } } // Completion should work inside certain JsDoc tags. For example: @@ -854,8 +874,8 @@ namespace ts.Completions { } } - if (isJsDocTagName) { - return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, isJsDocTagName }; + if (isJsDocTagName || shouldAppendAtSignBeforeJsDocTagName) { + return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, isJsDocTagName, shouldAppendAtSignBeforeJsDocTagName }; } if (!insideJsDocTagExpression) { @@ -983,7 +1003,7 @@ namespace ts.Completions { log("getCompletionData: Semantic work: " + (timestamp() - semanticStart)); - return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), isJsDocTagName }; + return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), isJsDocTagName, shouldAppendAtSignBeforeJsDocTagName }; function getTypeScriptMemberSymbols(): void { // Right of dot member completion list diff --git a/src/services/jsDoc.ts b/src/services/jsDoc.ts index 08a51a63e63..40ee276b0fa 100644 --- a/src/services/jsDoc.ts +++ b/src/services/jsDoc.ts @@ -42,7 +42,8 @@ namespace ts.JsDoc { "prop", "version" ]; - let jsDocCompletionEntries: CompletionEntry[]; + let jsDocTagNameCompletionEntries: CompletionEntry[]; + let jsDocTagNameWithAtSignCompletionEntries: CompletionEntry[]; export function getJsDocCommentsFromDeclarations(declarations: Declaration[]) { // Only collect doc comments from duplicate declarations once: @@ -88,15 +89,27 @@ namespace ts.JsDoc { return undefined; } - export function getAllJsDocCompletionEntries(): CompletionEntry[] { - return jsDocCompletionEntries || (jsDocCompletionEntries = ts.map(jsDocTagNames, tagName => { - return { - name: tagName, - kind: ScriptElementKind.keyword, - kindModifiers: "", - sortText: "0", - }; - })); + export function getAllJsDocCompletionEntries(shouldAppendAtSign: boolean): CompletionEntry[] { + if (!shouldAppendAtSign) { + return jsDocTagNameCompletionEntries || (jsDocTagNameCompletionEntries = ts.map(jsDocTagNames, tagName => { + return { + name: tagName, + kind: ScriptElementKind.keyword, + kindModifiers: "", + sortText: "0", + }; + })); + } + else { + return jsDocTagNameWithAtSignCompletionEntries || (jsDocTagNameWithAtSignCompletionEntries = ts.map(jsDocTagNames, tagName => { + return { + name: `@${tagName}`, + kind: ScriptElementKind.keyword, + kindModifiers: "", + sortText: "0" + } + })); + } } /** From 441c5880d75fe40c9d3d0fba75bd6b541a0d8573 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Tue, 28 Feb 2017 15:41:47 -0800 Subject: [PATCH 2/7] Update fourslash tests --- tests/cases/fourslash/completionInJsDoc.ts | 53 +++++++++++++------ .../completionListAtInvalidLocations.ts | 44 +++++++-------- 2 files changed, 57 insertions(+), 40 deletions(-) diff --git a/tests/cases/fourslash/completionInJsDoc.ts b/tests/cases/fourslash/completionInJsDoc.ts index 8ab9dbd0131..5eca52743a4 100644 --- a/tests/cases/fourslash/completionInJsDoc.ts +++ b/tests/cases/fourslash/completionInJsDoc.ts @@ -2,29 +2,39 @@ // @allowJs: true // @Filename: Foo.js -/////** @/*1*/ */ -////var v1; +//// /** @/*1*/ */ +//// var v1; //// -/////** @p/*2*/ */ -////var v2; +//// /** @p/*2*/ */ +//// var v2; //// -/////** @param /*3*/ */ -////var v3; +//// /** @param /*3*/ */ +//// var v3; //// -/////** @param { n/*4*/ } bar */ -////var v4; +//// /** @param { n/*4*/ } bar */ +//// var v4; //// -/////** @type { n/*5*/ } */ -////var v5; +//// /** @type { n/*5*/ } */ +//// var v5; //// -////// @/*6*/ -////var v6; +//// // @/*6*/ +//// var v6; //// -////// @pa/*7*/ -////var v7; +//// // @pa/*7*/ +//// var v7; //// -/////** @return { n/*8*/ } */ -////var v8; +//// /** @return { n/*8*/ } */ +//// var v8; +//// +//// /** /*9*/ */ +//// +//// /** +//// /*10*/ +//// */ +//// +//// /** +//// * /*11*/ +//// */ goTo.marker('1'); verify.completionListContains("constructor"); @@ -55,3 +65,14 @@ verify.completionListIsEmpty(); goTo.marker('8'); verify.completionListContains('number'); +goTo.marker('9'); +verify.completionListCount(40); +verify.completionListContains("@argument"); + +goTo.marker('10'); +verify.completionListCount(40); +verify.completionListContains("@return"); + +goTo.marker('11'); +verify.completionListCount(40); +verify.completionListContains("@argument"); diff --git a/tests/cases/fourslash/completionListAtInvalidLocations.ts b/tests/cases/fourslash/completionListAtInvalidLocations.ts index 0660f0e183b..171f63825f2 100644 --- a/tests/cases/fourslash/completionListAtInvalidLocations.ts +++ b/tests/cases/fourslash/completionListAtInvalidLocations.ts @@ -1,28 +1,24 @@ /// -////var v1 = ''; -////" /*openString1*/ -////var v2 = ''; -////"/*openString2*/ -////var v3 = ''; -////" bar./*openString3*/ -////var v4 = ''; -////// bar./*inComment1*/ -////var v6 = ''; -////// /*inComment2*/ -////var v7 = ''; -/////** /*inComment3*/ -////var v8 = ''; -/////** /*inComment4*/ **/ -////var v9 = ''; -/////* /*inComment5*/ -////var v11 = ''; -//// // /*inComment6*/ -////var v12 = ''; -////type htm/*inTypeAlias*/ -/// -////// /*inComment7*/ -////foo; -////var v10 = /reg/*inRegExp1*/ex/; +//// var v1 = ''; +//// " /*openString1*/ +//// var v2 = ''; +//// "/*openString2*/ +//// var v3 = ''; +//// " bar./*openString3*/ +//// var v4 = ''; +//// // bar./*inComment1*/ +//// var v6 = ''; +//// // /*inComment2*/ +//// var v7 = ''; +//// /* /*inComment3*/ +//// var v11 = ''; +//// // /*inComment4*/ +//// var v12 = ''; +//// type htm/*inTypeAlias*/ +//// +//// // /*inComment5*/ +//// foo; +//// var v10 = /reg/*inRegExp1*/ex/; goTo.eachMarker(() => verify.completionListIsEmpty()); From e74df1e38bbb911998defecb8899acf1a6806a12 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Tue, 28 Feb 2017 15:51:41 -0800 Subject: [PATCH 3/7] Remove unnecessary comment --- src/services/completions.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 4450be7cad6..1d446f0aa59 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -842,10 +842,6 @@ namespace ts.Completions { const lineStart = getLineStartPositionForPosition(position, sourceFile); shouldAppendAtSignBeforeJsDocTagName = sourceFile.text.substr(lineStart, position).indexOf("@") === -1; - // This is for the case - // /** - // * |completion here| - // **/ if (shouldAppendAtSignBeforeJsDocTagName) { isJsDocTagName = true; } From 9bef19c54c629e8b4a109706ef8bfa118afd66f9 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Wed, 1 Mar 2017 09:19:47 -0800 Subject: [PATCH 4/7] Fix JsDoc tagname in tests --- tests/cases/fourslash/completionInJsDoc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cases/fourslash/completionInJsDoc.ts b/tests/cases/fourslash/completionInJsDoc.ts index 5eca52743a4..cf8a448bf19 100644 --- a/tests/cases/fourslash/completionInJsDoc.ts +++ b/tests/cases/fourslash/completionInJsDoc.ts @@ -71,7 +71,7 @@ verify.completionListContains("@argument"); goTo.marker('10'); verify.completionListCount(40); -verify.completionListContains("@return"); +verify.completionListContains("@returns"); goTo.marker('11'); verify.completionListCount(40); From b5c6221e36c1e4d20300b07ebefcdcab9121b57c Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Wed, 1 Mar 2017 17:46:35 -0800 Subject: [PATCH 5/7] Address PR --- src/services/completions.ts | 56 +++++++++++++++++++++++-------------- src/services/jsDoc.ts | 43 ++++++++++++++-------------- 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 1d446f0aa59..042312c1240 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -16,11 +16,16 @@ namespace ts.Completions { return undefined; } - const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isJsDocTagName, shouldAppendAtSignBeforeJsDocTagName } = completionData; + const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, requestJsDocTagName, requestJsDocTag } = completionData; - if (isJsDocTagName) { + 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.getAllJsDocCompletionEntries(shouldAppendAtSignBeforeJsDocTagName) }; + 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() }; } const entries: CompletionEntry[] = []; @@ -54,7 +59,7 @@ namespace ts.Completions { } // Add keywords if this is not a member completion list - if (!isMemberCompletion && !isJsDocTagName) { + if (!isMemberCompletion && !(requestJsDocTag && requestJsDocTagName)) { addRange(entries, keywordCompletions); } @@ -814,13 +819,10 @@ namespace ts.Completions { function getCompletionData(typeChecker: TypeChecker, log: (message: string) => void, sourceFile: SourceFile, position: number) { const isJavaScriptFile = isSourceFileJavaScript(sourceFile); - let isJsDocTagName = false; - // This is for the case when users request completion in JsDoc without "@" - // i.e. - // /** - // * |completion here| - // **/ - let shouldAppendAtSignBeforeJsDocTagName = false; + // 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 start = timestamp(); const currentToken = getTokenAtPosition(sourceFile, position); @@ -836,15 +838,27 @@ namespace ts.Completions { // 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) { - isJsDocTagName = true; + requestJsDocTagName = true; } else { + // When completion is requested without "@", we will have check to make sure that + // there are no comments prefix the request position. We will only allow "*" and space. + // e.g + // /** |c| /* + // + // /** + // |c| + // */ + // + // /** + // * |c| + // */ + // + // /** + // * |c| + // */ const lineStart = getLineStartPositionForPosition(position, sourceFile); - shouldAppendAtSignBeforeJsDocTagName = sourceFile.text.substr(lineStart, position).indexOf("@") === -1; - - if (shouldAppendAtSignBeforeJsDocTagName) { - isJsDocTagName = true; - } + requestJsDocTag = !(sourceFile.text.substr(lineStart, position).match(/[^\*|\s|(/\*\*)]/)); } } @@ -855,7 +869,7 @@ namespace ts.Completions { const tag = getJsDocTagAtPosition(sourceFile, position); if (tag) { if (tag.tagName.pos <= position && position <= tag.tagName.end) { - isJsDocTagName = true; + requestJsDocTagName = true; } switch (tag.kind) { @@ -870,8 +884,8 @@ namespace ts.Completions { } } - if (isJsDocTagName || shouldAppendAtSignBeforeJsDocTagName) { - return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, isJsDocTagName, shouldAppendAtSignBeforeJsDocTagName }; + if (requestJsDocTagName || requestJsDocTag) { + return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, requestJsDocTagName, requestJsDocTag }; } if (!insideJsDocTagExpression) { @@ -999,7 +1013,7 @@ namespace ts.Completions { log("getCompletionData: Semantic work: " + (timestamp() - semanticStart)); - return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), isJsDocTagName, shouldAppendAtSignBeforeJsDocTagName }; + return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), requestJsDocTagName, requestJsDocTag }; function getTypeScriptMemberSymbols(): void { // Right of dot member completion list diff --git a/src/services/jsDoc.ts b/src/services/jsDoc.ts index 40ee276b0fa..59c6ad4b03b 100644 --- a/src/services/jsDoc.ts +++ b/src/services/jsDoc.ts @@ -43,7 +43,7 @@ namespace ts.JsDoc { "version" ]; let jsDocTagNameCompletionEntries: CompletionEntry[]; - let jsDocTagNameWithAtSignCompletionEntries: CompletionEntry[]; + let jsDocTagCompletionEntries: CompletionEntry[]; export function getJsDocCommentsFromDeclarations(declarations: Declaration[]) { // Only collect doc comments from duplicate declarations once: @@ -89,27 +89,26 @@ namespace ts.JsDoc { return undefined; } - export function getAllJsDocCompletionEntries(shouldAppendAtSign: boolean): CompletionEntry[] { - if (!shouldAppendAtSign) { - return jsDocTagNameCompletionEntries || (jsDocTagNameCompletionEntries = ts.map(jsDocTagNames, tagName => { - return { - name: tagName, - kind: ScriptElementKind.keyword, - kindModifiers: "", - sortText: "0", - }; - })); - } - else { - return jsDocTagNameWithAtSignCompletionEntries || (jsDocTagNameWithAtSignCompletionEntries = ts.map(jsDocTagNames, tagName => { - return { - name: `@${tagName}`, - kind: ScriptElementKind.keyword, - kindModifiers: "", - sortText: "0" - } - })); - } + export function getJSDocTagNameCompletions(): CompletionEntry[] { + return jsDocTagNameCompletionEntries || (jsDocTagNameCompletionEntries = ts.map(jsDocTagNames, tagName => { + return { + name: tagName, + kind: ScriptElementKind.keyword, + kindModifiers: "", + sortText: "0", + }; + })); + } + + export function getJSDocTagCompletions(): CompletionEntry[] { + return jsDocTagCompletionEntries || (jsDocTagCompletionEntries = ts.map(jsDocTagNames, tagName => { + return { + name: `@${tagName}`, + kind: ScriptElementKind.keyword, + kindModifiers: "", + sortText: "0" + } + })); } /** From 34b68095d213ed14a718afbb3074f73576d70189 Mon Sep 17 00:00:00 2001 From: Kanchalai Tanglertsampan Date: Wed, 1 Mar 2017 17:46:46 -0800 Subject: [PATCH 6/7] Add more tests --- tests/cases/fourslash/completionInJsDoc.ts | 30 ++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/cases/fourslash/completionInJsDoc.ts b/tests/cases/fourslash/completionInJsDoc.ts index cf8a448bf19..3fd92f42d52 100644 --- a/tests/cases/fourslash/completionInJsDoc.ts +++ b/tests/cases/fourslash/completionInJsDoc.ts @@ -36,6 +36,22 @@ //// * /*11*/ //// */ +//// /** +//// /*12*/ +//// */ + +//// /** +//// * /*13*/ +//// */ + +//// /** +//// * some comment /*14*/ +//// */ + +//// /** +//// * @param /*15*/ +//// */ + goTo.marker('1'); verify.completionListContains("constructor"); verify.completionListContains("param"); @@ -76,3 +92,17 @@ verify.completionListContains("@returns"); goTo.marker('11'); verify.completionListCount(40); verify.completionListContains("@argument"); + +goTo.marker('12'); +verify.completionListCount(40); +verify.completionListContains("@constructor"); + +goTo.marker('13'); +verify.completionListCount(40); +verify.completionListContains("@param"); + +goTo.marker('14'); +verify.completionListIsEmpty(); + +goTo.marker('15'); +verify.completionListIsEmpty(); \ No newline at end of file From da51f396955ffa416e37e985fc33f4cf35aebfe6 Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 1 Mar 2017 21:11:34 -0800 Subject: [PATCH 7/7] FIx minor stuffs --- src/services/completions.ts | 4 ++-- tests/cases/fourslash/completionInJsDoc.ts | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 042312c1240..6a4ee3485de 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -59,7 +59,7 @@ namespace ts.Completions { } // Add keywords if this is not a member completion list - if (!isMemberCompletion && !(requestJsDocTag && requestJsDocTagName)) { + if (!isMemberCompletion && !requestJsDocTag && !requestJsDocTagName) { addRange(entries, keywordCompletions); } @@ -858,7 +858,7 @@ namespace ts.Completions { // * |c| // */ const lineStart = getLineStartPositionForPosition(position, sourceFile); - requestJsDocTag = !(sourceFile.text.substr(lineStart, position).match(/[^\*|\s|(/\*\*)]/)); + requestJsDocTag = !(sourceFile.text.substring(lineStart, position).match(/[^\*|\s|(/\*\*)]/)); } } diff --git a/tests/cases/fourslash/completionInJsDoc.ts b/tests/cases/fourslash/completionInJsDoc.ts index 3fd92f42d52..60707905956 100644 --- a/tests/cases/fourslash/completionInJsDoc.ts +++ b/tests/cases/fourslash/completionInJsDoc.ts @@ -35,22 +35,24 @@ //// /** //// * /*11*/ //// */ - +//// //// /** //// /*12*/ //// */ - +//// //// /** //// * /*13*/ //// */ - +//// //// /** //// * some comment /*14*/ //// */ - +//// //// /** //// * @param /*15*/ //// */ +//// +//// /** @param /*16*/ */ goTo.marker('1'); verify.completionListContains("constructor"); @@ -105,4 +107,7 @@ goTo.marker('14'); verify.completionListIsEmpty(); goTo.marker('15'); +verify.completionListIsEmpty(); + +goTo.marker('16'); verify.completionListIsEmpty(); \ No newline at end of file