Expose getJSDocCommentsAndTags #53627

Gerrit0 · 2023-04-02T02:20:34Z

As discussed in the TS Discord, TypeDoc is now relying on this internal function in order parse comments more closely to TypeScript's parsing.

Fixes #45197

Closes microsoft#45197

typescript-bot · 2023-04-02T02:20:48Z

The TypeScript team hasn't accepted the linked issue #45197. If you can get it accepted, this PR will have a better chance of being reviewed.

jakebailey

I don't have any problem doing this, but, I don't know if we should bother trying to move all of the code. Frankly, we've been bad at sorting out public from non-public and I'm not sure there's a benefit of trying at the moment...

jakebailey · 2023-04-12T17:08:33Z

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -8507,6 +8507,7 @@ declare namespace ts {
    function getNameOfDeclaration(declaration: Declaration | Expression | undefined): DeclarationName | undefined;
    function getDecorators(node: HasDecorators): readonly Decorator[] | undefined;
    function getModifiers(node: HasModifiers): readonly Modifier[] | undefined;
+    function getJSDocCommentsAndTags(hostNode: Node, noCache?: boolean): readonly (JSDoc | JSDocTag)[];


Do we have noCache anywhere else in the public code? Is this useful externally?

I must not have been paying much attention, didn't mean to expose that parameter externally, I don't think it should be used

I've moved it back to utilities.ts, and avoided exposing noCache externally, which unfortunately required either an extra function, or an overload and disabling a rule.

sandersn

Needs some documentation that explains why you'd use this lower-level function (which will also help me understand why this change is needed).

sandersn · 2023-04-12T17:19:34Z

src/compiler/utilitiesPublic.ts

@@ -956,6 +961,52 @@ export function getModifiers(node: HasModifiers): readonly Modifier[] | undefine
    }
 }

+export function getJSDocCommentsAndTags(hostNode: Node, noCache?: boolean): readonly (JSDoc | JSDocTag)[] {


This function is wrapped in the language service as getCommentHavingNodes, for declarations only.

This function needs at least two kinds of documentation:

Why use it instead of getJSDocTags? They both look for multiple relevant comments, but getJSDocTags only returns tags that apply, not the entire comment. Does non-local comment text ever make sense [1]? Are the non-local comments useful in some way besides comments+tags? Would a caller need to know that one tag was part of a local comment and another was part of a non-local one?

What do the parameters mean and what preconditions are there for their use? Specifically:

Internally, getJSDocCommentsAndTags is called from two places; the first verifies canHaveJSDoc(hostNode) and the second passes only declarations. So I think this only works on specific kinds of node, or maybe on any node, but might hurt performance due to deoptimisation.

getJSDocTagsNoCache is labelled internal but find-all-refs doesn't find anything. Why wouldn't you want caching?

[1] eg

/** * Function documentation, non-local to x, but shouldn't be needed, right? * @param x non-local documentation for x, in a tag */ function f(/** also local, so maybe this is needed? */x) { }

It should be used when you want documentation for a node, personally, getJSDocTags is something I never see myself using, because if I want documentation for something, I want all related documentation, not only documentation that's nested within a tag. getJSDocTags would return nothing of use whatsoever for non-parameters/type parameters... I have no idea why you'd use that function, and was surprised it was exposed while this more useful version was not.

I experimented with changing this to accept a Declaration rather than Node, and while TypeDoc is fine with that signature, getJSDocTypeParameterTagsWorker isn't (InferTypeNode and JSDocTemplateTag)

Caching - I removed noCache and the other noCache parameters from functions that call this with that parameter, and found that getJSDocOverrideTagNoCache is public, but that looks like a mistake, the other getJSDoc*Tag functions all use the cache. The real user of all of this is getJSDocModifierFlagsNoCache, which is ultimately used by __debugModifierFlags in enableDebugInfo... not familiar enough with the compiler to say if these ought to be cleaned up.

jakebailey · 2023-04-17T19:32:56Z

Given this is now just a plain de-internalizing, this seems okay to me if the API seems reasonable, but I think @sandersn or @rbuckton know more here.

sandersn · 2023-04-19T23:38:47Z

I think it's probably fine, even though the function is still pretty complex for the simplish uses I think most people will want it for.

@Gerrit0 specifically it sounds like you're only interested in the jsdoc comment text. Would a function with no lookup loop work for you? Maybe a signature like getJSDocComments: (hostNode: Node) => JSDoc | undefined

It's easier to expose getJSDocCommentsAndTags, so probably not worthwhile making a new function, but I am curious about your exact needs.

Gerrit0 · 2023-04-21T03:15:24Z

Yes, that would work for me (mostly, should return readonly JSDoc[] to be able to handle JSDoc @overload)

If you think that's a better solution, I'd be happy to give implementing that a shot.

sandersn

Let's expose what we have, but rewrite the explanation.

sandersn · 2023-04-21T16:33:29Z

src/compiler/utilities.ts

@@ -4181,7 +4181,31 @@ export function canHaveJSDoc(node: Node): node is HasJSDoc {
    }
 }

