Skip to content

Use the new JSDoc parser to handle @param tags #6867

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

Closed
wants to merge 2 commits into from
Closed

Use the new JSDoc parser to handle @param tags #6867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
128e043
Use the new JSDoc parser to handle @param tags
RyanCavanaugh Feb 3, 2016
16f9a65
Clean up outdated comments / logic
RyanCavanaugh Feb 3, 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
232 changes: 80 additions & 152 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,68 @@ namespace ts {

return documentationComment;

function getCommentFromJsDocTag(tag: JSDocTag, rangeEnd: number, sourceFile: SourceFile): SymbolDisplayPart {
const text = sourceFile.text;

const parts: string[] = [];
let start = tag.end + 1;

// Eat any leading ignored characters
while (start < rangeEnd) {
const ch = text.charCodeAt(start);
if (ch === CharacterCodes.asterisk || isWhiteSpace(ch)) {
start++;
Copy link
Member

Choose a reason for hiding this comment

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

Adding the below to the if expression gets rid of the leading - in @param comments nicely for me 😄

|| (ch === CharacterCodes.minus && tag.kind === SyntaxKind.JSDocParameterTag)

}
else if (ch === CharacterCodes.at) {
// Most likely a malformed tag
return undefined;
}
else {
break;
}
}

let i: number;
for (i = start; i < rangeEnd; i++) {
const ch = text.charCodeAt(i);
if (isLineBreak(ch)) {
// Line break, push any non-empty content so far
if (start !== i) {
parts.push(text.substr(start, i - start));
}

// Keep scanning until we hit either a non-ws/non-asterisk char
while (i < rangeEnd) {
i++;
const nextCh = text.charCodeAt(i);
if ((nextCh === CharacterCodes.asterisk) || (isWhiteSpace(nextCh) || isLineBreak(nextCh))) {
// Eat *s and whitespace
Copy link
Member

Choose a reason for hiding this comment

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

Just write continue; to make it explicit

continue;
}
else if (nextCh === CharacterCodes.at) {
// Abort, we're running into another malformed jsdoc tag
Copy link
Member

Choose a reason for hiding this comment

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

Malformed is not necessarily true - you could just be in the next JSDoc tag, right?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, never mind. You figure out the start of the next tag later on.

Copy link
Member

Choose a reason for hiding this comment

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

But so does that mean that you'll always cut JSDoc comments off at an @?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only if the @ occurs at the start of a line (which is basically indistinguishable from a malformed comment)

i = rangeEnd;
break;
}
else {
// Scan this
break;
}
}
start = i;
}
}

if (start < rangeEnd) {
parts.push(text.substr(start, rangeEnd - start));
}

return {
kind: "documentation",
text: parts.join("\n")
};
}

function getJsDocCommentsSeparatedByNewLines() {
const paramTag = "@param";
const jsDocCommentParts: SymbolDisplayPart[] = [];
Expand All @@ -391,9 +453,24 @@ namespace ts {
// 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);
// getJsDocCommentTextRange removes leading /* and trailing */, so add those back (-2, +4 net)
const content = parseIsolatedJSDocComment(sourceFileOfDeclaration.text, jsDocCommentTextRange.pos - 2, jsDocCommentTextRange.end - jsDocCommentTextRange.pos + 4);
if (content && content.jsDocComment.tags) {
const tags = content.jsDocComment.tags;

for (let i = 0; i < tags.length; i++) {
const tag = tags[i];
if (tag.kind === SyntaxKind.JSDocParameterTag) {
const tagParamName = (tag as JSDocParameterTag).preParameterName || (tag as JSDocParameterTag).postParameterName;
const nextTagStart = (i === tags.length - 1) ? jsDocCommentTextRange.end : tags[i + 1].pos;
Copy link
Member

Choose a reason for hiding this comment

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

Why even bother with this if you immediately stop at an @? Just use the range end.

Copy link
Member Author

Choose a reason for hiding this comment

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

The @ might not be the range end

if (tagParamName && tagParamName.text === name) {
const comment = getCommentFromJsDocTag(tag, nextTagStart, sourceFileOfDeclaration);
if (comment) {
jsDocCommentParts.push(comment);
}
}
}
}
}
});
}
Expand Down Expand Up @@ -540,155 +617,6 @@ namespace ts {

return docComments;
}

