Skip to content

Merge JSDoc of assignments from function expressions #9010

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.

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 4 commits into from
Jun 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -603,9 +603,11 @@ namespace ts {
}

export function getJsDocCommentsFromText(node: Node, text: string) {
const commentRanges = (node.kind === SyntaxKind.Parameter || node.kind === SyntaxKind.TypeParameter) ?
concatenate(getTrailingCommentRanges(text, node.pos),
getLeadingCommentRanges(text, node.pos)) :
const commentRanges = (node.kind === SyntaxKind.Parameter ||
node.kind === SyntaxKind.TypeParameter ||
node.kind === SyntaxKind.FunctionExpression ||
node.kind === SyntaxKind.ArrowFunction) ?
concatenate(getTrailingCommentRanges(text, node.pos), getLeadingCommentRanges(text, node.pos)) :
getLeadingCommentRangesOfNodeFromText(node, text);
return filter(commentRanges, isJsDocComment);

Expand Down
47 changes: 33 additions & 14 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,37 +406,56 @@ namespace ts {
const sourceFileOfDeclaration = getSourceFileOfNode(declaration);
// If it is parameter - try and get the jsDoc comment with @param tag from function declaration's jsDoc comments
if (canUseParsedParamTagComments && declaration.kind === SyntaxKind.Parameter) {
ts.forEach(getJsDocCommentTextRange(declaration.parent, sourceFileOfDeclaration), jsDocCommentTextRange => {
const cleanedParamJsDocComment = getCleanedParamJsDocComment(jsDocCommentTextRange.pos, jsDocCommentTextRange.end, sourceFileOfDeclaration);
if (cleanedParamJsDocComment) {
addRange(jsDocCommentParts, cleanedParamJsDocComment);
}
});
if ((declaration.parent.kind === SyntaxKind.FunctionExpression || declaration.parent.kind === SyntaxKind.ArrowFunction) &&
declaration.parent.parent.kind === SyntaxKind.VariableDeclaration) {
addCommentParts(declaration.parent.parent.parent, sourceFileOfDeclaration, getCleanedParamJsDocComment);
}
addCommentParts(declaration.parent, sourceFileOfDeclaration, getCleanedParamJsDocComment);
}

// If this is left side of dotted module declaration, there is no doc comments associated with this node
if (declaration.kind === SyntaxKind.ModuleDeclaration && (<ModuleDeclaration>declaration).body.kind === SyntaxKind.ModuleDeclaration) {
return;
}

if ((declaration.kind === SyntaxKind.FunctionExpression || declaration.kind === SyntaxKind.ArrowFunction) &&
declaration.parent.kind === SyntaxKind.VariableDeclaration) {
addCommentParts(declaration.parent.parent, sourceFileOfDeclaration, getCleanedJsDocComment);
}

// If this is dotted module name, get the doc comments from the parent
while (declaration.kind === SyntaxKind.ModuleDeclaration && declaration.parent.kind === SyntaxKind.ModuleDeclaration) {
declaration = <ModuleDeclaration>declaration.parent;
}
addCommentParts(declaration.kind === SyntaxKind.VariableDeclaration ? declaration.parent.parent : declaration,
sourceFileOfDeclaration,
getCleanedJsDocComment);

// Get the cleaned js doc comment text from the declaration
ts.forEach(getJsDocCommentTextRange(
declaration.kind === SyntaxKind.VariableDeclaration ? declaration.parent.parent : declaration, sourceFileOfDeclaration), jsDocCommentTextRange => {
const cleanedJsDocComment = getCleanedJsDocComment(jsDocCommentTextRange.pos, jsDocCommentTextRange.end, sourceFileOfDeclaration);
if (cleanedJsDocComment) {
addRange(jsDocCommentParts, cleanedJsDocComment);
}
});
if (declaration.kind === SyntaxKind.VariableDeclaration) {
const init = (declaration as VariableDeclaration).initializer;
if (init && (init.kind === SyntaxKind.FunctionExpression || init.kind === SyntaxKind.ArrowFunction)) {
Copy link
Member

@DanielRosenwasser DanielRosenwasser Jun 7, 2016

Choose a reason for hiding this comment

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

There must be a function like isFunctionExpressionLike

Copy link
Member Author

Choose a reason for hiding this comment

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

There is isFunctionLike, but that includes tons of other things like methods. The problem is that we seem to need (almost) all combinations of the function-like kinds, so we'd end up with one function for each, each with its own very-long name.

// Get the cleaned js doc comment text from the initializer
addCommentParts(init, sourceFileOfDeclaration, getCleanedJsDocComment);
}
}
}
});

