From bdba8a301877d00beb29cb3fd7362b603f9d2317 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 27 Sep 2017 15:02:39 -0700 Subject: [PATCH 1/2] Zip @param tags and parameters together instead of looking for a name --- src/compiler/binder.ts | 39 ++++++++++ src/compiler/checker.ts | 20 ++--- src/compiler/types.ts | 1 + src/compiler/utilities.ts | 76 ++++++------------- src/services/jsDoc.ts | 2 +- .../reference/jsdocParamTagMatching1.symbols | 12 +++ .../reference/jsdocParamTagMatching1.types | 12 +++ .../cases/compiler/jsdocParamTagMatching1.ts | 11 +++ .../fourslash/jsDocFunctionSignatures12.ts | 4 +- .../signatureHelpWhenEditingCallExpression.ts | 27 ++----- 10 files changed, 113 insertions(+), 91 deletions(-) create mode 100644 tests/baselines/reference/jsdocParamTagMatching1.symbols create mode 100644 tests/baselines/reference/jsdocParamTagMatching1.types create mode 100644 tests/cases/compiler/jsdocParamTagMatching1.ts diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts index 796c960342384..1a9a4f39485b3 100644 --- a/src/compiler/binder.ts +++ b/src/compiler/binder.ts @@ -677,10 +677,49 @@ namespace ts { break; default: bindEachChild(node); + if (isFunctionLikeDeclaration(node)) { + bindJSDocParameters(node); + } break; } } + function bindJSDocParameters(node: SignatureDeclaration): void { + const { parameters } = node; + if (parameters.length === 0) { + return; + } + + // Clear any previous binding + for (const p of parameters) { + p.jsdocParamTags = undefined; + } + + let paramIndex = 0; + for (const tag of getJSDocTags(parent)) { + if (!isJSDocParameterTag(tag)) { + continue; + } + + // If we run out of parameters, just do nothing. + // Will error in `checkJSDocParameterTag` since it will have no symbol. + const startParamIndex = paramIndex; + do { + const param = parameters[paramIndex]; + paramIndex++; + if (paramIndex === parameters.length) { + paramIndex = 0; + } + + if (param.name.kind !== SyntaxKind.Identifier || getLeftmostName(tag.name).escapedText === param.name.escapedText) { + tag.symbol = param.symbol; + param.jsdocParamTags = append(param.jsdocParamTags, tag); + break; + } + } while (paramIndex !== startParamIndex); + } + } + function isNarrowingExpression(expr: Expression): boolean { switch (expr.kind) { case SyntaxKind.Identifier: diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f6aeec2f70420..267d088c0176e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6268,18 +6268,8 @@ namespace ts { if (node.type && node.type.kind === SyntaxKind.JSDocOptionalType) { return true; } - const paramTags = getJSDocParameterTags(node); - if (paramTags) { - for (const paramTag of paramTags) { - if (paramTag.isBracketed) { - return true; - } - - if (paramTag.typeExpression) { - return paramTag.typeExpression.type.kind === SyntaxKind.JSDocOptionalType; - } - } - } + return some(node.jsdocParamTags, paramTag => + paramTag.isBracketed || paramTag.typeExpression && paramTag.typeExpression.type.kind === SyntaxKind.JSDocOptionalType); } } @@ -19789,10 +19779,10 @@ namespace ts { function checkJSDocParameterTag(node: JSDocParameterTag) { checkSourceElement(node.typeExpression); - if (!getParameterSymbolFromJSDoc(node)) { + if (!node.symbol) { error(node.name, Diagnostics.JSDoc_param_tag_has_name_0_but_there_is_no_parameter_with_that_name, - unescapeLeadingUnderscores((node.name.kind === SyntaxKind.QualifiedName ? node.name.right : node.name).escapedText)); + idText(getLeftmostName(node.name))); } } @@ -23014,7 +23004,7 @@ namespace ts { } if (entityName.parent!.kind === SyntaxKind.JSDocParameterTag) { - return getParameterSymbolFromJSDoc(entityName.parent as JSDocParameterTag); + return entityName.parent.symbol; } if (entityName.parent.kind === SyntaxKind.TypeParameter && entityName.parent.parent.kind === SyntaxKind.JSDocTemplateTag) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 1f27edcaf89d9..e8ad6614753e4 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -742,6 +742,7 @@ namespace ts { questionToken?: QuestionToken; // Present on optional parameter type?: TypeNode; // Optional type annotation initializer?: Expression; // Optional initializer + /* @internal */ jsdocParamTags?: JSDocParameterTag[]; } export interface BindingElement extends NamedDeclaration { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 23888f0b6d566..1680e056c2a20 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -1505,11 +1505,17 @@ namespace ts { ((node as JSDocFunctionType).parameters[0].name as Identifier).escapedText === "new"; } - export function getAllJSDocs(node: Node): (JSDoc | JSDocTag)[] { + export function forEachJSDoc(node: Node, cb: (doc: JSDoc | JSDocTag) => void): void { if (isJSDocTypedefTag(node)) { - return [node.parent]; + cb(node.parent); + return; + } + if (isParameter(node)) { + forEach(node.jsdocParamTags, cb); + } + for (const tag of getJSDocCommentsAndTags(node)) { + cb(tag); } - return getJSDocCommentsAndTags(node); } export function getJSDocCommentsAndTags(node: Node): (JSDoc | JSDocTag)[] { @@ -1556,11 +1562,6 @@ namespace ts { getJSDocCommentsAndTagsWorker(parent); } - // Pull parameter comments from declaring function as well - if (node.kind === SyntaxKind.Parameter) { - result = addRange(result, getJSDocParameterTags(node as ParameterDeclaration)); - } - if (isVariableLike(node) && node.initializer && hasJSDocNodes(node.initializer)) { result = addRange(result, node.initializer.jsDoc); } @@ -1571,24 +1572,6 @@ namespace ts { } } - /** Does the opposite of `getJSDocParameterTags`: given a JSDoc parameter, finds the parameter corresponding to it. */ - export function getParameterSymbolFromJSDoc(node: JSDocParameterTag): Symbol | undefined { - if (node.symbol) { - return node.symbol; - } - if (!isIdentifier(node.name)) { - return undefined; - } - const name = node.name.escapedText; - const func = getJSDocHost(node); - if (!isFunctionLike(func)) { - return undefined; - } - const parameter = find(func.parameters, p => - p.name.kind === SyntaxKind.Identifier && p.name.escapedText === name); - return parameter && parameter.symbol; - } - export function getJSDocHost(node: JSDocTag): HasJSDoc { Debug.assert(node.parent!.kind === SyntaxKind.JSDocComment); return node.parent!.parent!; @@ -1609,17 +1592,12 @@ namespace ts { } export function isRestParameter(node: ParameterDeclaration) { - if (isInJavaScriptFile(node)) { - if (node.type && node.type.kind === SyntaxKind.JSDocVariadicType || - forEach(getJSDocParameterTags(node), - t => t.typeExpression && t.typeExpression.type.kind === SyntaxKind.JSDocVariadicType)) { - return true; - } - } - return isDeclaredRestParam(node); + return isDeclaredRestParam(node) || isInJavaScriptFile(node) && ( + node.type && node.type.kind === SyntaxKind.JSDocVariadicType || + some(node.jsdocParamTags, tag => tag.typeExpression && tag.typeExpression.type.kind === SyntaxKind.JSDocVariadicType)); } - export function isDeclaredRestParam(node: ParameterDeclaration) { + function isDeclaredRestParam(node: ParameterDeclaration) { return node && node.dotDotDotToken !== undefined; } @@ -3499,6 +3477,10 @@ namespace ts { return parent.parent && parent.parent.kind === SyntaxKind.ExpressionStatement ? AccessKind.Write : AccessKind.ReadWrite; } } + + export function getLeftmostName(node: EntityName): Identifier { + return node.kind === SyntaxKind.QualifiedName ? getLeftmostName(node.left) : node; + } } namespace ts { @@ -4057,17 +4039,9 @@ namespace ts { * initializer is the containing function. The tags closest to the * node are returned first, so in the previous example, the param * tag on the containing function expression would be first. - * - * Does not return tags for binding patterns, because JSDoc matches - * parameters by name and binding patterns do not have a name. */ export function getJSDocParameterTags(param: ParameterDeclaration): ReadonlyArray | undefined { - if (param.name && isIdentifier(param.name)) { - const name = param.name.escapedText; - return getJSDocTags(param.parent).filter((tag): tag is JSDocParameterTag => isJSDocParameterTag(tag) && isIdentifier(tag.name) && tag.name.escapedText === name) as JSDocParameterTag[]; - } - // a binding pattern doesn't have a name, so it's not possible to match it a JSDoc parameter, which is identified by name - return undefined; + return param.jsdocParamTags; } /** @@ -4122,15 +4096,13 @@ namespace ts { * tag directly on the node would be returned. */ export function getJSDocType(node: Node): TypeNode | undefined { - let tag: JSDocTypeTag | JSDocParameterTag = getFirstJSDocTag(node, SyntaxKind.JSDocTypeTag) as JSDocTypeTag; - if (!tag && node.kind === SyntaxKind.Parameter) { - const paramTags = getJSDocParameterTags(node as ParameterDeclaration); - if (paramTags) { - tag = find(paramTags, tag => !!tag.typeExpression); - } + const tag = getFirstJSDocTag(node, SyntaxKind.JSDocTypeTag) as JSDocTypeTag; + if (tag) { + return tag.typeExpression && tag.typeExpression.type; + } + else if (node.kind === SyntaxKind.Parameter) { + return forEach((node as ParameterDeclaration).jsdocParamTags, tag => tag.typeExpression && tag.typeExpression.type); } - - return tag && tag.typeExpression && tag.typeExpression.type; } /** diff --git a/src/services/jsDoc.ts b/src/services/jsDoc.ts index a135a9e8eef8e..f475929397096 100644 --- a/src/services/jsDoc.ts +++ b/src/services/jsDoc.ts @@ -53,7 +53,7 @@ namespace ts.JsDoc { // from Array - Array and Array const documentationComment = []; forEachUnique(declarations, declaration => { - forEach(getAllJSDocs(declaration), doc => { + forEachJSDoc(declaration, doc => { if (doc.comment) { if (documentationComment.length) { documentationComment.push(lineBreakPart()); diff --git a/tests/baselines/reference/jsdocParamTagMatching1.symbols b/tests/baselines/reference/jsdocParamTagMatching1.symbols new file mode 100644 index 0000000000000..1da79b29b0083 --- /dev/null +++ b/tests/baselines/reference/jsdocParamTagMatching1.symbols @@ -0,0 +1,12 @@ +=== /a.js === +/** + * @param {{ x: number, y: string }} p + * @param {number} m + */ +function f({ x, y }, m) { +>f : Symbol(f, Decl(a.js, 0, 0)) +>x : Symbol(x, Decl(a.js, 4, 12)) +>y : Symbol(y, Decl(a.js, 4, 15)) +>m : Symbol(m, Decl(a.js, 4, 20)) +} + diff --git a/tests/baselines/reference/jsdocParamTagMatching1.types b/tests/baselines/reference/jsdocParamTagMatching1.types new file mode 100644 index 0000000000000..6e80b68291115 --- /dev/null +++ b/tests/baselines/reference/jsdocParamTagMatching1.types @@ -0,0 +1,12 @@ +=== /a.js === +/** + * @param {{ x: number, y: string }} p + * @param {number} m + */ +function f({ x, y }, m) { +>f : ({ x, y }: { x: number; y: string; }, m: number) => void +>x : number +>y : string +>m : number +} + diff --git a/tests/cases/compiler/jsdocParamTagMatching1.ts b/tests/cases/compiler/jsdocParamTagMatching1.ts new file mode 100644 index 0000000000000..3880161389056 --- /dev/null +++ b/tests/cases/compiler/jsdocParamTagMatching1.ts @@ -0,0 +1,11 @@ +// @allowJs: true +// @checkJs: true +// @noEmit: true +// @Filename: /a.js + +/** + * @param {{ x: number, y: string }} p + * @param {number} m + */ +function f({ x, y }, m) { +} diff --git a/tests/cases/fourslash/jsDocFunctionSignatures12.ts b/tests/cases/fourslash/jsDocFunctionSignatures12.ts index 29a8ac4caeb9c..a24409f882aa1 100644 --- a/tests/cases/fourslash/jsDocFunctionSignatures12.ts +++ b/tests/cases/fourslash/jsDocFunctionSignatures12.ts @@ -9,5 +9,5 @@ ////function f1(o) { //// o/**/; ////} -goTo.marker(); -verify.quickInfoIs("(parameter) o: any"); + +verify.quickInfoAt("", `(parameter) o: any`); diff --git a/tests/cases/fourslash/signatureHelpWhenEditingCallExpression.ts b/tests/cases/fourslash/signatureHelpWhenEditingCallExpression.ts index 61e51d0a4e7c6..a8abce8d70257 100644 --- a/tests/cases/fourslash/signatureHelpWhenEditingCallExpression.ts +++ b/tests/cases/fourslash/signatureHelpWhenEditingCallExpression.ts @@ -1,30 +1,15 @@ /// +// @noLib: true /////** -//// * Returns the substring at the specified location within a String object. -//// * @param start The zero-based index integer indicating the beginning of the substring. -//// * @param end Zero-based index integer indicating the end of the substring. The substring includes the characters up to, but not including, the character indicated by end. -//// * If end is omitted, the characters from start through the end of the original string are returned. -//// */ -////function foo(start: number, end?: number) { +//// * @param start START +//// */ +////function foo(start: number) { //// return ""; ////} //// -////fo/*1*/ +////foo/*1*/ goTo.marker('1'); verify.not.signatureHelpPresent(); -edit.insert("o"); -verify.not.signatureHelpPresent(); edit.insert("("); -verify.currentParameterHelpArgumentDocCommentIs("The zero-based index integer indicating the beginning of the substring."); -edit.insert("10,"); -verify.currentParameterHelpArgumentDocCommentIs("Zero-based index integer indicating the end of the substring. The substring includes the characters up to, but not including, the character indicated by end.\nIf end is omitted, the characters from start through the end of the original string are returned."); -edit.insert(" "); -verify.currentParameterHelpArgumentDocCommentIs("Zero-based index integer indicating the end of the substring. The substring includes the characters up to, but not including, the character indicated by end.\nIf end is omitted, the characters from start through the end of the original string are returned."); -edit.insert(", "); -edit.backspace(3); -verify.currentParameterHelpArgumentDocCommentIs("Zero-based index integer indicating the end of the substring. The substring includes the characters up to, but not including, the character indicated by end.\nIf end is omitted, the characters from start through the end of the original string are returned."); -edit.insert("12"); -verify.currentParameterHelpArgumentDocCommentIs("Zero-based index integer indicating the end of the substring. The substring includes the characters up to, but not including, the character indicated by end.\nIf end is omitted, the characters from start through the end of the original string are returned."); -edit.insert(")"); -verify.not.signatureHelpPresent(); +verify.currentParameterHelpArgumentDocCommentIs("START"); From 95d405216bcf4a9f97a06dd80bead89032816b5d Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 29 Sep 2017 14:39:02 -0700 Subject: [PATCH 2/2] Add test for #17217 --- tests/cases/fourslash/findAllRefsJsDocParam.ts | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 tests/cases/fourslash/findAllRefsJsDocParam.ts diff --git a/tests/cases/fourslash/findAllRefsJsDocParam.ts b/tests/cases/fourslash/findAllRefsJsDocParam.ts new file mode 100644 index 0000000000000..949ba1fafd46a --- /dev/null +++ b/tests/cases/fourslash/findAllRefsJsDocParam.ts @@ -0,0 +1,6 @@ +/// + +/////** @param [|s|] */ +////const x = [|{| "isWriteAccess": true, "isDefinition": true |}s|] => [|s|]; + +verify.singleReferenceGroup("(parameter) s: any");