Conversion order should not affect link resolution

Resolves #2466
TypeStrong · Dec 31, 2023 · 65c83a9 · 65c83a9
1 parent a0aa060
commit 65c83a9
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,6 +27,7 @@
 -   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.
+-   Conversion order should no longer affect link resolution for classes with properties whose type does not rely on `this`, #2466.
 -   Keyword syntax highlighting introduced in 0.25.4 was not always applied to keywords.
 -   If all members in a group are hidden from the page, the group will be hidden in the page index on page load.
 

diff --git a/src/lib/converter/symbols.ts b/src/lib/converter/symbols.ts
@@ -182,12 +182,10 @@ export function convertSymbol(
         flags = removeFlag(flags, ts.SymbolFlags.Property);
     }
 
-    for (const flag of getEnumFlags(flags ^ allConverterFlags)) {
-        if (!(flag & allConverterFlags)) {
-            context.logger.verbose(
-                `Missing converter for symbol: ${symbol.name} with flag ${ts.SymbolFlags[flag]}`,
-            );
-        }
+    for (const flag of getEnumFlags(flags & ~allConverterFlags)) {
+        context.logger.verbose(
+            `Missing converter for symbol: ${symbol.name} with flag ${ts.SymbolFlags[flag]}`,
+        );
     }
 
     // Note: This method does not allow skipping earlier converters.
@@ -628,7 +626,7 @@ function convertClassOrInterface(
     // And finally, index signatures
     convertIndexSignature(reflectionContext, symbol);
 
-    // Normally this shouldn't matter, unless someone did something with skipLibCheck off.
+    // Normally this shouldn't matter, unless someone did something with skipLibCheck on.
     return ts.SymbolFlags.Alias;
 }
 

diff --git a/src/lib/models/reflections/project.ts b/src/lib/models/reflections/project.ts
@@ -6,7 +6,7 @@ import type { SignatureReflection } from "./signature";
 import type { ParameterReflection } from "./parameter";
 import { IntrinsicType } from "../types";
 import type { TypeParameterReflection } from "./type-parameter";
-import { removeIfPresent } from "../../utils";
+import { removeIf, removeIfPresent } from "../../utils";
 import type * as ts from "typescript";
 import { ReflectionKind } from "./kind";
 import { Comment, CommentDisplayPart } from "../comments";
@@ -103,7 +103,37 @@ export class ProjectReflection extends ContainerReflection {
 
         if (symbol) {
             this.reflectionIdToSymbolMap.set(reflection.id, symbol);
-            this.registerSymbolId(reflection, new ReflectionSymbolId(symbol));
+            const id = new ReflectionSymbolId(symbol);
+            this.registerSymbolId(reflection, id);
+
+            // #2466
+            // If we just registered a member of a class or interface, then we need to check if
+            // we've registered this symbol before under the wrong parent reflection.
+            // This can happen because the compiler API will use non-dependently-typed symbols
+            // for properties of classes/interfaces which inherit them, so we can't rely on the
+            // property being unique for each class.
+            if (
+                reflection.parent?.kindOf(ReflectionKind.ClassOrInterface) &&
+                reflection.kindOf(ReflectionKind.SomeMember)
+            ) {
+                const saved = this.symbolToReflectionIdMap.get(id);
+                const parentSymbolReflection =
+                    symbol.parent &&
+                    this.getReflectionFromSymbol(symbol.parent);
+
+                if (
+                    typeof saved === "object" &&
+                    saved.length > 1 &&
+                    parentSymbolReflection
+                ) {
+                    removeIf(
+                        saved,
+                        (item) =>
+                            this.getReflectionById(item)?.parent !==
+                            parentSymbolReflection,
+                    );
+                }
+            }
         }
     }
 

diff --git a/src/test/converter2/issues/gh2466.ts b/src/test/converter2/issues/gh2466.ts
@@ -0,0 +1,15 @@
+// Conversion order test for @link
+
+export interface One extends Two {}
+
+/** {@link method1} */
+export interface Two {
+    method1(): string;
+}
+
+/** {@link method2} */
+export interface Three {
+    method2(): string;
+}
+
+export interface Four extends Three {}
diff --git a/src/test/issues.c2.test.ts b/src/test/issues.c2.test.ts
@@ -1330,4 +1330,24 @@ describe("Issue Tests", () => {
         const is = querySig(project, "FooA.is");
         equal(is.type?.toString(), "this is Foo & Object");
     });
+
+    it("Does not care about conversion order for @link resolution, #2466", () => {
+        const project = convert();
+
+        const Two = query(project, "Two");
+        equal(getLinks(Two), [
+            {
+                display: "method1",
+                target: [ReflectionKind.Method, "Two.method1"],
+            },
+        ]);
+
+        const Three = query(project, "Three");
+        equal(getLinks(Three), [
+            {
+                display: "method2",
+                target: [ReflectionKind.Method, "Three.method2"],
+            },
+        ]);
+    });
 });