Skip to content

Fix C# ModelProvider constructor virtual dispatch - #11474

Open
ArcturusZhang wants to merge 1 commit into
microsoft:mainfrom
ArcturusZhang:arcturus/lazy-model-provider-discriminator
Open

Fix C# ModelProvider constructor virtual dispatch#11474
ArcturusZhang wants to merge 1 commit into
microsoft:mainfrom
ArcturusZhang:arcturus/lazy-model-provider-discriminator

Conversation

@ArcturusZhang

Copy link
Copy Markdown
Member

Summary

  • delay multi-level discriminator detection until model constructor generation needs it
  • avoid indirectly calling virtual BuildName() while a derived ModelProvider is only partially constructed
  • add a regression test covering derived providers whose generated name depends on derived constructor state

Fixes #11472

Validation

  • npm run build
  • npm test
  • npm run cop
  • eng/scripts/Generate.ps1

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: b3780828-9610-442a-b2ce-a43c6094e0b5
@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 Jul 30, 2026
@pkg-pr-new

pkg-pr-new Bot commented Jul 30, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 82f1081

@github-actions

Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@ArcturusZhang
ArcturusZhang marked this pull request as ready for review July 30, 2026 05:48
@ArcturusZhang

ArcturusZhang commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

@jorgerangel-msft jorgerangel-msft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like regen preview failed due to auth issues. I retriggered it. Otherwise changes lgtm

Assert.AreEqual("ProjectedModel", provider!.Name);
}

private sealed class DerivedModelProviderReadingOwnName : ModelProvider

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we please reuse the existing TestModelProvider type and just update the parameters it accepts?

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.

ModelProvider constructor indirectly calls virtual BuildName through discriminator calculation

2 participants