Keep type alias comments on the alias

Resolves #2372

Author: Gerrit0

CHANGELOG.md

@@ -14,6 +14,7 @@
 ### Bug Fixes
 
 -   Methods which return function types no longer have duplicated comments, #2336.
+-   Comments on function-like type aliases will now show up under the type alias, rather than nested within the type declaration, #2372.
 -   Fix crash when converting some complicated union/intersection types, #2451.
 -   Navigation triangle markers should no longer display on a separate line with some font settings, #2457.
 -   `@group` and `@category` organization is now applied later to allow inherited comments to create groups/categories, #2459.

src/lib/converter/plugins/CommentPlugin.ts

@@ -387,13 +387,6 @@ export class CommentPlugin extends ConverterComponent {
             ? undefined
             : reflection.comment;
 
-        // Since this reflection has signatures, remove the comment from the parent
-        // reflection. This is important so that in type aliases we don't end up with
-        // a comment rendered twice.
-        if (!reflection.kindOf(ReflectionKind.ClassOrInterface)) {
-            delete reflection.comment;
-        }
-
         for (const signature of signatures) {
             const childComment = (signature.comment ||= comment?.clone());
             if (!childComment) continue;
@@ -450,6 +443,27 @@ export class CommentPlugin extends ConverterComponent {
             childComment?.removeTags("@typeParam");
             childComment?.removeTags("@template");
         }
+
+        // Since this reflection has signatures, we need to remove the comment from the non-primary
+        // declaration location. For functions, this means removing it from the Function reflection.
+        // For type aliases, this means removing it from reflection.type.declaration.
+        // This is important so that in type aliases we don't end up with a comment rendered twice.
+        if (
+            reflection.kindOf(
+                ReflectionKind.FunctionOrMethod | ReflectionKind.Constructor,
+            )
+        ) {
+            delete reflection.comment;
+        }
+        if (reflection.kindOf(ReflectionKind.TypeAlias)) {
+            reflection.comment?.removeTags("@param");
+            reflection.comment?.removeTags("@typeParam");
+            reflection.comment?.removeTags("@template");
+
+            for (const sig of signatures) {
+                delete sig.comment;
+            }
+        }
     }
 
     private removeExcludedTags(comment: Comment) {

src/lib/validation/documentation.ts

@@ -56,19 +56,35 @@ export function validateDocumentation(
             }
         }
 
+        // Type aliases own their comments, even if they're function-likes.
+        // So if we're a type literal owned by a type alias, don't do anything.
+        if (
+            ref.kindOf(ReflectionKind.TypeLiteral) &&
+            ref.parent?.kindOf(ReflectionKind.TypeAlias)
+        ) {
+            toProcess.push(ref.parent);
+            continue;
+        }
+        // Ditto for signatures on type aliases.
+        if (
+            ref.kindOf(ReflectionKind.CallSignature) &&
+            ref.parent?.parent?.kindOf(ReflectionKind.TypeAlias)
+        ) {
+            toProcess.push(ref.parent.parent);
+            continue;
+        }
+
         if (ref instanceof DeclarationReflection) {
             const signatures =
                 ref.type instanceof ReflectionType
                     ? ref.type.declaration.getNonIndexSignatures()
                     : ref.getNonIndexSignatures();
 
             if (signatures.length) {
-                // We maybe used to have a comment, but the comment plugin has removed it.
-                // See CommentPlugin.onResolve. We've been asked to validate this reflection,
-                // (it's probably a type alias) so we should validate that signatures all have
-                // comments, but we shouldn't produce a warning here.
+                // We've been asked to validate this reflection, so we should validate that
+                // signatures all have comments, but we'll still have a comment here because
+                // type aliases always have their own comment.
                 toProcess.push(...signatures);
-                continue;
             }
         }
 
@@ -80,7 +96,9 @@ export function validateDocumentation(
             }
 
             logger.warn(
-                `${ref.getFriendlyFullName()}, defined in ${nicePath(
+                `${ref.getFriendlyFullName()} (${
+                    ReflectionKind[ref.kind]
+                }), defined in ${nicePath(
                     symbolId.fileName,
                 )}, does not have any documentation.`,
             );

src/test/converter/alias/specs.json

@@ -165,6 +165,14 @@
       "variant": "declaration",
       "kind": 2097152,
       "flags": {},
+      "comment": {
+        "summary": [
+          {
+            "kind": "text",
+            "text": "A type that describes a compare function, e.g. for array.sort()."
+          }
+        ]
+      },
       "sources": [
         {
           "fileName": "alias.ts",
@@ -205,14 +213,6 @@
               "variant": "signature",
               "kind": 4096,
               "flags": {},
-              "comment": {
-                "summary": [
-                  {
-                    "kind": "text",
-                    "text": "A type that describes a compare function, e.g. for array.sort()."
-                  }
-                ]
-              },
               "parameters": [
                 {
                   "id": 4,

src/test/converter/js/specs.json

@@ -685,6 +685,14 @@
           "variant": "declaration",
           "kind": 2097152,
           "flags": {},
+          "comment": {
+            "summary": [
+              {
+                "kind": "text",
+                "text": "even though in the same comment block"
+              }
+            ]
+          },
           "sources": [
             {
               "fileName": "index.js",
@@ -716,14 +724,6 @@
                   "variant": "signature",
                   "kind": 4096,
                   "flags": {},
-                  "comment": {
-                    "summary": [
-                      {
-                        "kind": "text",
-                        "text": "even though in the same comment block"
-                      }
-                    ]
-                  },
                   "type": {
                     "type": "intrinsic",
                     "name": "any"

src/test/converter/mixin/specs.json

@@ -1231,6 +1231,14 @@
       "variant": "declaration",
       "kind": 2097152,
       "flags": {},
+      "comment": {
+        "summary": [
+          {
+            "kind": "text",
+            "text": "Any constructor function"
+          }
+        ]
+      },
       "sources": [
         {
           "fileName": "mixin.ts",
@@ -1275,14 +1283,6 @@
               "variant": "signature",
               "kind": 16384,
               "flags": {},
-              "comment": {
-                "summary": [
-                  {
-                    "kind": "text",
-                    "text": "Any constructor function"
-                  }
-                ]
-              },
               "parameters": [
                 {
                   "id": 9,
@@ -1318,6 +1318,14 @@
       "variant": "declaration",
       "kind": 2097152,
       "flags": {},
+      "comment": {
+        "summary": [
+          {
+            "kind": "text",
+            "text": "Any function"
+          }
+        ]
+      },
       "sources": [
         {
           "fileName": "mixin.ts",
@@ -1362,14 +1370,6 @@
               "variant": "signature",
               "kind": 4096,
               "flags": {},
-              "comment": {
-                "summary": [
-                  {
-                    "kind": "text",
-                    "text": "Any function"
-                  }
-                ]
-              },
               "parameters": [
                 {
                   "id": 4,

src/test/converter2/issues/gh2372.ts

@@ -0,0 +1,5 @@
+/**
+ * The signature for a function acting as an event handler.
+ * @param e Param should work
+ */
+export type EventHandler = (e: Event) => void;

src/test/issues.c2.test.ts

@@ -414,16 +414,14 @@ describe("Issue Tests", () => {
     it("#1734", () => {
         const project = convert();
         const alias = query(project, "Foo");
-        const type = alias.type;
-        ok(type instanceof ReflectionType);
 
         const expectedComment = new Comment();
         expectedComment.blockTags = [
             new CommentTag("@asdf", [
                 { kind: "text", text: "Some example text" },
             ]),
         ];
-        equal(type.declaration.signatures?.[0].comment, expectedComment);
+        equal(alias.comment, expectedComment);
     });
 
     it("#1745", () => {
@@ -565,12 +563,9 @@ describe("Issue Tests", () => {
         equal(Type1.type?.type, "reflection" as const);
         equal(Type2.type?.type, "reflection" as const);
 
+        equal(Type1.comment, new Comment([{ kind: "text", text: "On Tag" }]));
         equal(
-            Type1.type.declaration.signatures?.[0].comment,
-            new Comment([{ kind: "text", text: "On Tag" }]),
-        );
-        equal(
-            Type2.type.declaration.signatures?.[0].comment,
+            Type2.comment,
             new Comment([{ kind: "text", text: "Some type 2." }]),
         );
     });
@@ -579,7 +574,7 @@ describe("Issue Tests", () => {
         const project = convert();
         app.validate(project);
         logger.expectMessage(
-            "warn: UnDocFn.__type, defined in */gh1898.ts, does not have any documentation.",
+            "warn: UnDocFn (TypeAlias), defined in */gh1898.ts, does not have any documentation.",
         );
     });
 
@@ -1166,6 +1161,22 @@ describe("Issue Tests", () => {
         equal(ns?.children?.map((c) => c.name), ["property"]);
     });
 
+    it("Puts delegate type alias comments on the type alias #2372", () => {
+        const project = convert();
+        equal(
+            getComment(project, "EventHandler"),
+            "The signature for a function acting as an event handler.",
+        );
+
+        const typeSig = query(project, "EventHandler").type?.visit({
+            reflection(r) {
+                return r.declaration.signatures![0];
+            },
+        });
+
+        equal(Comment.combineDisplayParts(typeSig?.comment?.summary), "");
+    });
+
     it("Handles spaces in JSDoc default parameter names #2384", () => {
         const project = convert();
         const Typed = query(project, "Typed");

src/test/validation.test.ts

@@ -172,7 +172,7 @@ describe("validateDocumentation", () => {
         validateDocumentation(project, logger, ["Function"]);
 
         logger.expectMessage(
-            "warn: bar, defined in */function.ts, does not have any documentation.",
+            "warn: bar (CallSignature), defined in */function.ts, does not have any documentation.",
         );
         logger.expectNoOtherMessages();
     });
@@ -183,7 +183,7 @@ describe("validateDocumentation", () => {
         validateDocumentation(project, logger, ["Accessor"]);
 
         logger.expectMessage(
-            "warn: Foo.foo, defined in */getSignature.ts, does not have any documentation.",
+            "warn: Foo.foo (GetSignature), defined in */getSignature.ts, does not have any documentation.",
         );
         logger.expectNoOtherMessages();
     });
@@ -194,7 +194,7 @@ describe("validateDocumentation", () => {
         validateDocumentation(project, logger, ["Constructor"]);
 
         logger.expectMessage(
-            "warn: Foo.constructor, defined in */class.ts, does not have any documentation.",
+            "warn: Foo.constructor (ConstructorSignature), defined in */class.ts, does not have any documentation.",
         );
         logger.expectNoOtherMessages();
     });
@@ -205,7 +205,7 @@ describe("validateDocumentation", () => {
         validateDocumentation(project, logger, ["Method"]);
 
         logger.expectMessage(
-            "warn: Foo.method, defined in */interface.ts, does not have any documentation.",
+            "warn: Foo.method (CallSignature), defined in */interface.ts, does not have any documentation.",
         );
         logger.expectNoOtherMessages();
     });