diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs index 5fe0a90bcfc..1d7ea756d1c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs @@ -125,6 +125,9 @@ public MrwSerializationTypeDefinition(InputModelType inputModel, ModelProvider m protected override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) => [.. originalMethods]; + protected override IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) + => [.. originalConstructors]; + private ConstructorProvider SerializationConstructor => _serializationConstructor ??= _model.FullConstructor; private PropertyProvider[] AdditionalProperties => _additionalProperties.Value; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs index eff1d0ac65b..08a2112faf5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs @@ -205,6 +205,75 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } + [Test] + public async Task BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor() + { + // The last contract published a parameterless `protected BaseModel()`. The current generation + // makes the discriminator required, so the abstract base's initialization constructor now takes a + // parameter and the parameterless constructor is dropped. It is restored, and the generated + // parameterless mocking constructor on the serialization partial is removed to avoid a duplicate. + var derivedInputModel = InputFactory.Model( + "derivedModel", + discriminatedKind: "one", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true) + ]); + var inputModel = InputFactory.Model( + "baseModel", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true) + ], + discriminatedModels: new Dictionary() { { "one", derivedInputModel } }); + + await MockHelpers.LoadMockGeneratorAsync( + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + inputModels: () => [inputModel]); + + var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType().Single(t => t.Name == "BaseModel"); + + model.ProcessTypeForBackCompatibility(); + + // The model gains the restored standalone parameterless constructor. + var modelContent = new TypeProviderWriter(model).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile("Model"), modelContent); + + // The serialization partial no longer carries the parameterless mocking constructor (avoids CS0111). + var serializationContent = new TypeProviderWriter(model.SerializationProviders.Single()).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile("Serialization"), serializationContent); + } + + [Test] + public async Task BackCompat_StructParameterlessConstructorNotMovedFromSerialization() + { + // A struct always exposes a public parameterless constructor via its serialization (mocking) + // constructor, so the last contract's parameterless constructor is already present. It must not + // be moved onto the model partial, which would be pointless churn with no public API change. + var inputModel = InputFactory.Model( + "structModel", + modelAsStruct: true, + properties: + [ + InputFactory.Property("prop", InputPrimitiveType.String, isRequired: true) + ]); + + await MockHelpers.LoadMockGeneratorAsync( + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + inputModels: () => [inputModel]); + + var model = ScmCodeModelGenerator.Instance.OutputLibrary.TypeProviders + .OfType().Single(t => t.Name == "StructModel"); + + model.ProcessTypeForBackCompatibility(); + + Assert.IsFalse(model.Constructors.Any(c => c.Signature.Parameters.Count == 0), + "Struct model must not gain a parameterless constructor on the model partial."); + Assert.IsTrue(model.SerializationProviders.Single().Constructors.Any(c => c.Signature.Parameters.Count == 0), + "Struct serialization partial must retain its parameterless constructor."); + } + [Test] public void TestDynamicModelWithUnionAdditionalProps() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor(Model).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor(Model).cs new file mode 100644 index 00000000000..621e16a44c3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor(Model).cs @@ -0,0 +1,31 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; + +namespace Sample.Models +{ + public abstract partial class BaseModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + private protected BaseModel(string kind) + { + Kind = kind; + } + + internal BaseModel(string kind, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Kind = kind; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + protected BaseModel() : this(default) + { + } + + internal string Kind { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor(Serialization).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor(Serialization).cs new file mode 100644 index 00000000000..31ddf954a8d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor(Serialization).cs @@ -0,0 +1,111 @@ +// + +#nullable disable + +using System; +using System.ClientModel.Primitives; +using System.Text.Json; +using Sample; + +namespace Sample.Models +{ + [global::System.ClientModel.Primitives.PersistableModelProxyAttribute(typeof(global::Sample.Models.UnknownBaseModel))] + public abstract partial class BaseModel : global::System.ClientModel.Primitives.IJsonModel + { + protected virtual global::Sample.Models.BaseModel PersistableModelCreateCore(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + switch (format) + { + case "J": + using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(data, global::Sample.ModelSerializationExtensions.JsonDocumentOptions)) + { + return global::Sample.Models.BaseModel.DeserializeBaseModel(document.RootElement, options); + } + default: + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support reading '{options.Format}' format."); + } + } + + protected virtual global::System.BinaryData PersistableModelWriteCore(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + switch (format) + { + case "J": + return global::System.ClientModel.Primitives.ModelReaderWriter.Write(this, options, global::Sample.SampleContext.Default); + default: + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support writing '{options.Format}' format."); + } + } + + global::System.BinaryData global::System.ClientModel.Primitives.IPersistableModel.Write(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelWriteCore(options); + + global::Sample.Models.BaseModel global::System.ClientModel.Primitives.IPersistableModel.Create(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelCreateCore(data, options); + + string global::System.ClientModel.Primitives.IPersistableModel.GetFormatFromOptions(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => "J"; + + void global::System.ClientModel.Primitives.IJsonModel.Write(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + writer.WriteStartObject(); + this.JsonModelWriteCore(writer, options); + writer.WriteEndObject(); + } + + protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + if ((format != "J")) + { + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support writing '{format}' format."); + } + writer.WritePropertyName("kind"u8); + writer.WriteStringValue(Kind); + if (((options.Format != "W") && (_additionalBinaryDataProperties != null))) + { + foreach (var item in _additionalBinaryDataProperties) + { + writer.WritePropertyName(item.Key); +#if NET6_0_OR_GREATER + writer.WriteRawValue(item.Value); +#else + using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(item.Value)) + { + global::System.Text.Json.JsonSerializer.Serialize(writer, document.RootElement); + } +#endif + } + } + } + + global::Sample.Models.BaseModel global::System.ClientModel.Primitives.IJsonModel.Create(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.JsonModelCreateCore(ref reader, options); + + protected virtual global::Sample.Models.BaseModel JsonModelCreateCore(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + if ((format != "J")) + { + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.BaseModel)} does not support reading '{format}' format."); + } + using global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.ParseValue(ref reader); + return global::Sample.Models.BaseModel.DeserializeBaseModel(document.RootElement, options); + } + + internal static global::Sample.Models.BaseModel DeserializeBaseModel(global::System.Text.Json.JsonElement element, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + if ((element.ValueKind == global::System.Text.Json.JsonValueKind.Null)) + { + return null; + } + if (element.TryGetProperty("kind"u8, out global::System.Text.Json.JsonElement discriminator)) + { + switch (discriminator.GetString()) + { + case "one": + return global::Sample.Models.DerivedModel.DeserializeDerivedModel(element, options); + } + } + return global::Sample.Models.UnknownBaseModel.DeserializeUnknownBaseModel(element, options); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor/BaseModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor/BaseModel.cs new file mode 100644 index 00000000000..2e50e2419d5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_ParameterlessConstructorRestoredRemovesMockingConstructor/BaseModel.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public abstract partial class BaseModel + { + /// Initializes a new instance of BaseModel. + protected BaseModel() + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_StructParameterlessConstructorNotMovedFromSerialization/StructModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_StructParameterlessConstructorNotMovedFromSerialization/StructModel.cs new file mode 100644 index 00000000000..7aa013b79fe --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BackCompat_StructParameterlessConstructorNotMovedFromSerialization/StructModel.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public partial struct StructModel + { + /// Initializes a new instance of StructModel. + public StructModel() + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs index 77cc9c53543..6a2aad4f357 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/BackCompatibilityChangeCategory.cs @@ -54,5 +54,11 @@ public enum BackCompatibilityChangeCategory /// A fixed enum member was re-added to preserve a member that existed in the last contract but is no longer produced by the current spec. EnumMemberAddedFromLastContract, + + /// A back-compat model constructor was re-added to preserve a public constructor that existed in the last contract but is no longer produced by the current spec. + ConstructorAddedFromLastContract, + + /// A back-compat model constructor could not be reconstructed from the last contract and was skipped. + ConstructorAddedFromLastContractSkipped, } } 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 d38a547f37f..43d2559a48b 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 @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; using Microsoft.TypeSpec.Generator.EmitterRpc; @@ -774,6 +775,280 @@ protected internal override ConstructorProvider[] BuildConstructors() return [.. constructors]; } + /// + /// Restores previously-published public constructors that the current generation would otherwise + /// drop. The primary scenario is a previously required property becoming optional: the corresponding + /// parameter is removed from the initialization constructor, which is a source-breaking change for + /// callers that construct the model positionally. When the previous public constructor can be safely + /// reconstructed - i.e. every one of its extra parameters still maps to a settable property whose + /// name and type are unchanged (or a property renamed via a codegen customization but keeping the + /// same type) - a back-compat overload is added that chains to the current public constructor and + /// assigns the extra properties. + /// + protected internal override IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) + { + if (LastContractView?.Constructors is not { Count: > 0 } previousConstructors) + { + return base.BuildConstructorsForBackCompatibility(originalConstructors); + } + + var constructors = new List(base.BuildConstructorsForBackCompatibility(originalConstructors)); + var restorablePropertyLookup = BuildRestorablePropertyLookup(); + IReadOnlyList candidateConstructors = CustomCodeView?.Constructors is { Count: > 0 } customConstructors + ? [.. constructors, .. customConstructors] + : constructors; + + foreach (var previousConstructor in previousConstructors) + { + if (!MethodSignatureHelper.IsPublicApi(previousConstructor.Signature.Modifiers)) + { + continue; + } + + var previousParameters = previousConstructor.Signature.Parameters; + + if (BackCompatHelper.IsConstructorRemovalAcceptedInBaseline(this, previousConstructor.Signature)) + { + continue; + } + + // 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) + { + 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))) + { + var parameterlessConstructor = BuildBackCompatParameterlessConstructor(previousConstructor, candidateConstructors); + RemoveGeneratedMockingConstructor(constructors); + constructors.Add(parameterlessConstructor); + CodeModelGenerator.Instance.Emitter.Info( + $"Restored parameterless constructor '{Name}()' to match last contract.", + BackCompatibilityChangeCategory.ConstructorAddedFromLastContract); + } + + continue; + } + + // 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. + if (constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters)) + || CanonicalView.Constructors.Any(c => BackCompatHelper.ParametersMatch(c.Signature.Parameters, previousParameters))) + { + continue; + } + + if (TryBuildRestoredConstructor(previousConstructor, candidateConstructors, restorablePropertyLookup, out var restoredConstructor)) + { + constructors.Add(restoredConstructor); + CodeModelGenerator.Instance.Emitter.Info( + $"Restored constructor '{Name}({string.Join(", ", previousParameters.Select(p => p.Type.Name))})' to match last contract.", + BackCompatibilityChangeCategory.ConstructorAddedFromLastContract); + } + else + { + CodeModelGenerator.Instance.Emitter.Info( + $"Could not restore constructor '{Name}({string.Join(", ", previousParameters.Select(p => p.Type.Name))})' from the last contract; a property name or type has changed.", + BackCompatibilityChangeCategory.ConstructorAddedFromLastContractSkipped); + } + } + + return constructors; + } + + private bool TryBuildRestoredConstructor( + ConstructorProvider previousConstructor, + IReadOnlyList currentConstructors, + Dictionary restorablePropertyLookup, + [NotNullWhen(true)] out ConstructorProvider? restoredConstructor) + { + restoredConstructor = null; + 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. + ConstructorProvider? targetConstructor = null; + foreach (var candidate in currentConstructors) + { + if (!MethodSignatureHelper.IsPublicApi(candidate.Signature.Modifiers) + || candidate.Signature.Parameters.Count >= previousParameters.Count) + { + continue; + } + + // Check whether this candidate would improve on the current target before performing the + // more expensive subsequence lookup. + if ((targetConstructor == null + || candidate.Signature.Parameters.Count > targetConstructor.Signature.Parameters.Count) + && IsParameterSubsequence(candidate.Signature.Parameters, previousParameters)) + { + targetConstructor = candidate; + } + } + + if (targetConstructor == null) + { + return false; + } + + 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)>(); + int targetIndex = 0; + + foreach (var previousParameter in previousParameters) + { + if (targetIndex < targetParameters.Count + && targetParameters[targetIndex].Equals(previousParameter)) + { + var keptParameter = PartialMethodCustomization.CloneParameterWithName( + targetParameters[targetIndex], + previousParameter.Name, + removeDefault: false, + validation: ParameterValidationType.None); + restoredParameters.Add(keptParameter); + initializerArguments.Add(keptParameter); + targetIndex++; + continue; + } + + if (!restorablePropertyLookup.TryGetValue(previousParameter.Name, out var property) + || !property.Type.AreNamesEqual(previousParameter.Type)) + { + return false; + } + + var restoredParameter = PartialMethodCustomization.CloneParameterWithName( + property.AsParameter, + previousParameter.Name, + removeDefault: true); + + 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) + { + return false; + } + + var bodyStatements = new List(extraAssignments.Count); + foreach (var (property, parameter) in extraAssignments) + { + ValueExpression assignee = property.BackingField is null ? property : property.BackingField; + ValueExpression value = parameter; + if (CSharpType.RequiresToList(parameter.Type, property.Type)) + { + value = parameter.Type.IsNullable ? value.NullConditional().ToList() : value.ToList(); + } + + bodyStatements.Add(assignee.Assign(value).Terminate()); + } + + var signature = new ConstructorSignature( + Type, + $"Initializes a new instance of {Type:C}", + previousConstructor.Signature.Modifiers, + restoredParameters, + initializer: new ConstructorInitializer(false, initializerArguments)); + + restoredConstructor = new ConstructorProvider(signature, bodyStatements, this); + return true; + } + + private ConstructorProvider BuildBackCompatParameterlessConstructor( + ConstructorProvider previousConstructor, + IReadOnlyList currentConstructors) + { + // Prefer the public or protected constructor with the fewest required parameters, then a + // private-protected one; a null target yields a standalone constructor. + const MethodSignatureModifiers privateProtected = MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected; + var target = currentConstructors + .Where(c => c.Signature.Parameters.Count > 0 + && (MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers) || (c.Signature.Modifiers & privateProtected) == privateProtected)) + .MinBy(c => (MethodSignatureHelper.IsPublicApi(c.Signature.Modifiers) ? 0 : 1, c.Signature.Parameters.Count(p => p.DefaultValue is null))); + + ConstructorInitializer? initializer = target is null + ? null + : new ConstructorInitializer(false, [.. target.Signature.Parameters.Select(_ => Snippet.Default)]); + + var signature = new ConstructorSignature( + Type, + $"Initializes a new instance of {Type:C}", + previousConstructor.Signature.Modifiers, + parameters: [], + initializer: initializer); + + return new ConstructorProvider(signature, MethodBodyStatement.Empty, this); + } + + private void RemoveGeneratedMockingConstructor(List constructors) + { + constructors.RemoveAll(c => c.Signature.Parameters.Count == 0); + + foreach (var serializationProvider in SerializationProviders) + { + var serializationConstructors = serializationProvider.Constructors; + if (serializationConstructors.Any(c => c.Signature.Parameters.Count == 0)) + { + serializationProvider.Update( + constructors: [.. serializationConstructors.Where(c => c.Signature.Parameters.Count != 0)]); + } + } + } + + private static bool IsParameterSubsequence( + IReadOnlyList subset, + IReadOnlyList full) + { + if (subset.Count > full.Count) + { + return false; + } + + int matched = 0; + foreach (var parameter in full) + { + if (matched == subset.Count) + { + break; + } + + if (subset[matched].Equals(parameter)) + { + matched++; + } + } + + return matched == subset.Count; + } + + private Dictionary BuildRestorablePropertyLookup() + { + var lookup = new Dictionary(); + foreach (var property in CanonicalView.Properties) + { + if (!MethodSignatureHelper.IsPublicApi(property.Modifiers) || !property.Body.HasSetter || property.WireInfo == null) + { + continue; + } + + lookup.TryAdd(property.AsParameter.Name, property); + + if (property.OriginalName != null) + { + lookup.TryAdd(property.OriginalName.ToVariableName(), property); + } + } + + return lookup; + } + /// /// Determines if this model should have a dual constructor pattern. /// This is needed when the model shares the same discriminator property name as its base model diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs index 0337b78c5a3..5dbcb7f0aa1 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/PartialMethodCustomization.cs @@ -216,9 +216,12 @@ public static MethodSignature BuildPartialSignature( internal static ParameterProvider CloneParameterWithName( ParameterProvider source, string newName, - bool removeDefault) + bool removeDefault, + ParameterValidationType? validation = null) { - if (source.Name == newName && !(removeDefault && source.DefaultValue != null)) + if (source.Name == newName + && !(removeDefault && source.DefaultValue != null) + && (validation == null || validation == source.Validation)) { return source; } @@ -238,7 +241,7 @@ internal static ParameterProvider CloneParameterWithName( initializationValue: source.InitializationValue, location: source.Location, wireInfo: source.WireInfo, - validation: source.Validation, + validation: validation ?? source.Validation, inputParameter: source.InputParameter) { SpreadSource = source.SpreadSource, diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs index 346b90d6a81..000c664e000 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs @@ -50,6 +50,35 @@ public static bool IsMethodRemovalAcceptedInBaseline(TypeProvider enclosingType, return true; } + /// + /// Returns true when the removal of a previously-published constructor — identified by the + /// enclosing type's fully-qualified name and the exact parameter types — has been accepted in the + /// ApiCompat baseline, in which case back compatibility must not restore it. Constructors are + /// recorded in the baseline as the .ctor member of their declaring type. Emits an + /// informational log entry when a suppression is honored. + /// + public static bool IsConstructorRemovalAcceptedInBaseline(TypeProvider enclosingType, ConstructorSignature previousSignature) + { + var parameterTypes = new CSharpType[previousSignature.Parameters.Count]; + for (int i = 0; i < parameterTypes.Length; i++) + { + parameterTypes[i] = previousSignature.Parameters[i].Type; + } + + if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMethodRemovalSuppressed( + enclosingType.Type.FullyQualifiedName, + ".ctor", + parameterTypes) != true) + { + return false; + } + + CodeModelGenerator.Instance.Emitter.Info( + $"Skipping back-compat for '{enclosingType.Type.FullyQualifiedName}..ctor'; removal is accepted in the ApiCompat baseline.", + BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped); + return true; + } + /// /// Finds the current method that has the same parameter set as /// (matched by name and return type) but in a different order, or null when there is none. 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 48b8d37e0e6..15a9ffec1c0 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,365 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual("baseProp", publicConstructor!.Signature.Parameters[0].Name); } + [Test] + public async Task BackCompat_ParameterlessConstructorRestored() + { + // The last contract published a parameterless `protected BaseModel()`. The current generation + // makes the discriminator required, so the initialization constructor now takes a parameter and + // the parameterless constructor is dropped. It should be restored, chaining to the private-protected + // initialization constructor (no public or protected constructor exists on the abstract base). + var derivedInputModel = InputFactory.Model( + "DerivedModel", + discriminatedKind: "one", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: true) + ]); + var inputModel = InputFactory.Model( + "BaseModel", + properties: + [ + InputFactory.Property("kind", InputPrimitiveType.String, isRequired: true, isDiscriminator: 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); + + modelProvider!.ProcessTypeForBackCompatibility(); + + var file = new TypeProviderWriter(modelProvider).Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [Test] + public async Task BackCompat_ParameterlessConstructorChainsToPublicConstructor() + { + // The last contract published a parameterless `public MockInputModel()`. The current generation + // makes "name" required, so the public initialization constructor now takes it. The restored + // parameterless constructor chains to that public constructor (not the internal full constructor). + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true) + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + + modelProvider!.ProcessTypeForBackCompatibility(); + + var file = new TypeProviderWriter(modelProvider).Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [Test] + public async Task BackCompat_RequiredToOptionalConstructorIsRestored() + { + // "resources" was required in the last contract (so the initialization constructor + // accepted it), but the current spec relaxes it to optional which would otherwise drop + // it from the constructor. The previously published constructor should be restored. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("resources", InputPrimitiveType.String, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + + // Before back-compat processing the public constructor only takes "name". + var publicCtorBefore = modelProvider!.Constructors.SingleOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicCtorBefore); + Assert.AreEqual(1, publicCtorBefore!.Signature.Parameters.Count); + + modelProvider.ProcessTypeForBackCompatibility(); + + // After back-compat processing the previously published (name, resources) constructor is restored. + var restoredCtor = modelProvider.Constructors.SingleOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNotNull(restoredCtor, "Expected the (name, resources) constructor to be restored for back compat"); + Assert.AreEqual("name", restoredCtor!.Signature.Parameters[0].Name); + Assert.AreEqual("resources", restoredCtor.Signature.Parameters[1].Name); + Assert.IsTrue(restoredCtor.Signature.Parameters[1].Type.Equals(typeof(string))); + + // It chains to the current (name) constructor and assigns the extra property in its body. + var initializer = restoredCtor.Signature.Initializer; + Assert.IsNotNull(initializer); + Assert.IsFalse(initializer!.IsBase); + Assert.AreEqual(1, initializer.Arguments.Count); + Assert.AreEqual("name", initializer.Arguments[0].ToDisplayString()); + + var body = restoredCtor.BodyStatements!.ToDisplayString(); + Assert.AreEqual(ParameterValidationType.None, restoredCtor.Signature.Parameters[0].Validation); + Assert.IsFalse(body.Contains("Argument.AssertNotNull(name"), $"Did not expect duplicated name validation in restored constructor, was: {body}"); + Assert.IsTrue(body.Contains("Resources = resources"), $"Expected the body to assign Resources, was: {body}"); + // "resources" is now an optional property, so the restored back-compat overload does not null-check it. + Assert.AreEqual(ParameterValidationType.None, restoredCtor.Signature.Parameters[1].Validation); + Assert.IsFalse(body.Contains("Argument.AssertNotNull(resources"), $"Did not expect null validation for the now-optional resources parameter, was: {body}"); + + // Validate the full generated model, including the restored constructor, against the expected output. + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [Test] + public async Task BackCompat_ConstructorNotRestoredWhenPropertyRemoved() + { + // "resources" existed in the last contract constructor but has been removed entirely from + // the current spec, so the previous constructor cannot be safely restored. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + + modelProvider!.ProcessTypeForBackCompatibility(); + + var twoParamPublicCtor = modelProvider.Constructors.FirstOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNull(twoParamPublicCtor, "The constructor should not be restored when a property was removed"); + + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [Test] + public async Task BackCompat_ConstructorNotRestoredWhenLastContractMissing() + { + // No last contract exists for the model, so nothing should be restored. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("resources", InputPrimitiveType.String, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + Assert.IsNull(modelProvider!.LastContractView); + + modelProvider.ProcessTypeForBackCompatibility(); + + var twoParamPublicCtor = modelProvider.Constructors.FirstOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNull(twoParamPublicCtor); + + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [Test] + public async Task BackCompat_OptionalValueTypeConstructorParameterIsRestored() + { + // "count" was a required value type in the last contract, so the initialization constructor + // accepted it. Relaxing it to optional drops it; the previously published constructor is + // restored, and because it is a value type no null validation is emitted. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("count", InputPrimitiveType.Int32, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + + modelProvider!.ProcessTypeForBackCompatibility(); + + var restoredCtor = modelProvider.Constructors.SingleOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNotNull(restoredCtor, "Expected the (name, count) constructor to be restored for back compat"); + Assert.AreEqual("count", restoredCtor!.Signature.Parameters[1].Name); + // The restored parameter keeps the previously published non-nullable value type. + Assert.IsTrue(restoredCtor.Signature.Parameters[1].Type.Equals(typeof(int))); + Assert.AreEqual(ParameterValidationType.None, restoredCtor.Signature.Parameters[1].Validation); + + var body = restoredCtor.BodyStatements!.ToDisplayString(); + Assert.IsTrue(body.Contains("Count = count"), $"Expected the body to assign Count, was: {body}"); + // Value types never emit a null check. + Assert.IsFalse(body.Contains("AssertNotNull(count"), $"Did not expect null validation for a value type, was: {body}"); + + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [Test] + public async Task BackCompat_RenamedPropertyConstructorIsRestored() + { + // The spec property "resources" is renamed to "ResourceList" via a [CodeGenMember] + // customization. The previously published constructor's "resources" parameter must still + // be matched to the renamed property (via its OriginalName) so the constructor is restored. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("resources", InputPrimitiveType.String, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync( + method: "BackCompat_RenamedPropertyConstructorIsRestored_LastContract")); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + + modelProvider!.ProcessTypeForBackCompatibility(); + + var restoredCtor = modelProvider.Constructors.SingleOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNotNull(restoredCtor, "Expected the (name, resources) constructor to be restored for back compat"); + // The restored parameter keeps the previously published (pre-rename) name. + Assert.AreEqual("resources", restoredCtor!.Signature.Parameters[1].Name); + + // The body assigns the current, renamed property. + var body = restoredCtor.BodyStatements!.ToDisplayString(); + Assert.IsTrue(body.Contains("ResourceList = resources"), $"Expected the body to assign the renamed property, was: {body}"); + + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [TestCase(".txt")] + [TestCase(".xml")] + public async Task BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline(string baselineExtension) + { + // "resources" was required in the last contract and is now optional, which would normally + // cause the previous (name, resources) constructor to be restored. However its removal is + // accepted in the ApiCompat baseline (tested in both the txt and xml formats), so the + // constructor must not be resurrected. + var baseline = Helpers.GetApiCompatBaselineFromFile(fileExtension: baselineExtension); + + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("resources", InputPrimitiveType.String, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync( + method: "BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline_LastContract"), + apiCompatBaseline: baseline); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + + modelProvider!.ProcessTypeForBackCompatibility(); + + // The (name, resources) constructor removal is accepted in the baseline, so it is not restored. + var restoredCtor = modelProvider.Constructors.FirstOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNull(restoredCtor, "The constructor should not be restored when its removal is accepted in the baseline"); + + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + + [Test] + public async Task BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode() + { + // "resources" was required in the last contract and is now optional, which would normally + // cause the previous (name, resources) constructor to be restored. Here the user has replaced + // that constructor with their own custom implementation, so the generator must not add a + // colliding back-compat overload. + var inputModel = InputFactory.Model( + "MockInputModel", + usage: InputModelTypeUsage.Input | InputModelTypeUsage.Json, + properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true), + InputFactory.Property("resources", InputPrimitiveType.String, isRequired: false), + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync( + method: "BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync( + method: "BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode_LastContract")); + + var modelProvider = CodeModelGenerator.Instance.OutputLibrary.TypeProviders.SingleOrDefault(t => t.Name == "MockInputModel") as ModelProvider; + Assert.IsNotNull(modelProvider); + + // The custom (name, resources) constructor lives in the canonical view. + var customCtor = modelProvider!.CanonicalView.Constructors.SingleOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNotNull(customCtor, "Expected the custom (name, resources) constructor to be present"); + + modelProvider.ProcessTypeForBackCompatibility(); + + // Because the custom code already provides the (name, resources) constructor, the generator + // must not restore a colliding overload of its own. + var restoredCtor = modelProvider.Constructors.FirstOrDefault(c => + c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public) + && c.Signature.Parameters.Count == 2); + Assert.IsNull(restoredCtor, "The constructor should not be restored when it is replaced by custom code"); + + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + [Test] public async Task TestBuildProperties_WithObjectAdditionalPropertiesBackwardCompatibility() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenLastContractMissing.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenLastContractMissing.cs new file mode 100644 index 00000000000..61958c1b8b5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenLastContractMissing.cs @@ -0,0 +1,33 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, string resources, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + Resources = resources; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string Name { get; } + + public string Resources { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenLastContractMissing/UnrelatedModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenLastContractMissing/UnrelatedModel.cs new file mode 100644 index 00000000000..78a0f6a28d2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenLastContractMissing/UnrelatedModel.cs @@ -0,0 +1,9 @@ +namespace Sample.Models +{ + // Note: this last-contract model has a different name than the spec model + // ("MockInputModel"), so no last contract view is found for the model. + public partial class UnrelatedModel + { + public int? Count { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenPropertyRemoved.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenPropertyRemoved.cs new file mode 100644 index 00000000000..ac6d11111d5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenPropertyRemoved.cs @@ -0,0 +1,30 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string Name { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenPropertyRemoved/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenPropertyRemoved/MockInputModel.cs new file mode 100644 index 00000000000..776ffa9d8d7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenPropertyRemoved/MockInputModel.cs @@ -0,0 +1,18 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + // In the last contract "resources" existed and was part of the constructor, but + // the current spec removes the property entirely. Because there is no matching + // property to assign, the previous constructor cannot be safely restored. + public MockInputModel(string name, string resources) + { + Name = name; + Resources = resources; + } + + public string Name { get; } + + public string Resources { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.cs new file mode 100644 index 00000000000..61958c1b8b5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.cs @@ -0,0 +1,33 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, string resources, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + Resources = resources; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string Name { get; } + + public string Resources { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.txt b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.txt new file mode 100644 index 00000000000..2c4258cf7d3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.txt @@ -0,0 +1 @@ +MembersMustExist : Member 'public Sample.Models.MockInputModel..ctor(System.String, System.String)' does not exist in the implementation but it does exist in the contract. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.xml b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.xml new file mode 100644 index 00000000000..36000f351a8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline.xml @@ -0,0 +1,7 @@ + + + + CP0002 + M:Sample.Models.MockInputModel.#ctor(System.String,System.String) + + diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline_LastContract/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline_LastContract/MockInputModel.cs new file mode 100644 index 00000000000..c6e9450a34e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenRemovalAcceptedInBaseline_LastContract/MockInputModel.cs @@ -0,0 +1,18 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + // In the last contract "resources" was required so the initialization constructor + // accepted it. The current spec relaxes it to optional, which would normally cause the + // previous constructor to be restored - but here its removal is accepted in the baseline. + public MockInputModel(string name, string resources) + { + Name = name; + Resources = resources; + } + + public string Name { get; } + + public string Resources { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode.cs new file mode 100644 index 00000000000..61958c1b8b5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode.cs @@ -0,0 +1,33 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, string resources, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + Resources = resources; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string Name { get; } + + public string Resources { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode/MockInputModel.cs new file mode 100644 index 00000000000..4dc7415bd15 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode/MockInputModel.cs @@ -0,0 +1,18 @@ +#nullable disable + +using Sample; +using SampleTypeSpec; + +namespace Sample.Models +{ + public partial class MockInputModel + { + // The user supplies their own (name, resources) constructor, replacing the one the + // generator would otherwise restore for back compat. Restoration must be skipped so the + // generated overload does not collide with this custom code. + public MockInputModel(string name, string resources) : this(name) + { + Resources = resources; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode_LastContract/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode_LastContract/MockInputModel.cs new file mode 100644 index 00000000000..d088ba570fa --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ConstructorNotRestoredWhenReplacedByCustomCode_LastContract/MockInputModel.cs @@ -0,0 +1,16 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + // The previously published constructor accepted the required "resources" parameter. + public MockInputModel(string name, string resources) + { + Name = name; + Resources = resources; + } + + public string Name { get; } + + public string Resources { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_OptionalValueTypeConstructorParameterIsRestored.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_OptionalValueTypeConstructorParameterIsRestored.cs new file mode 100644 index 00000000000..c6e78ea90c8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_OptionalValueTypeConstructorParameterIsRestored.cs @@ -0,0 +1,38 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, int count, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + Count = count; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public MockInputModel(string name, int count) : this(name) + { + Count = count; + } + + public string Name { get; } + + public int Count { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_OptionalValueTypeConstructorParameterIsRestored/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_OptionalValueTypeConstructorParameterIsRestored/MockInputModel.cs new file mode 100644 index 00000000000..adad3e228ab --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_OptionalValueTypeConstructorParameterIsRestored/MockInputModel.cs @@ -0,0 +1,18 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + // In the last contract "count" was required so the initialization constructor + // accepted it. The current spec relaxes it to optional, which drops it from the + // constructor unless it is restored for back compat. + public MockInputModel(string name, int count) + { + Name = name; + Count = count; + } + + public string Name { get; } + + public int Count { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorChainsToPublicConstructor.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorChainsToPublicConstructor.cs new file mode 100644 index 00000000000..75ea9c12218 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorChainsToPublicConstructor.cs @@ -0,0 +1,34 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public MockInputModel() : this(default) + { + } + + public string Name { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorChainsToPublicConstructor/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorChainsToPublicConstructor/MockInputModel.cs new file mode 100644 index 00000000000..bd04c9c92cc --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorChainsToPublicConstructor/MockInputModel.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + /// Initializes a new instance of MockInputModel. + public MockInputModel() + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorRestored.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorRestored.cs new file mode 100644 index 00000000000..621e16a44c3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorRestored.cs @@ -0,0 +1,31 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; + +namespace Sample.Models +{ + public abstract partial class BaseModel + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + private protected BaseModel(string kind) + { + Kind = kind; + } + + internal BaseModel(string kind, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Kind = kind; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + protected BaseModel() : this(default) + { + } + + internal string Kind { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorRestored/BaseModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorRestored/BaseModel.cs new file mode 100644 index 00000000000..2e50e2419d5 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_ParameterlessConstructorRestored/BaseModel.cs @@ -0,0 +1,10 @@ +namespace Sample.Models +{ + public abstract partial class BaseModel + { + /// Initializes a new instance of BaseModel. + protected BaseModel() + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored.cs new file mode 100644 index 00000000000..beb896a14e3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored.cs @@ -0,0 +1,36 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, string resourceList, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + ResourceList = resourceList; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public MockInputModel(string name, string resources) : this(name) + { + ResourceList = resources; + } + + public string Name { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored/MockInputModel.cs new file mode 100644 index 00000000000..e9ed6d5d4a3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored/MockInputModel.cs @@ -0,0 +1,14 @@ +#nullable disable + +using Sample; +using SampleTypeSpec; +using Microsoft.TypeSpec.Generator.Customizations; + +namespace Sample.Models +{ + public partial class MockInputModel + { + [CodeGenMember("Resources")] + public string ResourceList { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored_LastContract/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored_LastContract/MockInputModel.cs new file mode 100644 index 00000000000..83aedb284da --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RenamedPropertyConstructorIsRestored_LastContract/MockInputModel.cs @@ -0,0 +1,16 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + // The previously published constructor used the pre-rename parameter name "resources". + public MockInputModel(string name, string resources) + { + Name = name; + Resources = resources; + } + + public string Name { get; } + + public string Resources { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RequiredToOptionalConstructorIsRestored.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RequiredToOptionalConstructorIsRestored.cs new file mode 100644 index 00000000000..605759a88bd --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RequiredToOptionalConstructorIsRestored.cs @@ -0,0 +1,38 @@ +// + +#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) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal MockInputModel(string name, string resources, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + Resources = resources; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public MockInputModel(string name, string resources) : this(name) + { + Resources = resources; + } + + public string Name { get; } + + public string Resources { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RequiredToOptionalConstructorIsRestored/MockInputModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RequiredToOptionalConstructorIsRestored/MockInputModel.cs new file mode 100644 index 00000000000..bc469c58d83 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/BackCompat_RequiredToOptionalConstructorIsRestored/MockInputModel.cs @@ -0,0 +1,17 @@ +namespace Sample.Models +{ + public partial class MockInputModel + { + // In the last contract, both properties were required so the initialization + // constructor accepted both of them. + public MockInputModel(string name, string resources) + { + Name = name; + Resources = resources; + } + + public string Name { get; } + + public string Resources { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/ApiCompatBaselineTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/ApiCompatBaselineTests.cs index 0d0cfddde73..592b12204e2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/ApiCompatBaselineTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/ApiCompatBaselineTests.cs @@ -365,6 +365,53 @@ public void IsMethodRemovalSuppressedMatchesDictionaryWithModelValue() Assert.IsFalse(baseline.IsMethodRemovalSuppressed("Ns.Types", "WithDictionary", [dictionaryOfInt])); } + [Test] + public void IsMethodRemovalSuppressedMatchesConstructor() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(fileExtension: _fileExtension, method: "Baseline"); + + // Ns.Ctors..ctor(System.String, System.Int32) is accepted in the baseline. Constructors are + // recorded under the ".ctor" member name with their exact parameter types. + Assert.IsTrue(baseline.IsMethodRemovalSuppressed( + "Ns.Ctors", + ".ctor", + [new CSharpType(typeof(string)), new CSharpType(typeof(int))])); + + // The same types in a different order are a different constructor signature. + Assert.IsFalse(baseline.IsMethodRemovalSuppressed( + "Ns.Ctors", + ".ctor", + [new CSharpType(typeof(int)), new CSharpType(typeof(string))])); + + // A different arity must not match. + Assert.IsFalse(baseline.IsMethodRemovalSuppressed("Ns.Ctors", ".ctor", [new CSharpType(typeof(string))])); + + // A different parameter type in one slot must not match. + Assert.IsFalse(baseline.IsMethodRemovalSuppressed( + "Ns.Ctors", + ".ctor", + [new CSharpType(typeof(string)), new CSharpType(typeof(bool))])); + + // A different declaring type must not match. + Assert.IsFalse(baseline.IsMethodRemovalSuppressed( + "Ns.Other", + ".ctor", + [new CSharpType(typeof(string)), new CSharpType(typeof(int))])); + } + + [Test] + public void IsMethodRemovalSuppressedMatchesParameterlessConstructor() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(fileExtension: _fileExtension, method: "Baseline"); + + // Ns.Ctors..ctor() has no parameters; the canonical signature is empty on both sides. + Assert.IsTrue(baseline.IsMethodRemovalSuppressed("Ns.Ctors", ".ctor", [])); + + // Ns.Foo only has a constructor overload that takes arguments in the baseline, so querying + // its parameterless constructor must not match that overload. + Assert.IsFalse(baseline.IsMethodRemovalSuppressed("Ns.Foo", ".ctor", [])); + } + [Test] public void ReferencesSuppressedTypeMatchesDirectType() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.txt b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.txt index 725b47e3795..9769f92a6df 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.txt +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.txt @@ -3,6 +3,8 @@ TypesMustExist : Type 'Azure.AI.Projects.Agents.ProjectsAgentProtocol' does not MembersMustExist : Member 'public Azure.AI.Projects.Agents.ProtocolVersionRecord Azure.AI.Projects.Agents.ProjectsAgentsModelFactory.ProtocolVersionRecord(Azure.AI.Projects.Agents.ProjectsAgentProtocol, System.String)' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public System.Void Ns.Foo.Reset()' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Ns.Foo..ctor(Ns.Kind, System.String)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'public Ns.Ctors..ctor(System.String, System.Int32)' does not exist in the implementation but it does exist in the contract. +MembersMustExist : Member 'public Ns.Ctors..ctor()' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public Ns.Kind Ns.Foo.Kind.get()' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public System.Void Ns.Foo.Kind.set(Ns.Kind)' does not exist in the implementation but it does exist in the contract. MembersMustExist : Member 'public System.Void Ns.Foo.Configure(System.Collections.Generic.IDictionary, System.String)' does not exist in the implementation but it does exist in the contract. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.xml b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.xml index 6b8132093b2..b683202d4c6 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.xml +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/SourceInput/TestData/ApiCompatBaselineTests/Baseline.xml @@ -20,6 +20,14 @@ CP0002 M:Ns.Foo.#ctor(Ns.Kind,System.String) + + CP0002 + M:Ns.Ctors.#ctor(System.String,System.Int32) + + + CP0002 + M:Ns.Ctors.#ctor + CP0002 M:Ns.Foo.get_Kind diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index 80e8dc85ee2..e895d7b1b85 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -21,6 +21,8 @@ - [API Version Enum](#api-version-enum) - [Non-abstract Base Models](#non-abstract-base-models) - [Model Constructors](#model-constructors) + - [Required Property Becomes Optional](#scenario-required-property-becomes-optional) + - [Parameterless Constructor Becomes Parameterized](#scenario-parameterless-constructor-becomes-parameterized) - [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) @@ -638,6 +640,108 @@ public abstract partial class SearchIndexerDataIdentity - The modifier is changed from `private protected` to `public` - No additional constructors are generated; only the accessibility is adjusted +#### Scenario: Required Property Becomes Optional + +**Description:** When a required model property becomes optional, the current initialization constructor no longer includes that property. To preserve source compatibility for callers that construct the model positionally, the generator restores the previously published public constructor as an overload. The restored overload chains to the closest current public constructor and assigns the now-optional property. + +**Example:** + +Previous version required both properties: + +```csharp +public partial class Widget +{ + public Widget(string name, string description) + { + Name = name; + Description = description; + } + + public string Name { get; } + public string Description { get; } +} +``` + +Current TypeSpec makes `description` optional: + +```csharp +public partial class Widget +{ + public Widget(string name) + { + Name = name; + } + + public string Name { get; } + public string Description { get; set; } +} +``` + +**Generated Compatibility Result:** + +```csharp +public Widget(string name, string description) : this(name) +{ + Description = description; +} +``` + +**Key Points:** + +- The previous constructor must be public and no generated or custom constructor may already have the same parameters. +- Every parameter removed from the current constructor must map to a public, settable property with the same type. Properties renamed through a code-generation customization are supported. +- The current constructor used for chaining must have parameters that match an in-order subset of the previous constructor's parameters. +- If the constructor removal is accepted in an ApiCompat baseline, the generator does not restore it. + +#### Scenario: Parameterless Constructor Becomes Parameterized + +**Description:** When a model previously exposed an accessible parameterless constructor and a property later becomes required, generation replaces the parameterless constructor with one that accepts the required property. The generator restores the previous parameterless constructor and chains it to an appropriate current constructor with `default` values. + +**Example:** + +Previous version exposed a parameterless constructor: + +```csharp +public partial class Widget +{ + protected Widget() + { + } + + public string Name { get; set; } +} +``` + +Current TypeSpec makes `name` required: + +```csharp +public partial class Widget +{ + protected Widget(string name) + { + Name = name; + } + + public string Name { get; } +} +``` + +**Generated Compatibility Result:** + +```csharp +protected Widget() : this(default) +{ +} +``` + +**Key Points:** + +- The previous parameterless constructor must be accessible and no accessible generated or custom parameterless constructor may already exist. +- The restored constructor retains the previous accessibility. +- The generator prefers an accessible current constructor with the fewest required parameters as the chain target. When necessary, it can chain to a `private protected` initialization constructor. +- 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. + ### 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.