Skip to content

fix(http-client-csharp): forward discriminator to base ctor for external base models#11002

Merged
JoshLove-msft merged 3 commits into
microsoft:mainfrom
JoshLove-msft:joshlove-msft/csharp-external-base-discriminator
Jun 17, 2026
Merged

fix(http-client-csharp): forward discriminator to base ctor for external base models#11002
JoshLove-msft merged 3 commits into
microsoft:mainfrom
JoshLove-msft:joshlove-msft/csharp-external-base-discriminator

Conversation

@JoshLove-msft

Copy link
Copy Markdown
Contributor

Problem

Discriminated subtypes whose base model is marked as an external type (via the TCGC @alternateType decorator) did not forward the discriminator value in the generated base constructor call. When the base was external, it became a bare TypeProvider that cannot serve as a BaseModelProvider, so no : base(...) initializer was emitted and the discriminator literal was dropped.

Fix

Route external InputModelTypes through SystemObjectModelProvider (a real ModelProvider) so the existing constructor / discriminator-forwarding logic applies:

  • TypeFactory.CreateModel: external models now map to a SystemObjectModelProvider wrapping the resolved framework/referenced type, handled before the overridable CreateModelCore so all generators (including the Scm client generator, whose ScmTypeFactory.CreateModelCore override previously bypassed it) share the behavior.
  • OutputLibrary.BuildModels: external bases mapped to SystemObjectModelProvider are excluded from emission (the external type already exists in a referenced assembly).

Result

For a discriminated base Animal marked external, derived types now emit:

public partial class Pet : <ExternalBase>
{
    internal Pet(string name, bool trained) : base("pet", name) { ... }
}
public partial class Dog : Pet
{
    internal Dog(string name, bool trained, string breed) : base("dog", name, trained) { ... }
}

and the external base type itself is not generated.

Validation

  • Added unit + e2e tests in SystemObjectModelProviderTests.
  • Sample-TypeSpec end-to-end run confirmed the discriminator is forwarded through the real Scm emitter and the external base is excluded.
  • Full suites pass: Microsoft.TypeSpec.Generator.Tests (1536) and Microsoft.TypeSpec.Generator.ClientModel.Tests (1429); C# formatting clean.

…nal base models

External base models marked via @alternateType now map to a
SystemObjectModelProvider (a ModelProvider) in TypeFactory.CreateModel,
so derived discriminated types emit the correct base constructor call with
the discriminator value, and the external base type itself is not generated.
Handling this in CreateModel (rather than the overridable CreateModelCore)
ensures all generators, including the Scm client generator, share the behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Jun 17, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pkg-pr-new

pkg-pr-new Bot commented Jun 17, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: f6b56c2

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@azure-sdk-automation

Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

…r forwarding

- ScmTypeFactoryTests: verify an external base model maps to SystemObjectModelProvider
  and the discriminator is forwarded through the Scm factory (which overrides
  CreateModelCore) - guards against regressing the fix back into CreateModelCore only.
- SystemObjectModelProviderTests: external model that cannot be resolved falls back to
  normal generation; a property typed as an external model resolves to the external type
  while the external model itself is not generated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JoshLove-msft JoshLove-msft added this pull request to the merge queue Jun 17, 2026
Merged via the queue into microsoft:main with commit dffc2b0 Jun 17, 2026
29 checks passed
@JoshLove-msft JoshLove-msft deleted the joshlove-msft/csharp-external-base-discriminator branch June 17, 2026 17:25
chidozieononiwu pushed a commit to chidozieononiwu/typespec that referenced this pull request Jun 22, 2026
…e declares them (microsoft#11044)

Fixes microsoft#11042

## Problem
Models derived from an external/referenced MRW-generated base type (e.g.
`Azure.AI.Extensions.OpenAI` types deriving from OpenAI's
`ResponseItem`) generated `PersistableModelCreateCore` and the other
serialization `*Core` methods as `protected virtual` instead of
`protected override`. The compiler then reports CS0114 (`hides inherited
member ... add the override keyword`).

## Root cause
PR microsoft#11002 routes external `InputModelType`s through
`SystemObjectModelProvider` so derived models get a real `ModelProvider`
base. However
`SystemObjectModelProvider.ShouldSkipDerivedSerializationMethodOverrides`
unconditionally returns `true` (introduced in microsoft#9862 for hand-authored
ARM bases like `ResourceData` that do **not** declare the generated
`*Core` methods). Reusing it for *all* external bases forced derived
models to hide rather than override even when the external base is
itself MRW-generated and exposes the virtual `*Core` methods.

## Fix
`MrwSerializationTypeDefinition` now decides whether to skip the derived
overrides by reflecting on the wrapped framework type when the base is a
`SystemObjectModelProvider`: if it declares all four generated `*Core`
methods (`JsonModelWriteCore`, `JsonModelCreateCore`,
`PersistableModelWriteCore`, `PersistableModelCreateCore`), the derived
model emits `protected override`; otherwise existing behavior is
preserved (bases without those methods, e.g. `object`/`ResourceData`,
still emit `virtual`).

## Tests
Added regression tests in `SystemObjectModelSerializationTests` covering
an external base that follows the generated MRW pattern, asserting the
four `*Core` methods are emitted as `override`. Existing tests (object
base -> virtual) remain unchanged. Full ClientModel suite (1435 tests)
passes.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants