diff --git a/.chronus/changes/json-schema-fixes-2024-4-25-0-58-10.md b/.chronus/changes/json-schema-fixes-2024-4-25-0-58-10.md new file mode 100644 index 0000000000..878d132894 --- /dev/null +++ b/.chronus/changes/json-schema-fixes-2024-4-25-0-58-10.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/compiler" +--- + +Emitter framework: fix losing context when referencing circular types \ No newline at end of file diff --git a/.chronus/changes/json-schema-fixes-2024-4-25-0-58-28.md b/.chronus/changes/json-schema-fixes-2024-4-25-0-58-28.md new file mode 100644 index 0000000000..d9fcbef162 --- /dev/null +++ b/.chronus/changes/json-schema-fixes-2024-4-25-0-58-28.md @@ -0,0 +1,6 @@ +--- +changeKind: internal +packages: + - "@typespec/json-schema" +--- + diff --git a/packages/compiler/src/emitter-framework/asset-emitter.ts b/packages/compiler/src/emitter-framework/asset-emitter.ts index d42b786d3f..2d2f2d0854 100644 --- a/packages/compiler/src/emitter-framework/asset-emitter.ts +++ b/packages/compiler/src/emitter-framework/asset-emitter.ts @@ -733,8 +733,8 @@ export function createAssetEmitter( * Invoke the callback with the given context. */ function withContext(newContext: EmitterState, cb: () => void) { - const oldContext = newContext.context; - const oldTypeStack = newContext.lexicalTypeStack; + const oldContext = context; + const oldTypeStack = lexicalTypeStack; context = newContext.context; lexicalTypeStack = newContext.lexicalTypeStack; diff --git a/packages/compiler/test/emitter-framework/context.test.ts b/packages/compiler/test/emitter-framework/context.test.ts index 4f39d988c2..570a5f7ebe 100644 --- a/packages/compiler/test/emitter-framework/context.test.ts +++ b/packages/compiler/test/emitter-framework/context.test.ts @@ -504,4 +504,50 @@ describe("emitter-framework: emitter context", () => { }); }); }); + + describe("instantiation context", () => { + it("restores context when after referencing a type with a circular reference", async () => { + class Emitter extends CodeTypeEmitter { + programContext(program: Program): Context { + return { + scope: this.emitter.createSourceFile("foo.txt").globalScope, + }; + } + modelDeclarationContext(model: Model, name: string): Context { + return { + inModel: name, + }; + } + modelDeclaration(model: Model, name: string): EmitterOutput { + super.modelDeclaration(model, name); + return this.emitter.result.declaration(name, "Declaration for model " + name); + } + modelPropertyLiteralContext(property: ModelProperty): Context { + return { inProp: property.name }; + } + modelPropertyLiteral(property: ModelProperty): EmitterOutput { + const beforeContext = this.emitter.getContext(); + const res = super.modelPropertyLiteral(property); + assert.deepStrictEqual(beforeContext, this.emitter.getContext()); + return res; + } + } + + await emitTypeSpec( + Emitter, + ` + model A { + a: B; + } + + model B { + b: B; + } + + `, + {}, + false + ); + }); + }); }); diff --git a/packages/json-schema/src/json-schema-emitter.ts b/packages/json-schema/src/json-schema-emitter.ts index 81082bb7da..36da5fc205 100644 --- a/packages/json-schema/src/json-schema-emitter.ts +++ b/packages/json-schema/src/json-schema-emitter.ts @@ -96,7 +96,6 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche } this.#applyConstraints(model, schema); - return this.#createDeclaration(model, name, schema); } @@ -371,7 +370,7 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche // into the defs of the root schema which references this schema. return { $ref: "#/$defs/" + targetDeclaration.name }; } else { - // referencing a schema that is in a source file, but doesn't have an id. + // referencing a schema that is in a different source file, but doesn't have an id. // nb: this may be dead code. // when either targetSfScope or currentSfScope are undefined, we have a common scope @@ -385,7 +384,17 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche } } - throw new Error("$ref to $defs not yet supported."); + if (!currentSfScope && !targetSfScope) { + // referencing ourself, and we don't have an id (otherwise we'd have + // returned that above), so just return a ref. + // This should be accurate because the only case when this can happen is if + // the target declaration is not a root schema, and so it will be present in + // the defs of some root schema, and there is only one level of defs. + + return { $ref: "#/$defs/" + targetDeclaration.name }; + } + + throw new Error("JSON Pointer refs to arbitrary schemas is not supported"); } scalarInstantiation( @@ -719,6 +728,7 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche const decls = sourceFile.globalScope.declarations; compilerAssert(decls.length === 1, "Multiple decls in single schema per file mode"); const content: Record = { ...decls[0].value }; + const bundledDecls = new Set>(); if (sourceFile.meta.bundledRefs.length > 0) { // bundle any refs, including refs of refs @@ -726,6 +736,11 @@ export class JsonSchemaEmitter extends TypeEmitter, JSONSche const refsToBundle: Declaration[] = [...sourceFile.meta.bundledRefs]; while (refsToBundle.length > 0) { const decl = refsToBundle.shift()!; + if (bundledDecls.has(decl)) { + // already $def'd, no need to def it again. + continue; + } + bundledDecls.add(decl); content.$defs[decl.name] = decl.value; // all scopes are source file scopes in this emitter diff --git a/packages/json-schema/test/references.test.ts b/packages/json-schema/test/references.test.ts index 237422d025..ef71305e13 100644 --- a/packages/json-schema/test/references.test.ts +++ b/packages/json-schema/test/references.test.ts @@ -196,4 +196,28 @@ describe("referencing non-JSON Schema types", () => { assert(amSchemas["B.json"] !== undefined); assert(Object.keys(amSchemas).length === 2); }); + + it("handles circular refs among non-json schema types", async () => { + const schemas = await emitSchema( + ` + @jsonSchema + model R { + test: A; + } + + model A { + a: B; + } + + model B { + a: A; + } + `, + {}, + { emitNamespace: false, emitTypes: ["R"] } + ); + assert(schemas["R.json"] !== undefined); + assert(Object.keys(schemas).length === 1); + assert(Object.keys(schemas["R.json"].$defs).length === 2); + }); });