Skip to content

Parse comment on @property tag and use as documentation comment #21119

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
4 commits merged into from
Jan 11, 2018
Merged

Parse comment on @property tag and use as documentation comment #21119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ab446c
Parse comment on @property tag and use as documentation comment
Jan 9, 2018
5181017
Fix comment parsing bug -- back up after seeing `@` character
Jan 10, 2018
a15127f
Add test for indent
Jan 10, 2018
28aa3f1
Don't default comment to ""
Jan 11, 2018
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
46 changes: 23 additions & 23 deletions src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6268,7 +6268,6 @@ namespace ts {
scanner.scanRange(start + 3, length - 5, () => {
// Initially we can parse out a tag. We also have seen a starting asterisk.
// This is so that /** * @type */ doesn't parse.
let advanceToken = true;
let state = JSDocState.SawAsterisk;
let margin: number | undefined = undefined;
// + 4 for leading '/** '
Expand Down Expand Up @@ -6300,7 +6299,6 @@ namespace ts {
// Real-world comments may break this rule, so "BeginningOfLine" will not be a real line beginning
// for malformed examples like `/** @param {string} x @returns {number} the length */`
state = JSDocState.BeginningOfLine;
advanceToken = false;
margin = undefined;
indent++;
}
Expand Down Expand Up @@ -6352,13 +6350,7 @@ namespace ts {
pushComment(scanner.getTokenText());
break;
}
if (advanceToken) {
t = nextJSDocToken();
}
else {
advanceToken = true;
t = currentToken as JsDocSyntaxKind;
}
t = nextJSDocToken();
}
removeLeadingNewlines(comments);
removeTrailingNewlines(comments);
Expand Down Expand Up @@ -6454,10 +6446,11 @@ namespace ts {
// a badly malformed tag should not be added to the list of tags
return;
}
addTag(tag, parseTagComments(indent + tag.end - tag.pos));
tag.comment = parseTagComments(indent + tag.end - tag.pos);
addTag(tag);
}

function parseTagComments(indent: number) {
function parseTagComments(indent: number): string | undefined {
const comments: string[] = [];
let state = JSDocState.BeginningOfLine;
let margin: number | undefined;
Expand All @@ -6479,6 +6472,8 @@ namespace ts {
indent = 0;
break;
case SyntaxKind.AtToken:
scanner.setTextPos(scanner.getTextPos() - 1);
// falls through
case SyntaxKind.EndOfFileToken:
// Done
break loop;
Expand Down Expand Up @@ -6514,7 +6509,7 @@ namespace ts {

removeLeadingNewlines(comments);
removeTrailingNewlines(comments);
return comments;
return comments.length === 0 ? undefined : comments.join("");
}

function parseUnknownTag(atToken: AtToken, tagName: Identifier) {
Expand All @@ -6524,9 +6519,7 @@ namespace ts {
return finishNode(result);
}

function addTag(tag: JSDocTag, comments: string[]): void {
tag.comment = comments.join("");

function addTag(tag: JSDocTag): void {
if (!tags) {
tags = [tag];
tagsPos = tag.pos;
Expand Down Expand Up @@ -6571,9 +6564,7 @@ namespace ts {
}
}

function parseParameterOrPropertyTag(atToken: AtToken, tagName: Identifier, target: PropertyLikeParse.Parameter): JSDocParameterTag;
function parseParameterOrPropertyTag(atToken: AtToken, tagName: Identifier, target: PropertyLikeParse.Property): JSDocPropertyTag;
function parseParameterOrPropertyTag(atToken: AtToken, tagName: Identifier, target: PropertyLikeParse): JSDocPropertyLikeTag {
function parseParameterOrPropertyTag(atToken: AtToken, tagName: Identifier, target: PropertyLikeParse): JSDocParameterTag | JSDocPropertyTag {
let typeExpression = tryParseTypeExpression();
let isNameFirst = !typeExpression;
skipWhitespace();
Expand All @@ -6585,7 +6576,7 @@ namespace ts {
typeExpression = tryParseTypeExpression();
}

const result: JSDocPropertyLikeTag = target === PropertyLikeParse.Parameter ?
const result = target === PropertyLikeParse.Parameter ?
<JSDocParameterTag>createNode(SyntaxKind.JSDocParameterTag, atToken.pos) :
<JSDocPropertyTag>createNode(SyntaxKind.JSDocPropertyTag, atToken.pos);
const nestedTypeLiteral = parseNestedTypeLiteral(typeExpression, name);
Expand All @@ -6600,7 +6591,6 @@ namespace ts {
result.isNameFirst = isNameFirst;
result.isBracketed = isBracketed;
return finishNode(result);

}

function parseNestedTypeLiteral(typeExpression: JSDocTypeExpression, name: EntityName) {
Expand Down Expand Up @@ -6829,18 +6819,28 @@ namespace ts {
if (!tagName) {
return false;
}
let t: PropertyLikeParse;
switch (tagName.escapedText) {
case "type":
return target === PropertyLikeParse.Property && parseTypeTag(atToken, tagName);
case "prop":
case "property":
return target === PropertyLikeParse.Property && parseParameterOrPropertyTag(atToken, tagName, target);
t = PropertyLikeParse.Property;
break;
case "arg":
case "argument":
case "param":
return target === PropertyLikeParse.Parameter && parseParameterOrPropertyTag(atToken, tagName, target);
t = PropertyLikeParse.Parameter;
break;
default:
return false;
}
return false;
if (target !== t) {
return false;
}
const tag = parseParameterOrPropertyTag(atToken, tagName, target);
tag.comment = parseTagComments(tag.end - tag.pos);
Copy link
Member

Choose a reason for hiding this comment

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

just to make sure I'm not missing something: This is the only line required to make the tests pass, right? The rest looks like refactoring.

Copy link
Member

Choose a reason for hiding this comment

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

why isn't isn't indent needed to get the correct offset, like with the other call to parseTagComments? Does tryParseChildTag need to track indent?

Copy link
Author

Choose a reason for hiding this comment

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

I needed to make a few other changes to parser.ts to avoid skipping the @ token at the end of a comment and to use undefined instead of "" for the comment.

Copy link
Author

Choose a reason for hiding this comment

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

It looks like it will be needed to get the best comment text, that will be a bigger change though. Filed #21123

return tag;
}

function parseTemplateTag(atToken: AtToken, tagName: Identifier): JSDocTemplateTag | undefined {
Expand Down
17 changes: 5 additions & 12 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1586,42 +1586,35 @@ namespace ts {
((node as JSDocFunctionType).parameters[0].name as Identifier).escapedText === "new";
}

export function getAllJSDocs(node: Node): (JSDoc | JSDocTag)[] {
Copy link
Member

Choose a reason for hiding this comment

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

this was internal, right? I don't see a jsdoc on it, so I assume so.

Copy link
Author

Choose a reason for hiding this comment

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

API tests should ensure we don't delete internal things.

if (isJSDocTypedefTag(node)) {
return [node.parent];
}
return getJSDocCommentsAndTags(node);
}

export function getSourceOfAssignment(node: Node): Node {
function getSourceOfAssignment(node: Node): Node {
return isExpressionStatement(node) &&
node.expression && isBinaryExpression(node.expression) &&
node.expression.operatorToken.kind === SyntaxKind.EqualsToken &&
node.expression.right;
}

export function getSingleInitializerOfVariableStatement(node: Node, child?: Node): Node {
function getSingleInitializerOfVariableStatement(node: Node, child?: Node): Node {
return isVariableStatement(node) &&
node.declarationList.declarations.length > 0 &&
(!child || node.declarationList.declarations[0].initializer === child) &&
node.declarationList.declarations[0].initializer;
}

export function getSingleVariableOfVariableStatement(node: Node, child?: Node): Node {
function getSingleVariableOfVariableStatement(node: Node, child?: Node): Node {
return isVariableStatement(node) &&
node.declarationList.declarations.length > 0 &&
(!child || node.declarationList.declarations[0] === child) &&
node.declarationList.declarations[0];
}

export function getNestedModuleDeclaration(node: Node): Node {
function getNestedModuleDeclaration(node: Node): Node {
return node.kind === SyntaxKind.ModuleDeclaration &&
(node as ModuleDeclaration).body &&
(node as ModuleDeclaration).body.kind === SyntaxKind.ModuleDeclaration &&
(node as ModuleDeclaration).body;
}

export function getJSDocCommentsAndTags(node: Node): (JSDoc | JSDocTag)[] {
export function getJSDocCommentsAndTags(node: Node): ReadonlyArray<JSDoc | JSDocTag> {
let result: (JSDoc | JSDocTag)[] | undefined;
getJSDocCommentsAndTagsWorker(node);
return result || emptyArray;
Expand Down
36 changes: 25 additions & 11 deletions src/services/jsDoc.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,30 @@ namespace ts.JsDoc {
// Eg. const a: Array<string> | Array<number>; a.length
// The property length will have two declarations of property length coming
// from Array<T> - Array<string> and Array<number>
const documentationComment = <SymbolDisplayPart[]>[];
const documentationComment: SymbolDisplayPart[] = [];
forEachUnique(declarations, declaration => {
forEach(getAllJSDocs(declaration), doc => {
if (doc.comment) {
if (documentationComment.length) {
documentationComment.push(lineBreakPart());
}
documentationComment.push(textPart(doc.comment));
for (const { comment } of getCommentHavingNodes(declaration)) {
if (comment === undefined) continue;
if (documentationComment.length) {
documentationComment.push(lineBreakPart());
}
});
documentationComment.push(textPart(comment));
}
});
return documentationComment;
}

function getCommentHavingNodes(declaration: Declaration): ReadonlyArray<JSDoc | JSDocTag> {
switch (declaration.kind) {
case SyntaxKind.JSDocPropertyTag:
return [declaration as JSDocPropertyTag];
case SyntaxKind.JSDocTypedefTag:
return [(declaration as JSDocTypedefTag).parent];
default:
return getJSDocCommentsAndTags(declaration);
}
}

export function getJsDocTagsFromDeclarations(declarations?: Declaration[]): JSDocTagInfo[] {
// Only collect doc comments from duplicate declarations once.
const tags: JSDocTagInfo[] = [];
Expand All @@ -77,7 +87,7 @@ namespace ts.JsDoc {
return tags;
}

function getCommentText(tag: JSDocTag): string {
function getCommentText(tag: JSDocTag): string | undefined {
const { comment } = tag;
switch (tag.kind) {
case SyntaxKind.JSDocAugmentsTag:
Expand All @@ -96,11 +106,15 @@ namespace ts.JsDoc {
}

function withNode(node: Node) {
return `${node.getText()} ${comment}`;
return addComment(node.getText());
}

function withList(list: NodeArray<Node>): string {
return `${list.map(x => x.getText())} ${comment}`;
return addComment(list.map(x => x.getText()).join(", "));
}

function addComment(s: string) {
return comment === undefined ? s : `${s} ${comment}`;
}
}

Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.leadingAsterisk.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@
"pos": 15,
"end": 21
}
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.noLeadingAsterisk.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@
"pos": 15,
"end": 21
}
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.noReturnType.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
"pos": 9,
"end": 15,
"escapedText": "return"
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.oneParamTag.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@
"escapedText": "name1"
},
"isNameFirst": false,
"isBracketed": false,
"comment": ""
"isBracketed": false
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions ...s/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@
"escapedText": "name1"
},
"isNameFirst": true,
"isBracketed": false,
"comment": ""
"isBracketed": false
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramWithoutType.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@
"escapedText": "foo"
},
"isNameFirst": true,
"isBracketed": false,
"comment": ""
"isBracketed": false
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.returnTag1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@
"pos": 17,
"end": 23
}
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.returnsTag1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@
"pos": 18,
"end": 24
}
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@
"length": 1,
"pos": 18,
"end": 20
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@
"length": 2,
"pos": 18,
"end": 22
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag3.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@
"length": 2,
"pos": 18,
"end": 23
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag4.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@
"length": 2,
"pos": 18,
"end": 23
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
3 changes: 1 addition & 2 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.templateTag5.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@
"length": 2,
"pos": 18,
"end": 24
},
"comment": ""
}
},
"length": 1,
"pos": 8,
Expand Down
6 changes: 2 additions & 4 deletions tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTag2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@
"escapedText": "name1"
},
"isNameFirst": false,
"isBracketed": false,
"comment": ""
"isBracketed": false
},
"1": {
"kind": "JSDocParameterTag",
Expand Down Expand Up @@ -70,8 +69,7 @@
"escapedText": "name2"
},
"isNameFirst": false,
"isBracketed": false,
"comment": ""
"isBracketed": false
},
"length": 2,
"pos": 8,
Expand Down
Loading