Fix issue with referencing cycle with an inline type (#2602)

Change to the circularReference hook to pass a new `ReferenceCyle`
class. Referencing a cycle which has a declaration in it shouldn't be a
problem even if we reference an inline type in it. So this data
structure helps with detecting such scenarios.

common/changes/@typespec/compiler/circular-ref-fixes_2023-10-23-19-10.json

@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@typespec/compiler",
+      "comment": "",
+      "type": "none"
+    }
+  ],
+  "packageName": "@typespec/compiler"
+}

packages/compiler/src/emitter-framework/asset-emitter.ts

@@ -12,6 +12,7 @@ import {
 import { CustomKeyMap } from "./custom-key-map.js";
 import { Placeholder } from "./placeholder.js";
 import { resolveDeclarationReferenceScope } from "./ref-scope.js";
+import { ReferenceCycle } from "./reference-cycle.js";
 import { TypeEmitter } from "./type-emitter.js";
 import {
   AssetEmitter,
@@ -25,14 +26,22 @@ import {
   NamespaceScope,
   NoEmit,
   RawCode,
-  ReferenceChainEntry,
   Scope,
   SourceFile,
   SourceFileScope,
   TypeEmitterMethod,
   TypeSpecDeclaration,
 } from "./types.js";
 
+/**
+ * Represent an entry in the reference chain.
+ */
+interface ReferenceChainEntry {
+  method: string;
+  type: Type;
+  context: ContextState;
+}
+
 export function createAssetEmitter<T, TOptions extends object>(
   program: Program,
   TypeEmitterClass: typeof TypeEmitter<T, TOptions>,
@@ -223,13 +232,19 @@ export function createAssetEmitter<T, TOptions extends object>(
           waitingCircularRefs.set(entity.emitEntityKey, waiting);
         }
 
-        const circularChain = getCircularChain(referenceTypeChain, entity);
+        const typeChainSnapshot = referenceTypeChain;
         waiting.push({
           state: {
             lexicalTypeStack,
             context,
           },
-          cb: (entity) => invokeReference(this, entity, true, circularChain),
+          cb: (resolvedEntity) =>
+            invokeReference(
+              this,
+              resolvedEntity,
+              true,
+              resolveReferenceCycle(typeChainSnapshot, entity, typeToEmitEntity as any)
+            ),
         });
 
         placeholder = new Placeholder();
@@ -242,13 +257,13 @@ export function createAssetEmitter<T, TOptions extends object>(
         assetEmitter: AssetEmitter<T, TOptions>,
         entity: EmitEntity<T>,
         circular: boolean,
-        circularChain?: ReferenceChainEntry[]
+        cycle?: ReferenceCycle
       ): EmitEntity<T> {
         let ref;
         const scope = currentScope();
 
         if (circular) {
-          ref = typeEmitter.circularReference(entity, scope, circularChain!);
+          ref = typeEmitter.circularReference(entity, scope, cycle!);
         } else {
           if (entity.kind !== "declaration") {
             return entity;
@@ -567,10 +582,6 @@ export function createAssetEmitter<T, TOptions extends object>(
       ];
     }
 
-    if (!isInternalMethod(method)) {
-      referenceTypeChain = [...referenceTypeChain, stackEntryInterner.intern({ type })];
-    }
-
     lexicalTypeStack = newTypeStack;
 
     if (!programContext) {
@@ -642,6 +653,17 @@ export function createAssetEmitter<T, TOptions extends object>(
       knownContexts.set([entry, context], newContextState);
       context = newContextState;
     }
+
+    if (!isInternalMethod(method)) {
+      referenceTypeChain = [
+        ...referenceTypeChain,
+        stackEntryInterner.intern({
+          method,
+          type,
+          context,
+        }),
+      ];
+    }
   }
 
   /**
@@ -657,6 +679,7 @@ export function createAssetEmitter<T, TOptions extends object>(
     const oldRefTypeStack = referenceTypeChain;
 
     setContextForType(method, args);
+
     cb();
 
     context = oldContext;
@@ -846,10 +869,21 @@ function keyHasReferenceContext(key: keyof TypeEmitter<any, any>): boolean {
   return !noReferenceContext.has(key);
 }
 
-function getCircularChain(stack: ReferenceChainEntry[], entity: CircularEmit) {
+function resolveReferenceCycle(
+  stack: ReferenceChainEntry[],
+  entity: CircularEmit,
+  typeToEmitEntity: CustomKeyMap<[string, Type, ContextState], EmitEntity<unknown>>
+): ReferenceCycle {
   for (let i = stack.length - 1; i >= 0; i--) {
     if (stack[i].type === entity.emitEntityKey[1]) {
-      return stack.slice(i);
+      return new ReferenceCycle(
+        stack.slice(i).map((x) => {
+          return {
+            type: x.type,
+            entity: typeToEmitEntity.get([x.method, x.type, x.context])!,
+          };
+        })
+      );
     }
   }
   throw new Error(

packages/compiler/src/emitter-framework/index.ts

@@ -3,5 +3,6 @@ export * from "./builders/array-builder.js";
 export * from "./builders/object-builder.js";
 export * from "./builders/string-builder.js";
 export * from "./placeholder.js";
+export { ReferenceCycle } from "./reference-cycle.js";
 export * from "./type-emitter.js";
 export * from "./types.js";

packages/compiler/src/emitter-framework/reference-cycle.ts

@@ -0,0 +1,44 @@
+import { Type, getTypeName } from "../index.js";
+import { EmitEntity } from "./types.js";
+
+export interface ReferenceCycleEntry {
+  type: Type;
+  entity: EmitEntity<unknown>;
+}
+
+/**
+ * Represent a reference cycle.
+ * The cycle entries will always start with a declaration if there is one in the cycle.
+ */
+export class ReferenceCycle implements Iterable<ReferenceCycleEntry> {
+  /**
+   * If this cycle contains a declaration.
+   */
+  readonly containsDeclaration: boolean;
+
+  #entries: ReferenceCycleEntry[];
+
+  constructor(entries: ReferenceCycleEntry[]) {
+    const firstDeclarationIndex = entries.findIndex((entry) => entry.entity.kind === "declaration");
+    this.containsDeclaration = firstDeclarationIndex !== -1;
+    this.#entries = this.containsDeclaration
+      ? [...entries.slice(firstDeclarationIndex), ...entries.slice(0, firstDeclarationIndex)]
+      : entries;
+  }
+
+  get first(): ReferenceCycleEntry {
+    return this.#entries[0];
+  }
+
+  [Symbol.iterator](): Iterator<ReferenceCycleEntry> {
+    return this.#entries[Symbol.iterator]();
+  }
+
+  [Symbol.toStringTag](): string {
+    return [...this.#entries, this.#entries[0]].map((x) => getTypeName(x.type)).join(" -> ");
+  }
+
+  toString(): string {
+    return this[Symbol.toStringTag]();
+  }
+}

packages/compiler/src/emitter-framework/type-emitter.ts

@@ -23,13 +23,13 @@ import {
 import { code, StringBuilder } from "./builders/string-builder.js";
 import { Placeholder } from "./placeholder.js";
 import { resolveDeclarationReferenceScope } from "./ref-scope.js";
+import { ReferenceCycle } from "./reference-cycle.js";
 import {
   AssetEmitter,
   Context,
   Declaration,
   EmitEntity,
   EmittedSourceFile,
-  ReferenceChainEntry,
   Scope,
   SourceFile,
   TypeSpecDeclaration,
@@ -722,10 +722,15 @@ export class TypeEmitter<T, TOptions extends object = Record<string, never>> {
   circularReference(
     target: EmitEntity<T>,
     scope: Scope<T> | undefined,
-    circularChain: ReferenceChainEntry[]
+    cycle: ReferenceCycle
   ): EmitEntity<T> | T {
+    if (!cycle.containsDeclaration) {
+      throw new Error(
+        `Circular references to non-declarations are not supported by this emitter. Cycle:\n${cycle}`
+      );
+    }
     if (target.kind !== "declaration") {
-      throw new Error("Circular references to non-declarations are not supported by this emitter.");
+      return target;
     }
     compilerAssert(
       scope,

packages/compiler/src/emitter-framework/types.ts

@@ -185,13 +185,6 @@ export interface LexicalTypeStackEntry {
   args: any[];
 }
 
-/**
- * Represent an entry in the reference chain.
- */
-export interface ReferenceChainEntry {
-  type: Type;
-}
-
 export interface EmitterState {
   lexicalTypeStack: LexicalTypeStackEntry[];
   context: ContextState;

packages/compiler/test/emitter-framework/circular-ref.test.ts

@@ -1,12 +1,12 @@
 import { deepStrictEqual, strictEqual } from "assert";
-import { Model, ModelProperty, Program, getTypeName } from "../../src/core/index.js";
+import { Model, ModelProperty, Program, Type, getTypeName } from "../../src/core/index.js";
 import {
   ArrayBuilder,
   Context,
   EmitEntity,
   EmitterOutput,
   ObjectBuilder,
-  ReferenceChainEntry,
+  ReferenceCycle,
   Scope,
   TypeEmitter,
   createAssetEmitter,
@@ -15,13 +15,13 @@ import { getHostForTypeSpecFile } from "./host.js";
 
 describe("compiler: emitter-framework: circular references", () => {
   interface FindOptions {
-    noDeclaration: boolean;
+    modelsInline: boolean;
     circleReference: boolean;
   }
 
   interface CircularRefEntry {
     target: EmitEntity<any>;
-    circularChain: ReferenceChainEntry[];
+    cycle: ReferenceCycle;
   }
   async function findCircularReferences(code: string, options: FindOptions) {
     const invalidReferences: CircularRefEntry[] = [];
@@ -30,7 +30,7 @@ describe("compiler: emitter-framework: circular references", () => {
       modelDeclaration(model: Model, _: string): EmitterOutput<object> {
         const obj = new ObjectBuilder();
         obj.set("props", this.emitter.emitModelProperties(model));
-        if (options.noDeclaration) {
+        if (options.modelsInline) {
           return obj; // Never make a declaration
         } else {
           return this.emitter.result.declaration(model.name, obj);
@@ -55,21 +55,21 @@ describe("compiler: emitter-framework: circular references", () => {
         }
       }
 
+      arrayLiteral(array: Model, elementType: Type) {
+        return { type: "array", items: this.emitter.emitTypeReference(elementType) };
+      }
+
       programContext(program: Program): Context {
         const sourceFile = this.emitter.createSourceFile("main");
         return { scope: sourceFile.globalScope };
       }
 
-      circularReference(
-        target: EmitEntity<any>,
-        scope: Scope<any>,
-        circularChain: ReferenceChainEntry[]
-      ) {
-        if (target.kind !== "declaration") {
-          invalidReferences.push({ target, circularChain });
+      circularReference(target: EmitEntity<any>, scope: Scope<any>, cycle: ReferenceCycle) {
+        if (!cycle.containsDeclaration) {
+          invalidReferences.push({ target, cycle });
           return target;
         }
-        return super.circularReference(target, scope, circularChain);
+        return super.circularReference(target, scope, cycle);
       }
     };
 
@@ -86,23 +86,23 @@ describe("compiler: emitter-framework: circular references", () => {
   const selfRef = `model Foo { foo: Foo }`;
   it("self referencing with declaration works fine", async () => {
     const invalidReferences = await findCircularReferences(selfRef, {
-      noDeclaration: false,
+      modelsInline: false,
       circleReference: true,
     });
     strictEqual(invalidReferences.length, 0);
   });
 
   it("self referencing without declaration report circular reference", async () => {
     const invalidReferences = await findCircularReferences(selfRef, {
-      noDeclaration: true,
+      modelsInline: true,
       circleReference: true,
     });
     strictEqual(invalidReferences.length, 1);
   });
 
   it("without circular reference inline types cause no issue", async () => {
     const invalidReferences = await findCircularReferences(selfRef, {
-      noDeclaration: true,
+      modelsInline: true,
       circleReference: false,
     });
     strictEqual(invalidReferences.length, 0);
@@ -115,14 +115,40 @@ describe("compiler: emitter-framework: circular references", () => {
       model Bar { bar: Foo }
     `;
     const result = await findCircularReferences(code, {
-      noDeclaration: true,
+      modelsInline: true,
       circleReference: true,
     });
     strictEqual(result.length, 1);
 
+    deepStrictEqual(result[0].cycle.containsDeclaration, false);
     deepStrictEqual(
-      result[0].circularChain.map((x) => getTypeName(x.type)),
+      [...result[0].cycle].map((x) => getTypeName(x.type)),
       ["Foo", "Foo.foo", "Bar", "Bar.bar"]
     );
   });
+
+  context("cycle with declaration inside", () => {
+    it("doesn't report issue if referencing a declaration in the cycle", async () => {
+      const code = `
+        model First { foo: Foo }
+        model Foo { foo: Foo[] }
+      `;
+      const result = await findCircularReferences(code, {
+        modelsInline: false,
+        circleReference: true,
+      });
+      strictEqual(result.length, 0);
+    });
+    it("doesn't report issue if referencing an inline type in the cycle", async () => {
+      const code = `
+        model First { foo: Foo[] }
+        model Foo { foo: Foo[] }
+      `;
+      const result = await findCircularReferences(code, {
+        modelsInline: false,
+        circleReference: true,
+      });
+      strictEqual(result.length, 0);
+    });
+  });
 });