Skip to content

Commit

Permalink
Fix references to Symbols with multiple reflections
Browse files Browse the repository at this point in the history
Fixes #2106
  • Loading branch information
Gerrit0 committed Dec 28, 2023
1 parent 8eb2261 commit 6370490
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 11 additions & 11 deletions src/lib/converter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -666,13 +666,13 @@ const queryConverter: TypeConverter<ts.TypeQueryNode> = {
);
}

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 =
Expand All @@ -683,12 +683,12 @@ const queryConverter: TypeConverter<ts.TypeQueryNode> = {
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);
},
};

Expand Down
15 changes: 15 additions & 0 deletions src/lib/models/reflections/kind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 32 additions & 12 deletions src/lib/models/reflections/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ export class ProjectReflection extends ContainerReflection {
readonly variant = "project";

// Used to resolve references.
private symbolToReflectionIdMap: Map<ReflectionSymbolId, number> =
new StableKeyMap();
private symbolToReflectionIdMap: Map<
ReflectionSymbolId,
number | number[]
> = new StableKeyMap();

private reflectionIdToSymbolIdMap = new Map<number, ReflectionSymbolId>();

Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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 */
Expand All @@ -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);
}
}
Expand Down
29 changes: 27 additions & 2 deletions src/lib/models/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -984,6 +1004,10 @@ export class ReferenceType extends Type {
result.refersToTypeParameter = true;
}

if (typeof this._target !== "number" && this.preferValues) {
result.preferValues = true;
}

return result;
}

Expand Down Expand Up @@ -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;
}
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/serialization/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ export interface ReferenceType
target: number | ReflectionSymbolId;
qualifiedName?: string;
refersToTypeParameter?: boolean;
preferValues?: boolean;
}

/** @category Types */
Expand Down
22 changes: 11 additions & 11 deletions src/test/converter/inheritance/specs.json
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@
},
"inheritedFrom": {
"type": "reference",
"target": -1,
"target": 33,
"name": "My.instanceProp"
}
},
Expand Down Expand Up @@ -801,7 +801,7 @@
"extendedTypes": [
{
"type": "reference",
"target": 31,
"target": 32,
"name": "My",
"package": "typedoc"
}
Expand Down Expand Up @@ -855,6 +855,13 @@
"character": 21,
"url": "typedoc://mergable-class.ts#L10"
}
],
"extendedBy": [
{
"type": "reference",
"target": 34,
"name": "MySubClass"
}
]
},
{
Expand Down Expand Up @@ -895,7 +902,7 @@
],
"type": {
"type": "reference",
"target": 31,
"target": 32,
"name": "My",
"package": "typedoc"
}
Expand Down Expand Up @@ -970,14 +977,7 @@
"target": 27,
"name": "MyCtor",
"package": "typedoc"
},
"extendedBy": [
{
"type": "reference",
"target": 34,
"name": "MySubClass"
}
]
}
}
],
"groups": [
Expand Down
17 changes: 17 additions & 0 deletions src/test/converter2/issues/gh2106.ts
Original file line number Diff line number Diff line change
@@ -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;
11 changes: 11 additions & 0 deletions src/test/issues.c2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down

0 comments on commit 6370490

Please sign in to comment.