Add JSDoc's @inheritDoc Support for Static Class Members for TypeScript #46719

Zzzen · 2021-11-07T15:41:44Z

src/services/services.ts

Zzzen · 2021-11-07T15:48:15Z

tests/cases/fourslash/jsDocInheritDoc.ts

@@ -52,6 +52,6 @@

 verify.quickInfoAt("1", "constructor Bar(value: number): Bar", undefined); // constructors aren't actually inherited


Does anyone have ideas why constructors aren't actually inherited?

sandersn · 2021-11-15T23:46:37Z

#23215 is kind of old, and it leaves a lot of questions underspecified.

Is @inheritDoc required for inheritance? jsdoc.app says no.
Are other tags allowed? jsdoc.app doesn't give an example with other tags, so maybe not.
Are comments allowed? jsdoc.app also doesn't give an example with a comment on the derived method.
If other tags are allowed, how are they integrated? The current method seems to combine the base and derived tags all together, which doesn't make sense for param tags; the parameter names might be the same, or they might be different.
If the parameter names are the same, or if both base and derived have return tags, should the inherited tags replace the old one? Probably yes.
Is the case of the tag important? Probably no.
Why does this only work on static members?

Answering the inheritance questions seems like a lot of work. If I were writing this, I'd be very tempted to say that (1) the derived jsdoc must have @inheritdoc (2) the derived jsdoc must have no other tags or comments.

src/services/services.ts

sandersn · 2021-11-15T23:34:30Z

src/services/services.ts

+            if (declaration.modifiers?.some(modifier => modifier.kind === SyntaxKind.StaticKeyword)) {
+            // For nodes like `class Foo extends Bar {}`, superTypeNode refers to an ExpressionWithTypeArguments with its expression being `Bar`.
+            const baseClassSymbol = checker.getSymbolAtLocation(isExpressionWithTypeArguments(superTypeNode) && !superTypeNode.typeArguments?.length ? superTypeNode.expression : superTypeNode);
+                const symbol = baseClassSymbol?.exports?.get(declaration.symbol.name as __String);


I think the usual way is getPropertiesOfType(getDeclaredTypeOfSymbol(s)) (though I'm not 100% sure)

Updated. It's actually getTypeOfSymbolAtLocation because we are looking for static members of class.

sandersn · 2021-11-15T23:38:24Z

tests/cases/fourslash/quickInfoJsDocTags7.ts

+////abstract class BaseClass {
+////    /**
+////     * Useful description always applicable
+////     * 


The test needs to show what happens when there's a param tag for stuff, and what happens when there's a param tag for a parameter whose name is the same between base/derived.

sandersn · 2021-11-15T23:42:57Z

tests/cases/fourslash/quickInfoJsDocTags7.ts

+////    }
+////
+////    /**
+////     * @inheritDoc


What happens if there's comment text on someProperty? Is it discarded, appended, prepended, or is inheritdoc ignored?

They should be concatenated.

sandersn · 2022-03-25T23:59:17Z

@Zzzen Do you want to keep working on this? Otherwise I'd like to close it to reduce the number of open PRs.

Zzzen · 2022-03-26T13:45:56Z

Thanks for reminding me. And yes, I'm going to finish it.

Zzzen · 2022-03-26T14:00:10Z

jsdoc.app says that it is provided for compatibility with Closure Compiler, which is written in java. Haven't check the source code of Closure Compiler, I think they should work like javadoc.

Inherits (copies) documentation from the "nearest" inheritable class or implementable interface into the current doc comment at this tag's location. This allows you to write more general comments higher up the inheritance tree, and to write around the copied text.
This tag is valid only in these places in a doc comment:

In the main description block of a method. In this case, the main description is copied from a class or interface up the hierarchy.

In the text arguments of the @return, @param and @throws tags of a method. In this case, the tag text is copied from the corresponding tag up the hierarchy.

See Automatic Copying of Method Comments for a more precise description of how comments are found in the inheritance hierarchy. Note that if this tag is missing, the comment is or is not automatically inherited according to rules described in that section.

sandersn

Couple of questions, although it's quite possible that neither are fixable in this PR.

sandersn · 2022-03-30T22:34:45Z

tests/baselines/reference/quickInfoInheritDoc.baseline

+          "text": "Applicable description always.",
+          "kind": "text"
+        },
+        {
+          "text": "\n",
+          "kind": "lineBreak"
+        },
+        {
+          "text": "text over tag",


I thought @inheritDoc was supposed to insert the inherited text at the location of the tag. Here, it looks like it's getting prepended.

It's appended in webstorm. 😆

src/services/services.ts

sandersn

It looks to me like @inheritDoc is dropping the subclass documentation in the new tests.

tests/baselines/reference/quickInfoInheritDoc2.baseline

src/services/services.ts

sandersn

This is OK as-is, but I don't want to merge until after 4.7 beta is over.

In the meantime, it's OK for

/**
 * over
 * @inheritDoc
 * under
 */

To produce "Base\nover\under", but I still think it'd be better to produce "over\nBase\under".

DanielRosenwasser · 2022-05-11T02:42:44Z

src/services/services.ts

-            const symbol = checker.getPropertyOfType(checker.getTypeAtLocation(superTypeNode), declaration.symbol.name);
+            const baseType = checker.getTypeAtLocation(superTypeNode);
+            const symbol = isStaticMember
+                ? find(checker.getExportsOfModule(baseType.symbol), s => s.escapedName === declaration.symbol.name)


getExportsOfModule on the non-merged symbol of something that isn't a module? This doesn't seem right.

Add JSDoc's @inheritdoc Support for Static Class Members for TypeScript

d9afcd9

typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Nov 7, 2021

Zzzen commented Nov 7, 2021

View reviewed changes

src/services/services.ts Outdated Show resolved Hide resolved

Zzzen commented Nov 7, 2021

View reviewed changes

orta requested a review from sandersn November 12, 2021 11:12

sandersn requested changes Nov 15, 2021

View reviewed changes

sandersn self-assigned this Dec 8, 2021

Zzzen added 4 commits March 27, 2022 15:06

use public api

b1bf52e

Merge branch 'main' into jsDocInheritDoc

8f93a48

fix

46fb1c3

add tests

4d25f7f

sandersn requested changes Mar 30, 2022

View reviewed changes

simplify implementation

53ec474

sandersn requested changes Apr 5, 2022

View reviewed changes

tests/baselines/reference/quickInfoInheritDoc2.baseline Show resolved Hide resolved

extract comments from inherticDoc

dfea5d0

Zzzen force-pushed the jsDocInheritDoc branch from 1d9ba28 to dfea5d0 Compare April 16, 2022 09:27

Zzzen commented Apr 16, 2022

View reviewed changes

src/services/services.ts Show resolved Hide resolved

sandersn approved these changes Apr 27, 2022

View reviewed changes

sandersn merged commit 604e5fc into microsoft:main May 9, 2022

DanielRosenwasser mentioned this pull request May 11, 2022

Language service crashes for static member in class with unresolved base members #49058

Closed

DanielRosenwasser reviewed May 11, 2022

View reviewed changes

DanielRosenwasser mentioned this pull request May 12, 2022

fix(49058): Language service crashes for static member in class with unresolved base members #49059

Merged

		@@ -52,6 +52,6 @@

		verify.quickInfoAt("1", "constructor Bar(value: number): Bar", undefined); // constructors aren't actually inherited

Add JSDoc's @inheritDoc Support for Static Class Members for TypeScript #46719

Add JSDoc's @inheritDoc Support for Static Class Members for TypeScript #46719

Uh oh!

Conversation

Zzzen commented Nov 7, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandersn commented Nov 15, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandersn commented Mar 25, 2022

Uh oh!

Zzzen commented Mar 26, 2022

Uh oh!

Zzzen commented Mar 26, 2022

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sandersn left a comment

Choose a reason for hiding this comment

Uh oh!

DanielRosenwasser May 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanielRosenwasser May 11, 2022 •

edited

Loading