Skip to content

[csharp] Auto-resolve ModelProvider.BaseModelProvider from BaseType via CSharpTypeMap#10600

Merged
jorgerangel-msft merged 12 commits intomainfrom
copilot/fix-mismatched-base-type-values
May 6, 2026
Merged

[csharp] Auto-resolve ModelProvider.BaseModelProvider from BaseType via CSharpTypeMap#10600
jorgerangel-msft merged 12 commits intomainfrom
copilot/fix-mismatched-base-type-values

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 5, 2026

ModelProvider.BuildBaseType is virtual and emitters override it to redirect the C# base class (e.g., to a framework type they own), but BuildBaseModelProvider was private, so BaseModelProvider kept walking InputModelType.BaseModel and diverged from the generated class hierarchy. Visitors traversing BaseModelProvider would see "inherited" properties from a parent that is no longer the actual C# base.

Changes

  • Inverted the dependency between BuildBaseType and BuildBaseModelProvider so that BaseType is the source of truth and BaseModelProvider derives from it:
    • BuildBaseType now resolves the C# base CSharpType directly from CustomCodeView.BaseType (including the empty-namespace TypeProvidersByName fallback with the CreateModel warm-up loop, plus a CSharpTypeMap fallback so a customized base that matches an existing generated ModelProvider returns that provider's Type) or from _inputModel.BaseModel via CreateModel(...).Type. It no longer reads BaseModelProvider, so the previous chicken-and-egg cycle is broken.
    • BuildBaseModelProvider remains private and now looks up BaseType in CSharpTypeMap, returning the registered ModelProvider if any, else null. No recursion since BuildBaseType doesn't read BaseModelProvider.
  • Unit tests in ModelProviderTests covering:
    • Override-only-BuildBaseType redirect to another generated model auto-resolves BaseModelProvider via the map.
    • Override-only-BuildBaseType redirect to a framework / external type auto-resolves BaseModelProvider to null.
    • Override-only-BuildBaseType redirect to a non-ModelProvider TypeProvider registered in CSharpTypeMap auto-resolves BaseModelProvider to null.
    • Default case (no overrides) resolves both consistently.
    • No-base-model case yields null for both.
  • Tests use MockHelpers.LoadMockGenerator(createModelCore: ...) to mock the type factory so it returns the custom ModelProvider subclass for the relevant InputModelType, matching the pattern in existing tests like InputLibraryVisitorTests.

Example: emitter redirecting to a framework base

Emitters now only need to override BuildBaseType. BaseModelProvider is automatically null because the framework CSharpType isn't a ModelProvider in CSharpTypeMap:

internal class MyModelProvider : ModelProvider
{
    private readonly CSharpType _frameworkBase;

    public MyModelProvider(InputModelType inputModel, CSharpType frameworkBase)
        : base(inputModel) => _frameworkBase = frameworkBase;

    protected override CSharpType? BuildBaseType() => _frameworkBase;
    // No need to also override BuildBaseModelProvider — it auto-resolves to null.
}

If BuildBaseType is redirected to another generated model's CSharpType, BaseModelProvider automatically resolves to that model's ModelProvider. BaseType and BaseModelProvider stay in lock-step in all cases.

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/96294dad-fa6c-4b21-a8c7-8fcfb0107c92

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@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 May 5, 2026
Copilot AI changed the title [WIP] Fix mismatched values returned by ModelProvider.BaseType and BaseModelProvider [csharp] Make ModelProvider base resolution methods virtual to keep BaseType and BaseModelProvider consistent May 5, 2026
Copilot AI requested a review from jorgerangel-msft May 5, 2026 19:48
Comment thread .chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md Outdated
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/60279f78-9af0-440e-97b7-7026b39783c9

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 5, 2026

Open in StackBlitz

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

commit: e35a22e

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/5b6fcac7-58a2-4f8e-be3e-2b2870fbeed0

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copilot AI changed the title [csharp] Make ModelProvider base resolution methods virtual to keep BaseType and BaseModelProvider consistent [csharp] Make ModelProvider.BuildBaseModelProvider virtual to keep BaseType and BaseModelProvider consistent May 5, 2026
Copilot AI requested a review from jorgerangel-msft May 5, 2026 20:12
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/cab911a0-953a-4889-b96d-18aaccf1af83

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

No changes needing a change description found.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 6, 2026

@copilot what about this check? Why are we not porting the full checks over?

That check was an early-exit in the old BuildBaseModelProvider to prevent fall-through to walking _inputModel.BaseModel once we knew the customized base was an external type. In the new BuildBaseType it's structurally redundant: the entire CustomCodeView?.BaseType != null branch always returns (either via the empty-namespace TypeProvidersByName lookup, the new CSharpTypeMap fallback, or the final return customBase; at line 228), so we never fall through to _inputModel.BaseModel when there's a customized base. Adding if (!string.IsNullOrEmpty(customBase.Namespace)) return null; would actually break the external-type case — BuildBaseType is now the source of truth for the C# base, so for an external custom base we want to return customBase, not null. BuildBaseModelProvider then returns null for it (because the CSharpTypeMap entry, if present, is a SystemObjectTypeProvider, not a ModelProvider), giving the same end result the old code achieved via the explicit namespace check.

Copy link
Copy Markdown
Contributor

@jorgerangel-msft jorgerangel-msft left a comment

Choose a reason for hiding this comment

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

@copilot can we add a test that validates that if BuildBaseType is overridden with a custom TypeProvider (not a ModelProvider) that BaseModelProvider returns null?

…es BaseModelProvider to null

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/b96d6016-7d00-4f8f-904a-f073b5f2e121

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 6, 2026

@copilot can we add a test that validates that if BuildBaseType is overridden with a custom TypeProvider (not a ModelProvider) that BaseModelProvider returns null?

Added in a856150. The new test OverridingBuildBaseType_AutoResolvesBaseModelProviderToNullForNonModelTypeProvider registers a non-ModelProvider TypeProvider (NonModelTypeProvider) in CSharpTypeMap and overrides BuildBaseType to return its TypeBaseType resolves to the redirected CSharpType and BaseModelProvider is null since the registered provider is not a ModelProvider.

…ed test

Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/2e5a0923-a771-4653-b62b-63eb879324ab

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/18ff54a4-2957-47ba-a9fe-dad844a23b49

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
Agent-Logs-Url: https://github.com/microsoft/typespec/sessions/488ed6a5-0f9f-409a-896f-0c806507dc52

Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com>
@jorgerangel-msft jorgerangel-msft added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit 1d5f379 May 6, 2026
29 checks passed
@jorgerangel-msft jorgerangel-msft deleted the copilot/fix-mismatched-base-type-values branch May 6, 2026 20:40
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.

[csharp] ModelProvider.BaseType and BaseModelProvider can return mismatched values

3 participants