typescript-go icon indicating copy to clipboard operation
typescript-go copied to clipboard

Inherited methods not merging JSDoc comments

Open LukeAbby opened this issue 2 weeks ago • 6 comments

Steps to reproduce

class Foo {
   /** foo bar */
   method(): void {}
}

class Bar extends Foo {
   /** @remarks lorem ipsum */
   method(): void {}
}

Behavior with [email protected]

Hovering over Bar#method gives:

(method) Bar.method(): void
foo bar
@remarks — lorem ipsum

Behavior with tsgo

Hovering over Bar#method gives:

(method) Bar.method(): void
@remarks — lorem ipsum

Note that the foo bar is gone. I believe that blocks like @remarks being merged is a case not handled by #2111

LukeAbby avatar Dec 06 '25 22:12 LukeAbby

Hmm, I definitely agree that derived methods with no JSDoc should inherit JSDoc from base methods, but I'm not sure I like the notion of always merging the inherited JSDoc. Often the base documentation states what should happen in an override, which may be confusing to show in the description of an override that is actually present.

ahejlsberg avatar Dec 07 '25 11:12 ahejlsberg

Actually, the old language service doesn't merge always merge inherited JSDoc. Rather, the summary and remarks sections are processed independently so that you might see the inherited summary of the base method but with the @remarks of the derived method. Or vice versa. I'm pretty sure things weren't intentionally designed that way, it just happened to work that way. I have to say I find the new behavior more understandable.

ahejlsberg avatar Dec 07 '25 12:12 ahejlsberg

I had always assumed this was intentional and there isn't really a great workaround if it's removed. If there were a way to simulalte the old behavior, say writing @inheritDoc, then I'd be happy.

class Foo {
   /** foo bar */
   method(): void {}
}

class Bar extends Foo {
   /**
     * @inheritDoc
     * @remarks lorem ipsum
     */
   method(): void {}
}

As it stands I have no idea how common this pattern in general but it's used quite a bit in one of the repos I maintain. In the range of hundreds of times.

And, yeah, I should have been more precise with my language. Indeed you won't end up with text merged from both summary blocks or text from both remarks blocks.

LukeAbby avatar Dec 07 '25 22:12 LukeAbby

FWIW there is a PR for this in #2262. @LukeAbby does it do what you expect?

jakebailey avatar Dec 08 '25 18:12 jakebailey

Unfortunately it appears that PR does not entirely work. This is how it renders SomeClass.doSomethingUseful: Image (Notice the "some description" in @returns)

abstract class BaseClass {
  /**
   * Useful description always applicable
   *
   * @returns {string} Useful description of return value always applicable.
   */
  public static doSomethingUseful(stuff?: any): string {
    throw new Error("Must be implemented by subclass");
  }
}

class SubClass extends BaseClass {
  /**
   * some description
   *
   * @inheritDoc
   *
   * @param {{ tiger: string; lion: string; }} [mySpecificStuff] Description of my specific parameter.
   */
  public static doSomethingUseful(mySpecificStuff?: { tiger: string; lion: string }): string {}
}

This leads me to believe there's a literal concatenation of the JSDoc going on.

I do think the idea of restoring the old behavior OR allowing users to opt in to merging documentation with @inheritDoc sounds fine to me.

LukeAbby avatar Dec 08 '25 20:12 LukeAbby

@LukeAbby, I have updated the pull request and added a test. Could you confirm if this test addresses the expected behavior?

=== /quickInfoJsDocInheritDocTag.js ===
// abstract class A {
//   /**
//    * A.f description
//    * @returns {string} A.f return value.
//    */
//   public static f(props?: any): string {
//     throw new Error("Must be implemented by subclass");
//   }
// }
// 
// class B extends A {
//   /**
//    * B.f description
//    * @inheritDoc
//    * @param {{ a: string; b: string; }} [props] description of props
//    */
//   public static f(props?: { a: string; b: string }): string {}
//                 ^
// | ----------------------------------------------------------------------
// | ```tsx
// | (method) B.f(props?: { a: string; b: string; }): string
// | ```
// | A.f description B.f description
// | 
// | *@returns* — A.f return value.
// | 
// | 
// | *@inheritDoc*
// | 
// | *@param* `props` — description of props
// | 
// | ----------------------------------------------------------------------
// }

This closely matches how it’s handled in 5.9

(method) B.f(props?: {
    a: string;
    b: string;
}): string
A.f description B.f description

@returns — A.f return value.

@inheritDoc

@param props — description of props

a-tarasyuk avatar Dec 12 '25 09:12 a-tarasyuk