function getCleanedParamJsDocComment(pos: number, end: number, sourceFile: SourceFile) {
let paramHelpStringMargin: number;
const paramDocComments: SymbolDisplayPart[] = [];
while (pos < end) {
if (isParamTag(pos, end, sourceFile)) {
let blankLineCount = 0;
let recordedParamTag = false;
// Consume leading spaces
pos = consumeWhiteSpaces(pos + paramTag.length);
if (pos >= end) {
break;
}

// Ignore type expression
if (sourceFile.text.charCodeAt(pos) === CharacterCodes.openBrace) {
pos++;
for (let curlies = 1; pos < end; pos++) {
const charCode = sourceFile.text.charCodeAt(pos);

// { character means we need to find another } to match the found one
if (charCode === CharacterCodes.openBrace) {
curlies++;
continue;
}

// } char
if (charCode === CharacterCodes.closeBrace) {
curlies--;
if (curlies === 0) {
// We do not have any more } to match the type expression is ignored completely
pos++;
break;
}
else {
// there are more { to be matched with }
continue;
}
}

// Found start of another tag
if (charCode === CharacterCodes.at) {
break;
}
}

// Consume white spaces
pos = consumeWhiteSpaces(pos);
if (pos >= end) {
break;
}
}

// Parameter name
if (isName(pos, end, sourceFile, name)) {
// Found the parameter we are looking for consume white spaces
pos = consumeWhiteSpaces(pos + name.length);
if (pos >= end) {
break;
}

let paramHelpString = "";
const firstLineParamHelpStringPos = pos;
while (pos < end) {
const ch = sourceFile.text.charCodeAt(pos);

// at line break, set this comment line text and go to next line
if (isLineBreak(ch)) {
if (paramHelpString) {
pushDocCommentLineText(paramDocComments, paramHelpString, blankLineCount);
paramHelpString = "";
blankLineCount = 0;
recordedParamTag = true;
}
else if (recordedParamTag) {
blankLineCount++;
}

// Get the pos after cleaning start of the line
setPosForParamHelpStringOnNextLine(firstLineParamHelpStringPos);
continue;
}

// Done scanning param help string - next tag found
if (ch === CharacterCodes.at) {
break;
}

paramHelpString += sourceFile.text.charAt(pos);

// Go to next character
pos++;
}

// If there is param help text, add it top the doc comments
if (paramHelpString) {
pushDocCommentLineText(paramDocComments, paramHelpString, blankLineCount);
}
paramHelpStringMargin = undefined;
}

// If this is the start of another tag, continue with the loop in seach of param tag with symbol name
if (sourceFile.text.charCodeAt(pos) === CharacterCodes.at) {
continue;
}
}

// Next character
pos++;
}

return paramDocComments;

function consumeWhiteSpaces(pos: number) {
while (pos < end && isWhiteSpace(sourceFile.text.charCodeAt(pos))) {
pos++;
}

return pos;
}

function setPosForParamHelpStringOnNextLine(firstLineParamHelpStringPos: number) {
// Get the pos after consuming line breaks
pos = consumeLineBreaks(pos, end, sourceFile);
if (pos >= end) {
return;
}

if (paramHelpStringMargin === undefined) {
paramHelpStringMargin = sourceFile.getLineAndCharacterOfPosition(firstLineParamHelpStringPos).character;
}

// Now consume white spaces max
const startOfLinePos = pos;
pos = consumeWhiteSpacesOnTheLine(pos, end, sourceFile, paramHelpStringMargin);
if (pos >= end) {
return;
}

const consumedSpaces = pos - startOfLinePos;
if (consumedSpaces < paramHelpStringMargin) {
const ch = sourceFile.text.charCodeAt(pos);
if (ch === CharacterCodes.asterisk) {
// Consume more spaces after asterisk
pos = consumeWhiteSpacesOnTheLine(pos + 1, end, sourceFile, paramHelpStringMargin - consumedSpaces - 1);
}
}
}
}
}
}

Expand Down
16 changes: 8 additions & 8 deletions tests/cases/fourslash/commentsCommentParsing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,9 @@ verify.quickInfoIs("(parameter) b: number", "");

goTo.marker('21');
verify.currentSignatureHelpDocCommentIs("This is multiplication function\n@anotherTag\n@anotherTag");
verify.currentParameterHelpArgumentDocCommentIs("{");
verify.currentParameterHelpArgumentDocCommentIs("");
goTo.marker('21aq');
verify.quickInfoIs("(parameter) c: number", "{");
verify.quickInfoIs("(parameter) c: number", "");

goTo.marker('22');
verify.currentSignatureHelpDocCommentIs("This is multiplication function\n@anotherTag\n@anotherTag");
Expand Down Expand Up @@ -372,9 +372,9 @@ verify.quickInfoIs("(parameter) c: () => string", "this is optional param c");

