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 43d2559a48b..f641397b605 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 @@ -792,11 +792,15 @@ protected internal override IReadOnlyList BuildConstructors return base.BuildConstructorsForBackCompatibility(originalConstructors); } - var constructors = new List(base.BuildConstructorsForBackCompatibility(originalConstructors)); - var restorablePropertyLookup = BuildRestorablePropertyLookup(); + var originalConstructorList = originalConstructors as IReadOnlyList ?? [.. originalConstructors]; IReadOnlyList candidateConstructors = CustomCodeView?.Constructors is { Count: > 0 } customConstructors - ? [.. constructors, .. customConstructors] - : constructors; + ? [.. originalConstructorList, .. customConstructors] + : originalConstructorList; + + RestorePreviousConstructorParameterNames(originalConstructorList, candidateConstructors, previousConstructors); + + var constructors = new List(base.BuildConstructorsForBackCompatibility(originalConstructorList)); + var restorablePropertyLookup = BuildRestorablePropertyLookup(); foreach (var previousConstructor in previousConstructors) { @@ -815,11 +819,14 @@ protected internal override IReadOnlyList BuildConstructors // A previously published accessible parameterless constructor is dropped when the current // generation makes a property required. Restore it and drop the generated mocking constructor // so it is not a duplicate. An accessible parameterless constructor (generated or custom code) - // counts as already present; an inaccessible generated mocking constructor does not. - if (!Type.IsStruct && previousParameters.Count == 0) + // counts as already present; an inaccessible generated mocking constructor does not. A struct + // always exposes a public parameterless constructor via its serialization (mocking) + // constructor, so there is nothing to restore on the model partial. + if (previousParameters.Count == 0) { - if (!constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers)) - && !CanonicalView.Constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))) + if (!Type.IsStruct + && !constructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers)) + && !candidateConstructors.Any(c => c.Signature.Parameters.Count == 0 && MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers))) { var parameterlessConstructor = BuildBackCompatParameterlessConstructor(previousConstructor, candidateConstructors); RemoveGeneratedMockingConstructor(constructors); @@ -833,9 +840,9 @@ protected internal override IReadOnlyList BuildConstructors } // If a constructor with the same parameters already exists - either still generated or - // supplied by custom code (which lives in the canonical view) - there is nothing to restore. + // supplied by custom code - there is nothing to restore. if (constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters)) - || CanonicalView.Constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters))) + || candidateConstructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters))) { continue; } @@ -858,6 +865,70 @@ protected internal override IReadOnlyList BuildConstructors return constructors; } + private void RestorePreviousConstructorParameterNames( + IReadOnlyList currentConstructors, + IReadOnlyList candidateConstructors, + IReadOnlyList previousConstructors) + { + const MethodSignatureModifiers privateProtected = MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected; + foreach (var previousConstructor in previousConstructors) + { + if (!MethodSignatureHelper.IsPublicApi(previousConstructor.Signature.Modifiers)) + { + continue; + } + + var previousParameters = previousConstructor.Signature.Parameters; + + // A generated or custom constructor that already matches the previous signature (types and + // names) satisfies the contract; renaming another constructor into it would collide. + if (candidateConstructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters))) + { + continue; + } + + var currentConstructor = currentConstructors.FirstOrDefault(c => + (MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers) + || (c.Signature.Modifiers & privateProtected) == privateProtected) + && MethodSignatureBase.SignatureComparer.Equals(c.Signature, previousConstructor.Signature)); + if (currentConstructor is null) + { + continue; + } + + var currentParameters = currentConstructor.Signature.Parameters; + + // A swap or rotation keeps every previous name, so realign the existing parameter objects + // to the previous order - renaming positionally would mis-bind a caller's named argument to + // the wrong property. Otherwise restore names positionally where the types line up. + var currentByName = currentParameters.ToDictionary(p => p.Name); + IReadOnlyList restoredParameters = previousParameters.All(p => currentByName.ContainsKey(p.Name)) + ? [.. previousParameters.Select(p => currentByName[p.Name])] + : currentParameters; + if (!restoredParameters.Select((p, i) => p.Type.AreNamesEqual(previousParameters[i].Type)).All(match => match)) + { + continue; + } + + for (int i = 0; i < restoredParameters.Count; i++) + { + var restoredName = previousParameters[i].Name; + if (string.Equals(restoredParameters[i].Name, restoredName, StringComparison.Ordinal)) + { + continue; + } + + CodeModelGenerator.Instance.Emitter.Debug( + $"Preserved parameter name '{restoredName}' at position {i} on constructor '{Name}' from last contract (instead of '{restoredParameters[i].Name}').", + BackCompatibilityChangeCategory.ParameterNamePreserved); + restoredParameters[i].Update(name: restoredName); + } + + currentConstructor.Signature.Update(parameters: [.. restoredParameters]); + currentConstructor.Update(signature: currentConstructor.Signature); + } + } + private bool TryBuildRestoredConstructor( ConstructorProvider previousConstructor, IReadOnlyList currentConstructors, @@ -868,7 +939,8 @@ private bool TryBuildRestoredConstructor( var previousParameters = previousConstructor.Signature.Parameters; // Find the public constructor to chain to: its parameters must form an in-order subsequence of - // the previous constructor's parameters. Prefer the closest one. + // the previous constructor's parameters. Prefer the closest one. When none exists the restored + // constructor is standalone, assigning each parameter to its matching property. ConstructorProvider? targetConstructor = null; foreach (var candidate in currentConstructors) { @@ -888,12 +960,7 @@ private bool TryBuildRestoredConstructor( } } - if (targetConstructor == null) - { - return false; - } - - var targetParameters = targetConstructor.Signature.Parameters; + var targetParameters = targetConstructor?.Signature.Parameters ?? []; var restoredParameters = new List(previousParameters.Count); var initializerArguments = new List(targetParameters.Count); var extraAssignments = new List<(PropertyProvider Property, ParameterProvider Parameter)>(); @@ -915,26 +982,53 @@ private bool TryBuildRestoredConstructor( continue; } - if (!restorablePropertyLookup.TryGetValue(previousParameter.Name, out var property) - || !property.Type.AreNamesEqual(previousParameter.Type)) + // The chained path restores settable, wire-backed properties by parameter name (already + // indexed). A standalone path also accepts required get-only auto-properties (assignable in a + // constructor) and matches by wire name, so it can fully initialize every property itself. + PropertyProvider? property; + if (targetConstructor != null) + { + property = restorablePropertyLookup.TryGetValue(previousParameter.Name, out var chained) + && chained.Type.AreNamesEqual(previousParameter.Type) + ? chained + : null; + } + else + { + property = CanonicalView.Properties.FirstOrDefault(p => + MethodSignatureHelper.IsPublicApi(p.Modifiers) + && (p.Body.HasSetter || p.Body is AutoPropertyBody) + && p.Type.AreNamesEqual(previousParameter.Type) + && (string.Equals(p.AsParameter.Name, previousParameter.Name, StringComparison.Ordinal) + || string.Equals(p.WireInfo?.SerializedName, previousParameter.Name, StringComparison.Ordinal))); + } + + if (property is null || extraAssignments.Any(a => a.Property == property)) { return false; } - var restoredParameter = PartialMethodCustomization.CloneParameterWithName( - property.AsParameter, - previousParameter.Name, - removeDefault: true); + var restoredParameter = targetConstructor != null + ? PartialMethodCustomization.CloneParameterWithName(property.AsParameter, previousParameter.Name, removeDefault: true) + : previousParameter; restoredParameters.Add(restoredParameter); extraAssignments.Add((property, restoredParameter)); } - // Every target parameter must be consumed and at least one extra property must be assigned, - // otherwise the restored constructor would be redundant or would produce an invalid chained call. - if (targetIndex != targetParameters.Count || extraAssignments.Count == 0) + if (targetConstructor != null) { - return false; + if (targetIndex != targetParameters.Count || extraAssignments.Count == 0) + { + return false; + } + } + else + { + if (currentConstructors.Any(c => MethodSignatureBase.SignatureComparer.Equals(c.Signature, previousConstructor.Signature))) + { + return false; + } } var bodyStatements = new List(extraAssignments.Count); @@ -955,7 +1049,7 @@ private bool TryBuildRestoredConstructor( $"Initializes a new instance of {Type:C}", previousConstructor.Signature.Modifiers, restoredParameters, - initializer: new ConstructorInitializer(false, initializerArguments)); + initializer: targetConstructor is null ? null : new ConstructorInitializer(false, initializerArguments)); restoredConstructor = new ConstructorProvider(signature, bodyStatements, this); return true; 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 15a9ffec1c0..6e9bf1f3151 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 @@ -2346,6 +2346,226 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual("baseProp", publicConstructor!.Signature.Parameters[0].Name); } + [Test] + public async Task BackCompat_ConstructorParameterSwapRestoredBySignatureMatch() + { + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("vmSkuName", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "MockInputModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + var constructor = modelProvider.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.That(constructor.Signature.Parameters.Select(p => p.Name), Is.EqualTo(new[] { "name", "vmSkuName" })); + Assert.That(constructor.Signature.Parameters.Select(p => p.Property?.Name), Is.EqualTo(new[] { "Name", "VmSkuName" })); + + var content = new TypeProviderWriter(modelProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + [Test] + public async Task BackCompat_ConstructorParameterRenameRestoredBySignatureMatch() + { + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("unchanged", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("newName", InputPrimitiveType.String, isRequired: true), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "MockInputModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + var constructor = modelProvider.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.That(constructor.Signature.Parameters.Select(p => p.Name), Is.EqualTo(new[] { "unchanged", "oldName" })); + Assert.That(constructor.Signature.Parameters.Select(p => p.Property?.Name), Is.EqualTo(new[] { "Unchanged", "NewName" })); + + var content = new TypeProviderWriter(modelProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + [Test] + public async Task BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch() + { + var casingModel = InputFactory.Model( + "CasingModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("vMwareSiteId", InputPrimitiveType.String, isRequired: true), + ]); + var rotationModel = InputFactory.Model( + "RotationModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("third", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("first", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("second", InputPrimitiveType.String, isRequired: true), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [casingModel, rotationModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var models = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Where(t => t.Name is "CasingModel" or "RotationModel") + .ToDictionary(t => t.Name); + + foreach (var model in models.Values) + { + model.ProcessTypeForBackCompatibility(); + } + + var casingContent = new TypeProviderWriter(models["CasingModel"]).Write().Content; + var rotationContent = new TypeProviderWriter(models["RotationModel"]).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile("Casing"), casingContent); + Assert.AreEqual(Helpers.GetExpectedFromFile("Rotation"), rotationContent); + } + + [Test] + public async Task BackCompat_ConstructorParameterChainedRenameRestoredBySignatureMatch() + { + // Current ctor is (skuName, name); the last contract was (name, vmName). Restoring position 0 to + // "name" transiently collides with position 1's current "name", which is itself restored to + // "vmName" - so both names must still be preserved rather than skipped on the intermediate clash. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input, + properties: + [ + InputFactory.Property("skuName", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "MockInputModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + var constructor = modelProvider.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.That(constructor.Signature.Parameters.Select(p => p.Name), Is.EqualTo(new[] { "name", "vmName" })); + Assert.That(constructor.Signature.Parameters.Select(p => p.Property?.Name), Is.EqualTo(new[] { "SkuName", "Name" })); + + var content = new TypeProviderWriter(modelProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + [Test] + public async Task BackCompat_StandaloneConstructorRestoredWhenNoChainTarget() + { + // The last contract published `MockInputModel(string beta, string alpha, int gamma)`. The current + // generation reorders the required properties (so the public `(alpha, beta)` constructor is not an + // in-order subsequence to chain to) and relaxes `gamma` to optional. Because every required + // property is still covered, the previous constructor is restored as a standalone constructor that + // assigns each parameter to its matching property. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("alpha", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("beta", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("gamma", InputPrimitiveType.Int32, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "MockInputModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + var restoredCtor = modelProvider.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 3); + Assert.That(restoredCtor.Signature.Parameters.Select(p => p.Name), + Is.EqualTo(new[] { "beta", "alpha", "gamma" })); + Assert.IsNull(restoredCtor.Signature.Initializer); + + var content = new TypeProviderWriter(modelProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + [Test] + public async Task BackCompat_StandaloneConstructorRestoredWhenRequiredPropertyUnassigned() + { + // The last contract published `MockInputModel(string workloadProfileType, int minimumCount, int maximumCount)`. + // The current generation adds a required `name` property that the previous constructor has no value + // for. The constructor is still restored - reproducing the previous behavior - assigning the + // properties it can and leaving the newly-required `Name` unset (valid because the generator does + // not emit the C# `required` modifier). This is a round-trip model, so the required properties have + // setters; required-ness is tracked independently of the setter. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Output | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("workloadProfileType", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("minimumCount", InputPrimitiveType.Int32, isRequired: false), + InputFactory.Property("maximumCount", InputPrimitiveType.Int32, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType() + .Single(t => t.Name == "MockInputModel"); + + modelProvider.ProcessTypeForBackCompatibility(); + + var restoredCtor = modelProvider.Constructors.Single(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 3); + Assert.That(restoredCtor.Signature.Parameters.Select(p => p.Name), + Is.EqualTo(new[] { "workloadProfileType", "minimumCount", "maximumCount" })); + Assert.IsNull(restoredCtor.Signature.Initializer); + + // `Name` is required but has no source parameter, so the restored constructor leaves it unset. + var body = restoredCtor.BodyStatements!.ToDisplayString(); + Assert.IsFalse(body.Contains("Name ="), $"Did not expect the restored constructor to assign Name, was: {body}"); + + var content = new TypeProviderWriter(modelProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + [Test] public async Task BackCompat_ParameterlessConstructorRestored() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch(Casing).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch(Casing).cs new file mode 100644 index 00000000000..2ce0ded32f7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch(Casing).cs @@ -0,0 +1,30 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using Sample; + +namespace Sample.Models +{ + public partial class CasingModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public CasingModel(string vmwareSiteId) + { + global::Sample.Argument.AssertNotNull(vmwareSiteId, nameof(vmwareSiteId)); + + VMwareSiteId = vmwareSiteId; + } + + internal CasingModel(string vmwareSiteId, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + VMwareSiteId = vmwareSiteId; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string VMwareSiteId { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch(Rotation).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch(Rotation).cs new file mode 100644 index 00000000000..8bb76269a08 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch(Rotation).cs @@ -0,0 +1,40 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using Sample; + +namespace Sample.Models +{ + public partial class RotationModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public RotationModel(string first, string second, string third) + { + global::Sample.Argument.AssertNotNull(third, nameof(third)); + global::Sample.Argument.AssertNotNull(first, nameof(first)); + global::Sample.Argument.AssertNotNull(second, nameof(second)); + + Third = third; + First = first; + Second = second; + } + + internal RotationModel(string third, string first, string second, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Third = third; + First = first; + Second = second; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string Third { get; } + + public string First { get; } + + public string Second { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch/Models.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch/Models.cs new file mode 100644 index 00000000000..6cb0f17f4d5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterCasingAndRotationRestoredBySignatureMatch/Models.cs @@ -0,0 +1,16 @@ +namespace Sample.Models +{ + public partial class CasingModel + { + public CasingModel(string vmwareSiteId) + { + } + } + + public partial class RotationModel + { + public RotationModel(string first, string second, string third) + { + } + } +} \ No newline at end of file diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterChainedRenameRestoredBySignatureMatch.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterChainedRenameRestoredBySignatureMatch.cs new file mode 100644 index 00000000000..2722d0e2ba4 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterChainedRenameRestoredBySignatureMatch.cs @@ -0,0 +1,35 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using Sample; + +namespace Sample.Models +{ + public partial class MockInputModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public MockInputModel(string name, string vmName) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + global::Sample.Argument.AssertNotNull(vmName, nameof(vmName)); + + SkuName = name; + Name = vmName; + } + + internal MockInputModel(string name, string vmName, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + SkuName = name; + Name = vmName; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string SkuName { get; } + + public string Name { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterChainedRenameRestoredBySignatureMatch/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterChainedRenameRestoredBySignatureMatch/MockInputModel.cs new file mode 100644 index 00000000000..e51bc8cf455 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterChainedRenameRestoredBySignatureMatch/MockInputModel.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + // Previous contract named position 0 "name" (now "skuName") and position 1 "vmName" (now "name"). + public MockInputModel(string name, string vmName) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameRestoredBySignatureMatch.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameRestoredBySignatureMatch.cs new file mode 100644 index 00000000000..3fc7eee2f25 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameRestoredBySignatureMatch.cs @@ -0,0 +1,35 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using Sample; + +namespace Sample.Models +{ + public partial class MockInputModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public MockInputModel(string unchanged, string oldName) + { + global::Sample.Argument.AssertNotNull(unchanged, nameof(unchanged)); + global::Sample.Argument.AssertNotNull(oldName, nameof(oldName)); + + Unchanged = unchanged; + NewName = oldName; + } + + internal MockInputModel(string unchanged, string oldName, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Unchanged = unchanged; + NewName = oldName; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string Unchanged { get; } + + public string NewName { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameRestoredBySignatureMatch/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameRestoredBySignatureMatch/MockInputModel.cs new file mode 100644 index 00000000000..7ad384b7eff --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterRenameRestoredBySignatureMatch/MockInputModel.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + public MockInputModel(string unchanged, string oldName) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapRestoredBySignatureMatch.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapRestoredBySignatureMatch.cs new file mode 100644 index 00000000000..b8d12181c3e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapRestoredBySignatureMatch.cs @@ -0,0 +1,35 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using Sample; + +namespace Sample.Models +{ + public partial class MockInputModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public MockInputModel(string name, string vmSkuName) + { + global::Sample.Argument.AssertNotNull(vmSkuName, nameof(vmSkuName)); + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + VmSkuName = vmSkuName; + Name = name; + } + + internal MockInputModel(string vmSkuName, string name, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + VmSkuName = vmSkuName; + Name = name; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string VmSkuName { get; } + + public string Name { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapRestoredBySignatureMatch/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapRestoredBySignatureMatch/MockInputModel.cs new file mode 100644 index 00000000000..c901270f15d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorParameterSwapRestoredBySignatureMatch/MockInputModel.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + public MockInputModel(string name, string vmSkuName) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenNoChainTarget.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenNoChainTarget.cs new file mode 100644 index 00000000000..56673e8b2ab --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenNoChainTarget.cs @@ -0,0 +1,45 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using Sample; + +namespace Sample.Models +{ + public partial class MockInputModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public MockInputModel(string alpha, string beta) + { + global::Sample.Argument.AssertNotNull(alpha, nameof(alpha)); + global::Sample.Argument.AssertNotNull(beta, nameof(beta)); + + Alpha = alpha; + Beta = beta; + } + + internal MockInputModel(string alpha, string beta, int? gamma, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Alpha = alpha; + Beta = beta; + Gamma = gamma; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public MockInputModel(string beta, string alpha, int gamma) + { + Beta = beta; + Alpha = alpha; + Gamma = gamma; + } + + public string Alpha { get; } + + public string Beta { get; } + + public int? Gamma { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenNoChainTarget/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenNoChainTarget/MockInputModel.cs new file mode 100644 index 00000000000..57c956d4334 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenNoChainTarget/MockInputModel.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + public MockInputModel(string beta, string alpha, int gamma) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenRequiredPropertyUnassigned.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenRequiredPropertyUnassigned.cs new file mode 100644 index 00000000000..825927b8dad --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenRequiredPropertyUnassigned.cs @@ -0,0 +1,48 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using Sample; + +namespace Sample.Models +{ + public partial class MockInputModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public MockInputModel(string name, string workloadProfileType) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + global::Sample.Argument.AssertNotNull(workloadProfileType, nameof(workloadProfileType)); + + Name = name; + WorkloadProfileType = workloadProfileType; + } + + internal MockInputModel(string name, string workloadProfileType, int? minimumCount, int? maximumCount, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + WorkloadProfileType = workloadProfileType; + MinimumCount = minimumCount; + MaximumCount = maximumCount; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public MockInputModel(string workloadProfileType, int minimumCount, int maximumCount) + { + WorkloadProfileType = workloadProfileType; + MinimumCount = minimumCount; + MaximumCount = maximumCount; + } + + public string Name { get; set; } + + public string WorkloadProfileType { get; set; } + + public int? MinimumCount { get; set; } + + public int? MaximumCount { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenRequiredPropertyUnassigned/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenRequiredPropertyUnassigned/MockInputModel.cs new file mode 100644 index 00000000000..990e61a2471 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_StandaloneConstructorRestoredWhenRequiredPropertyUnassigned/MockInputModel.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + public MockInputModel(string workloadProfileType, int minimumCount, int maximumCount) + { + } + } +} diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index e895d7b1b85..ff6991f9e2d 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -23,6 +23,8 @@ - [Model Constructors](#model-constructors) - [Required Property Becomes Optional](#scenario-required-property-becomes-optional) - [Parameterless Constructor Becomes Parameterized](#scenario-parameterless-constructor-becomes-parameterized) + - [Constructor Parameter Name Restored by Signature Match](#scenario-constructor-parameter-name-restored-by-signature-match) + - [Previous Constructor Restored as a Standalone Constructor](#scenario-previous-constructor-restored-as-a-standalone-constructor) - [Parameter Naming](#parameter-naming) - [Page Size Parameter Casing Correction](#scenario-page-size-parameter-casing-correction) - [Top Parameter Conversion to MaxCount](#scenario-top-parameter-conversion-to-maxcount) @@ -742,6 +744,69 @@ protected Widget() : this(default) - The generated parameterless mocking constructor is removed so it does not duplicate the restored constructor. - If the constructor removal is accepted in an ApiCompat baseline, the generator does not restore it. +#### Scenario: Constructor Parameter Name Restored by Signature Match + +**Description:** When a constructor keeps the same parameter types (in the same order and count) but one or more parameters would be renamed — because of a `@@clientName`, a spec/property rename, a generator naming-rule change, or a casing correction — the new names would appear on the generated constructor. Renaming a constructor parameter is source-breaking for callers using named arguments and is not flagged by ApiCompat / binary-compat tooling. To avoid this, the generator matches the current constructor to the previously published one by signature and restores the previous parameter names. + +This covers straight renames, casing corrections, and parameter **swaps/rotations**. For a swap or rotation (every previous name is still present, just in a different position) the generator realigns the existing parameter objects to the previous positional order instead of renaming positionally, so a caller's named argument stays bound to the same property. + +**Example:** + +Previous version published `(name, vmSkuName)`: + +```csharp +public MockInputModel(string name, string vmSkuName) +{ + Name = name; + VmSkuName = vmSkuName; +} +``` + +Current TypeSpec orders the properties differently, which would normally produce `(vmSkuName, name)`. The generator restores the previous parameter order and names: + +```csharp +public MockInputModel(string name, string vmSkuName) +{ + VmSkuName = vmSkuName; + Name = name; +} +``` + +#### Scenario: Previous Constructor Restored as a Standalone Constructor + +**Description:** The [Required Property Becomes Optional](#scenario-required-property-becomes-optional) scenario restores a previous public constructor as an overload that **chains** to a current public constructor. When no current public constructor's parameters form an in-order subsequence of the previous constructor's parameters — for example, the required properties were reordered so there is nothing to chain to — the generator instead restores the previous constructor as a **standalone** constructor that assigns each parameter directly to its matching property. + +**Example:** + +Previous version published `(beta, alpha, gamma)` with all three required: + +```csharp +public MockInputModel(string beta, string alpha, int gamma) +{ + Beta = beta; + Alpha = alpha; + Gamma = gamma; +} +``` + +Current TypeSpec reorders the required properties (so the current public `(alpha, beta)` constructor is not an in-order subsequence to chain to) and relaxes `gamma` to optional. The previous constructor is restored as a standalone constructor (no `this(...)` initializer): + +```csharp +public MockInputModel(string beta, string alpha, int gamma) +{ + Beta = beta; + Alpha = alpha; + Gamma = gamma; +} +``` + +**Key Points:** + +- Used only when no current public constructor can serve as a chain target; otherwise the chaining overload from the "required property becomes optional" scenario is generated. +- Each parameter must map to a public, settable (or required get-only, constructor-assignable) property of the same type, matched by parameter or wire name. A standalone constructor can also assign required get-only auto-properties, which the chained path cannot. +- The constructor is still restored even if a newly-required property has no value in the previous signature; the generator assigns what it can and leaves the new property unset (valid because it does not emit the C# `required` modifier). +- Not restored when a constructor with the same signature already exists, or when the removal is accepted in an ApiCompat baseline. + ### Parameter Naming The generator maintains backward compatibility for parameter names to ensure that existing code continues to compile when parameter names are corrected, standardized, or converted to follow naming conventions.