Skip to content

Commit

Permalink
Improve recursive type detection
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerrit0 committed Jan 1, 2024
1 parent 914f9bb commit 99a8da5
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 19 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
25 changes: 9 additions & 16 deletions src/lib/converter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ts.Symbol>();
const seenTypes = new Set<number>();

function maybeConvertType(
context: Context,
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down
2 changes: 0 additions & 2 deletions src/lib/models/reflections/abstract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,6 @@ export abstract class Reflection {

/**
* Url safe alias for this reflection.
*
* @see {@link BaseReflection.getAlias}
*/
private _alias?: string;

Expand Down
4 changes: 4 additions & 0 deletions src/lib/types/ts-internal/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
1 change: 0 additions & 1 deletion src/lib/utils/options/readers/typedoc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions src/test/behavior.c2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
});
29 changes: 29 additions & 0 deletions src/test/converter2/behavior/refusingToRecurse.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
type OptionalKeys<T> = {
[K in keyof T]: T[K] extends { $opt: any } ? K : never;
}[keyof T];

type FromSchema<T> = T extends typeof String
? string
: T extends readonly [typeof Array, infer U]
? FromSchema<U>[]
: {
[K in OptionalKeys<T>]?: FromSchema<
(T[K] & { $opt: unknown })["$opt"]
>;
} & {
[K in Exclude<keyof T, OptionalKeys<T>>]: FromSchema<T[K]>;
};

export const schema = {
x: [
Array,
{
z: String,
y: { $opt: String },
},
],
} as const;

export type Schema = FromSchema<typeof schema>;

export const schemaTypeBased = null! as Schema;

0 comments on commit 99a8da5

Please sign in to comment.