Fix ModelProvider base-ctor virtual-dispatch anti-pattern

[ArcturusZhang]

Fixes #10626.

Problem

ModelProvider's base constructor was reaching back into derived-class state via virtual dispatch — the classic CA2214 anti-pattern. C# guarantees derived ctor bodies run only after base(...) returns, so any code reachable from ModelProvider..ctor that overrides BuildBaseType() (or other virtuals) and reads a derived-class field gets a NullReferenceException deep in framework code, with no obvious link back to "your derived ctor body has not run yet."

This is especially harmful for explicit extension points like BuildBaseType — extension authors discover the contract only via runtime NREs.

Two distinct call chains in ModelProvider..ctor triggered this anti-pattern:

Site 1 — AddTypeToKeep(this) (filed in #10626)

ModelProvider..ctor
 └─ CodeModelGenerator.AddTypeToKeep(this)
     └─ this.Type
         └─ this.BaseType
             └─ virtual BuildBaseType()   ← dispatches onto partially-constructed derived class

Site 2 — EnsureDiscriminatorValueExpression()

Surfaced while validating the fix end-to-end against the Azure.Provisioning.Cdn migration. After fixing site 1 above, regen still NRE'd:

ModelProvider..ctor
 └─ if (_inputModel.BaseModel is not null) DiscriminatorValueExpression = EnsureDiscriminatorValueExpression()
     └─ BaseModelProvider
         └─ BuildBaseModelProvider
             └─ get_BaseType
                 └─ virtual BuildBaseType()   ← same anti-pattern, different path

Real stack trace from a ProvisioningResourceProvider derived from ProvisioningModelProvider : ModelProvider:

at Azure.Generator.Provisioning.Providers.ProvisioningResourceProvider.BuildBaseType()
at TypeProvider.get_BaseType()
at ModelProvider.BuildBaseModelProvider()
at ModelProvider.get_BaseModelProvider()
at ModelProvider.EnsureDiscriminatorValueExpression()
at ModelProvider..ctor(InputModelType inputModel)

Fix

Three complementary changes that together remove every ctor-time reach-into-derived-state in ModelProvider:

1. Move keep-set registration out of the constructor (site 1, root cause)

Removed the AddTypeToKeep(this) call from ModelProvider..ctor. TypeFactory.CreateModel now performs the registration after CreateModelCore and PreVisitModel have completed:

// TypeFactory.cs
modelProvider = CreateModelCore(model);
foreach (var visitor in Visitors)
    modelProvider = visitor.PreVisitModel(model, modelProvider);

InputTypeToModelProvider[model] = modelProvider;
if (modelProvider != null)
{
    if (model.Access == "public")
        CodeModelGenerator.Instance.AddTypeToKeep(modelProvider);   // ← moved here
    ...
}

This mirrors the existing EnumProvider / TypeFactory.CreateEnum lifecycle, where the same if (Access == "public") AddTypeToKeep(...) registration is already performed post-construction.

2. Defer FQN resolution in AddTypeToKeep(TypeProvider) (defense in depth)

AddTypeToKeep(TypeProvider) previously called type.Type.FullyQualifiedName immediately, which is what triggered the virtual BuildBaseType dispatch. Now it stores the TypeProvider reference and resolves the FQN lazily when AdditionalRootTypes / NonRootTypes is enumerated. The keep sets are only consumed by GeneratedCodeWorkspace.PostProcessAsync (and unit tests), well after every provider has been fully constructed.

This hardens the API against any future ctor-time caller — including third-party extensions that subclass other TypeProvider types and call AddTypeToKeep from their own constructors.

The public API surface of AddTypeToKeep(string) and AddTypeToKeep(TypeProvider) is unchanged; the storage is split internally and unioned at read time.

3. Defer DiscriminatorValueExpression evaluation (site 2)

Converted DiscriminatorValueExpression from an eager-init property assigned in the ctor to a Lazy<ValueExpression?> materialized on first read:

// before
public ValueExpression? DiscriminatorValueExpression { get; init; }

if (_inputModel.BaseModel is not null)
    DiscriminatorValueExpression = EnsureDiscriminatorValueExpression();   // virtual dispatch via BaseModelProvider

// after
private readonly Lazy<ValueExpression?> _discriminatorValueExpression;
public ValueExpression? DiscriminatorValueExpression => _discriminatorValueExpression.Value;

_discriminatorValueExpression = new Lazy<ValueExpression?>(() =>
    _inputModel.BaseModel is not null ? EnsureDiscriminatorValueExpression() : null);

All DiscriminatorValueExpression consumers (ModelProvider ctor body for non-primary constructors, ModelFactoryProvider) read it during emission/serialization, far after construction. No external callers write to the property — verified across microsoft/typespec and Azure/azure-sdk-for-net.

Tests

Two regression tests in ModelProviderTests, both using a DerivedModelProviderReadingOwnField fixture whose BuildBaseType() override reads a derived-class field — would NRE before the fix:

• DerivedModelProviderConstructionDoesNotForceTypeEvaluation — covers sites 1 & 2 (registration + lazy FQN). Constructs the derived provider, calls AddTypeToKeep(TypeProvider) explicitly, verifies FQN appears in AdditionalRootTypes.
• DerivedModelProviderConstructionDoesNotForceDiscriminatorEvaluation — covers site 3. Constructs a derived provider for a model with a base + discriminator value, asserts no NRE during ctor and that DiscriminatorValueExpression is still computable on demand.

Existing PublicModelsAreIncludedInAdditionalRootTypes / InternalModelsAreNotIncludedInAdditionalRootTypes tests continue to pass — they query the keep set after MockHelpers.LoadMockGenerator(...) builds the full output library, which goes through the new TypeFactory.CreateModel registration path.

Test results:

• Microsoft.TypeSpec.Generator.Tests: 1452 / 1452 passing
• Microsoft.TypeSpec.Generator.ClientModel.Tests: 1323 / 1323 passing

End-to-end validation: re-ran the Azure.Provisioning.Cdn regen (which initially exposed both NRE sites) with this PR's binaries overlaid. Generation now completes cleanly.

Behavioral notes

• Anyone instantiating ModelProvider directly via new ModelProvider(...) instead of TypeFactory.CreateModel(...) will no longer get the auto-keep behavior. This matches EnumProvider's existing contract and is consistent with the framework's intended factory entry point. None of the in-tree generators (mgmt / provisioning / azure / scm) rely on the bypass path; the only direct new ModelProvider(...) calls are in unit tests, which assert behavior that does not depend on keep-set membership.
• DiscriminatorValueExpression is now computed on first access. Result is identical for all current call sites; the only observable difference is timing, which is irrelevant because all readers run during emission.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10628

commit: 4c534b4

[github-actions]

No changes needing a change description found.

[jorgerangel-msft]

Could we avoid recomputing this on every access of the properties ?

[ArcturusZhang]

Good point. I updated this to cache the materialized keep sets so repeated property reads don't re-enumerate providers or resolve FQNs again.

I also invalidate the cached set when AddTypeToKeep(...) actually adds a new name/provider. That path is not expected in the normal generation lifecycle after the keep sets are consumed, but it keeps the cache behavior correct for any future or test-only caller that reads the set before all registrations have completed, while preserving the main goal of not forcing TypeProvider.Type at registration time.

---

🤖 arcturus-copilot