Skip to content

[http-client-csharp] Consolidate InputType graph walkers into a single visitor #10669

@JoshLove-msft

Description

@JoshLove-msft

Background

While reviewing #10666 (dynamic loading of ExternalTypes), @JoshLove-msft noted that ExternalTypeReferenceResolver.CollectExternalTypes/VisitType duplicates the recursive type-graph traversal already implemented in InputModelTypeConverter.MarkModelsAsDynamicRecursive.

A third (or fourth) walker is likely needed in the future, so we should consolidate to a single canonical visitor.

Current duplication

Two callers in the repo today:

  1. InputModelTypeConverter.MarkModelsAsDynamicRecursive (Microsoft.TypeSpec.Generator.Input/src/InputTypes/Serialization/InputModelTypeConverter.cs)

    • Seed: a single InputModelType (during deserialization, before the library is fully built)
    • Side effect: sets modelType.IsDynamicModel = true
    • Special rule: walks BaseModel only when the base has a DiscriminatorProperty / DiscriminatorValue
  2. ExternalTypeReferenceResolver.CollectExternalTypes/VisitType (Microsoft.TypeSpec.Generator/src/Utilities/ExternalTypeReferenceResolver.cs)

    • Seed: the whole InputLibrary (Models + Enums + Constants + every client method parameter & response body)
    • Side effect: collects InputExternalTypeMetadata entries with non-null Package into a dictionary keyed by (package, version, identity)

Both walk: InputModelType → derived/discriminated subtypes → BaseModel → properties; and InputArrayType / InputDictionaryType / InputNullableType / InputUnionType value/variant types. Cycle detection via identity-based HashSet<InputType>.

Proposed shape

A static class InputTypeWalker (or instance InputTypeVisitor) in Microsoft.TypeSpec.Generator.Input that takes:

  • A seed (InputType, IEnumerable<InputType>, InputNamespace, or InputLibrary overloads)
  • A visit callback that returns a VisitAction enum: Continue / SkipChildren / Stop
public enum VisitAction { Continue, SkipChildren, Stop }

public static class InputTypeWalker
{
    public static void Walk(InputType root, Func<InputType, VisitAction> visit);
    public static void Walk(InputNamespace ns, Func<InputType, VisitAction> visit);
    public static void Walk(InputLibrary library, Func<InputType, VisitAction> visit);
}

Why a visitor-with-callback instead of IEnumerable<InputType> on InputLibrary (the original suggestion):

  • Each caller needs different control (side effects, pruning, cycle handling).
  • IEnumerable would force callers to either accept the full enumeration cost or re-implement filtering.
  • A callback also lets the special-case rules (e.g. MarkModelsAsDynamicRecursive's base-discriminator walk) live inside a single owner-decision point rather than being scattered across the visitor and the consumer.

Migration

  1. Land InputTypeWalker with a comprehensive set of unit tests (cycles, every container type, every entry-point overload).
  2. Migrate MarkModelsAsDynamicRecursive — must preserve the base-discriminator special rule and produce the same set of dynamically-marked models on the existing test corpus.
  3. Migrate ExternalTypeReferenceResolver.CollectExternalTypes — must produce the same metadata set on the existing tests.
  4. Delete the duplicated traversal code.

Acceptance criteria

  • One canonical type-graph walker in Microsoft.TypeSpec.Generator.Input.
  • All existing MarkModelsAsDynamicRecursive and ExternalTypeReferenceResolver tests pass unchanged.
  • New unit tests cover the walker directly (every node type, cycles, prune/stop semantics).

cc @JoshLove-msft (per discussion in #10666 (review thread r3231221827))

Metadata

Metadata

Assignees

No one assigned

    Labels

    emitter:client:csharpIssue for the C# client emitter: @typespec/http-client-csharpfeatureNew feature or request

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions