From 63704904523ec025fc08713b4c1156208972783e Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Thu, 28 Dec 2023 16:55:17 -0700 Subject: [PATCH] Fix references to Symbols with multiple reflections Fixes #2106 --- CHANGELOG.md | 1 + src/lib/converter/types.ts | 22 ++++++------ src/lib/models/reflections/kind.ts | 15 ++++++++ src/lib/models/reflections/project.ts | 44 ++++++++++++++++------- src/lib/models/types.ts | 29 +++++++++++++-- src/lib/serialization/schema.ts | 1 + src/test/converter/inheritance/specs.json | 22 ++++++------ src/test/converter2/issues/gh2106.ts | 17 +++++++++ src/test/issues.c2.test.ts | 11 ++++++ 9 files changed, 126 insertions(+), 36 deletions(-) create mode 100644 src/test/converter2/issues/gh2106.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d97475173..ca7590bab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ ### Bug Fixes +- If both an interface and a variable share a name/symbol, TypeDoc will no longer link to the variable when referenced in a type position, #2106. - `notDocumented` validation will no longer require documentation for data within parameters that cannot be documented via `@param`, #2291. - "defined in" locations for signatures will now always be contained within the function declaration's location. This prevents defined in sometimes pointing to node_modules, #2307. - Type parameters will now be resolved for arrow-methods on classes like regular class methods, #2320. diff --git a/src/lib/converter/types.ts b/src/lib/converter/types.ts index a54fab61c..8c766adc3 100644 --- a/src/lib/converter/types.ts +++ b/src/lib/converter/types.ts @@ -666,13 +666,13 @@ const queryConverter: TypeConverter = { ); } - return new QueryType( - ReferenceType.createSymbolReference( - context.resolveAliasedSymbol(querySymbol), - context, - node.exprName.getText(), - ), + const ref = ReferenceType.createSymbolReference( + context.resolveAliasedSymbol(querySymbol), + context, + node.exprName.getText(), ); + ref.preferValues = true; + return new QueryType(ref); }, convertType(context, type, node) { const symbol = @@ -683,12 +683,12 @@ const queryConverter: TypeConverter = { type, )}. This is a bug.`, ); - return new QueryType( - ReferenceType.createSymbolReference( - context.resolveAliasedSymbol(symbol), - context, - ), + const ref = ReferenceType.createSymbolReference( + context.resolveAliasedSymbol(symbol), + context, ); + ref.preferValues = true; + return new QueryType(ref); }, }; diff --git a/src/lib/models/reflections/kind.ts b/src/lib/models/reflections/kind.ts index c816fe4fc..cc3c13eef 100644 --- a/src/lib/models/reflections/kind.ts +++ b/src/lib/models/reflections/kind.ts @@ -103,6 +103,21 @@ export namespace ReflectionKind { ReflectionKind.Function | ReflectionKind.Method; + // The differences between Type/Value here only really matter for + // possibly merged declarations where we have multiple reflections. + /** @internal */ + export const TypeReferenceTarget = + ReflectionKind.Interface | + ReflectionKind.TypeAlias | + ReflectionKind.Class | + ReflectionKind.Enum; + /** @internal */ + export const ValueReferenceTarget = + ReflectionKind.Module | + ReflectionKind.Namespace | + ReflectionKind.Variable | + ReflectionKind.Function; + /** * Note: This does not include Class/Interface, even though they technically could contain index signatures * @internal diff --git a/src/lib/models/reflections/project.ts b/src/lib/models/reflections/project.ts index 637107e46..f786724ee 100644 --- a/src/lib/models/reflections/project.ts +++ b/src/lib/models/reflections/project.ts @@ -26,8 +26,10 @@ export class ProjectReflection extends ContainerReflection { readonly variant = "project"; // Used to resolve references. - private symbolToReflectionIdMap: Map = - new StableKeyMap(); + private symbolToReflectionIdMap: Map< + ReflectionSymbolId, + number | number[] + > = new StableKeyMap(); private reflectionIdToSymbolIdMap = new Map(); @@ -100,13 +102,8 @@ export class ProjectReflection extends ContainerReflection { this.reflections[reflection.id] = reflection; if (symbol) { - const id = new ReflectionSymbolId(symbol); - this.symbolToReflectionIdMap.set( - id, - this.symbolToReflectionIdMap.get(id) ?? reflection.id, - ); - this.reflectionIdToSymbolIdMap.set(reflection.id, id); this.reflectionIdToSymbolMap.set(reflection.id, symbol); + this.registerSymbolId(reflection, new ReflectionSymbolId(symbol)); } } @@ -212,8 +209,11 @@ export class ProjectReflection extends ContainerReflection { const symbol = this.reflectionIdToSymbolMap.get(reflection.id); if (symbol) { const id = new ReflectionSymbolId(symbol); - if (this.symbolToReflectionIdMap.get(id) === reflection.id) { + const saved = this.symbolToReflectionIdMap.get(id); + if (saved === reflection.id) { this.symbolToReflectionIdMap.delete(id); + } else if (typeof saved === "object") { + removeIfPresent(saved, reflection.id); } } @@ -239,13 +239,25 @@ export class ProjectReflection extends ContainerReflection { /** * Gets the reflection associated with the given symbol id, if it exists. + * If there are multiple reflections associated with this symbol, gets the first one. * @internal */ - getReflectionFromSymbolId(symbolId: ReflectionSymbolId) { + getReflectionFromSymbolId( + symbolId: ReflectionSymbolId, + ): Reflection | undefined { + return this.getReflectionsFromSymbolId(symbolId)[0]; + } + + /** @internal */ + getReflectionsFromSymbolId(symbolId: ReflectionSymbolId) { const id = this.symbolToReflectionIdMap.get(symbolId); if (typeof id === "number") { - return this.getReflectionById(id); + return [this.getReflectionById(id)!]; + } else if (typeof id === "object") { + return id.map((id) => this.getReflectionById(id)!); } + + return []; } /** @internal */ @@ -256,7 +268,15 @@ export class ProjectReflection extends ContainerReflection { /** @internal */ registerSymbolId(reflection: Reflection, id: ReflectionSymbolId) { this.reflectionIdToSymbolIdMap.set(reflection.id, id); - if (!this.symbolToReflectionIdMap.has(id)) { + + const previous = this.symbolToReflectionIdMap.get(id); + if (previous) { + if (typeof previous === "number") { + this.symbolToReflectionIdMap.set(id, [previous, reflection.id]); + } else { + previous.push(reflection.id); + } + } else { this.symbolToReflectionIdMap.set(id, reflection.id); } } diff --git a/src/lib/models/types.ts b/src/lib/models/types.ts index 5a01b6b52..5d425a9fa 100644 --- a/src/lib/models/types.ts +++ b/src/lib/models/types.ts @@ -8,6 +8,7 @@ import { getQualifiedName } from "../utils/tsutils"; import { ReflectionSymbolId } from "./reflections/ReflectionSymbolId"; import type { DeclarationReference } from "../converter/comments/declarationReference"; import { findPackageForPath } from "../utils/fs"; +import { ReflectionKind } from "./reflections/kind"; /** * Base class of all type definitions. @@ -798,8 +799,21 @@ export class ReferenceType extends Type { if (typeof this._target === "number") { return this._project?.getReflectionById(this._target); } - const resolved = this._project?.getReflectionFromSymbolId(this._target); - if (resolved) this._target = resolved.id; + const resolvePotential = this._project?.getReflectionsFromSymbolId( + this._target, + ); + if (!resolvePotential?.length) { + return; + } + + const kind = this.preferValues + ? ReflectionKind.ValueReferenceTarget + : ReflectionKind.TypeReferenceTarget; + + const resolved = + resolvePotential.find((refl) => refl.kindOf(kind)) || + resolvePotential.find((refl) => refl.kindOf(~kind))!; + this._target = resolved.id; return resolved; } @@ -859,6 +873,12 @@ export class ReferenceType extends Type { */ refersToTypeParameter = false; + /** + * If set, will prefer reflections with {@link ReflectionKind | ReflectionKinds} which represent + * values rather than those which represent types. + */ + preferValues = false; + private _target: ReflectionSymbolId | number; private _project: ProjectReflection | null; @@ -984,6 +1004,10 @@ export class ReferenceType extends Type { result.refersToTypeParameter = true; } + if (typeof this._target !== "number" && this.preferValues) { + result.preferValues = true; + } + return result; } @@ -1015,6 +1039,7 @@ export class ReferenceType extends Type { this.qualifiedName = obj.qualifiedName ?? obj.name; this.package = obj.package; this.refersToTypeParameter = !!obj.refersToTypeParameter; + this.preferValues = !!obj.preferValues; } } diff --git a/src/lib/serialization/schema.ts b/src/lib/serialization/schema.ts index 35cd0a300..1cf7f34ee 100644 --- a/src/lib/serialization/schema.ts +++ b/src/lib/serialization/schema.ts @@ -292,6 +292,7 @@ export interface ReferenceType target: number | ReflectionSymbolId; qualifiedName?: string; refersToTypeParameter?: boolean; + preferValues?: boolean; } /** @category Types */ diff --git a/src/test/converter/inheritance/specs.json b/src/test/converter/inheritance/specs.json index deb966ddb..e94b85589 100644 --- a/src/test/converter/inheritance/specs.json +++ b/src/test/converter/inheritance/specs.json @@ -744,7 +744,7 @@ }, "inheritedFrom": { "type": "reference", - "target": -1, + "target": 33, "name": "My.instanceProp" } }, @@ -801,7 +801,7 @@ "extendedTypes": [ { "type": "reference", - "target": 31, + "target": 32, "name": "My", "package": "typedoc" } @@ -855,6 +855,13 @@ "character": 21, "url": "typedoc://mergable-class.ts#L10" } + ], + "extendedBy": [ + { + "type": "reference", + "target": 34, + "name": "MySubClass" + } ] }, { @@ -895,7 +902,7 @@ ], "type": { "type": "reference", - "target": 31, + "target": 32, "name": "My", "package": "typedoc" } @@ -970,14 +977,7 @@ "target": 27, "name": "MyCtor", "package": "typedoc" - }, - "extendedBy": [ - { - "type": "reference", - "target": 34, - "name": "MySubClass" - } - ] + } } ], "groups": [ diff --git a/src/test/converter2/issues/gh2106.ts b/src/test/converter2/issues/gh2106.ts new file mode 100644 index 000000000..6b167c772 --- /dev/null +++ b/src/test/converter2/issues/gh2106.ts @@ -0,0 +1,17 @@ +export function balance(address: string): Coin { + return { + amount: "", + denom: "", + }; +} + +export interface Coin { + denom: string; + amount: string; +} + +export const Coin = { + thisIsAValue: true, +}; + +export type TypeOf = typeof Coin; diff --git a/src/test/issues.c2.test.ts b/src/test/issues.c2.test.ts index a068b9e90..f6d5d2c46 100644 --- a/src/test/issues.c2.test.ts +++ b/src/test/issues.c2.test.ts @@ -928,6 +928,17 @@ describe("Issue Tests", () => { ); }); + it("Handles types/values with same name #2106", () => { + const project = convert(); + const balance = querySig(project, "balance"); + equal(balance.type?.type, "reference"); + equal(balance.type.reflection?.kind, ReflectionKind.Interface); + + const TypeOf = query(project, "TypeOf"); + equal(TypeOf.type?.type, "query"); + equal(TypeOf.type.queryType.reflection?.kind, ReflectionKind.Variable); + }); + it("#2135", () => { const project = convert(); const hook = query(project, "Camera.useCameraPermissions");