-/** @internal */
+/**
+ * Gets either the {@link JSDoc} object or {@link JSDocTag} providing documentation for the provided node.


Suggested change

* Gets either the {@link JSDoc} object or {@link JSDocTag} providing documentation for the provided node.

* Gets an array of each {@link JSDoc} object or {@link JSDocTag} that documents the provided node at every position that related JSDoc could exist.

sandersn · 2023-04-21T16:37:20Z

src/compiler/utilities.ts

+ *
+ * @param hostNode node to get the associated JSDoc comment for.


This is redundant, so please delete it.

sandersn · 2023-04-21T16:45:13Z

src/compiler/utilities.ts

+ * const a = 0
+ * /**
+ *  * Entire JSDoc will be returned for `b`
+ *  * JSDocTag will be returned for `c`


Suggested change

* * JSDocTag will be returned for `c`

* @param c JSDocTag will be returned for `c`

sandersn · 2023-04-21T16:51:56Z

src/compiler/utilities.ts

-/** @internal */
+/**
+ * Gets either the {@link JSDoc} object or {@link JSDocTag} providing documentation for the provided node.
+ * A {@link JSDoc} object will be returned if an entire comment is associated with this node. This is always


I would replace this paragraph with the one below:

This function checks multiple locations for JSDoc comments that apply to a host node. At each location, the whole comment may apply to the node, or only a specific tag in the comment. In the first case, location adds the entire {@link JSDoc} object. In the second case, it adds the applicable {@link JSDocTag}. For example, a JSDoc comment before a parameter adds the entire {@link JSDoc}. But a `@param` tag on the parent function only adds the {@link JSDocTag} for the `@param`.

(writing probably needs to be rewrapped)

Gerrit0 added 2 commits April 1, 2023 18:07

Expose getJSDocCommentsAndTags

a9acb2d

Closes microsoft#45197

Update API baseline

6daf333

typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 2, 2023

sandersn self-assigned this Apr 12, 2023

sandersn requested review from sandersn and jakebailey April 12, 2023 17:05

jakebailey reviewed Apr 12, 2023

View reviewed changes

sandersn requested changes Apr 12, 2023

View reviewed changes

Gerrit0 added 2 commits April 15, 2023 17:42

getJSDocCommentsAndTags -> utilities.ts, add docs

23c32fc

Accept baseline, fix eslint

0481b73

Gerrit0 requested review from sandersn and jakebailey April 16, 2023 00:32

sandersn requested changes Apr 21, 2023

View reviewed changes

Update doc comment with suggested wording

dcff7c9

Gerrit0 requested a review from sandersn April 21, 2023 17:14

sandersn approved these changes Apr 24, 2023

View reviewed changes

sandersn merged commit a1df8f7 into microsoft:main Apr 24, 2023

Gerrit0 deleted the fix/45197 branch April 24, 2023 23:18

jakebailey mentioned this pull request May 17, 2023

A minimal custom transformer plugin proposal #54276

Closed

molisani mentioned this pull request May 13, 2024

Expose API for getting JSDoc nodes in TypeScript files #7393

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose getJSDocCommentsAndTags #53627

Expose getJSDocCommentsAndTags #53627

Gerrit0 commented Apr 2, 2023

typescript-bot commented Apr 2, 2023

jakebailey left a comment

jakebailey Apr 12, 2023

Gerrit0 Apr 15, 2023

Gerrit0 Apr 16, 2023

sandersn left a comment

sandersn Apr 12, 2023

Gerrit0 Apr 16, 2023

jakebailey commented Apr 17, 2023

sandersn commented Apr 19, 2023

Gerrit0 commented Apr 21, 2023

sandersn left a comment

sandersn Apr 21, 2023

sandersn Apr 21, 2023

sandersn Apr 21, 2023

sandersn Apr 21, 2023

	* Gets either the {@link JSDoc} object or {@link JSDocTag} providing documentation for the provided node.
	* Gets an array of each {@link JSDoc} object or {@link JSDocTag} that documents the provided node at every position that related JSDoc could exist.

		*
		* @param hostNode node to get the associated JSDoc comment for.

	* * JSDocTag will be returned for `c`
	* @param c JSDocTag will be returned for `c`

Expose getJSDocCommentsAndTags #53627

Expose getJSDocCommentsAndTags #53627

Conversation

Gerrit0 commented Apr 2, 2023

typescript-bot commented Apr 2, 2023

jakebailey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakebailey commented Apr 17, 2023

sandersn commented Apr 19, 2023

Gerrit0 commented Apr 21, 2023

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment