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 37f85d828bf..2f1fa32e1c5 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 @@ -642,6 +642,68 @@ private ConstructorProvider BuildFullConstructor() this); } + protected internal override IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) + { + // Only handle the case of changing modifiers on abstract base types + if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)) + { + return [.. originalConstructors]; + } + + if (LastContractView?.Constructors == null || LastContractView.Constructors.Count == 0) + { + return [.. originalConstructors]; + } + + List constructors = [.. originalConstructors]; + + // Check if the last contract had a public constructor with matching parameters + foreach (var previousConstructor in LastContractView.Constructors) + { + if (!previousConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)) + { + continue; + } + + // Find a matching constructor in the current version by parameter signature + for (int i = 0; i < constructors.Count; i++) + { + var currentConstructor = constructors[i]; + + // Check if parameters match (same count and types) + if (ParametersMatch(currentConstructor.Signature.Parameters, previousConstructor.Signature.Parameters)) + { + // Change the modifier from private protected to public + if (currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private) && + currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)) + { + currentConstructor.Signature.Update(modifiers: MethodSignatureModifiers.Public); + } + } + } + } + + return [.. constructors]; + } + + private bool ParametersMatch(IReadOnlyList params1, IReadOnlyList params2) + { + if (params1.Count != params2.Count) + { + return false; + } + + for (int i = 0; i < params1.Count; i++) + { + if (!params1[i].Type.AreNamesEqual(params2[i].Type) || params1[i].Name != params2[i].Name) + { + return false; + } + } + + return true; + } + private IEnumerable GetAllBasePropertiesForConstructorInitialization(bool includeAllHierarchyDiscriminator = false) { var properties = new Stack>(); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs index fdafe9ca32c..c5ed0328e12 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs @@ -565,17 +565,26 @@ internal void EnsureBuilt() internal void ProcessTypeForBackCompatibility() { - if (LastContractView?.Methods == null || LastContractView?.Methods.Count == 0) + var hasMethods = LastContractView?.Methods != null && LastContractView.Methods.Count > 0; + var hasConstructors = LastContractView?.Constructors != null && LastContractView.Constructors.Count > 0; + + if (!hasMethods && !hasConstructors) { return; } - Update(methods: BuildMethodsForBackCompatibility(Methods)); + var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null; + var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null; + + Update(methods: newMethods, constructors: newConstructors); } protected internal virtual IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) => [.. originalMethods]; + protected internal virtual IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) + => [.. originalConstructors]; + private IReadOnlyList? _enumValues; private bool ShouldGenerate(ConstructorProvider constructor) 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 defefe73703..4ae6598c305 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 @@ -1760,5 +1760,52 @@ protected override TypeProvider[] BuildSerializationProviders() return [new Mock() { CallBase = true }.Object]; } } + + [Test] + public async Task BackCompat_AbstractTypeConstructorAccessibility() + { + var discriminatorEnum = InputFactory.StringEnum("kindEnum", [("One", "one"), ("Two", "two")]); + var derivedInputModel = InputFactory.Model( + "DerivedModel", + discriminatedKind: "one", + properties: + [ + InputFactory.Property("kind", InputFactory.EnumMember.String("One", "one", discriminatorEnum), isRequired: true, isDiscriminator: true), + InputFactory.Property("derivedProp", InputPrimitiveType.Int32, isRequired: true) + ]); + var inputModel = InputFactory.Model( + "BaseModel", + properties: + [ + InputFactory.Property("kind", discriminatorEnum, isRequired: false, isDiscriminator: true), + InputFactory.Property("baseProp", InputPrimitiveType.String, isRequired: true) + ], + discriminatedModels: new Dictionary() { { "one", derivedInputModel }}); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "BaseModel") as ModelProvider; + + Assert.IsNotNull(modelProvider); + + // Without ProcessTypeForBackCompatibility, constructor should be private protected + var privateProtectedConstructor = modelProvider!.Constructors + .FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private) + && c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected) + && c.Signature.Parameters.Count == 1); + Assert.IsNotNull(privateProtectedConstructor, "Expected a private protected constructor before back compat processing"); + + // Call ProcessTypeForBackCompatibility to apply backward compatibility logic + modelProvider.ProcessTypeForBackCompatibility(); + + // After ProcessTypeForBackCompatibility, constructor should be public to match last contract + var publicConstructor = modelProvider.Constructors + .FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 1); + Assert.IsNotNull(publicConstructor, "Constructor modifier should be changed to public for backward compatibility"); + Assert.AreEqual("baseProp", publicConstructor!.Signature.Parameters[0].Name); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_AbstractTypeConstructorAccessibility/BaseModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_AbstractTypeConstructorAccessibility/BaseModel.cs new file mode 100644 index 00000000000..2b7d1d63862 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_AbstractTypeConstructorAccessibility/BaseModel.cs @@ -0,0 +1,13 @@ +using System.Collections.Generic; + +namespace Sample.Models +{ + public abstract partial class BaseModel + { + /// Initializes a new instance of BaseModel. + public BaseModel(string baseProp) + { + BaseProp = baseProp; + } + } +} diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index b4822db0035..2b44139c827 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -9,6 +9,7 @@ - [Model Properties](#model-properties) - [API Version Enum](#api-version-enum) - [Non-abstract Base Models](#non-abstract-base-models) + - [Model Constructors](#model-constructors) ## Overview @@ -212,3 +213,67 @@ public class BaseModel public string CommonProperty { get; set; } } ``` + +### Model Constructors + +The generator maintains backward compatibility for model constructors on abstract base types to prevent breaking changes when constructor accessibility changes. + +#### Scenario: Public Constructor on Abstract Base Type + +**Description:** When an abstract base type had a public constructor in the previous version, but the current TypeSpec generation would create a `private protected` constructor, the generator automatically changes the modifier to `public` to maintain backward compatibility. + +This commonly occurs when: + +- Migrating from autorest-generated code to TypeSpec-generated code +- Abstract base types with discriminators had public parameterless constructors in previous versions + +**Example:** + +Previous version had a public parameterless constructor: + +```csharp +public abstract partial class SearchIndexerDataIdentity +{ + /// Initializes a new instance of SearchIndexerDataIdentity. + public SearchIndexerDataIdentity() + { + } +} +``` + +Current TypeSpec would generate a private protected constructor: + +```csharp +public abstract partial class SearchIndexerDataIdentity +{ + /// Initializes a new instance of SearchIndexerDataIdentity. + /// A URI fragment specifying the type of identity. + private protected SearchIndexerDataIdentity(string odataType) + { + OdataType = odataType; + } +} +``` + +**Generated Compatibility Result:** + +When a matching public constructor exists in the last contract, the modifier is changed from `private protected` to `public`: + +```csharp +public abstract partial class SearchIndexerDataIdentity +{ + /// Initializes a new instance of SearchIndexerDataIdentity. + /// A URI fragment specifying the type of identity. + public SearchIndexerDataIdentity(string odataType) + { + OdataType = odataType; + } +} +``` + +**Key Points:** + +- Only applies to abstract base types +- The constructor must have matching parameters (same count, types, and names) +- The modifier is changed from `private protected` to `public` +- No additional constructors are generated; only the accessibility is adjusted