Fix/wrapper alias handling (#1876)

* test: add failing test for anonymous recursive type in wrapper

Reveals bug where anonymous recursive types inside wrappers
(e.g., Default<{ self: Root }, {}>) create dangling $ref because
the synthetic name is created by cycle detection but never stored
in definitions due to isWrapperContext check.

The test shows that when an anonymous object type containing a
recursive reference is wrapped in Default/Cell/Stream, the cycle
detection correctly creates AnonymousType_1 and emits a $ref to it,
but the definition is never stored in $defs because line 224 in
schema-generator.ts prevents synthetic names from being used when
isWrapperContext is true.

Reported by automated PR review bot.

* Bug Fixed: Anonymous recursive types in wrappers were creating dangling  references.

  Root Cause: When a named type (like SchemaRoot) was already in progress or existed in definitions, the code was incorrectly deleting it from the
  definitionStack at line 188-190. This broke cycle detection for any child types being formatted, because the parent type was no longer on the stack
  to detect the cycle.

  The Fix: Removed the incorrect definitionStack.delete() call at lines 188-190. This deletion was logically wrong because:
  1. We never pushed to the stack at that point (we return early before line 210's push)
  2. If something with that key WAS on the stack, it was from a parent call, and deleting it broke cycle detection

  Test Updates:
  - Updated anonymous-recursive-wrapper.expected.json to reflect the correct behavior: anonymous objects that appear only once should be inlined, not
  hoisted to separate definitions
  - The test now passes with the schema correctly inlining the anonymous object and using  for SchemaRoot's cycle

* fix(schema-generator): handle wrapper type aliases with generic parameters

  Fixes improper type extraction from wrapper alias declarations that contain
  generic type parameters instead of concrete types.

  When processing wrapper aliases like 'type MyCell<T> = Cell<T>' or nested
  aliases like 'Cell<NumberList<number>>' where 'NumberList<T> = T[]', the
  code was incorrectly extracting type information from alias declarations
  (which contain generic 'T') instead of from usage sites (which have
  concrete types like 'string' or 'number').

  Key changes:
  - Filter out type parameter nodes in wrapper and array formatters by checking
    if extracted types have TypeFlags.TypeParameter flag
  - Fix formatChildType to properly clear parent typeNode context using
    destructuring, preventing mismatched type/node pairs in recursive calls
  - Improve Default wrapper node selection to prefer usage-site nodes with
    concrete type arguments over alias declaration nodes
  - Update wrapper-aliases test fixture to use realistic interface definitions
    matching actual CommonTools wrappers instead of simple type erasure

  Before: 'Cell<NumberList<number>>' generated 'items: {}' (empty object)
  After: 'Cell<NumberList<number>>' generates 'items: { type: number }'

  Related to commit 16aceaf which fixed anonymous recursive wrapper refs.

* fmt, lint

* update fixture expectation for new improved behavior

* fix fixture

Author: mathpirate

packages/schema-generator/src/formatters/common-tools-formatter.ts

@@ -5,6 +5,7 @@ import type {
   TypeFormatter,
 } from "../interface.ts";
 import type { SchemaGenerator } from "../schema-generator.ts";
+import { detectWrapperViaNode, resolveWrapperNode } from "../type-utils.ts";
 
 type WrapperKind = "Cell" | "Stream" | "OpaqueRef";
 
@@ -24,35 +25,66 @@ export class CommonToolsFormatter implements TypeFormatter {
   }
 
   supportsType(type: ts.Type, context: GenerationContext): boolean {
-    // Prefer Default via node (erased at type-level)
-    const n = context.typeNode;
-    if (n && ts.isTypeReferenceNode(n)) {
-      const nodeName = this.getTypeRefIdentifierName(n);
-      if (
-        nodeName === "Default" || this.isAliasToDefault(n, context.typeChecker)
-      ) {
-        return true;
-      }
+    // Check via typeNode for Default (erased at type-level) and all wrapper aliases
+    const wrapperViaNode = detectWrapperViaNode(
+      context.typeNode,
+      context.typeChecker,
+    );
+    if (wrapperViaNode) {
+      return true;
     }
 
-    // Check if this is a wrapper type (Cell/Stream/OpaqueRef)
-    return this.getWrapperTypeInfo(type) !== undefined;
+    // Check if this is a wrapper type (Cell/Stream/OpaqueRef) via type structure
+    const wrapperInfo = this.getWrapperTypeInfo(type);
+    return wrapperInfo !== undefined;
   }
 
   formatType(type: ts.Type, context: GenerationContext): SchemaDefinition {
     const n = context.typeNode;
 
+    // Check via typeNode for all wrapper types (handles both direct usage and aliases)
+    const resolvedWrapper = n
+      ? resolveWrapperNode(n, context.typeChecker)
+      : undefined;
+
     // Handle Default via node (direct or alias)
-    if (n && ts.isTypeReferenceNode(n)) {
-      const isDefaultNode =
-        (ts.isIdentifier(n.typeName) && n.typeName.text === "Default") ||
-        this.isAliasToDefault(n, context.typeChecker);
-      if (isDefaultNode) {
-        return this.formatDefaultType(n, context);
+    if (resolvedWrapper?.kind === "Default") {
+      // For Default, we need the node with concrete type arguments.
+      // If the original node has type arguments, use it.
+      // Otherwise, use the resolved node (for direct Default references).
+      const nodeForDefault = n && ts.isTypeReferenceNode(n) && n.typeArguments
+        ? n // Original has type args, use it for concrete types
+        : resolvedWrapper.node; // Direct reference or fallback
+
+      if (nodeForDefault && ts.isTypeReferenceNode(nodeForDefault)) {
+        return this.formatDefaultType(nodeForDefault, context);
+      }
+    }
+
+    // Handle Cell/Stream/OpaqueRef via node (direct or alias)
+    if (resolvedWrapper && resolvedWrapper.kind !== "Default") {
+      // Use the ACTUAL type from the usage site (which has concrete type arguments)
+      const wrapperInfo = this.getWrapperTypeInfo(type);
+      if (wrapperInfo) {
+        // For choosing which node to pass to formatWrapperType:
+        // - If original node has type arguments: use it (has concrete types from usage site)
+        // - If original node is just identifier (alias): use resolved node
+        //   formatWrapperType will check if node has type args before extracting inner types
+        const nodeToPass = n && ts.isTypeReferenceNode(n) && n.typeArguments
+          ? n // Original has type args, use it
+          : resolvedWrapper.node; // Original is just alias, use resolved (but won't extract inner types from it)
+
+        return this.formatWrapperType(
+          wrapperInfo.typeRef,
+          nodeToPass,
+          context,
+          wrapperInfo.kind,
+        );
       }
     }
 
-    // Get wrapper type information (Cell/Stream/OpaqueRef)
+    // Fallback: try to get wrapper type information from type structure
+    // (for cases where we don't have a typeNode)
     const wrapperInfo = this.getWrapperTypeInfo(type);
     if (wrapperInfo) {
       return this.formatWrapperType(
@@ -76,9 +108,29 @@ export class CommonToolsFormatter implements TypeFormatter {
     wrapperKind: WrapperKind,
   ): SchemaDefinition {
     const innerTypeFromType = typeRef.typeArguments?.[0];
-    const innerTypeNode = typeRefNode && ts.isTypeReferenceNode(typeRefNode)
-      ? typeRefNode.typeArguments?.[0]
-      : undefined;
+
+    // Only extract innerTypeNode if the typeRefNode has type arguments AND
+    // those arguments are not generic type parameters.
+    // If typeRefNode has no type arguments, or if the arguments are generic parameters
+    // (e.g., T from an alias declaration), we should NOT extract inner types from it.
+    let innerTypeNode: ts.TypeNode | undefined = undefined;
+    if (
+      typeRefNode && ts.isTypeReferenceNode(typeRefNode) &&
+      typeRefNode.typeArguments
+    ) {
+      const firstArg = typeRefNode.typeArguments[0];
+      if (firstArg) {
+        // Check if this node represents a type parameter
+        const argType = context.typeChecker.getTypeFromTypeNode(firstArg);
+        const isTypeParameter =
+          (argType.flags & ts.TypeFlags.TypeParameter) !== 0;
+        if (!isTypeParameter) {
+          // Not a type parameter, safe to use
+          innerTypeNode = firstArg;
+        }
+        // Otherwise leave innerTypeNode as undefined (don't use type parameter nodes)
+      }
+    }
 
     // Resolve inner type, preferring type information; fall back to node if needed
     let innerType: ts.Type | undefined = innerTypeFromType;
@@ -91,13 +143,23 @@ export class CommonToolsFormatter implements TypeFormatter {
       );
     }
 
+    // When we resolve aliases (e.g., StringCell -> Cell<string>), the resolved node's
+    // type arguments may contain unbound generics (e.g., T) from the alias declaration.
+    // In that case, we must NOT pass the node, since the type information has the
+    // concrete types (e.g., string) from the usage site.
+    // We detect this by checking if the inner type is a type parameter.
+    const innerTypeIsGeneric =
+      (innerType.flags & ts.TypeFlags.TypeParameter) !== 0;
+
     // Don't pass synthetic TypeNodes - they lose type information (especially for arrays)
     // Synthetic nodes have pos === -1 and end === -1
     // But DO pass real TypeNodes from source code for proper type detection (e.g., Default)
     const isSyntheticNode = innerTypeNode && innerTypeNode.pos === -1 &&
       innerTypeNode.end === -1;
 
-    const shouldPassTypeNode = innerTypeNode && !isSyntheticNode;
+    // Only pass typeNode if it's real (not synthetic) AND not a generic type parameter
+    const shouldPassTypeNode = innerTypeNode && !isSyntheticNode &&
+      !innerTypeIsGeneric;
     const innerSchema = this.schemaGenerator.formatChildType(
       innerType,
       context,
@@ -186,62 +248,6 @@ export class CommonToolsFormatter implements TypeFormatter {
       (type as ts.TypeReference).target?.symbol?.name === "Stream";
   }
 
-  private isAliasToDefault(
-    typeNode: ts.TypeReferenceNode,
-    typeChecker: ts.TypeChecker,
-  ): boolean {
-    return this.followAliasChain(typeNode, typeChecker, new Set());
-  }
-
-  private followAliasChain(
-    typeNode: ts.TypeReferenceNode,
-    typeChecker: ts.TypeChecker,
-    visited: Set<string>,
-  ): boolean {
-    if (!ts.isIdentifier(typeNode.typeName)) {
-      return false;
-    }
-
-    const typeName = typeNode.typeName.text;
-
-    // Detect circular aliases and throw descriptive error
-    if (visited.has(typeName)) {
-      const aliasChain = Array.from(visited).join(" -> ");
-      throw new Error(
-        `Circular type alias detected: ${aliasChain} -> ${typeName}`,
-      );
-    }
-    visited.add(typeName);
-
-    // Check if we've reached "Default"
-    if (typeName === "Default") {
-      return true;
-    }
-
-    // Look up the symbol for this type name
-    const symbol = typeChecker.getSymbolAtLocation(typeNode.typeName);
-    if (!symbol || !(symbol.flags & ts.SymbolFlags.TypeAlias)) {
-      return false;
-    }
-
-    const aliasDeclaration = symbol.valueDeclaration ||
-      symbol.declarations?.[0];
-    if (!aliasDeclaration || !ts.isTypeAliasDeclaration(aliasDeclaration)) {
-      return false;
-    }
-
-    const aliasedType = aliasDeclaration.type;
-    if (
-      ts.isTypeReferenceNode(aliasedType) &&
-      ts.isIdentifier(aliasedType.typeName)
-    ) {
-      // Recursively follow the alias chain
-      return this.followAliasChain(aliasedType, typeChecker, visited);
-    }
-
-    return false;
-  }
-
   private formatDefaultType(
     typeRefNode: ts.TypeReferenceNode,
     context: GenerationContext,

packages/schema-generator/src/schema-generator.ts

@@ -14,6 +14,7 @@ import { CommonToolsFormatter } from "./formatters/common-tools-formatter.ts";
 import { UnionFormatter } from "./formatters/union-formatter.ts";
 import { IntersectionFormatter } from "./formatters/intersection-formatter.ts";
 import {
+  detectWrapperViaNode,
   getNamedTypeKey,
   isDefaultTypeRef,
   safeGetIndexTypeOfType,
@@ -85,7 +86,11 @@ export class SchemaGenerator implements ISchemaGenerator {
     context: GenerationContext,
     typeNode?: ts.TypeNode,
   ): SchemaDefinition {
-    const childContext = typeNode ? { ...context, typeNode } : context;
+    // IMPORTANT: Always create a new context, replacing typeNode (even if undefined).
+    // If we pass the parent context as-is when typeNode is undefined, the child will
+    // inherit the parent's typeNode which leads to mismatched type/node pairs.
+    const { typeNode: _, ...baseContext } = context;
+    const childContext = typeNode ? { ...context, typeNode } : baseContext;
     return this.formatType(type, childContext, false);
   }
 
@@ -160,13 +165,12 @@ export class SchemaGenerator implements ISchemaGenerator {
     // Check if we're in a wrapper context (Default/Cell/Stream/OpaqueRef).
     // Wrapper types erase to their inner type, so we must check typeNode to
     // distinguish wrapper context from inner context.
-    const typeNodeName =
-      context.typeNode && ts.isTypeReferenceNode(context.typeNode) &&
-        ts.isIdentifier(context.typeNode.typeName)
-        ? context.typeNode.typeName.text
-        : undefined;
-    const isWrapperContext = typeNodeName &&
-      ["Default", "Cell", "Stream", "OpaqueRef"].includes(typeNodeName);
+    // This now handles both direct wrappers and aliases (e.g., type MyDefault<T> = Default<T, T>)
+    const wrapperKind = detectWrapperViaNode(
+      context.typeNode,
+      context.typeChecker,
+    );
+    const isWrapperContext = wrapperKind !== undefined;
 
     let namedKey = getNamedTypeKey(type, context.typeNode);
 
@@ -175,15 +179,14 @@ export class SchemaGenerator implements ISchemaGenerator {
       const synthetic = context.anonymousNames.get(type);
       if (synthetic) namedKey = synthetic;
     }
+
+    // Check if this type is already being built or exists
     if (namedKey) {
       if (
         context.inProgressNames.has(namedKey) || context.definitions[namedKey]
       ) {
         // Already being built or exists: emit a ref
         context.emittedRefs.add(namedKey);
-        context.definitionStack.delete(
-          this.createStackKey(type, context.typeNode, context.typeChecker),
-        );
         return { "$ref": `#/$defs/${namedKey}` };
       }
       // Start building this named type; we'll store the result below