From 81978a9f34a830803c1065aa53ff435e8318e961 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 19:19:57 +0000 Subject: [PATCH 1/5] Initial plan From a5b47383294b3c541ea2baa9a7396d0d1f910cf0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 19:48:07 +0000 Subject: [PATCH 2/5] make ModelProvider base resolution methods virtual 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> --- ...lprovider-base-virtual-2026-5-5-19-21-0.md | 7 ++ .../src/Providers/ModelProvider.cs | 18 ++- .../ModelProviders/ModelProviderTests.cs | 111 ++++++++++++++++++ 3 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 .chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md diff --git a/.chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md b/.chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md new file mode 100644 index 00000000000..c465ca8d1ed --- /dev/null +++ b/.chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md @@ -0,0 +1,7 @@ +--- +changeKind: fix +packages: + - "@typespec/http-client-csharp" +--- + +Make `ModelProvider.BuildBaseTypeProvider` and `ModelProvider.BuildBaseModelProvider` `protected virtual` so emitters that override `BuildBaseType` can keep `BaseType`, `BaseTypeProvider`, and `BaseModelProvider` consistent. Previously, overriding `BuildBaseType` could leave `BaseModelProvider` walking the original `InputModelType.BaseModel`, producing a base model chain that did not match the generated C# class hierarchy. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index de172f444a8..8925d25703c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -127,7 +127,14 @@ private IReadOnlyList BuildDerivedModels() internal override TypeProvider? BaseTypeProvider => _baseTypeProvider ??= BuildBaseTypeProvider(); private TypeProvider? _baseTypeProvider; - private TypeProvider? BuildBaseTypeProvider() + /// + /// Builds the representing the base type of this model. + /// Emitters that need to redirect a model's base class can override this method and/or + /// alongside so that + /// , , and + /// remain consistent. + /// + protected virtual TypeProvider? BuildBaseTypeProvider() { // First check if there's a generated base model if (BaseModelProvider != null) @@ -291,7 +298,14 @@ private static bool IsDiscriminator(InputProperty property) return property is InputModelProperty modelProperty && modelProperty.IsDiscriminator; } - private ModelProvider? BuildBaseModelProvider() + /// + /// Builds the representing the base model of this model. + /// Emitters that override to redirect the generated C# base + /// class should also override this method (and/or ) so + /// that , , and + /// stay consistent. + /// + protected virtual ModelProvider? BuildBaseModelProvider() { // consider models that have been customized to inherit from a different generated model if (CustomCodeView?.BaseType != null) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index 7c8e88e49d1..a4c66ec03ca 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -15,6 +15,8 @@ namespace Microsoft.TypeSpec.Generator.Tests.Providers.ModelProviders { + public class MyFrameworkBase { } + public class ModelProviderTests { [SetUp] @@ -422,6 +424,115 @@ public void BuildBaseType() Assert.AreEqual(baseModel!.Type, derivedModel!.Type.BaseType); } + // Reproduces the scenario from the issue: an emitter overrides BuildBaseType to redirect a + // model's base class to a framework type. Now that BuildBaseTypeProvider and + // BuildBaseModelProvider are virtual, the emitter can override them together so that + // BaseType, BaseTypeProvider, and BaseModelProvider remain in agreement. + [Test] + public void BaseTypeAndBaseModelProvider_AreConsistent_WhenEmitterRedirectsBaseClass() + { + var inputBase = InputFactory.Model("baseModel", usage: InputModelTypeUsage.Input, properties: []); + var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: [], baseModel: inputBase); + + // Force-create the input base provider so that, without overrides, BaseModelProvider + // would have walked the input model and returned the ModelProvider for "baseModel". + var inputBaseProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase); + Assert.IsNotNull(inputBaseProvider); + + var redirectedBaseType = new CSharpType(typeof(MyFrameworkBase)); + var redirectedProvider = new SystemObjectTypeProvider(redirectedBaseType); + var derivedProvider = new RedirectedBaseModelProvider(inputDerived, redirectedProvider); + + // BaseTypeProvider is the redirected provider (not the input model's base provider). + Assert.AreSame(redirectedProvider, derivedProvider.GetBaseTypeProviderForTest()); + + // BaseType is the redirected framework type, NOT the input model's base type. + Assert.AreSame(redirectedProvider.Type, derivedProvider.BaseType); + Assert.AreNotSame(inputBaseProvider!.Type, derivedProvider.BaseType); + + // BaseModelProvider agrees with BaseType. Since the redirected base is not a ModelProvider, + // BaseModelProvider is null instead of returning the original input-model-derived parent. + Assert.IsNull(derivedProvider.BaseModelProvider); + } + + // Verifies that BaseModelProvider and BaseTypeProvider stay consistent in the default flow. + [Test] + public void BaseModelProvider_MatchesBaseTypeProvider_InDefaultFlow() + { + var inputBase = InputFactory.Model("baseModel", usage: InputModelTypeUsage.Input, properties: []); + var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: [], baseModel: inputBase); + + // Force-create the input base provider so it is registered in the type factory. + CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase); + + var derivedProvider = new TestableModelProvider(inputDerived); + Assert.IsNotNull(derivedProvider.BaseModelProvider); + Assert.AreSame(derivedProvider.BaseModelProvider, derivedProvider.GetBaseTypeProviderForTest()); + } + + // Demonstrates that BuildBaseType can also be overridden together with BuildBaseModelProvider + // so that BaseType and BaseModelProvider stay in agreement (the core requirement of the + // referenced issue). + [Test] + public void OverridingBuildBaseTypeAndBuildBaseModelProvider_KeepsBaseTypeConsistent() + { + var inputBase = InputFactory.Model("baseModel", usage: InputModelTypeUsage.Input, properties: []); + var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: [], baseModel: inputBase); + CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase); + + var redirectedBaseType = new CSharpType(typeof(MyFrameworkBase)); + var derivedProvider = new BuildBaseTypeOverridingModelProvider(inputDerived, redirectedBaseType); + + Assert.AreEqual(redirectedBaseType, derivedProvider.BaseType); + // The emitter explicitly returned null from BuildBaseModelProvider, so it agrees with + // BaseType (no in-spec ModelProvider matches the redirected framework type). + Assert.IsNull(derivedProvider.BaseModelProvider); + } + + private class RedirectedBaseModelProvider : ModelProvider + { + private readonly TypeProvider _baseTypeProvider; + + public RedirectedBaseModelProvider(InputModelType inputModel, TypeProvider baseTypeProvider) : base(inputModel) + { + _baseTypeProvider = baseTypeProvider; + } + + protected override TypeProvider? BuildBaseTypeProvider() => _baseTypeProvider; + + // Per the recommended pattern, providers that override the base provider should also + // delegate BuildBaseType to BaseTypeProvider so that all three values stay in sync. + protected override CSharpType? BuildBaseType() => BaseTypeProvider?.Type; + + // Override BuildBaseModelProvider together with BuildBaseTypeProvider so the values + // never disagree — _baseTypeProvider may not be a ModelProvider. + protected override ModelProvider? BuildBaseModelProvider() => _baseTypeProvider as ModelProvider; + + internal TypeProvider? GetBaseTypeProviderForTest() => BaseTypeProvider; + } + + private class BuildBaseTypeOverridingModelProvider : ModelProvider + { + private readonly CSharpType _redirectedBaseType; + + public BuildBaseTypeOverridingModelProvider(InputModelType inputModel, CSharpType redirectedBaseType) : base(inputModel) + { + _redirectedBaseType = redirectedBaseType; + } + + protected override CSharpType? BuildBaseType() => _redirectedBaseType; + + // The emitter overrides BuildBaseModelProvider to keep it in sync with the redirected BaseType. + protected override ModelProvider? BuildBaseModelProvider() => null; + } + + private class TestableModelProvider : ModelProvider + { + public TestableModelProvider(InputModelType inputModel) : base(inputModel) { } + + internal TypeProvider? GetBaseTypeProviderForTest() => BaseTypeProvider; + } + [Test] public void BuildModelAsStruct() { From 90148c5972c35ad4718fe0b1d6baa9280f99eef4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 19:50:58 +0000 Subject: [PATCH 3/5] revert changelog entry per review feedback 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> --- .../fix-modelprovider-base-virtual-2026-5-5-19-21-0.md | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 .chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md diff --git a/.chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md b/.chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md deleted file mode 100644 index c465ca8d1ed..00000000000 --- a/.chronus/changes/fix-modelprovider-base-virtual-2026-5-5-19-21-0.md +++ /dev/null @@ -1,7 +0,0 @@ ---- -changeKind: fix -packages: - - "@typespec/http-client-csharp" ---- - -Make `ModelProvider.BuildBaseTypeProvider` and `ModelProvider.BuildBaseModelProvider` `protected virtual` so emitters that override `BuildBaseType` can keep `BaseType`, `BaseTypeProvider`, and `BaseModelProvider` consistent. Previously, overriding `BuildBaseType` could leave `BaseModelProvider` walking the original `InputModelType.BaseModel`, producing a base model chain that did not match the generated C# class hierarchy. From b54661235109b1e202720c1c919cf1886e34a252 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 20:12:06 +0000 Subject: [PATCH 4/5] revert BuildBaseTypeProvider to private; tighten test comment 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> --- .../src/Providers/ModelProvider.cs | 12 +-- .../ModelProviders/ModelProviderTests.cs | 80 +------------------ 2 files changed, 4 insertions(+), 88 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs index 8925d25703c..0b84b814211 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelProvider.cs @@ -127,14 +127,7 @@ private IReadOnlyList BuildDerivedModels() internal override TypeProvider? BaseTypeProvider => _baseTypeProvider ??= BuildBaseTypeProvider(); private TypeProvider? _baseTypeProvider; - /// - /// Builds the representing the base type of this model. - /// Emitters that need to redirect a model's base class can override this method and/or - /// alongside so that - /// , , and - /// remain consistent. - /// - protected virtual TypeProvider? BuildBaseTypeProvider() + private TypeProvider? BuildBaseTypeProvider() { // First check if there's a generated base model if (BaseModelProvider != null) @@ -301,8 +294,7 @@ private static bool IsDiscriminator(InputProperty property) /// /// Builds the representing the base model of this model. /// Emitters that override to redirect the generated C# base - /// class should also override this method (and/or ) so - /// that , , and + /// class should also override this method so that and /// stay consistent. /// protected virtual ModelProvider? BuildBaseModelProvider() diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index a4c66ec03ca..d5c8716d23a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -424,55 +424,8 @@ public void BuildBaseType() Assert.AreEqual(baseModel!.Type, derivedModel!.Type.BaseType); } - // Reproduces the scenario from the issue: an emitter overrides BuildBaseType to redirect a - // model's base class to a framework type. Now that BuildBaseTypeProvider and - // BuildBaseModelProvider are virtual, the emitter can override them together so that - // BaseType, BaseTypeProvider, and BaseModelProvider remain in agreement. - [Test] - public void BaseTypeAndBaseModelProvider_AreConsistent_WhenEmitterRedirectsBaseClass() - { - var inputBase = InputFactory.Model("baseModel", usage: InputModelTypeUsage.Input, properties: []); - var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: [], baseModel: inputBase); - - // Force-create the input base provider so that, without overrides, BaseModelProvider - // would have walked the input model and returned the ModelProvider for "baseModel". - var inputBaseProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase); - Assert.IsNotNull(inputBaseProvider); - - var redirectedBaseType = new CSharpType(typeof(MyFrameworkBase)); - var redirectedProvider = new SystemObjectTypeProvider(redirectedBaseType); - var derivedProvider = new RedirectedBaseModelProvider(inputDerived, redirectedProvider); - - // BaseTypeProvider is the redirected provider (not the input model's base provider). - Assert.AreSame(redirectedProvider, derivedProvider.GetBaseTypeProviderForTest()); - - // BaseType is the redirected framework type, NOT the input model's base type. - Assert.AreSame(redirectedProvider.Type, derivedProvider.BaseType); - Assert.AreNotSame(inputBaseProvider!.Type, derivedProvider.BaseType); - - // BaseModelProvider agrees with BaseType. Since the redirected base is not a ModelProvider, - // BaseModelProvider is null instead of returning the original input-model-derived parent. - Assert.IsNull(derivedProvider.BaseModelProvider); - } - - // Verifies that BaseModelProvider and BaseTypeProvider stay consistent in the default flow. - [Test] - public void BaseModelProvider_MatchesBaseTypeProvider_InDefaultFlow() - { - var inputBase = InputFactory.Model("baseModel", usage: InputModelTypeUsage.Input, properties: []); - var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: [], baseModel: inputBase); - - // Force-create the input base provider so it is registered in the type factory. - CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase); - - var derivedProvider = new TestableModelProvider(inputDerived); - Assert.IsNotNull(derivedProvider.BaseModelProvider); - Assert.AreSame(derivedProvider.BaseModelProvider, derivedProvider.GetBaseTypeProviderForTest()); - } - - // Demonstrates that BuildBaseType can also be overridden together with BuildBaseModelProvider - // so that BaseType and BaseModelProvider stay in agreement (the core requirement of the - // referenced issue). + // Verifies that overriding BuildBaseType together with BuildBaseModelProvider keeps + // BaseType and BaseModelProvider in agreement. [Test] public void OverridingBuildBaseTypeAndBuildBaseModelProvider_KeepsBaseTypeConsistent() { @@ -489,28 +442,6 @@ public void OverridingBuildBaseTypeAndBuildBaseModelProvider_KeepsBaseTypeConsis Assert.IsNull(derivedProvider.BaseModelProvider); } - private class RedirectedBaseModelProvider : ModelProvider - { - private readonly TypeProvider _baseTypeProvider; - - public RedirectedBaseModelProvider(InputModelType inputModel, TypeProvider baseTypeProvider) : base(inputModel) - { - _baseTypeProvider = baseTypeProvider; - } - - protected override TypeProvider? BuildBaseTypeProvider() => _baseTypeProvider; - - // Per the recommended pattern, providers that override the base provider should also - // delegate BuildBaseType to BaseTypeProvider so that all three values stay in sync. - protected override CSharpType? BuildBaseType() => BaseTypeProvider?.Type; - - // Override BuildBaseModelProvider together with BuildBaseTypeProvider so the values - // never disagree — _baseTypeProvider may not be a ModelProvider. - protected override ModelProvider? BuildBaseModelProvider() => _baseTypeProvider as ModelProvider; - - internal TypeProvider? GetBaseTypeProviderForTest() => BaseTypeProvider; - } - private class BuildBaseTypeOverridingModelProvider : ModelProvider { private readonly CSharpType _redirectedBaseType; @@ -526,13 +457,6 @@ public BuildBaseTypeOverridingModelProvider(InputModelType inputModel, CSharpTyp protected override ModelProvider? BuildBaseModelProvider() => null; } - private class TestableModelProvider : ModelProvider - { - public TestableModelProvider(InputModelType inputModel) : base(inputModel) { } - - internal TypeProvider? GetBaseTypeProviderForTest() => BaseTypeProvider; - } - [Test] public void BuildModelAsStruct() { From 893720d96dd3abb04dfc8fd70f9fd54c195d1579 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 5 May 2026 20:19:13 +0000 Subject: [PATCH 5/5] test: redirect BaseModelProvider to a custom ModelProvider type 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> --- .../ModelProviders/ModelProviderTests.cs | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index d5c8716d23a..663ccd88e28 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -15,8 +15,6 @@ namespace Microsoft.TypeSpec.Generator.Tests.Providers.ModelProviders { - public class MyFrameworkBase { } - public class ModelProviderTests { [SetUp] @@ -424,8 +422,9 @@ public void BuildBaseType() Assert.AreEqual(baseModel!.Type, derivedModel!.Type.BaseType); } - // Verifies that overriding BuildBaseType together with BuildBaseModelProvider keeps - // BaseType and BaseModelProvider in agreement. + // Verifies an emitter can override BuildBaseType and BuildBaseModelProvider together to + // redirect the C# base class to a custom ModelProvider, keeping BaseType and + // BaseModelProvider in agreement. [Test] public void OverridingBuildBaseTypeAndBuildBaseModelProvider_KeepsBaseTypeConsistent() { @@ -433,28 +432,33 @@ public void OverridingBuildBaseTypeAndBuildBaseModelProvider_KeepsBaseTypeConsis var inputDerived = InputFactory.Model("derivedModel", usage: InputModelTypeUsage.Input, properties: [], baseModel: inputBase); CodeModelGenerator.Instance.TypeFactory.CreateModel(inputBase); - var redirectedBaseType = new CSharpType(typeof(MyFrameworkBase)); - var derivedProvider = new BuildBaseTypeOverridingModelProvider(inputDerived, redirectedBaseType); + var customBaseProvider = new CustomBaseModelProvider(inputBase); + var derivedProvider = new BuildBaseTypeOverridingModelProvider(inputDerived, customBaseProvider); + + Assert.AreEqual(customBaseProvider.Type, derivedProvider.BaseType); + Assert.AreSame(customBaseProvider, derivedProvider.BaseModelProvider); + } + + private class CustomBaseModelProvider : ModelProvider + { + public CustomBaseModelProvider(InputModelType inputModel) : base(inputModel) { } - Assert.AreEqual(redirectedBaseType, derivedProvider.BaseType); - // The emitter explicitly returned null from BuildBaseModelProvider, so it agrees with - // BaseType (no in-spec ModelProvider matches the redirected framework type). - Assert.IsNull(derivedProvider.BaseModelProvider); + protected override string BuildName() => "CustomBase"; } private class BuildBaseTypeOverridingModelProvider : ModelProvider { - private readonly CSharpType _redirectedBaseType; + private readonly ModelProvider _redirectedBaseProvider; - public BuildBaseTypeOverridingModelProvider(InputModelType inputModel, CSharpType redirectedBaseType) : base(inputModel) + public BuildBaseTypeOverridingModelProvider(InputModelType inputModel, ModelProvider redirectedBaseProvider) : base(inputModel) { - _redirectedBaseType = redirectedBaseType; + _redirectedBaseProvider = redirectedBaseProvider; } - protected override CSharpType? BuildBaseType() => _redirectedBaseType; + protected override CSharpType? BuildBaseType() => _redirectedBaseProvider?.Type; // The emitter overrides BuildBaseModelProvider to keep it in sync with the redirected BaseType. - protected override ModelProvider? BuildBaseModelProvider() => null; + protected override ModelProvider? BuildBaseModelProvider() => _redirectedBaseProvider; } [Test]