Merge pull request #2522 from kraenhansen/variable-as-function-from-i…

…nterface

Adding a failing test to reproduce #2521

CHANGELOG.md

@@ -45,6 +45,7 @@ markedOptions -> markdownItOptions, markdownItLoader, navigation
 ### Thanks!
 
 -   @HarelM
+-   @kraenhansen
 
 # Unreleased
 

src/lib/converter/comments/index.ts

@@ -31,13 +31,15 @@ const jsDocCommentKinds = [
     ts.SyntaxKind.JSDocEnumTag,
 ];
 
+let commentDiscoveryId = 0;
 let commentCache = new WeakMap<ts.SourceFile, Map<number, Comment>>();
 
 // We need to do this for tests so that changing the tsLinkResolution option
 // actually works. Without it, we'd get the old parsed comment which doesn't
 // have the TS symbols attached.
 export function clearCommentCache() {
     commentCache = new WeakMap();
+    commentDiscoveryId = 0;
 }
 
 function getCommentWithCache(
@@ -82,6 +84,7 @@ function getCommentWithCache(
             assertNever(ranges[0].kind);
     }
 
+    comment.discoveryId = ++commentDiscoveryId;
     cache.set(ranges[0].pos, comment);
     commentCache.set(file, cache);
 

src/lib/converter/factories/signature.ts

@@ -1,7 +1,6 @@
 import ts from "typescript";
 import assert from "assert";
 import {
-    ConversionFlags,
     DeclarationReflection,
     IntrinsicType,
     ParameterReflection,
@@ -68,15 +67,11 @@ export function createSignature(
         parentReflection = parentReflection.parent;
     }
 
-    if (
-        declaration &&
-        (!parentReflection.comment ||
-            !(
-                parentReflection.conversionFlags &
-                ConversionFlags.VariableOrPropertySource
-            ))
-    ) {
-        sigRef.comment = context.getSignatureComment(declaration);
+    if (declaration) {
+        const sigComment = context.getSignatureComment(declaration);
+        if (parentReflection.comment?.discoveryId !== sigComment?.discoveryId) {
+            sigRef.comment = sigComment;
+        }
     }
 
     sigRef.typeParameters = convertTypeParameters(

src/lib/converter/symbols.ts

@@ -8,7 +8,6 @@ import {
     type Reflection,
     ReflectionFlag,
     ReflectionKind,
-    ConversionFlags,
 } from "../models";
 import {
     getEnumFlags,
@@ -697,7 +696,6 @@ function convertProperty(
         symbol,
         exportSymbol,
     );
-    reflection.conversionFlags |= ConversionFlags.VariableOrPropertySource;
 
     const declaration = symbol.getDeclarations()?.[0];
     let parameterType: ts.TypeNode | undefined;
@@ -1041,7 +1039,6 @@ function convertVariableAsFunction(
         exportSymbol,
     );
     setModifiers(symbol, accessDeclaration, reflection);
-    reflection.conversionFlags |= ConversionFlags.VariableOrPropertySource;
     context.finalizeDeclarationReflection(reflection);
 
     const reflectionContext = context.withScope(reflection);

src/lib/models/comments/comment.ts

@@ -4,6 +4,7 @@ import { ReflectionKind } from "../reflections/kind";
 import { ReflectionSymbolId } from "../reflections/ReflectionSymbolId";
 
 import type { Serializer, Deserializer, JSONOutput } from "../../serialization";
+import { NonEnumerable } from "../../utils/general";
 
 /**
  * Represents a parsed piece of a comment.
@@ -394,6 +395,14 @@ export class Comment {
      */
     label?: string;
 
+    /**
+     * Internal discovery ID used to prevent symbol comments from
+     * being duplicated on signatures. Only set when the comment was created
+     * @internal
+     */
+    @NonEnumerable
+    discoveryId?: number;
+
     /**
      * Creates a new Comment instance.
      */
@@ -412,11 +421,15 @@ export class Comment {
      * Create a deep clone of this comment.
      */
     clone() {
-        return new Comment(
+        const comment = new Comment(
             Comment.cloneDisplayParts(this.summary),
             this.blockTags.map((tag) => tag.clone()),
             new Set(this.modifierTags),
         );
+        if (this.discoveryId) {
+            comment.discoveryId = this.discoveryId;
+        }
+        return comment;
     }
 
     /**

src/lib/models/reflections/declaration.ts

@@ -37,14 +37,6 @@ export interface DeclarationHierarchy {
     isTarget?: boolean;
 }
 
-/**
- * @internal
- */
-export enum ConversionFlags {
-    None = 0,
-    VariableOrPropertySource = 1,
-}
-
 /**
  * A reflection that represents a single declaration emitted by the TypeScript compiler.
  *
@@ -172,12 +164,6 @@ export class DeclarationReflection extends ContainerReflection {
      */
     packageVersion?: string;
 
-    /**
-     * Flags for information about a reflection which is needed solely during conversion.
-     * @internal
-     */
-    conversionFlags = ConversionFlags.None;
-
     override isDeclaration(): this is DeclarationReflection {
         return true;
     }

src/lib/models/reflections/index.ts

@@ -6,7 +6,7 @@ export {
 } from "./abstract";
 export type { TraverseCallback, ReflectionVisitor } from "./abstract";
 export { ContainerReflection } from "./container";
-export { DeclarationReflection, ConversionFlags } from "./declaration";
+export { DeclarationReflection } from "./declaration";
 export type { DeclarationHierarchy } from "./declaration";
 export { ReflectionKind } from "./kind";
 export { ParameterReflection } from "./parameter";

src/test/converter/js/index.js

@@ -47,8 +47,8 @@
 
 /**
  * @callback Foo
- * @param {...string} args
- * @returns {number}
+ * @param {...string} args Foo param description
+ * @returns {number} Foo return description
  */
 
 /** @type {Foo} */

src/test/converter/js/specs.json

@@ -383,6 +383,14 @@
                       "flags": {
                         "isRest": true
                       },
+                      "comment": {
+                        "summary": [
+                          {
+                            "kind": "text",
+                            "text": "Foo param description"
+                          }
+                        ]
+                      },
                       "type": {
                         "type": "array",
                         "elementType": {
@@ -902,6 +910,20 @@
               "variant": "signature",
               "kind": 4096,
               "flags": {},
+              "comment": {
+                "summary": [],
+                "blockTags": [
+                  {
+                    "tag": "@returns",
+                    "content": [
+                      {
+                        "kind": "text",
+                        "text": "Foo return description"
+                      }
+                    ]
+                  }
+                ]
+              },
               "sources": [
                 {
                   "fileName": "index.js",
@@ -919,6 +941,14 @@
                   "flags": {
                     "isRest": true
                   },
+                  "comment": {
+                    "summary": [
+                      {
+                        "kind": "text",
+                        "text": "Foo param description"
+                      }
+                    ]
+                  },
                   "type": {
                     "type": "array",
                     "elementType": {

src/test/converter2/issues/gh2521.d.ts

@@ -0,0 +1,18 @@
+/**
+ * Original comment.
+ */
+export interface Foo {
+    /** Overload 1 */
+    (): void;
+    /** Overload 2 */
+    (baz: number): void;
+}
+
+// Inherits overload comments, but not Foo comment
+// Foo comment could be inherited with {@inheritDoc Foo}
+export const fooWithoutComment: Foo;
+
+/**
+ * New comment.
+ */
+export const fooWithComment: Foo;

src/test/issues.c2.test.ts

@@ -27,7 +27,7 @@ import {
     getConverter2Program,
 } from "./programs";
 import { TestLogger } from "./TestLogger";
-import { getComment, getLinks, query, querySig } from "./utils";
+import { getComment, getLinks, getSigComment, query, querySig } from "./utils";
 import { DefaultTheme, PageEvent } from "..";
 
 const base = getConverter2Base();
@@ -891,9 +891,9 @@ describe("Issue Tests", () => {
         const project = convert();
         for (const [name, docs, sigDocs] of [
             ["built", "", "inner docs"],
-            ["built2", "outer docs", ""],
+            ["built2", "outer docs", "inner docs"],
             ["fn", "", "inner docs"],
-            ["fn2", "outer docs", ""],
+            ["fn2", "outer docs", "inner docs"],
         ]) {
             const refl = query(project, name);
             ok(refl.signatures?.[0]);
@@ -1467,6 +1467,18 @@ describe("Issue Tests", () => {
         equal(cb2.type.declaration.signatures![0].comment, undefined);
     });
 
+    it("Specifying comment on variable still inherits signature comments, #2521", () => {
+        const project = convert();
+
+        equal(getComment(project, "fooWithoutComment"), "");
+        equal(getSigComment(project, "fooWithoutComment", 0), "Overload 1");
+        equal(getSigComment(project, "fooWithoutComment", 1), "Overload 2");
+
+        equal(getComment(project, "fooWithComment"), "New comment.");
+        equal(getSigComment(project, "fooWithComment", 0), "Overload 1");
+        equal(getSigComment(project, "fooWithComment", 1), "Overload 2");
+    });
+
     it("Ignores @license and @import comments at the top of the file, #2552", () => {
         const project = convert();
         equal(

src/test/utils.ts

@@ -35,6 +35,16 @@ export function getComment(project: ProjectReflection, name: string) {
     return Comment.combineDisplayParts(query(project, name).comment?.summary);
 }
 
+export function getSigComment(
+    project: ProjectReflection,
+    name: string,
+    index = 0,
+) {
+    return Comment.combineDisplayParts(
+        querySig(project, name, index).comment?.summary,
+    );
+}
+
 export function getLinks(refl: Reflection): Array<{
     display: string;
     target: undefined | string | [ReflectionKind, string];