Parse comment on @property tag and use as documentation comment #21119
Conversation
|
@sandersn can you please review this change. |
| //// */ | ||
| //// | ||
| /////** @type {I} */ | ||
| ////const obj = { /**/x: 10 }; |
There was a problem hiding this comment.
what about obj.x/**/ ? Did that work before this change? Does it work now?
There was a problem hiding this comment.
Yes, these both get the same symbol, and the documentation-getting code just depends on the symbol.
| @@ -1586,35 +1586,28 @@ namespace ts { | |||
| ((node as JSDocFunctionType).parameters[0].name as Identifier).escapedText === "new"; | |||
| } | |||
|
|
|||
| export function getAllJSDocs(node: Node): (JSDoc | JSDocTag)[] { | |||
There was a problem hiding this comment.
this was internal, right? I don't see a jsdoc on it, so I assume so.
There was a problem hiding this comment.
API tests should ensure we don't delete internal things.
src/services/jsDoc.ts
Outdated
| }); | ||
| return documentationComment; | ||
| } | ||
|
|
||
| function getCommentsFromDeclaration(declaration: Declaration): string[] { | ||
| switch (declaration.kind) { | ||
| case SyntaxKind.JSDocPropertyTag: |
There was a problem hiding this comment.
why is this special-cased here instead of in getAllJSDocs?
There was a problem hiding this comment.
Updated, now there's only one function.
| return false; | ||
| } | ||
| const tag = parseParameterOrPropertyTag(atToken, tagName, target); | ||
| tag.comment = parseTagComments(tag.end - tag.pos); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
It looks like it will be needed to get the best comment text, that will be a bigger change though. Filed #21123
| // @Filename: /a.js | ||
| /////** | ||
| //// * @typedef I | ||
| //// * @property {number} x Doc |
There was a problem hiding this comment.
Can you make the documentation longer and multiline, with varying indents, to make sure that it works correctly?
ghost
force-pushed
the
quickInfoProperty
branch
from
d142fcb to
af26984
Compare
| @@ -63,7 +63,7 @@ | |||
| ], | |||
| "documentation": [ | |||
| { | |||
| "text": "Doc{T} A template", | |||
| "text": "DocT} A template", | |||
ghost
force-pushed
the
quickInfoProperty
branch
from
af26984 to
5181017
Compare
src/services/jsDoc.ts
Outdated
| @@ -106,15 +102,19 @@ namespace ts.JsDoc { | |||
| const { name } = tag as JSDocTypedefTag | JSDocPropertyTag | JSDocParameterTag; | |||
| return name ? withNode(name) : comment; | |||
| default: | |||
| return comment; | |||
| return comment || ""; | |||
There was a problem hiding this comment.
adding || "" goes the opposite direction of adding | undefined to the return type. Is the latter still correct?
There was a problem hiding this comment.
Whoops, shouldn't have left this in.
* origin/master: (114 commits) LEGO: check in for master to temporary branch. LEGO: check in for master to temporary branch. Enable substitution for object literal shorthand property assignments in the system transform (microsoft#21106) Skip outer expressions when checking for super keyword in binder (microsoft#20164) Group intersection code in getSimplifiedIndexedAccessType Use filter instead of unnecessary laziness Remove mapped types to never from intersections Handle jsconfig.json in fourslash tests (microsoft#16484) Add a `getFullText()` helper method to `IScriptSnapshot` (microsoft#21155) Test Diff and Omit Keep string index from intersections in base constraint of index type LEGO: check in for master to temporary branch. Fix bug: get merged symbol of symbol.parent before checking against module symbol (microsoft#21147) Do not trigger the failed lookup location invalidation for creation of program emit files Handles microsoft#20934 Add test where emit of the file results in invalidating resolution and hence invoking recompile Parse comment on @Property tag and use as documentation comment (microsoft#21119) Update baselines push/popTypeResolution for circular base constraints LEGO: check in for master to temporary branch. Test:incorrect mapped type doesn't infinitely recur ...
Fixes #21105