goTo.marker('31');
verify.currentSignatureHelpDocCommentIs("This is subtract function");
verify.currentParameterHelpArgumentDocCommentIs("");
verify.currentParameterHelpArgumentDocCommentIs("this is optional param d");
goTo.marker('31aq');
verify.quickInfoIs("(parameter) d: () => string", "");
verify.quickInfoIs("(parameter) d: () => string", "this is optional param d");

goTo.marker('32');
verify.currentSignatureHelpDocCommentIs("This is subtract function");
Expand All @@ -384,9 +384,9 @@ verify.quickInfoIs("(parameter) e: () => string", "this is optional param e");

goTo.marker('33');
verify.currentSignatureHelpDocCommentIs("This is subtract function");
verify.currentParameterHelpArgumentDocCommentIs("");
verify.currentParameterHelpArgumentDocCommentIs("this is optional param f");
goTo.marker('33aq');
verify.quickInfoIs("(parameter) f: () => string", "");
verify.quickInfoIs("(parameter) f: () => string", "this is optional param f");

goTo.marker('34');
verify.currentSignatureHelpDocCommentIs("this is square function\n@paramTag { number } a this is input number of paramTag\n@returnType { number } it is return type");
Expand Down Expand Up @@ -481,9 +481,9 @@ verify.quickInfoIs("(parameter) a: string", "this is info about a\nspanning on t

goTo.marker('48');
verify.currentSignatureHelpDocCommentIs("This is function comment\n And aligned with 4 space char margin");
verify.currentParameterHelpArgumentDocCommentIs("this is info about b\nspanning on two lines and aligned perfectly\nspanning one more line alined perfectly\n spanning another line with more margin");
verify.currentParameterHelpArgumentDocCommentIs("this is info about b\nspanning on two lines and aligned perfectly\nspanning one more line alined perfectly\nspanning another line with more margin");
goTo.marker('48aq');
verify.quickInfoIs("(parameter) b: any", "this is info about b\nspanning on two lines and aligned perfectly\nspanning one more line alined perfectly\n spanning another line with more margin");
verify.quickInfoIs("(parameter) b: any", "this is info about b\nspanning on two lines and aligned perfectly\nspanning one more line alined perfectly\nspanning another line with more margin");

goTo.marker('49');
verify.currentSignatureHelpDocCommentIs("This is function comment\n And aligned with 4 space char margin");
Expand Down
8 changes: 4 additions & 4 deletions tests/cases/fourslash/commentsLinePreservation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ verify.quickInfoIs(undefined, "");
goTo.marker('f');
verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@random tag This should be third line");
goTo.marker('3');
verify.quickInfoIs(undefined, "first line of param\n\nparam information third line");
verify.quickInfoIs(undefined, "first line of param\nparam information third line");

goTo.marker('g');
verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@random tag This should be third line");
Expand All @@ -138,17 +138,17 @@ verify.quickInfoIs(undefined, "param information first line");
goTo.marker('h');
verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@random tag This should be third line");
goTo.marker('5');
verify.quickInfoIs(undefined, "param information first line\n\nparam information third line");
verify.quickInfoIs(undefined, "param information first line\nparam information third line");

goTo.marker('i');
verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line");
goTo.marker('6');
verify.quickInfoIs(undefined, "param information first line\n\nparam information third line");
verify.quickInfoIs(undefined, "param information first line\nparam information third line");

goTo.marker('j');
verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line");
goTo.marker('7');
verify.quickInfoIs(undefined, "param information first line\n\nparam information third line");
verify.quickInfoIs(undefined, "param information first line\nparam information third line");

goTo.marker('k');
verify.quickInfoIs(undefined, "This is firstLine\nThis is second Line\n@randomtag \n\n random information first line\n\n random information third line");
Expand Down
20 changes: 20 additions & 0 deletions tests/cases/fourslash/jsDocFunctionSignatures1.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
///<reference path="fourslash.ts" />

// @allowNonTsExtensions: true
// @Filename: Foo.js

//// /**
//// * @param {string} p1 parameter 1
//// * @param {string?} p2 parameter 2
//// * @param {string} [p3] parameter 3
//// * @param {string} [p4="test"] parameter 4
//// */
//// function f1(p1, p2, p3, p4) {
//// }
//// f1(''/*1*/, ''/*2*/, ''/*3*/, ''/*4*/)


for(const m of ['1', '2', '3', '4']) {
goTo.marker(m);
verify.currentParameterHelpArgumentDocCommentIs('parameter ' + m);
}