From 99a8da5c0cb5a2ec32a58ae7f0a15c3e38703148 Mon Sep 17 00:00:00 2001 From: Gerrit Birkeland Date: Mon, 1 Jan 2024 09:28:10 -0700 Subject: [PATCH] Improve recursive type detection --- CHANGELOG.md | 1 + src/lib/converter/types.ts | 25 ++++++---------- src/lib/models/reflections/abstract.ts | 2 -- src/lib/types/ts-internal/index.d.ts | 4 +++ src/lib/utils/options/readers/typedoc.ts | 1 - src/test/behavior.c2.test.ts | 9 ++++++ .../converter2/behavior/refusingToRecurse.ts | 29 +++++++++++++++++++ 7 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 src/test/converter2/behavior/refusingToRecurse.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c7735a907..8a535119d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Bug Fixes - Fixed infinite loop caused by a fix for some complicated union/intersection types, #2468. +- Improved infinite loop detection in type converter to reduce false positives. ## v0.25.5 (2024-01-01) diff --git a/src/lib/converter/types.ts b/src/lib/converter/types.ts index 9cb752a65..7de8e0cbd 100644 --- a/src/lib/converter/types.ts +++ b/src/lib/converter/types.ts @@ -102,7 +102,7 @@ export function loadConverters() { // This ought not be necessary, but we need some way to discover recursively // typed symbols which do not have type nodes. See the `recursive` symbol in the variables test. -const seenTypeSymbols = new Set(); +const seenTypes = new Set(); function maybeConvertType( context: Context, @@ -153,20 +153,12 @@ export function convertType( ); assert(node); // According to the TS source of typeToString, this is a bug if it does not hold. - const symbol = typeOrNode.getSymbol(); - if (symbol) { - if ( - node.kind !== ts.SyntaxKind.TypeReference && - node.kind !== ts.SyntaxKind.ArrayType && - seenTypeSymbols.has(symbol) - ) { - const typeString = context.checker.typeToString(typeOrNode); - context.logger.verbose( - `Refusing to recurse when converting type: ${typeString}`, - ); - return new UnknownType(typeString); - } - seenTypeSymbols.add(symbol); + if (seenTypes.has(typeOrNode.id)) { + const typeString = context.checker.typeToString(typeOrNode); + context.logger.verbose( + `Refusing to recurse when converting type: ${typeString}`, + ); + return new UnknownType(typeString); } let converter = converters.get(node.kind); @@ -179,8 +171,9 @@ export function convertType( converter = typeLiteralConverter; } + seenTypes.add(typeOrNode.id); const result = converter.convertType(context, typeOrNode, node); - if (symbol) seenTypeSymbols.delete(symbol); + seenTypes.delete(typeOrNode.id); return result; } diff --git a/src/lib/models/reflections/abstract.ts b/src/lib/models/reflections/abstract.ts index b7f582a86..00a9f6357 100644 --- a/src/lib/models/reflections/abstract.ts +++ b/src/lib/models/reflections/abstract.ts @@ -314,8 +314,6 @@ export abstract class Reflection { /** * Url safe alias for this reflection. - * - * @see {@link BaseReflection.getAlias} */ private _alias?: string; diff --git a/src/lib/types/ts-internal/index.d.ts b/src/lib/types/ts-internal/index.d.ts index 3228c5685..31b66c0c6 100644 --- a/src/lib/types/ts-internal/index.d.ts +++ b/src/lib/types/ts-internal/index.d.ts @@ -15,6 +15,10 @@ declare module "typescript" { parent?: ts.Symbol; } + interface Type { + id: number; + } + // https://github.com/microsoft/TypeScript/blob/v5.0.2/src/compiler/utilities.ts#L7432 export function getCheckFlags(symbol: ts.Symbol): CheckFlags; diff --git a/src/lib/utils/options/readers/typedoc.ts b/src/lib/utils/options/readers/typedoc.ts index f47ec57bc..6344ca81a 100644 --- a/src/lib/utils/options/readers/typedoc.ts +++ b/src/lib/utils/options/readers/typedoc.ts @@ -160,7 +160,6 @@ export class TypeDocReader implements OptionsReader { * * @param path Path to the typedoc.(js|json) file. If path is a directory * typedoc file will be attempted to be found at the root of this path - * @param logger * @return the typedoc.(js|json) file path or undefined */ private findTypedocFile(path: string): string | undefined { diff --git a/src/test/behavior.c2.test.ts b/src/test/behavior.c2.test.ts index 851393d5e..ddec3b2f5 100644 --- a/src/test/behavior.c2.test.ts +++ b/src/test/behavior.c2.test.ts @@ -1085,4 +1085,13 @@ describe("Behavior Tests", () => { } logger.expectNoOtherMessages(); }); + + it("Should not produce warnings when processing an object type twice due to intersection", () => { + const project = convert("refusingToRecurse"); + const schemaTypeBased = query(project, "schemaTypeBased"); + equal(schemaTypeBased.type?.toString(), "Object & Object"); + + logger.expectMessage("debug: Begin readme.md*"); + logger.expectNoOtherMessages({ ignoreDebug: false }); + }); }); diff --git a/src/test/converter2/behavior/refusingToRecurse.ts b/src/test/converter2/behavior/refusingToRecurse.ts new file mode 100644 index 000000000..1de022e82 --- /dev/null +++ b/src/test/converter2/behavior/refusingToRecurse.ts @@ -0,0 +1,29 @@ +type OptionalKeys = { + [K in keyof T]: T[K] extends { $opt: any } ? K : never; +}[keyof T]; + +type FromSchema = T extends typeof String + ? string + : T extends readonly [typeof Array, infer U] + ? FromSchema[] + : { + [K in OptionalKeys]?: FromSchema< + (T[K] & { $opt: unknown })["$opt"] + >; + } & { + [K in Exclude>]: FromSchema; + }; + +export const schema = { + x: [ + Array, + { + z: String, + y: { $opt: String }, + }, + ], +} as const; + +export type Schema = FromSchema; + +export const schemaTypeBased = null! as Schema;