return jsDocCommentParts;

function addCommentParts(commented: Node,
sourceFileOfDeclaration: SourceFile,
getCommentPart: (pos: number, end: number, file: SourceFile) => SymbolDisplayPart[]): void {
const ranges = getJsDocCommentTextRange(commented, sourceFileOfDeclaration);
// Get the cleaned js doc comment text from the declaration
ts.forEach(ranges, jsDocCommentTextRange => {
const cleanedComment = getCommentPart(jsDocCommentTextRange.pos, jsDocCommentTextRange.end, sourceFileOfDeclaration);
if (cleanedComment) {
addRange(jsDocCommentParts, cleanedComment);
}
});
}

function getJsDocCommentTextRange(node: Node, sourceFile: SourceFile): TextRange[] {
return ts.map(getJsDocComments(node, sourceFile),
jsDocComment => {
Expand Down
131 changes: 0 additions & 131 deletions tests/cases/fourslash/commentsFunction.ts

This file was deleted.

61 changes: 61 additions & 0 deletions tests/cases/fourslash/commentsFunctionDeclaration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/// <reference path='fourslash.ts' />

/////** This comment should appear for foo*/
////function f/*1*/oo() {
////}
////f/*2*/oo/*3*/(/*4*/);
/////** This is comment for function signature*/
////function fo/*5*/oWithParameters(/** this is comment about a*/a: string,
//// /** this is comment for b*/
//// b: number) {
//// var /*6*/d = /*7*/a;
////}
////fooWithParam/*8*/eters/*9*/(/*10*/"a",/*11*/10);

// ambient declaration
/////**
////* Does something
////* @param a a string
////*/
////declare function fn(a: string);
////fn(/*12*/"hello");

goTo.marker('1');
verify.quickInfoIs("function foo(): void", "This comment should appear for foo");

goTo.marker('2');
verify.quickInfoIs("function foo(): void", "This comment should appear for foo");

goTo.marker('3');
verify.completionListContains('foo', 'function foo(): void', 'This comment should appear for foo');

goTo.marker('4');
verify.currentSignatureHelpDocCommentIs("This comment should appear for foo");

goTo.marker('5');
verify.quickInfoIs("function fooWithParameters(a: string, b: number): void", "This is comment for function signature");

goTo.marker('6');
verify.quickInfoIs('(local var) d: string', '');

goTo.marker('7');
verify.completionListContains('a', '(parameter) a: string', 'this is comment about a');
verify.completionListContains('b', '(parameter) b: number', 'this is comment for b');

goTo.marker('8');
verify.quickInfoIs("function fooWithParameters(a: string, b: number): void", "This is comment for function signature");

goTo.marker('9');
verify.completionListContains('fooWithParameters', 'function fooWithParameters(a: string, b: number): void', 'This is comment for function signature');

goTo.marker('10');
verify.currentSignatureHelpDocCommentIs("This is comment for function signature");
verify.currentParameterHelpArgumentDocCommentIs("this is comment about a");

goTo.marker('11');
verify.currentSignatureHelpDocCommentIs("This is comment for function signature");
verify.currentParameterHelpArgumentDocCommentIs("this is comment for b");

goTo.marker('12');
verify.currentSignatureHelpDocCommentIs("Does something");
verify.currentParameterHelpArgumentDocCommentIs("a string");
88 changes: 88 additions & 0 deletions tests/cases/fourslash/commentsFunctionExpression.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/// <reference path='fourslash.ts' />

// test arrow doc comments
/////** lamdaFoo var comment*/
////var lamb/*1*/daFoo = /** this is lambda comment*/ (/**param a*/a: number, /**param b*/b: number) => /*2*/a + b;
////var lambddaN/*3*/oVarComment = /** this is lambda multiplication*/ (/**param a*/a: number, /**param b*/b: number) => a * b;
/////*4*/lambdaFoo(/*5*/10, /*6*/20);

// test nested arrow doc comments
////function /*7*/anotherFunc(a: number) {
//// /** documentation
//// @param b {string} inner parameter */
//// var /*8*/lambdaVar = /** inner docs */(/*9*/b: string) => {
//// var /*10*/localVar = "Hello ";
//// return /*11*/localVar + /*12*/b;
//// }
//// return lamb/*13*/daVar("World") + a;
////}

// test function expression doc comments
/////**
//// * On variable
//// * @param s the first parameter!
//// * @returns the parameter's length
//// */
////var assi/*14*/gned = /**
//// * Summary on expression
//// * @param s param on expression
//// * @returns return on expression
//// */function(/** On parameter */s: string) {
//// return /*15*/s.length;
////}
////assig/*16*/ned/*17*/(/*18*/"hey");



goTo.marker('1');
verify.quickInfoIs("var lambdaFoo: (a: number, b: number) => number", "lamdaFoo var comment\nthis is lambda comment");

goTo.marker('2');
verify.completionListContains('a', '(parameter) a: number', 'param a');
verify.completionListContains('b', '(parameter) b: number', 'param b');

goTo.marker('3');
// pick up doccomments from the lambda itself
verify.quickInfoIs("var lambddaNoVarComment: (a: number, b: number) => number", "this is lambda multiplication");

goTo.marker('4');
verify.completionListContains('lambdaFoo', 'var lambdaFoo: (a: number, b: number) => number', 'lamdaFoo var comment\nthis is lambda comment');
verify.completionListContains('lambddaNoVarComment', 'var lambddaNoVarComment: (a: number, b: number) => number', 'this is lambda multiplication');

goTo.marker('5');
verify.currentParameterHelpArgumentDocCommentIs("param a");

goTo.marker('6');
verify.currentParameterHelpArgumentDocCommentIs("param b");




goTo.marker('7');
// no documentation from nested lambda
verify.quickInfoIs('function anotherFunc(a: number): string', '');
goTo.marker('8');
verify.quickInfoIs('(local var) lambdaVar: (b: string) => string', 'documentation\ninner docs ');
goTo.marker('9');
verify.quickInfoIs('(parameter) b: string', '{string} inner parameter ');
goTo.marker('10');
verify.quickInfoIs('(local var) localVar: string', '');
goTo.marker('11');
verify.quickInfoIs('(local var) localVar: string', '');
goTo.marker('12');
verify.quickInfoIs('(parameter) b: string', '{string} inner parameter ');
goTo.marker('13');
verify.quickInfoIs('(local var) lambdaVar: (b: string) => string', 'documentation\ninner docs ');

goTo.marker('14');
verify.quickInfoIs("var assigned: (s: string) => number", "On variable\n@returns the parameter's length\nSummary on expression\n@returns return on expression");
goTo.marker('15');
verify.completionListContains('s', '(parameter) s: string', "the first parameter!\nparam on expression\nOn parameter ");
goTo.marker('16');
verify.quickInfoIs("var assigned: (s: string) => number", "On variable\n@returns the parameter's length\nSummary on expression\n@returns return on expression");
goTo.marker('17');
verify.completionListContains("assigned", "var assigned: (s: string) => number", "On variable\n@returns the parameter's length\nSummary on expression\n@returns return on expression");
goTo.marker('18');
verify.currentSignatureHelpDocCommentIs("On variable\n@returns the parameter's length\nSummary on expression\n@returns return on expression");
verify.currentParameterHelpArgumentDocCommentIs("the first parameter!\nparam on expression\nOn parameter ");

4 changes: 3 additions & 1 deletion tests/cases/fourslash/getJavaScriptQuickInfo7.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@
//// x - /**/a1()

goTo.marker();
verify.quickInfoExists();
verify.quickInfoExists();
verify.quickInfoIs('function a1(p: any): number',
'This is a very cool function that is very nice.\n@returns something');