diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index 947b717b5ec..b6e2d9cc78a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -1263,149 +1263,59 @@ protected override ScmMethodProvider[] BuildMethods() protected sealed override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) { - List materializedMethods = [.. originalMethods]; - if (LastContractView?.Methods == null || LastContractView.Methods.Count == 0) { - return materializedMethods; + return [.. originalMethods]; } - var currentMethodSignatures = BuildCurrentMethodSignatures(materializedMethods); - - ProcessBackCompatForParameterReordering(materializedMethods, currentMethodSignatures); - ProcessBackCompatForNewOptionalParameters(materializedMethods, currentMethodSignatures); + // Snapshot the current signatures before the base reorders any of them in place, so we + // can fix up convenience method bodies that call reordered protocol methods afterwards. + var originalSignatures = new Dictionary(ReferenceEqualityComparer.Instance); + foreach (var method in originalMethods) + { + originalSignatures.TryAdd(method, method.Signature); + } - return materializedMethods; - } + List result = [.. base.BuildMethodsForBackCompatibility(originalMethods)]; - private void ProcessBackCompatForParameterReordering( - IList materializedMethods, - Dictionary currentMethodSignatures) - { + // Determine which existing methods had their parameters reordered by the base and fix up + // convenience method bodies that call the reordered protocol methods. var updatedSignatureToOriginal = new Dictionary(MethodSignature.MethodSignatureComparer); var methodsWithReorderedParams = new List(); - - foreach (var previousMethod in LastContractView!.Methods) + foreach (var (method, originalSignature) in originalSignatures) { - if (!ShouldProcessMethodForBackCompat(previousMethod.Signature, currentMethodSignatures)) + if (method.Signature.Name.Equals(originalSignature.Name) + && !MethodSignatureHelper.HaveSameParametersInSameOrder(method.Signature, originalSignature)) { - continue; - } - - var methodToUpdate = FindMethodWithSameParametersButDifferentOrder( - previousMethod.Signature, - currentMethodSignatures); - - if (methodToUpdate != null && TryReorderCurrentMethodParameters( - methodToUpdate, - previousMethod.Signature, - updatedSignatureToOriginal)) - { - methodsWithReorderedParams.Add(methodToUpdate); - CodeModelGenerator.Instance.Emitter.Debug( - $"Reordered parameters of '{Name}.{methodToUpdate.Signature.Name}' to match last contract.", - BackCompatibilityChangeCategory.MethodParameterReordering); + updatedSignatureToOriginal.TryAdd(method.Signature, originalSignature); + methodsWithReorderedParams.Add(method); } } if (methodsWithReorderedParams.Count > 0) { - UpdateConvenienceMethodsForBackCompat(materializedMethods, methodsWithReorderedParams, updatedSignatureToOriginal); - } - } - - private Dictionary BuildCurrentMethodSignatures(IEnumerable originalMethods) - { - var allMethods = CustomCodeView?.Methods != null - ? originalMethods.Concat(CustomCodeView.Methods) - : originalMethods; - - var result = new Dictionary(MethodSignature.MethodSignatureComparer); - foreach (var method in allMethods) - { - result.TryAdd(method.Signature, method); - } - return result; - } - - private static bool ShouldProcessMethodForBackCompat( - MethodSignature previousSignature, - Dictionary currentMethodSignatures) - { - if (currentMethodSignatures.ContainsKey(previousSignature)) - { - return false; - } - - var modifiers = previousSignature.Modifiers; - return modifiers.HasFlag(MethodSignatureModifiers.Public) || - modifiers.HasFlag(MethodSignatureModifiers.Protected); - } - - private static MethodProvider? FindMethodWithSameParametersButDifferentOrder( - MethodSignature previousSignature, - Dictionary currentMethodSignatures) - { - foreach (var kvp in currentMethodSignatures) - { - var currentSignature = kvp.Key; - if (currentSignature.Name.Equals(previousSignature.Name) - && currentSignature.ReturnType?.AreNamesEqual(previousSignature.ReturnType) == true - && MethodSignatureHelper.ContainsSameParameters(previousSignature, currentSignature)) - { - return kvp.Value; - } - } - - return null; - } - - private bool TryReorderCurrentMethodParameters( - MethodProvider methodToUpdate, - MethodSignature previousSignature, - Dictionary updatedSignatureToOriginal) - { - var currentSignature = methodToUpdate.Signature; - // Early exit: Check if parameters are already in the same order - if (MethodSignatureHelper.HaveSameParametersInSameOrder(currentSignature, previousSignature)) - { - return false; + UpdateConvenienceMethodsForBackCompat(result, methodsWithReorderedParams, updatedSignatureToOriginal); } - var parametersByName = currentSignature.Parameters.ToDictionary(p => p.Name.ToVariableName()); - var reorderedParameters = new List(currentSignature.Parameters.Count); - - foreach (var previousParam in previousSignature.Parameters) + // The base adds hidden overloads for methods that gained new optional non-body parameters. + // Where such an overload preserves a previous signature whose trailing parameter was a + // CancellationToken, suppress AZC0002: making that parameter optional would create an + // ambiguous call with the current method. + foreach (var method in result) { - if (parametersByName.TryGetValue(previousParam.Name, out var matchingParam)) + if (!originalSignatures.ContainsKey(method) && PreviousSignatureEndsWithCancellationToken(method.Signature)) { - reorderedParameters.Add(matchingParam); + method.Update(suppressions: + [ + new SuppressionStatement( + inner: null, + code: Literal("AZC0002"), + justification: "Back-compat overload preserves the previous method signature where CancellationToken was the trailing parameter. Making it optional would introduce an ambiguous call with the new method.") + ]); } } - if (reorderedParameters.Count != currentSignature.Parameters.Count) - { - return false; - } - - var updatedSignature = new MethodSignature( - currentSignature.Name, - currentSignature.Description, - currentSignature.Modifiers, - currentSignature.ReturnType, - currentSignature.ReturnDescription, - reorderedParameters, - currentSignature.Attributes, - currentSignature.GenericArguments, - currentSignature.GenericParameterConstraints, - currentSignature.ExplicitInterface, - currentSignature.NonDocumentComment); - updatedSignatureToOriginal.TryAdd(updatedSignature, currentSignature); - - UpdateXmlDocProviderForParamReorder(methodToUpdate.XmlDocs, updatedSignature); - methodToUpdate.Update(signature: updatedSignature, xmlDocProvider: methodToUpdate.XmlDocs); - - return true; + return result; } private ParameterProvider BuildClientEndpointParameter() @@ -1773,104 +1683,6 @@ private static void ReorderMethodInvocationArguments( } } - private static void UpdateXmlDocProviderForParamReorder( - XmlDocProvider xmlDocs, - MethodSignature updatedSignature) - { - var paramDocsByName = xmlDocs.Parameters.ToDictionary(s => s.Parameter.Name); - var reorderedParamDocs = new List(updatedSignature.Parameters.Count); - - foreach (var param in updatedSignature.Parameters) - { - if (paramDocsByName.TryGetValue(param.Name, out var paramDoc)) - { - reorderedParamDocs.Add(paramDoc); - } - } - - if (reorderedParamDocs.Count == xmlDocs.Parameters.Count && - !reorderedParamDocs.SequenceEqual(xmlDocs.Parameters)) - { - xmlDocs.Update(parameters: reorderedParamDocs); - } - } - - private void ProcessBackCompatForNewOptionalParameters( - List methods, - Dictionary currentMethodSignatures) - { - var currentMethodsByName = new Dictionary>(); - foreach (var method in currentMethodSignatures.Values) - { - if (method is ScmMethodProvider { Kind: ScmMethodKind.CreateRequest }) - { - continue; - } - - if (!currentMethodsByName.TryGetValue(method.Signature.Name, out var list)) - { - list = []; - currentMethodsByName[method.Signature.Name] = list; - } - list.Add(method); - } - - foreach (var previousMethod in LastContractView!.Methods) - { - var previousSignature = previousMethod.Signature; - - if (!previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Public) && - !previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)) - { - continue; - } - - if (currentMethodSignatures.ContainsKey(previousSignature) || - !currentMethodsByName.TryGetValue(previousSignature.Name, out var candidates)) - { - continue; - } - - ScmMethodProvider? matchedCurrent = null; - foreach (var candidate in candidates) - { - if (candidate is ScmMethodProvider { Kind: ScmMethodKind.Convenience or ScmMethodKind.Protocol } scmCandidate && - HasNewOptionalNonBodyParametersOnly(previousSignature, scmCandidate.Signature)) - { - matchedCurrent = scmCandidate; - break; - } - } - - if (matchedCurrent is null) - { - continue; - } - - var overload = BuildBackCompatOverloadForNewOptionalParameters(previousMethod, matchedCurrent); - if (overload == null || !currentMethodSignatures.TryAdd(overload.Signature, overload)) - { - continue; - } - - if (PreviousSignatureEndsWithCancellationToken(previousSignature)) - { - overload.Update(suppressions: - [ - new SuppressionStatement( - inner: null, - code: Literal("AZC0002"), - justification: "Back-compat overload preserves the previous method signature where CancellationToken was the trailing parameter. Making it optional would introduce an ambiguous call with the new method.") - ]); - } - - methods.Add(overload); - CodeModelGenerator.Instance.Emitter.Debug( - $"Added back-compat overload for '{Name}.{previousSignature.Name}' to handle new optional parameter(s) introduced relative to the last contract.", - BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded); - } - } - private static bool PreviousSignatureEndsWithCancellationToken(MethodSignature previousSignature) { if (previousSignature.Parameters.Count == 0) @@ -1881,87 +1693,5 @@ private static bool PreviousSignatureEndsWithCancellationToken(MethodSignature p var lastParam = previousSignature.Parameters[previousSignature.Parameters.Count - 1]; return new CSharpType.CSharpTypeIgnoreNullableComparer().Equals(lastParam.Type, new CSharpType(typeof(CancellationToken))); } - - // Returns true when currentSignature contains all parameters of previousSignature in the same - // relative order, every "extra" parameter is optional, and none of the extras are body parameters. - private static bool HasNewOptionalNonBodyParametersOnly( - MethodSignature previousSignature, - MethodSignature currentSignature) - { - if (currentSignature.Parameters.Count <= previousSignature.Parameters.Count) - { - return false; - } - - if (previousSignature.ReturnType is null - ? currentSignature.ReturnType is not null - : !previousSignature.ReturnType.AreNamesEqual(currentSignature.ReturnType)) - { - return false; - } - - // Walk current parameters and ensure previous parameters appear in the same relative order - // (matched by variable name and type), with every "extra" parameter being optional and non-body. - int previousIndex = 0; - for (int currentIndex = 0; currentIndex < currentSignature.Parameters.Count; currentIndex++) - { - var currentParam = currentSignature.Parameters[currentIndex]; - - if (previousIndex < previousSignature.Parameters.Count) - { - var previousParam = previousSignature.Parameters[previousIndex]; - if (currentParam.Name.ToVariableName() == previousParam.Name.ToVariableName() && - currentParam.Type.AreNamesEqual(previousParam.Type)) - { - previousIndex++; - continue; - } - } - - if (currentParam.DefaultValue is null) - { - return false; - } - - if (currentParam.Location == ParameterLocation.Body) - { - return false; - } - } - - return previousIndex == previousSignature.Parameters.Count; - } - - private ScmMethodProvider? BuildBackCompatOverloadForNewOptionalParameters( - MethodProvider previousMethod, - ScmMethodProvider currentMethod) - { - var previousSignature = previousMethod.Signature; - var currentSignature = currentMethod.Signature; - - var previousParamsByName = new Dictionary(); - foreach (var p in previousSignature.Parameters) - { - previousParamsByName.TryAdd(p.Name.ToVariableName(), p); - } - - var arguments = new List(currentSignature.Parameters.Count); - foreach (var currentParam in currentSignature.Parameters) - { - var currentParamVariableName = currentParam.Name.ToVariableName(); - ValueExpression value = previousParamsByName.TryGetValue(currentParamVariableName, out var prevParam) - ? prevParam - : (currentParam.DefaultValue ?? Default); - arguments.Add(PositionalReference(currentParamVariableName, value)); - } - - return new ScmMethodProvider( - signature: MethodSignatureHelper.BuildBackCompatMethodSignature(previousSignature, hideMethod: true), - bodyStatements: Return(This.Invoke(currentSignature.Name, arguments)), - enclosingType: this, - methodKind: currentMethod.Kind, - xmlDocProvider: previousMethod.XmlDocs, - serviceMethod: currentMethod.ServiceMethod); - } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ExtensibleEnumSerializationProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ExtensibleEnumSerializationProvider.cs index f3c5a792c95..a026a17b325 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ExtensibleEnumSerializationProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ExtensibleEnumSerializationProvider.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.IO; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Primitives; @@ -34,6 +35,9 @@ protected override string BuildRelativeFilePath() protected override TypeSignatureModifiers BuildDeclarationModifiers() => _enumProvider.DeclarationModifiers; + protected override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) + => [.. originalMethods]; + protected override MethodProvider[] BuildMethods() { // for string-based extensible enums, we are using `ToString` as its serialization 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 8202a2af405..16ac19e14e9 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 @@ -117,6 +117,10 @@ public MrwSerializationTypeDefinition(InputModelType inputModel, ModelProvider m protected override string BuildNamespace() => _model.Type.Namespace; protected override TypeSignatureModifiers BuildDeclarationModifiers() => _model.DeclarationModifiers; + + protected override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) + => [.. originalMethods]; + private ConstructorProvider SerializationConstructor => _serializationConstructor ??= _model.FullConstructor; private PropertyProvider[] AdditionalProperties => _additionalProperties.Value; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MultipartFormDataSerializationDefinition.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MultipartFormDataSerializationDefinition.cs index 88cb97b16e7..3ef2c7c4c62 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MultipartFormDataSerializationDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MultipartFormDataSerializationDefinition.cs @@ -46,6 +46,9 @@ public MultipartFormDataSerializationDefinition(InputModelType inputModel, Model protected override TypeSignatureModifiers BuildDeclarationModifiers() => _model.DeclarationModifiers; + protected override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) + => [.. originalMethods]; + protected override string BuildRelativeFilePath() { return Path.Combine("src", "Generated", "Models", $"{Name}.Serialization.Multipart.cs"); 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 1c322568a7b..38f74bddf37 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 @@ -45,5 +45,8 @@ public enum BackCompatibilityChangeCategory /// A back-compat change was skipped because the removal was accepted in the ApiCompat baseline. BaselineAcceptedRemovalSkipped, + + /// 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, } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs index e170e8a98a3..83d965b4ed0 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/EmitterRpc/Emitter.cs @@ -182,6 +182,7 @@ public void WriteBufferedMessages() BackCompatibilityChangeCategory.ModelFactoryMethodSkipped => "Model Factory Method Back-Compat Skipped", BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded => "Method Back-Compat Overload Added For New Optional Parameter", BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped => "Back-Compat Skipped For ApiCompat Baseline Accepted Removal", + BackCompatibilityChangeCategory.EnumMemberAddedFromLastContract => "Enum Member Added From Last Contract", _ => category.ToString(), }; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs index 409e5dc2b62..219b72470aa 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/VariableExpression.cs @@ -34,7 +34,7 @@ public void Update(CSharpType? type = null, string? name = null) } if (name != null) { - Declaration = new CodeWriterDeclaration(name); + Declaration.Update(name: name); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Primitives/CodeWriterDeclaration.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Primitives/CodeWriterDeclaration.cs index 3894d8f1c57..87035d3114c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Primitives/CodeWriterDeclaration.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Primitives/CodeWriterDeclaration.cs @@ -33,7 +33,7 @@ public CodeWriterDeclaration(string name) RequestedName = name; } - public string RequestedName { get; } + public string RequestedName { get; private set; } internal string GetActualName(CodeWriter.CodeScope scope) { @@ -49,5 +49,13 @@ internal void SetActualName(string actualName, CodeWriter.CodeScope scope) { _actualNames[scope] = actualName; } + + internal void Update(string? name = null) + { + if (name != null) + { + RequestedName = name; + } + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ApiVersionEnumProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ApiVersionEnumProvider.cs index 642a8bf85e2..6bceafce8a5 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ApiVersionEnumProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ApiVersionEnumProvider.cs @@ -168,15 +168,8 @@ private List BuildApiVersionEnumValuesForBackwardCompatibility(L return currentApiVersions; } - var processedNames = new HashSet(lastContractFields.Select(f => f.Name), StringComparer.OrdinalIgnoreCase); - // Then, add new versions in the wire order - foreach (var currentVersion in currentApiVersions) - { - if (!processedNames.Contains(currentVersion.Name)) - { - allMembers.Add(currentVersion); - } - } + // Then, add new versions in the wire order. + AppendMembersNotInLastContract(currentApiVersions, lastContractFields, allMembers); for (int i = 0; i < allMembers.Count; i++) { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/FixedEnumProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/FixedEnumProvider.cs index 524b3bf2627..854ea4a6b6a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/FixedEnumProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/FixedEnumProvider.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.TypeSpec.Generator.EmitterRpc; using Microsoft.TypeSpec.Generator.Expressions; @@ -128,29 +129,49 @@ protected override IReadOnlyList BuildEnumValues() { if (currentLookup.TryGetValue(field.Name, out var existingMember)) { + // By default, preserve the last contract's explicit value for integer enums so + // members keep their exact values. If the baseline accepts a value change for this + // member (EnumValuesMustMatch), honor the current value instead of restoring the old. + ValueExpression? initializationValue = existingMember.Field.InitializationValue; + var memberValue = existingMember.Value; + if (IsIntValueType + && field.InitializationValue is LiteralExpression { Literal: { } lastContractValue } + && CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMemberSuppressed(Type.FullyQualifiedName, field.Name, 0) != true) + { + initializationValue = field.InitializationValue; + memberValue = lastContractValue; + } + var updatedField = new FieldProvider( existingMember.Field.Modifiers, existingMember.Field.Type, existingMember.Name, existingMember.Field.EnclosingType, - existingMember.Field.Description); - allMembers.Add(new EnumTypeMember(existingMember.Name, updatedField, existingMember.Value)); + existingMember.Field.Description, + initializationValue); + allMembers.Add(new EnumTypeMember(existingMember.Name, updatedField, memberValue)); } - } - - // Then, add new members that weren't in the last contract (in their original input order) - var processedNames = new HashSet(lastContractFields.Select(f => f.Name), StringComparer.OrdinalIgnoreCase); - foreach (var current in currentValues) - { - if (!processedNames.Contains(current.Name)) + else if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMemberSuppressed(Type.FullyQualifiedName, field.Name, 0) == true) { - allMembers.Add(current); + CodeModelGenerator.Instance.Emitter.Debug( + $"Skipping re-add of enum member '{Name}.{field.Name}'; the removal is accepted in the ApiCompat baseline.", + BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped); + } + else if (TryResurrectRemovedMember(field, out var resurrectedMember)) + { + allMembers.Add(resurrectedMember); + CodeModelGenerator.Instance.Emitter.Debug( + $"Re-added enum member '{field.Name}' to enum '{Name}' to preserve a member from the last contract.", + BackCompatibilityChangeCategory.EnumMemberAddedFromLastContract); } } - // Report a summary-level change only if the relative order of shared members - // was actually altered to match the last contract. - if (!EnumMemberOrderMatches(currentValues, allMembers)) + // Then, add new members that weren't in the last contract (in their original input order). + AppendMembersNotInLastContract(currentValues, lastContractFields, allMembers); + + // Report a reordering only when the relative order of members present in BOTH the + // current values and the resulting set was actually altered. + if (SharedMemberOrderChanged(currentValues, allMembers)) { CodeModelGenerator.Instance.Emitter.Debug( $"Reordered members of enum '{Name}' to match last contract.", @@ -160,22 +181,71 @@ protected override IReadOnlyList BuildEnumValues() return allMembers; } - private static bool EnumMemberOrderMatches( - IReadOnlyList left, - IReadOnlyList right) + private bool TryResurrectRemovedMember(FieldProvider lastContractField, [NotNullWhen(true)] out EnumTypeMember? member) { - if (left.Count != right.Count) + member = null; + if (!IsIntValueType || lastContractField.InitializationValue is not LiteralExpression { Literal: { } literalValue }) { return false; } - for (int i = 0; i < left.Count; i++) + + var field = new FieldProvider( + FieldModifiers.Public | FieldModifiers.Static, + EnumUnderlyingType, + lastContractField.Name, + this, + lastContractField.Description, + Literal(literalValue)); + member = new EnumTypeMember(lastContractField.Name, field, literalValue); + return true; + } + + // Appends the members of that were not present in the last + // contract (i.e. newly introduced by the current spec), preserving their original input order. + // Shared by the fixed-enum and API-version back-compatibility passes. + private protected static void AppendMembersNotInLastContract( + IReadOnlyList currentValues, + IReadOnlyList lastContractFields, + List allMembers) + { + var lastContractNames = new HashSet(lastContractFields.Select(f => f.Name), StringComparer.OrdinalIgnoreCase); + foreach (var current in currentValues) + { + if (!lastContractNames.Contains(current.Name)) + { + allMembers.Add(current); + } + } + } + + private static bool SharedMemberOrderChanged( + IReadOnlyList currentValues, + IReadOnlyList result) + { + var currentNames = new HashSet(currentValues.Count, StringComparer.Ordinal); + foreach (var member in currentValues) { - if (!string.Equals(left[i].Name, right[i].Name, StringComparison.Ordinal)) + currentNames.Add(member.Name); + } + + var index = 0; + foreach (var member in result) + { + if (!currentNames.Contains(member.Name)) + { + continue; + } + + if (index >= currentValues.Count + || !string.Equals(member.Name, currentValues[index].Name, StringComparison.Ordinal)) { - return false; + return true; } + + index++; } - return true; + + return false; } protected internal override FieldProvider[] BuildFields() diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs index c30d3a00bff..ae2495a64ba 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ModelFactoryProvider.cs @@ -12,6 +12,7 @@ using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Snippets; using Microsoft.TypeSpec.Generator.Statements; +using Microsoft.TypeSpec.Generator.Utilities; using static Microsoft.TypeSpec.Generator.Snippets.Snippet; namespace Microsoft.TypeSpec.Generator.Providers @@ -105,7 +106,7 @@ protected internal sealed override IReadOnlyList BuildMethodsFor // change between the previous and current contract is a parameter rename. This // avoids source-breaking changes for callers using named arguments (e.g. when a // property is renamed via @@clientName, spec rename, or naming-rule change). - PreservePreviousParameterNames(factoryMethods); + BackCompatHelper.RestorePreviousParameterNames(this, factoryMethods); HashSet currentMethodSignatures = new List([.. factoryMethods, .. CustomCodeView?.Methods ?? []]) .Select(m => m.Signature) @@ -120,14 +121,8 @@ protected internal sealed override IReadOnlyList BuildMethodsFor // If the removal of this factory method has already been accepted in the ApiCompat // baseline, honor that decision and do not resurrect a compatibility shim for it. - if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMemberSuppressed( - Type.FullyQualifiedName, - previousMethod.Signature.Name, - previousMethod.Signature.Parameters.Count) == true) + if (BackCompatHelper.IsMethodRemovalAcceptedInBaseline(this, previousMethod.Signature)) { - CodeModelGenerator.Instance.Emitter.Info( - $"Skipping back-compat shim for '{Type.FullyQualifiedName}.{previousMethod.Signature.Name}'; removal is accepted in the ApiCompat baseline.", - BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped); continue; } @@ -212,85 +207,6 @@ protected internal sealed override IReadOnlyList BuildMethodsFor return [.. factoryMethods]; } - // Preserve original parameter names from the previous contract when a current factory - // method matches a previous one by name + parameter types/order but differs only in - // parameter names. The rename is applied in-place via ParameterProvider.Update which - // also updates the cached variable/argument expressions used by the method body and - // XML docs, so the body and docs serialize with the preserved names automatically. - private void PreservePreviousParameterNames(List currentFactoryMethods) - { - if (LastContractView?.Methods == null) - { - return; - } - - foreach (var previousMethod in LastContractView.Methods) - { - MethodProvider? matchingCurrent = null; - foreach (var current in currentFactoryMethods) - { - // MethodSignatureComparer matches on method name + parameter count + parameter - // types (positional); it does not consider parameter names. So a previous - // method whose only difference from a current method is parameter names will - // still match here. - if (MethodSignature.MethodSignatureComparer.Equals(current.Signature, previousMethod.Signature)) - { - matchingCurrent = current; - break; - } - } - - if (matchingCurrent is null) - { - continue; - } - - var previousParameters = previousMethod.Signature.Parameters; - var currentParameters = matchingCurrent.Signature.Parameters; - if (previousParameters.Count != currentParameters.Count) - { - continue; - } - - for (int i = 0; i < previousParameters.Count; i++) - { - var previousName = previousParameters[i].Name; - var currentParam = currentParameters[i]; - if (string.IsNullOrEmpty(previousName) || currentParam.Name == previousName) - { - continue; - } - - // Skip the rename when applying it would create a name collision with another - // current parameter (e.g. two same-typed parameters in swapped order between - // the previous and current contracts). A positional rename in that case would - // silently swap which parameter feeds which constructor field via name-based - // lookup in GetCtorArgs, producing semantically wrong (and source-breaking) code. - var previousNameToApply = previousName; - bool wouldCollide = false; - for (int j = 0; j < currentParameters.Count; j++) - { - if (j != i && string.Equals(currentParameters[j].Name, previousNameToApply, StringComparison.Ordinal)) - { - wouldCollide = true; - break; - } - } - - if (wouldCollide) - { - continue; - } - - CodeModelGenerator.Instance.Emitter.Debug( - $"Preserved parameter name '{previousName}' on '{Name}.{matchingCurrent.Signature.Name}' from last contract (instead of '{currentParam.Name}').", - BackCompatibilityChangeCategory.ParameterNamePreserved); - - currentParam.Update(name: previousName); - } - } - } - private bool TryBuildCompatibleMethodForPreviousContract( MethodProvider previousMethod, MethodSignature? currentMethodSignature, 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 9f28c6bc482..53142b6c74c 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 @@ -27,7 +27,7 @@ protected override FormattableString BuildDescription() { var description = DocHelpers.GetFormattableDescription(_inputModel.Summary, _inputModel.Doc) ?? $"The {Name}."; - if (IsAbstract) + if (DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)) { _derivedModels = BuildDerivedModels(); var publicDerivedModels = _derivedModels.Where(m => m.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public)).ToList(); @@ -66,7 +66,6 @@ protected override FormattableString BuildDescription() private ConstructorProvider? _fullConstructor; internal PropertyProvider? DiscriminatorProperty { get; private set; } private ValueExpression DiscriminatorLiteral => Literal(_inputModel.DiscriminatorValue ?? ""); - private bool IsAbstract => _inputModel.DiscriminatorProperty is not null && _inputModel.DiscriminatorValue is null && LastContractView?.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract) != false; public ModelProvider(InputModelType inputModel) : base(inputModel) { @@ -325,7 +324,7 @@ protected override TypeSignatureModifiers BuildDeclarationModifiers() declarationModifiers |= TypeSignatureModifiers.Internal; } - if (IsAbstract) + if (_inputModel.DiscriminatorProperty is not null && _inputModel.DiscriminatorValue is null) { declarationModifiers |= TypeSignatureModifiers.Abstract; } @@ -777,7 +776,7 @@ protected internal override ConstructorProvider[] BuildConstructors() private bool ComputeIsMultiLevelDiscriminator() { // Only applies to non-abstract models with a base model - if (IsAbstract || _inputModel.BaseModel == null) + if (DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract) || _inputModel.BaseModel == null) { return false; } @@ -848,71 +847,6 @@ private ConstructorProvider BuildFullConstructor() this); } - protected internal override IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) - { - // Only handle the case of changing modifiers on abstract base types - if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)) - { - return [.. originalConstructors]; - } - - if (LastContractView?.Constructors == null || LastContractView.Constructors.Count == 0) - { - return [.. originalConstructors]; - } - - List constructors = [.. originalConstructors]; - - // Check if the last contract had a public constructor with matching parameters - foreach (var previousConstructor in LastContractView.Constructors) - { - if (!previousConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)) - { - continue; - } - - // Find a matching constructor in the current version by parameter signature - for (int i = 0; i < constructors.Count; i++) - { - var currentConstructor = constructors[i]; - - // Check if parameters match (same count and types) - if (ParametersMatch(currentConstructor.Signature.Parameters, previousConstructor.Signature.Parameters)) - { - // Change the modifier from private protected to public - if (currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private) && - currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)) - { - currentConstructor.Signature.Update(modifiers: MethodSignatureModifiers.Public); - CodeModelGenerator.Instance.Emitter.Debug( - $"Promoted constructor '{Name}({string.Join(", ", currentConstructor.Signature.Parameters.Select(p => p.Type.ToString()))})' from 'private protected' to 'public' to match last contract.", - BackCompatibilityChangeCategory.ConstructorModifierPreserved); - } - } - } - } - - return [.. constructors]; - } - - private bool ParametersMatch(IReadOnlyList params1, IReadOnlyList params2) - { - if (params1.Count != params2.Count) - { - return false; - } - - for (int i = 0; i < params1.Count; i++) - { - if (!params1[i].Type.AreNamesEqual(params2[i].Type) || params1[i].Name != params2[i].Name) - { - return false; - } - } - - return true; - } - private IEnumerable GetAllBasePropertiesForConstructorInitialization(bool includeAllHierarchyDiscriminator = false) { var properties = new Stack>(); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs index 3d71670d5a9..83380c4299b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs @@ -215,6 +215,16 @@ private TypeSignatureModifiers BuildDeclarationModifiersInternal() severity: EmitterDiagnosticSeverity.Warning); } + // Back-compat: a type that the last contract published as non-abstract must not become + // abstract, which would be a source-breaking change for existing derived types and + // callers. Preserve the previously-published non-abstract shape. + if (modifiers.HasFlag(TypeSignatureModifiers.Abstract) && + LastContractView is { } lastContractView && + !lastContractView.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)) + { + modifiers &= ~TypeSignatureModifiers.Abstract; + } + // we always add partial when possible if (!modifiers.HasFlag(TypeSignatureModifiers.Enum) && DeclaringTypeProvider is null) { @@ -813,11 +823,122 @@ private static IReadOnlyList VisitNewMembers( protected internal virtual IReadOnlyList? BuildEnumValuesForBackCompatibility(IReadOnlyList originalEnumValues) => null; + /// + /// Returns this type's methods with backward compatibility applied against + /// . The default implementation restores the previous + /// parameter order on a current method when it matches a last-contract method by name and + /// return type with the same parameter set but in a different order. Reordering is done in + /// place, so a method's body (which references its parameters by object) remains valid. + /// Override and call base to extend this behavior; override without calling + /// base to replace it. + /// protected internal virtual IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) - => [.. originalMethods]; + { + var methods = new List(originalMethods); + + if (LastContractView?.Methods is not { Count: > 0 } previousMethods) + { + return methods; + } + + var currentMethodSignatures = BuildCurrentMethodSignatureMap(methods); + + foreach (var previousMethod in previousMethods) + { + if (!BackCompatHelper.ShouldApplyMethodBackCompatibility(previousMethod.Signature, currentMethodSignatures) + || BackCompatHelper.IsMethodRemovalAcceptedInBaseline(this, previousMethod.Signature)) + { + continue; + } + + var methodToReorder = BackCompatHelper.FindMethodWithSameParametersDifferentOrder(previousMethod.Signature, currentMethodSignatures); + if (methodToReorder != null && BackCompatHelper.TryRestorePreviousParameterOrder(methodToReorder, previousMethod.Signature)) + { + CodeModelGenerator.Instance.Emitter.Debug( + $"Reordered parameters of '{Name}.{methodToReorder.Signature.Name}' to match last contract.", + BackCompatibilityChangeCategory.MethodParameterReordering); + } + } + + BackCompatHelper.RestorePreviousParameterNames(this, methods); + BackCompatHelper.AddOverloadsForNewOptionalParameters(this, methods); + + return methods; + } + + /// + /// Builds a lookup of the type's current method signatures (including custom code methods) + /// used to match against last-contract methods. + /// + private Dictionary BuildCurrentMethodSignatureMap(IEnumerable methods) + { + var allMethods = CustomCodeView?.Methods != null + ? methods.Concat(CustomCodeView.Methods) + : methods; + + var result = new Dictionary(MethodSignature.MethodSignatureComparer); + foreach (var method in allMethods) + { + result.TryAdd(method.Signature, method); + } + return result; + } + /// + /// Returns this type's constructors with backward compatibility applied against + /// . The default implementation preserves a previously-published + /// public constructor on an abstract base type: when the current generation would emit a + /// private protected constructor whose parameters match a public constructor in + /// the last contract, the modifier is promoted back to public. Override and call + /// base to extend this behavior. + /// protected internal virtual IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) - => [.. originalConstructors]; + { + // Only handle the case of changing modifiers on abstract base types. + if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)) + { + return [.. originalConstructors]; + } + + if (LastContractView?.Constructors == null || LastContractView.Constructors.Count == 0) + { + return [.. originalConstructors]; + } + + List constructors = [.. originalConstructors]; + + // Check if the last contract had a public constructor with matching parameters + foreach (var previousConstructor in LastContractView.Constructors) + { + if (!previousConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)) + { + continue; + } + + // Find a matching constructor in the current version by parameter signature + for (int i = 0; i < constructors.Count; i++) + { + var currentConstructor = constructors[i]; + if (!currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private) || + !currentConstructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)) + { + continue; + } + + // Check if parameters match (same count and types) + if (BackCompatHelper.ParametersMatch(currentConstructor.Signature.Parameters, previousConstructor.Signature.Parameters)) + { + // Change the modifier from private protected to public + currentConstructor.Signature.Update(modifiers: MethodSignatureModifiers.Public); + CodeModelGenerator.Instance.Emitter.Debug( + $"Promoted constructor '{Name}({string.Join(", ", currentConstructor.Signature.Parameters.Select(p => p.Type.ToString()))})' from 'private protected' to 'public' to match last contract.", + BackCompatibilityChangeCategory.ConstructorModifierPreserved); + } + } + } + + return [.. constructors]; + } private IReadOnlyList? _enumValues; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/ApiCompatBaseline.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/ApiCompatBaseline.cs index 287c0ead838..5b514d404aa 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/ApiCompatBaseline.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/ApiCompatBaseline.cs @@ -26,6 +26,7 @@ public sealed class ApiCompatBaseline { private const string TypesMustExist = "TypesMustExist"; private const string MembersMustExist = "MembersMustExist"; + private const string EnumValuesMustMatch = "EnumValuesMustMatch"; private readonly HashSet _suppressedTypes; private readonly HashSet _suppressedMembers; @@ -98,11 +99,17 @@ public static ApiCompatBaseline Parse(IEnumerable lines) suppressedTypes.Add(quoted); break; case MembersMustExist: - if (TryParseMember(quoted, out var memberKey)) + if (TryParseMember(quoted, out var memberKey) || TryParseField(quoted, out memberKey)) { suppressedMembers.Add(memberKey); } break; + case EnumValuesMustMatch: + if (TryParseField(quoted, out var enumMemberKey)) + { + suppressedMembers.Add(enumMemberKey); + } + break; // Other rule ids (e.g. CannotRemoveAttribute) do not describe a removed // member/type that the back-compat system would resurrect, so they are ignored. default: @@ -256,6 +263,39 @@ private static bool TryParseMember(string signature, out MemberKey memberKey) return true; } + // Parses an ApiCompat field/enum-value signature (which has no parameter list) of the form: + // [modifiers] FieldType Namespace.DeclaringType.MemberName + // for example: + // Ns.CapacityLevel Ns.CapacityLevel.FiftyThousand + // public Ns.Kind Ns.Foo.Kind + // Enum members are recorded with a parameter count of zero. + private static bool TryParseField(string signature, out MemberKey memberKey) + { + memberKey = default; + + // Field/enum-value signatures never contain a parameter list. + if (signature.Contains('(')) + { + return false; + } + + // The fully-qualified member path is the last whitespace-delimited token (the tokens + // before it are access modifiers and the field/enum type). + var lastSpace = signature.LastIndexOf(' '); + var memberPath = lastSpace >= 0 ? signature.Substring(lastSpace + 1) : signature; + + var lastDot = memberPath.LastIndexOf('.'); + if (lastDot <= 0 || lastDot == memberPath.Length - 1) + { + return false; + } + + var declaringTypeFullName = memberPath.Substring(0, lastDot); + var memberName = memberPath.Substring(lastDot + 1); + memberKey = new MemberKey(declaringTypeFullName, memberName, 0); + return true; + } + private static int CountParameters(string parameterList) { var trimmed = parameterList.Trim(); 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 new file mode 100644 index 00000000000..72bcebb2ba8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs @@ -0,0 +1,507 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.TypeSpec.Generator.EmitterRpc; +using Microsoft.TypeSpec.Generator.Expressions; +using Microsoft.TypeSpec.Generator.Input.Extensions; +using Microsoft.TypeSpec.Generator.Primitives; +using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Statements; +using static Microsoft.TypeSpec.Generator.Snippets.Snippet; + +namespace Microsoft.TypeSpec.Generator.Utilities +{ + /// + /// Shared helpers for applying backward compatibility to generated methods by comparing the + /// current generation against a type's last contract. + /// + internal static class BackCompatHelper + { + /// + /// Returns true when a last-contract method should be considered for back compatibility: + /// it is public or protected and has no exact match among the current signatures. + /// + public static bool ShouldApplyMethodBackCompatibility( + MethodSignature previousSignature, + Dictionary currentMethodSignatures) + { + if (currentMethodSignatures.ContainsKey(previousSignature)) + { + return false; + } + + var modifiers = previousSignature.Modifiers; + return modifiers.HasFlag(MethodSignatureModifiers.Public) || + modifiers.HasFlag(MethodSignatureModifiers.Protected); + } + + /// + /// Returns true when the removal of a previously-published method — identified by the enclosing + /// type's fully-qualified name, the method name, and the parameter count — has been accepted in + /// the ApiCompat baseline, in which case back compatibility must not resurrect or restore it. + /// Emits an informational log entry when a suppression is honored. + /// + public static bool IsMethodRemovalAcceptedInBaseline(TypeProvider enclosingType, MethodSignature previousSignature) + { + if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMemberSuppressed( + enclosingType.Type.FullyQualifiedName, + previousSignature.Name, + previousSignature.Parameters.Count) != true) + { + return false; + } + + CodeModelGenerator.Instance.Emitter.Info( + $"Skipping back-compat for '{enclosingType.Type.FullyQualifiedName}.{previousSignature.Name}'; 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. + /// + public static MethodProvider? FindMethodWithSameParametersDifferentOrder( + MethodSignature previousSignature, + Dictionary currentMethodSignatures) + { + foreach (var kvp in currentMethodSignatures) + { + var currentSignature = kvp.Key; + if (currentSignature.Name.Equals(previousSignature.Name) + && currentSignature.ReturnType?.AreNamesEqual(previousSignature.ReturnType) == true + && MethodSignatureHelper.ContainsSameParameters(previousSignature, currentSignature)) + { + return kvp.Value; + } + } + + return null; + } + + /// + /// Returns true when two parameter lists match positionally by type name and parameter name + /// (and have the same count). Used to align a current member with its last-contract counterpart. + /// + public static bool ParametersMatch(IReadOnlyList params1, IReadOnlyList params2) + { + if (params1.Count != params2.Count) + { + return false; + } + + for (int i = 0; i < params1.Count; i++) + { + if (!params1[i].Type.AreNamesEqual(params2[i].Type) || params1[i].Name != params2[i].Name) + { + return false; + } + } + + return true; + } + + /// + /// Returns the previously-published name of a parameter whose original (spec) name is + /// , looked up in . When + /// is supplied, the search is scoped to last-contract methods + /// whose name matches it (allowing for a sync/async pair) so a parameter name shared across + /// methods cannot cross-match. Returns null when no match exists. + /// + public static string? FindPreviousParameterName( + TypeProvider? lastContractView, + string originalName, + string? methodName = null) + { + var lastContractMethods = lastContractView?.Methods; + if (lastContractMethods is null || lastContractMethods.Count == 0) + { + return null; + } + + IEnumerable scopedMethods = lastContractMethods; + if (methodName != null) + { + scopedMethods = lastContractMethods.Where(m => + string.Equals(m.Signature.Name, methodName, StringComparison.OrdinalIgnoreCase) || + string.Equals(m.Signature.Name, methodName + "Async", StringComparison.OrdinalIgnoreCase)); + } + + return scopedMethods + .SelectMany(method => method.Signature.Parameters) + .FirstOrDefault(p => string.Equals(p.Name, originalName, StringComparison.OrdinalIgnoreCase)) + ?.Name; + } + + public static void RestorePreviousParameterNames( + TypeProvider enclosingType, + IReadOnlyList currentMethods) + { + var lastContractView = enclosingType.LastContractView; + if (lastContractView?.Methods is not { Count: > 0 } previousMethods) + { + return; + } + + foreach (var method in currentMethods) + { + var modifiers = method.Signature.Modifiers; + if (!modifiers.HasFlag(MethodSignatureModifiers.Public) && !modifiers.HasFlag(MethodSignatureModifiers.Protected)) + { + continue; + } + + var currentParameters = method.Signature.Parameters; + MethodProvider? matchingPrevious = null; + bool matchingPreviousResolved = false; + + for (int i = 0; i < currentParameters.Count; i++) + { + var parameter = currentParameters[i]; + string? preservedName; + + var inputParameter = parameter.InputParameter; + if (inputParameter is not null) + { + if (!string.Equals(parameter.Name, inputParameter.Name, StringComparison.Ordinal)) + { + continue; + } + + var originalName = inputParameter.OriginalName; + if (string.IsNullOrEmpty(originalName)) + { + continue; + } + + preservedName = FindPreviousParameterName(lastContractView, originalName, method.Signature.Name); + } + else + { + // Positional fallback for synthesized parameters (e.g. model factory methods). + if (!matchingPreviousResolved) + { + matchingPrevious = FindMethodWithSameSignatureIgnoringNames(previousMethods, method.Signature); + matchingPreviousResolved = true; + } + + var previousParameters = matchingPrevious?.Signature.Parameters; + preservedName = previousParameters is not null && previousParameters.Count == currentParameters.Count + ? previousParameters[i].Name + : null; + } + + // A casing-only difference is still a source-breaking rename for named arguments, + // so compare case-sensitively and restore the previously-published spelling. + if (string.IsNullOrEmpty(preservedName) || string.Equals(parameter.Name, preservedName, StringComparison.Ordinal)) + { + continue; + } + + // Skip the rename when applying it would collide with another current parameter's + // name (e.g. two same-typed parameters whose order changed between the previous and + // current contracts). A rename in that case would produce a duplicate parameter name + // and, for name-based argument lookups, silently wire the wrong value. + bool wouldCollide = false; + for (int j = 0; j < currentParameters.Count; j++) + { + if (j != i && string.Equals(currentParameters[j].Name, preservedName, StringComparison.Ordinal)) + { + wouldCollide = true; + break; + } + } + + if (wouldCollide) + { + continue; + } + + CodeModelGenerator.Instance.Emitter.Debug( + $"Preserved parameter name '{preservedName}' on '{enclosingType.Name}.{method.Signature.Name}' from last contract (instead of '{parameter.Name}').", + BackCompatibilityChangeCategory.ParameterNamePreserved); + parameter.Update(name: preservedName); + } + } + } + + /// + /// Finds the last-contract method whose signature matches on + /// method name plus parameter count and types (ignoring parameter names), or null when none. + /// + private static MethodProvider? FindMethodWithSameSignatureIgnoringNames( + IReadOnlyList previousMethods, + MethodSignature currentSignature) + { + foreach (var previousMethod in previousMethods) + { + if (MethodSignature.MethodSignatureComparer.Equals(currentSignature, previousMethod.Signature)) + { + return previousMethod; + } + } + + return null; + } + + /// + /// Restores the previous parameter order on in place + /// (updating XML docs). Returns true when a change was made. + /// + public static bool TryRestorePreviousParameterOrder( + MethodProvider methodToReorder, + MethodSignature previousSignature) + { + var currentSignature = methodToReorder.Signature; + if (MethodSignatureHelper.HaveSameParametersInSameOrder(currentSignature, previousSignature)) + { + return false; + } + + var parametersByName = currentSignature.Parameters.ToDictionary(p => p.Name.ToVariableName()); + var reorderedParameters = new List(currentSignature.Parameters.Count); + + foreach (var previousParam in previousSignature.Parameters) + { + if (parametersByName.TryGetValue(previousParam.Name.ToVariableName(), out var matchingParam)) + { + reorderedParameters.Add(matchingParam); + } + } + + if (reorderedParameters.Count != currentSignature.Parameters.Count) + { + return false; + } + + foreach (var previousParam in previousSignature.Parameters) + { + if (parametersByName.TryGetValue(previousParam.Name.ToVariableName(), out var matchingParam) + && matchingParam.DefaultValue is not null + && previousParam.DefaultValue is not null) + { + matchingParam.Update(defaultValue: previousParam.DefaultValue); + } + } + + var updatedSignature = new MethodSignature( + currentSignature.Name, + currentSignature.Description, + currentSignature.Modifiers, + currentSignature.ReturnType, + currentSignature.ReturnDescription, + reorderedParameters, + currentSignature.Attributes, + currentSignature.GenericArguments, + currentSignature.GenericParameterConstraints, + currentSignature.ExplicitInterface, + currentSignature.NonDocumentComment); + + UpdateXmlDocProviderForParamReorder(methodToReorder.XmlDocs, updatedSignature); + methodToReorder.Update(signature: updatedSignature, xmlDocProvider: methodToReorder.XmlDocs); + + return true; + } + + private static void UpdateXmlDocProviderForParamReorder( + XmlDocProvider xmlDocs, + MethodSignature updatedSignature) + { + var paramDocsByName = xmlDocs.Parameters.ToDictionary(s => s.Parameter.Name); + var reorderedParamDocs = new List(updatedSignature.Parameters.Count); + + foreach (var param in updatedSignature.Parameters) + { + if (paramDocsByName.TryGetValue(param.Name, out var paramDoc)) + { + reorderedParamDocs.Add(paramDoc); + } + } + + if (reorderedParamDocs.Count == xmlDocs.Parameters.Count && + !reorderedParamDocs.SequenceEqual(xmlDocs.Parameters)) + { + xmlDocs.Update(parameters: reorderedParamDocs); + } + } + + /// + /// Adds hidden back-compat overloads for public/protected methods that gained one or more new + /// optional non-body parameters relative to the last contract. Each added overload matches a + /// previously-published signature and delegates to the current method, forwarding the previous + /// arguments and passing default for every new parameter. Renaming a method's parameter + /// set this way is otherwise a source-breaking change for existing callers. + /// + public static void AddOverloadsForNewOptionalParameters(TypeProvider enclosingType, List methods) + { + if (enclosingType.LastContractView?.Methods is not { Count: > 0 } previousMethods) + { + return; + } + + var currentMethods = enclosingType.CustomCodeView?.Methods is { } customMethods + ? methods.Concat(customMethods) + : methods; + if (!currentMethods.Any()) + { + return; + } + + var currentMethodsByName = new Dictionary>(); + foreach (var method in currentMethods) + { + if (!currentMethodsByName.TryGetValue(method.Signature.Name, out var bucket)) + { + bucket = []; + currentMethodsByName[method.Signature.Name] = bucket; + } + bucket.Add(method); + } + + foreach (var previousMethod in previousMethods) + { + var previousSignature = previousMethod.Signature; + if ((!previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Public) && + !previousSignature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)) || + !currentMethodsByName.TryGetValue(previousSignature.Name, out var candidates)) + { + continue; + } + + // One pass over the same-named methods: bail if the previous signature still exists, + // otherwise remember the first public/protected method that merely gained optional + // non-body parameters. + MethodProvider? matchedCurrent = null; + bool previousStillExists = false; + foreach (var candidate in candidates) + { + if (MethodSignature.MethodSignatureComparer.Equals(candidate.Signature, previousSignature)) + { + previousStillExists = true; + break; + } + + var modifiers = candidate.Signature.Modifiers; + if (matchedCurrent is null && + (modifiers.HasFlag(MethodSignatureModifiers.Public) || modifiers.HasFlag(MethodSignatureModifiers.Protected)) && + HasNewOptionalNonBodyParametersOnly(previousSignature, candidate.Signature)) + { + matchedCurrent = candidate; + } + } + + if (previousStillExists || matchedCurrent is null || IsMethodRemovalAcceptedInBaseline(enclosingType, previousSignature)) + { + continue; + } + + var overload = BuildNewOptionalParameterOverload(enclosingType, previousMethod, matchedCurrent); + candidates.Add(overload); + methods.Add(overload); + CodeModelGenerator.Instance.Emitter.Debug( + $"Added back-compat overload for '{enclosingType.Name}.{previousSignature.Name}' to handle new optional parameter(s) introduced relative to the last contract.", + BackCompatibilityChangeCategory.SvcMethodNewOptionalParameterOverloadAdded); + } + } + + /// + /// Returns true when contains all parameters of + /// in the same relative order (matched by variable name and + /// type) with the same return type, every "extra" current parameter is optional, and none of the + /// extras are body parameters. + /// + public static bool HasNewOptionalNonBodyParametersOnly( + MethodSignature previousSignature, + MethodSignature currentSignature) + { + if (currentSignature.Parameters.Count <= previousSignature.Parameters.Count) + { + return false; + } + + if (previousSignature.ReturnType is null + ? currentSignature.ReturnType is not null + : !previousSignature.ReturnType.AreNamesEqual(currentSignature.ReturnType)) + { + return false; + } + + // Walk current parameters and ensure previous parameters appear in the same relative order + // (matched by variable name and type), with every "extra" parameter being optional and non-body. + int previousIndex = 0; + for (int currentIndex = 0; currentIndex < currentSignature.Parameters.Count; currentIndex++) + { + var currentParam = currentSignature.Parameters[currentIndex]; + + if (previousIndex < previousSignature.Parameters.Count) + { + var previousParam = previousSignature.Parameters[previousIndex]; + if (currentParam.Name.ToVariableName() == previousParam.Name.ToVariableName() && + currentParam.Type.AreNamesEqual(previousParam.Type)) + { + previousIndex++; + continue; + } + } + + if (currentParam.DefaultValue is null) + { + return false; + } + + if (currentParam.Location is ParameterLocation.Body or ParameterLocation.Unknown) + { + return false; + } + } + + return previousIndex == previousSignature.Parameters.Count; + } + + private static MethodProvider BuildNewOptionalParameterOverload( + TypeProvider enclosingType, + MethodProvider previousMethod, + MethodProvider currentMethod) + { + var previousSignature = previousMethod.Signature; + var currentSignature = currentMethod.Signature; + + var previousParametersByName = new Dictionary(); + foreach (var parameter in previousSignature.Parameters) + { + previousParametersByName.TryAdd(parameter.Name.ToVariableName(), parameter); + } + + // Build the delegating call: forward each previous parameter and pass default for the new ones. + var arguments = new List(currentSignature.Parameters.Count); + foreach (var currentParam in currentSignature.Parameters) + { + var variableName = currentParam.Name.ToVariableName(); + ValueExpression value = previousParametersByName.TryGetValue(variableName, out var previousParam) + ? previousParam + : currentParam.DefaultValue ?? Default; + arguments.Add(PositionalReference(variableName, value)); + } + + var invocationTarget = currentSignature.Modifiers.HasFlag(MethodSignatureModifiers.Static) + ? Static(enclosingType.Type) + : This; + var delegatingCall = invocationTarget.Invoke(currentSignature.Name, arguments); + MethodBodyStatement body = IsVoidReturnType(currentSignature.ReturnType) + ? delegatingCall.Terminate() + : Return(delegatingCall); + + return new MethodProvider( + MethodSignatureHelper.BuildBackCompatMethodSignature(previousSignature, hideMethod: true), + body, + enclosingType, + previousMethod.XmlDocs); + } + + private static bool IsVoidReturnType(CSharpType? returnType) => + returnType is null || (returnType.IsFrameworkType && returnType.FrameworkType == typeof(void)); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/EnumProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/EnumProviderTests.cs index 3bbc8cb463e..fe18ff059a3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/EnumProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/EnumProviderTests.cs @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +// cspell:ignore readded + using System; using System.Linq; using System.Threading.Tasks; @@ -394,9 +396,9 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual("Recover", fields[0].Name); Assert.AreEqual("Default", fields[1].Name); - // No explicit initialization values - compiler auto-assigns based on order - Assert.IsNull(fields[0].InitializationValue); - Assert.IsNull(fields[1].InitializationValue); + // Shared members keep their explicit values from the last contract (Recover = 0, Default = 1). + Assert.AreEqual(0, (fields[0].InitializationValue as LiteralExpression)?.Literal); + Assert.AreEqual(1, (fields[1].InitializationValue as LiteralExpression)?.Literal); } // Validates that int enum member order is preserved and new values are appended @@ -429,18 +431,75 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual("Default", fields[1].Name); Assert.AreEqual("Third", fields[2].Name); - // No explicit initialization values for reordered members - compiler auto-assigns based on order - Assert.IsNull(fields[0].InitializationValue); - Assert.IsNull(fields[1].InitializationValue); + // Shared members keep their explicit values from the last contract (Recover = 0, Default = 1). + Assert.AreEqual(0, (fields[0].InitializationValue as LiteralExpression)?.Literal); + Assert.AreEqual(1, (fields[1].InitializationValue as LiteralExpression)?.Literal); // New value keeps its initialization value from the input var value3 = fields[2].InitializationValue as LiteralExpression; Assert.IsNotNull(value3); Assert.AreEqual(2, value3?.Literal); } - // Validates that removed enum values from last contract are not included + // Verifies that the last contract (reconstructed from Roslyn) exposes each enum member's + // explicit numeric value, so back-compat can preserve non-contiguous values (e.g. 100, 200, + // 500) rather than reassigning positional ordinals. + [Test] + public async Task BackCompat_LastContractExposesEnumMemberValues() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(int), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var input = InputFactory.Int32Enum("mockInputEnum", [ + ("OneHundred", 100), + ("TwoHundred", 200), + ("FiveHundred", 500), + ]); + + var enumType = EnumProvider.Create(input); + enumType.EnsureBuilt(); + + var lastContractFields = enumType.LastContractView?.Fields; + Assert.IsNotNull(lastContractFields); + Assert.AreEqual(3, lastContractFields!.Count); + + var values = lastContractFields.Select(f => (f.InitializationValue as LiteralExpression)?.Literal).ToArray(); + CollectionAssert.AreEqual(new object[] { 100, 200, 500 }, values); + } + [Test] - public async Task BackCompat_IntEnumValueRemoved() + public async Task BackCompat_IntEnumNonContiguousValuesPreserved() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(int), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var input = InputFactory.Int32Enum("mockInputEnum", [ + ("OneHundred", 100), + ("TwoHundred", 200), + ("FiveHundred", 500), + ]); + + var enumType = EnumProvider.Create(input); + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + var fields = enumType.Fields; + Assert.AreEqual(3, fields.Count); + Assert.AreEqual("OneHundred", fields[0].Name); + Assert.AreEqual("TwoHundred", fields[1].Name); + Assert.AreEqual("FiveHundred", fields[2].Name); + + // Each member keeps its exact explicit value from the last contract. + Assert.AreEqual(100, (fields[0].InitializationValue as LiteralExpression)?.Literal); + Assert.AreEqual(200, (fields[1].InitializationValue as LiteralExpression)?.Literal); + Assert.AreEqual(500, (fields[2].InitializationValue as LiteralExpression)?.Literal); + } + + // Validates that an integer enum value removed from the current spec is re-added from the + // last contract (preserving its explicit numeric value) to keep the previously shipped API. + [Test] + public async Task BackCompat_IntEnumRemovedValueReadded() { await MockHelpers.LoadMockGeneratorAsync( createCSharpTypeCore: (inputType) => typeof(int), @@ -459,18 +518,211 @@ await MockHelpers.LoadMockGeneratorAsync( enumType.EnsureBuilt(); enumType.ProcessTypeForBackCompatibility(); + var fields = enumType.Fields; + Assert.AreEqual(3, fields.Count); + + // Order preserved from last contract, and the removed "Third" is re-added at its + // original position. + Assert.AreEqual("Recover", fields[0].Name); + Assert.AreEqual("Default", fields[1].Name); + Assert.AreEqual("Third", fields[2].Name); + + // Shared members keep their explicit values from the last contract (Recover = 0, Default = 1). + Assert.AreEqual(0, (fields[0].InitializationValue as LiteralExpression)?.Literal); + Assert.AreEqual(1, (fields[1].InitializationValue as LiteralExpression)?.Literal); + + // The re-added member keeps its explicit numeric value from the last contract. + var readdedValue = fields[2].InitializationValue as LiteralExpression; + Assert.IsNotNull(readdedValue); + Assert.AreEqual(2, readdedValue?.Literal); + } + + // Validates that a value removed from the MIDDLE of an integer enum is re-added at its + // original position, preserving both surrounding order and its explicit numeric value. + [Test] + public async Task BackCompat_IntEnumRemovedMiddleValueReadded() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(int), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Last contract: Alpha = 0, Beta = 1, Gamma = 2. Current input removes the middle "Beta". + var input = InputFactory.Int32Enum("mockInputEnum", [ + ("Alpha", 0), + ("Gamma", 2), + ]); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + var fields = enumType.Fields; + Assert.AreEqual(3, fields.Count); + + // "Beta" is re-added at its original middle position. + Assert.AreEqual("Alpha", fields[0].Name); + Assert.AreEqual("Beta", fields[1].Name); + Assert.AreEqual("Gamma", fields[2].Name); + + // All members keep their explicit last-contract values (Alpha = 0, Beta = 1, Gamma = 2). + Assert.AreEqual(0, (fields[0].InitializationValue as LiteralExpression)?.Literal); + var betaValue = fields[1].InitializationValue as LiteralExpression; + Assert.IsNotNull(betaValue); + Assert.AreEqual(1, betaValue?.Literal); + Assert.AreEqual(2, (fields[2].InitializationValue as LiteralExpression)?.Literal); + } + + // Validates that a value removed from a long-backed integer enum is re-added with its exact + // (potentially > int.MaxValue) numeric value preserved from the last contract. + [Test] + public async Task BackCompat_LongEnumRemovedValueReadded() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(long), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Last contract: Small = 0, Large = 5000000000 (> int.MaxValue). Current removes "Large". + var input = InputFactory.Int64Enum("mockInputEnum", [ + ("Small", 0), + ]); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + var fields = enumType.Fields; Assert.AreEqual(2, fields.Count); - // Order should be preserved from last contract for members that still exist + Assert.AreEqual("Small", fields[0].Name); + Assert.AreEqual("Large", fields[1].Name); + + // The shared member keeps its explicit last-contract value (Small = 0, as a long). + Assert.AreEqual(0L, (fields[0].InitializationValue as LiteralExpression)?.Literal); + + // The re-added member preserves its long value exactly (not truncated to int). + var largeValue = fields[1].InitializationValue as LiteralExpression; + Assert.IsNotNull(largeValue); + Assert.AreEqual(5000000000L, largeValue?.Literal); + Assert.IsInstanceOf(largeValue?.Literal); + } + + // Validates that removed members are NOT re-added for string-backed enums, because the + // serialized wire value is not recoverable from the last contract (only the C# ordinal is). + [Test] + public async Task BackCompat_StringEnumRemovedValueNotReadded() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(string), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Last contract: Recover, Default, Third. Current input removes "Third". + var input = InputFactory.StringEnum("mockInputEnum", [ + ("Default", "default"), + ("Recover", "recover"), + ]); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + var fields = enumType.Fields; + + // "Third" must NOT be re-added for a string-based enum; only the two current members remain. + Assert.AreEqual(2, fields.Count); Assert.AreEqual("Recover", fields[0].Name); Assert.AreEqual("Default", fields[1].Name); + Assert.IsFalse(fields.Any(f => f.Name == "Third")); - // No explicit initialization values - compiler auto-assigns based on order + // String-based enum members never carry an explicit initialization value. Assert.IsNull(fields[0].InitializationValue); Assert.IsNull(fields[1].InitializationValue); } + // Validates that a removed integer enum member is NOT re-added when its removal is accepted + // in the ApiCompat baseline (here recorded as an EnumValuesMustMatch suppression), so the + // generator honors the intentional removal instead of resurrecting it. + [Test] + public async Task BackCompat_IntEnumRemovedValueNotReaddedWhenBaselineAccepts() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(); + + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(int), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + apiCompatBaseline: baseline); + + // Last contract: Recover, Default, Third. Current input removes "Third", but the baseline + // accepts that removal, so it must NOT be re-added. + var input = InputFactory.Int32Enum("mockInputEnum", [ + ("Default", 0), + ("Recover", 1), + ]); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + var fields = enumType.Fields; + Assert.AreEqual(2, fields.Count); + Assert.IsFalse(fields.Any(f => f.Name == "Third")); + Assert.AreEqual("Recover", fields[0].Name); + Assert.AreEqual("Default", fields[1].Name); + + // The surviving shared members keep their explicit last-contract values (Recover = 0, Default = 1). + Assert.AreEqual(0, (fields[0].InitializationValue as LiteralExpression)?.Literal); + Assert.AreEqual(1, (fields[1].InitializationValue as LiteralExpression)?.Literal); + } + + // Validates that when a shared integer enum member's value was intentionally changed and the + // baseline accepts it (recorded as an EnumValuesMustMatch suppression), back-compat honors the + // CURRENT value instead of restoring the old last-contract value. + [Test] + public async Task BackCompat_IntEnumChangedValueHonoredWhenBaselineAccepts() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(); + + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(int), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + apiCompatBaseline: baseline); + + // Last contract: Recover = 0, Default = 5. Current changes Default to 1; the baseline accepts + // the value change, so back-compat must keep the current value (1) rather than restoring 5. + var input = InputFactory.Int32Enum("mockInputEnum", [ + ("Default", 1), + ("Recover", 0), + ]); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + var fields = enumType.Fields; + Assert.AreEqual(2, fields.Count); + + // Order is preserved from the last contract: Recover first, Default second. + Assert.AreEqual("Recover", fields[0].Name); + Assert.AreEqual("Default", fields[1].Name); + + // Recover was unchanged, so it keeps its (identical) value of 0. + Assert.AreEqual(0, (fields[0].InitializationValue as LiteralExpression)?.Literal); + + // Default's accepted value change is honored: the CURRENT value (1) is kept, not the old (5). + var defaultValue = fields[1].InitializationValue as LiteralExpression; + Assert.IsNotNull(defaultValue); + Assert.AreEqual(1, defaultValue?.Literal); + } + // Validates that string enum order is also preserved from last contract [Test] public async Task BackCompat_StringEnumOrderPreserved() diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumChangedValueHonoredWhenBaselineAccepts.txt b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumChangedValueHonoredWhenBaselineAccepts.txt new file mode 100644 index 00000000000..ea63b24a4c6 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumChangedValueHonoredWhenBaselineAccepts.txt @@ -0,0 +1,4 @@ +# The Default member's value was intentionally changed from 5 (contract) to 1 (implementation). +# Accepting the difference in the baseline means back-compat must honor the new value instead of +# restoring the old one. +EnumValuesMustMatch : Enum value 'Sample.Models.MockInputEnum Sample.Models.MockInputEnum.Default' is (System.Int32)1 in the implementation but (System.Int32)5 in the contract. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumChangedValueHonoredWhenBaselineAccepts/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumChangedValueHonoredWhenBaselineAccepts/MockInputEnum.cs new file mode 100644 index 00000000000..cc5d8599434 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumChangedValueHonoredWhenBaselineAccepts/MockInputEnum.cs @@ -0,0 +1,10 @@ +#nullable disable + +namespace Sample.Models +{ + public enum MockInputEnum + { + Recover = 0, + Default = 5, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumNonContiguousValuesPreserved/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumNonContiguousValuesPreserved/MockInputEnum.cs new file mode 100644 index 00000000000..6a1ca3ff51b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumNonContiguousValuesPreserved/MockInputEnum.cs @@ -0,0 +1,11 @@ +#nullable disable + +namespace Sample.Models +{ + public enum MockInputEnum + { + OneHundred = 100, + TwoHundred = 200, + FiveHundred = 500, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedMiddleValueReadded/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedMiddleValueReadded/MockInputEnum.cs new file mode 100644 index 00000000000..4c8303d7c42 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedMiddleValueReadded/MockInputEnum.cs @@ -0,0 +1,11 @@ +#nullable disable + +namespace Sample.Models +{ + public enum MockInputEnum + { + Alpha = 0, + Beta = 1, + Gamma = 2, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueNotReaddedWhenBaselineAccepts.txt b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueNotReaddedWhenBaselineAccepts.txt new file mode 100644 index 00000000000..a49039dcdac --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueNotReaddedWhenBaselineAccepts.txt @@ -0,0 +1,4 @@ +# The Third enum member was intentionally removed during migration; suppress the difference so the +# back-compat system honors the removal instead of re-adding it. ApiCompat reports a removed enum +# member as a MembersMustExist difference. +MembersMustExist : Member 'public Sample.Models.MockInputEnum Sample.Models.MockInputEnum.Third' 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/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumValueRemoved/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueNotReaddedWhenBaselineAccepts/MockInputEnum.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumValueRemoved/MockInputEnum.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueNotReaddedWhenBaselineAccepts/MockInputEnum.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueReadded/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueReadded/MockInputEnum.cs new file mode 100644 index 00000000000..e93e6f85a4a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_IntEnumRemovedValueReadded/MockInputEnum.cs @@ -0,0 +1,11 @@ +#nullable disable + +namespace Sample.Models +{ + public enum MockInputEnum + { + Recover = 0, + Default = 1, + Third = 2, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_LastContractExposesEnumMemberValues/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_LastContractExposesEnumMemberValues/MockInputEnum.cs new file mode 100644 index 00000000000..6a1ca3ff51b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_LastContractExposesEnumMemberValues/MockInputEnum.cs @@ -0,0 +1,11 @@ +#nullable disable + +namespace Sample.Models +{ + public enum MockInputEnum + { + OneHundred = 100, + TwoHundred = 200, + FiveHundred = 500, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_LongEnumRemovedValueReadded/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_LongEnumRemovedValueReadded/MockInputEnum.cs new file mode 100644 index 00000000000..507bb126c3f --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_LongEnumRemovedValueReadded/MockInputEnum.cs @@ -0,0 +1,10 @@ +#nullable disable + +namespace Sample.Models +{ + public enum MockInputEnum : long + { + Small = 0, + Large = 5000000000, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_StringEnumRemovedValueNotReadded/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_StringEnumRemovedValueNotReadded/MockInputEnum.cs new file mode 100644 index 00000000000..e93e6f85a4a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_StringEnumRemovedValueNotReadded/MockInputEnum.cs @@ -0,0 +1,11 @@ +#nullable disable + +namespace Sample.Models +{ + public enum MockInputEnum + { + Recover = 0, + Default = 1, + Third = 2, + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityKeepsModifierOnNonAbstractType/KeepCtorType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityKeepsModifierOnNonAbstractType/KeepCtorType.cs new file mode 100644 index 00000000000..9b45621f6df --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityKeepsModifierOnNonAbstractType/KeepCtorType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public class KeepCtorType + { + public KeepCtorType(string baseProp) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityPromotesPrivateProtectedToPublic/PromoteCtorType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityPromotesPrivateProtectedToPublic/PromoteCtorType.cs new file mode 100644 index 00000000000..180db065877 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildConstructorsForBackCompatibilityPromotesPrivateProtectedToPublic/PromoteCtorType.cs @@ -0,0 +1,9 @@ +namespace Test +{ + public abstract class PromoteCtorType + { + public PromoteCtorType(string baseProp) + { + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildDeclarationModifiersPreservesNonAbstractFromLastContract/NonAbstractPreservedType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildDeclarationModifiersPreservesNonAbstractFromLastContract/NonAbstractPreservedType.cs new file mode 100644 index 00000000000..688e1888072 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildDeclarationModifiersPreservesNonAbstractFromLastContract/NonAbstractPreservedType.cs @@ -0,0 +1,6 @@ +namespace Test +{ + public class NonAbstractPreservedType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsOverloadForNewOptionalParameter.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsOverloadForNewOptionalParameter.cs new file mode 100644 index 00000000000..27ada66502b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsOverloadForNewOptionalParameter.cs @@ -0,0 +1,22 @@ +// + +#nullable disable + +using System.ComponentModel; + +namespace Test +{ + public partial class NewOptionalParamType + { + public string GetData(int param1, bool param2 = default) + { + return null; + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public string GetData(int param1) + { + return this.GetData(param1: param1, param2: default); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsOverloadForNewOptionalParameter/NewOptionalParamType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsOverloadForNewOptionalParameter/NewOptionalParamType.cs new file mode 100644 index 00000000000..69e7f0621e2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsOverloadForNewOptionalParameter/NewOptionalParamType.cs @@ -0,0 +1,7 @@ +namespace Test +{ + public class NewOptionalParamType + { + public string GetData(int param1) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsStaticOverloadForNewOptionalParameter.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsStaticOverloadForNewOptionalParameter.cs new file mode 100644 index 00000000000..4c768fd2b46 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsStaticOverloadForNewOptionalParameter.cs @@ -0,0 +1,22 @@ +// + +#nullable disable + +using System.ComponentModel; + +namespace Test +{ + public partial class StaticOverloadType + { + public static string GetData(int param1, bool param2 = default) + { + return null; + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public static string GetData(int param1) + { + return global::Test.StaticOverloadType.GetData(param1: param1, param2: default); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsStaticOverloadForNewOptionalParameter/StaticOverloadType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsStaticOverloadForNewOptionalParameter/StaticOverloadType.cs new file mode 100644 index 00000000000..514ea6bdb9c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsStaticOverloadForNewOptionalParameter/StaticOverloadType.cs @@ -0,0 +1,7 @@ +namespace Test +{ + public class StaticOverloadType + { + public static string GetData(int param1) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsVoidOverloadForNewOptionalParameter.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsVoidOverloadForNewOptionalParameter.cs new file mode 100644 index 00000000000..027f0f2b3cd --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsVoidOverloadForNewOptionalParameter.cs @@ -0,0 +1,21 @@ +// + +#nullable disable + +using System.ComponentModel; + +namespace Test +{ + public partial class VoidOverloadType + { + public void DoWork(int param1, bool param2 = default) + { + } + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public void DoWork(int param1) + { + this.DoWork(param1: param1, param2: default); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsVoidOverloadForNewOptionalParameter/VoidOverloadType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsVoidOverloadForNewOptionalParameter/VoidOverloadType.cs new file mode 100644 index 00000000000..e4890ff568f --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityAddsVoidOverloadForNewOptionalParameter/VoidOverloadType.cs @@ -0,0 +1,7 @@ +namespace Test +{ + public class VoidOverloadType + { + public void DoWork(int param1) { } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName.cs new file mode 100644 index 00000000000..a1fadf5dd08 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName.cs @@ -0,0 +1,18 @@ +// + +#nullable disable + +using Sample; + +namespace Test +{ + public partial class TestClient + { + public string Foo(string brandNewParam) + { + global::Sample.Argument.AssertNotNullOrEmpty(brandNewParam, nameof(brandNewParam)); + + return brandNewParam; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs new file mode 100644 index 00000000000..afe96ffaaf7 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName/TestClient.cs @@ -0,0 +1,10 @@ +namespace Test +{ + /// + /// Previously-published contract that does not contain the "brandNewParam" parameter. + /// + public class TestClient + { + public string Foo(string oldParam) { return null; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoredNameDoesNotSplitDeclarations.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoredNameDoesNotSplitDeclarations.cs new file mode 100644 index 00000000000..c611ba3d45d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoredNameDoesNotSplitDeclarations.cs @@ -0,0 +1,19 @@ +// + +#nullable disable + +using Sample; + +namespace Test +{ + public partial class TestClient + { + public string Foo(string oldParam) + { + global::Sample.Argument.AssertNotNullOrEmpty(oldParam, nameof(oldParam)); + + this.Validate(oldParam); + return oldParam; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoredNameDoesNotSplitDeclarations/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoredNameDoesNotSplitDeclarations/TestClient.cs new file mode 100644 index 00000000000..c413df7e7f8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoredNameDoesNotSplitDeclarations/TestClient.cs @@ -0,0 +1,10 @@ +namespace Test +{ + /// + /// Previously-published contract: the parameter was published as "oldParam". + /// + public class TestClient + { + public string Foo(string oldParam) { return null; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName.cs new file mode 100644 index 00000000000..c611ba3d45d --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName.cs @@ -0,0 +1,19 @@ +// + +#nullable disable + +using Sample; + +namespace Test +{ + public partial class TestClient + { + public string Foo(string oldParam) + { + global::Sample.Argument.AssertNotNullOrEmpty(oldParam, nameof(oldParam)); + + this.Validate(oldParam); + return oldParam; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName/TestClient.cs new file mode 100644 index 00000000000..c413df7e7f8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilityRestoresPreviousParameterName/TestClient.cs @@ -0,0 +1,10 @@ +namespace Test +{ + /// + /// Previously-published contract: the parameter was published as "oldParam". + /// + public class TestClient + { + public string Foo(string oldParam) { return null; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsOverloadForBaselineAcceptedRemoval.txt b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsOverloadForBaselineAcceptedRemoval.txt new file mode 100644 index 00000000000..ec668690582 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsOverloadForBaselineAcceptedRemoval.txt @@ -0,0 +1 @@ +MembersMustExist : Member 'public System.String Test.SkipOverloadType.GetData(System.Int32)' 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/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsOverloadForBaselineAcceptedRemoval/SkipOverloadType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsOverloadForBaselineAcceptedRemoval/SkipOverloadType.cs new file mode 100644 index 00000000000..1e1b8d634c4 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsOverloadForBaselineAcceptedRemoval/SkipOverloadType.cs @@ -0,0 +1,7 @@ +namespace Test +{ + public class SkipOverloadType + { + public string GetData(int param1) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline.cs new file mode 100644 index 00000000000..c0647c0cd30 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +namespace Test +{ + public partial class SkipReorderType + { + public string Foo(int second, string first) + { + return null; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline.txt b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline.txt new file mode 100644 index 00000000000..7e78b1188d0 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline.txt @@ -0,0 +1 @@ +MembersMustExist : Member 'public System.String Test.SkipReorderType.Foo(System.String, System.Int32)' 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/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline/SkipReorderType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline/SkipReorderType.cs new file mode 100644 index 00000000000..0374f273cfb --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline/SkipReorderType.cs @@ -0,0 +1,7 @@ +namespace Test +{ + public class SkipReorderType + { + public string Foo(string first, int second) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat.cs new file mode 100644 index 00000000000..a4c2bd4cf62 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +namespace Test +{ + public partial class CustomReorderType + { + public string Foo(string first, int second) + { + return null; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat/CustomReorderType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat/CustomReorderType.cs new file mode 100644 index 00000000000..0c0a8860a69 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderInheritsParameterReorderBackCompat/CustomReorderType.cs @@ -0,0 +1,7 @@ +namespace Test +{ + public class CustomReorderType + { + public string Foo(string first, int second) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults.cs new file mode 100644 index 00000000000..f41eb686ec3 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +namespace Test +{ + public partial class CustomReorderValueTypeDefaultsType + { + public string Foo(int count = 0, bool flag = false) + { + return null; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults/CustomReorderValueTypeDefaultsType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults/CustomReorderValueTypeDefaultsType.cs new file mode 100644 index 00000000000..d45afd10149 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/CustomTypeProviderReorderPreservesValueTypeDefaults/CustomReorderValueTypeDefaultsType.cs @@ -0,0 +1,13 @@ +namespace Test +{ + /// + /// Previous contract whose Foo method declares value-type optional parameters with literal + /// defaults (count = 0, flag = false) in the order (count, flag). The current generation + /// declares them in a different order with `default`-keyword defaults; restoring the order must + /// also preserve the previously published literal default representation. + /// + public class CustomReorderValueTypeDefaultsType + { + public string Foo(int count = 0, bool flag = false) => null; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/FindPreviousParameterNameLooksUpLastContract/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/FindPreviousParameterNameLooksUpLastContract/TestClient.cs new file mode 100644 index 00000000000..2c11ceaee1e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/FindPreviousParameterNameLooksUpLastContract/TestClient.cs @@ -0,0 +1,15 @@ +namespace Test +{ + /// + /// Represents the previously-published contract for a type whose parameters may have since + /// been renamed by the generator. + /// + public class TestClient + { + public void Foo(string oldParam) { } + + public void BarAsync(string oldAsyncParam) { } + + public void Other(string otherParam) { } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 9ec0b21c32d..5c2d2a087d8 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs @@ -12,6 +12,7 @@ using Microsoft.TypeSpec.Generator.Snippets; using Microsoft.TypeSpec.Generator.Statements; using Microsoft.TypeSpec.Generator.Tests.Common; +using Microsoft.TypeSpec.Generator.Utilities; using NUnit.Framework; namespace Microsoft.TypeSpec.Generator.Tests.Providers @@ -57,6 +58,505 @@ public async Task TestLoadLastContractView() Assert.AreEqual("p1", signature.Parameters[0].Name); } + // Validates that a custom TypeProvider inherits the parameter-reordering back-compat behavior + // from the base TypeProvider. + [Test] + public async Task CustomTypeProviderInheritsParameterReorderBackCompat() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Current generation declares Foo(second, first) — the reverse of the last contract's + // Foo(first, second). + var first = new ParameterProvider("first", $"", new CSharpType(typeof(string))); + var second = new ParameterProvider("second", $"", new CSharpType(typeof(int))); + var currentFoo = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [second, first]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "CustomReorderType", ns: "Test", methods: [currentFoo]); + + // The custom type does not override BuildMethodsForBackCompatibility; the base default + // restores the previous parameter order in place. + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // Validates that when restoring a previous parameter order, the previously published default + // value representation is preserved (e.g. a value-type parameter keeps its literal `= 0` + // rather than flipping to `= default`). + [Test] + public async Task CustomTypeProviderReorderPreservesValueTypeDefaults() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Current generation declares Foo(flag, count) with `default`-keyword defaults — the + // reverse of the last contract's Foo(int count = 0, bool flag = false). + var flag = new ParameterProvider("flag", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default); + var count = new ParameterProvider("count", $"", new CSharpType(typeof(int)), defaultValue: Snippet.Default); + var currentFoo = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [flag, count]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "CustomReorderValueTypeDefaultsType", ns: "Test", methods: [currentFoo]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // Validates that the base BuildMethodsForBackCompatibility default does NOT restore the + // previous parameter order when that removal has been accepted in the ApiCompat baseline. + [Test] + public async Task BuildMethodsForBackCompatibilitySkipsReorderAcceptedInBaseline() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(); + + await MockHelpers.LoadMockGeneratorAsync( + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + apiCompatBaseline: baseline); + + // Current generation declares Foo(second, first) — the reverse of the last contract's + // Foo(first, second) — but the reorder is an accepted removal in the ApiCompat baseline, + // so the base default must leave the current order untouched. + var first = new ParameterProvider("first", $"", new CSharpType(typeof(string))); + var second = new ParameterProvider("second", $"", new CSharpType(typeof(int))); + var currentFoo = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [second, first]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "SkipReorderType", ns: "Test", methods: [currentFoo]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // Validates that the base TypeProvider generalizes the non-abstract base model back-compat to + // any TypeProvider: a type the current generation would declare abstract is kept non-abstract + // when the last contract published it as a non-abstract class. + [Test] + public async Task BuildDeclarationModifiersPreservesNonAbstractFromLastContract() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var typeProvider = new TestTypeProvider( + name: "NonAbstractPreservedType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Abstract | TypeSignatureModifiers.Class); + + Assert.IsFalse( + typeProvider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract), + "Expected the abstract modifier to be removed to match the non-abstract last contract."); + } + + // Validates the negative case: without a last contract, a type the current generation declares + // abstract stays abstract (the preservation only applies against a non-abstract last contract). + [Test] + public void BuildDeclarationModifiersKeepsAbstractWithoutLastContract() + { + var typeProvider = new TestTypeProvider( + name: "AbstractNoContractType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Abstract | TypeSignatureModifiers.Class); + + Assert.IsTrue(typeProvider.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Abstract)); + } + + // Validates that the base TypeProvider generalizes the model-constructor back-compat to any + // abstract TypeProvider: a private-protected constructor is promoted to public when the last + // contract published a matching public constructor. + [Test] + public async Task BuildConstructorsForBackCompatibilityPromotesPrivateProtectedToPublic() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var baseProp = new ParameterProvider("baseProp", $"", new CSharpType(typeof(string))); + var currentConstructor = new ConstructorProvider( + new ConstructorSignature( + new CSharpType(typeof(object)), + $"", + MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected, + [baseProp]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider( + name: "PromoteCtorType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Abstract | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var promoted = typeProvider.Constructors.Single(); + Assert.IsTrue(promoted.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsFalse(promoted.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private)); + Assert.IsFalse(promoted.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)); + } + + // Validates the negative case: a non-abstract type does not get its private-protected + // constructor promoted, even with a matching public constructor in the last contract. + [Test] + public async Task BuildConstructorsForBackCompatibilityKeepsModifierOnNonAbstractType() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var baseProp = new ParameterProvider("baseProp", $"", new CSharpType(typeof(string))); + var currentConstructor = new ConstructorProvider( + new ConstructorSignature( + new CSharpType(typeof(object)), + $"", + MethodSignatureModifiers.Private | MethodSignatureModifiers.Protected, + [baseProp]), + Snippet.ThrowExpression(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider( + name: "KeepCtorType", + ns: "Test", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + constructors: [currentConstructor]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var constructor = typeProvider.Constructors.Single(); + Assert.IsTrue(constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Private)); + Assert.IsTrue(constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Protected)); + Assert.IsFalse(constructor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + } + + // Validates that the base TypeProvider generalizes the new-optional-parameter back-compat to any + // TypeProvider: a public method that gained an optional non-body parameter relative to the last + // contract gets a hidden overload matching the previous signature that delegates to the current one. + [Test] + public async Task BuildMethodsForBackCompatibilityAddsOverloadForNewOptionalParameter() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // The last contract published GetData(int param1); the current generation adds an optional + // non-body parameter param2. + var param1 = new ParameterProvider("param1", $"", new CSharpType(typeof(int))); + var param2 = new ParameterProvider("param2", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default, location: ParameterLocation.Query); + var getData = new MethodProvider( + new MethodSignature("GetData", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [param1, param2]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "NewOptionalParamType", ns: "Test", methods: [getData]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // TypeProvider: a STATIC method that gained an optional non-body parameter gets a hidden overload + // whose body delegates through the declaring type (not `this`), since a static method cannot use + // an instance receiver. + [Test] + public async Task BuildMethodsForBackCompatibilityAddsStaticOverloadForNewOptionalParameter() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // The last contract published static GetData(int param1); the current generation adds an + // optional non-body parameter param2. + var param1 = new ParameterProvider("param1", $"", new CSharpType(typeof(int))); + var param2 = new ParameterProvider("param2", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default, location: ParameterLocation.Query); + var getData = new MethodProvider( + new MethodSignature("GetData", $"", MethodSignatureModifiers.Public | MethodSignatureModifiers.Static, new CSharpType(typeof(string)), $"", [param1, param2]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "StaticOverloadType", ns: "Test", methods: [getData]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // TypeProvider: a VOID method that gained an optional non-body parameter gets a hidden overload + // whose body invokes the current method as a statement (no `return`), since a void method has no + // value to return. + [Test] + public async Task BuildMethodsForBackCompatibilityAddsVoidOverloadForNewOptionalParameter() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // The last contract published void DoWork(int param1); the current generation adds an + // optional non-body parameter param2. + var param1 = new ParameterProvider("param1", $"", new CSharpType(typeof(int))); + var param2 = new ParameterProvider("param2", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default, location: ParameterLocation.Query); + var doWork = new MethodProvider( + new MethodSignature("DoWork", $"", MethodSignatureModifiers.Public, null, $"", [param1, param2]), + MethodBodyStatement.Empty, + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "VoidOverloadType", ns: "Test", methods: [doWork]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // TypeProvider: when a previous signature's removal is accepted in the ApiCompat baseline, the + // optional-parameter overload pass must NOT resurrect it even though the replacement method added + // optional parameters. + [Test] + public async Task BuildMethodsForBackCompatibilitySkipsOverloadForBaselineAcceptedRemoval() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(); + + await MockHelpers.LoadMockGeneratorAsync( + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + apiCompatBaseline: baseline); + + // The last contract published GetData(int param1); the current generation adds an optional + // param2. The baseline accepts the removal of GetData(int), so no overload must be added. + var param1 = new ParameterProvider("param1", $"", new CSharpType(typeof(int))); + var param2 = new ParameterProvider("param2", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default, location: ParameterLocation.Query); + var getData = new MethodProvider( + new MethodSignature("GetData", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [param1, param2]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "SkipOverloadType", ns: "Test", methods: [getData]); + + typeProvider.ProcessTypeForBackCompatibility(); + + // Only the current method remains; the accepted removal is not resurrected as an overload. + Assert.AreEqual(1, typeProvider.Methods.Count); + Assert.AreEqual(2, typeProvider.Methods[0].Signature.Parameters.Count); + } + + // Validates the shared lookup that any provider can use to restore a previously-published + // parameter name from its last contract. + [Test] + public async Task FindPreviousParameterNameLooksUpLastContract() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var lastContractView = new TestTypeProvider(name: "TestClient").LastContractView; + + // The last contract published Foo(string oldParam); scoped to that method it is found. + Assert.AreEqual("oldParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldParam", "Foo")); + + // The exact casing from the contract is returned even when the lookup name differs only in casing. + Assert.AreEqual("oldParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldparam", "Foo")); + + // The lookup is scoped: "oldParam" is not published on Other. + Assert.IsNull(BackCompatHelper.FindPreviousParameterName(lastContractView, "oldParam", "Other")); + + // An unscoped lookup finds the parameter on any last-contract method. + Assert.AreEqual("oldParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldParam")); + + // A parameter that does not exist in the last contract returns null. + Assert.IsNull(BackCompatHelper.FindPreviousParameterName(lastContractView, "missing", "Foo")); + + // A sync method name matches its async counterpart (BarAsync) in the contract. + Assert.AreEqual("oldAsyncParam", BackCompatHelper.FindPreviousParameterName(lastContractView, "oldAsyncParam", "Bar")); + } + + // Validates that the lookup returns null when there is no last contract. + [Test] + public void FindPreviousParameterNameReturnsNullWithoutLastContract() + { + var typeProvider = new TestTypeProvider(name: "TestClient"); + Assert.IsNull(typeProvider.LastContractView); + Assert.IsNull(BackCompatHelper.FindPreviousParameterName(typeProvider.LastContractView, "oldParam", "Foo")); + } + + [Test] + public async Task TryRestorePreviousParameterOrderMatchesNonCanonicalParameterNames() + { + await MockHelpers.LoadMockGeneratorAsync(); + + // Current generation declares Foo(second_param, first_param) — the reverse of the + // previous Foo(first_param, second_param). The raw names are snake_case, so matching + // requires normalizing both sides via ToVariableName. + var firstParam = new ParameterProvider("first_param", $"", new CSharpType(typeof(string))); + var secondParam = new ParameterProvider("second_param", $"", new CSharpType(typeof(int))); + var currentFoo = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [secondParam, firstParam]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + var previousSignature = new MethodSignature( + "Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", + [ + new ParameterProvider("first_param", $"", new CSharpType(typeof(string))), + new ParameterProvider("second_param", $"", new CSharpType(typeof(int))), + ]); + + var reordered = BackCompatHelper.TryRestorePreviousParameterOrder(currentFoo, previousSignature); + + Assert.IsTrue(reordered); + CollectionAssert.AreEqual( + new[] { "first_param", "second_param" }, + currentFoo.Signature.Parameters.Select(p => p.Name).ToArray()); + } + + // Validates that the default-value preservation performed while reordering also matches by + // the normalized parameter identifier, restoring the previously-published default onto the + // current parameter even when the raw name is snake_case. + [Test] + public async Task TryRestorePreviousParameterOrderPreservesPreviousDefaultForNonCanonicalNames() + { + await MockHelpers.LoadMockGeneratorAsync(); + + // Current declares Foo(second_param = default, first_param) — reverse of previous order. + var firstParam = new ParameterProvider("first_param", $"", new CSharpType(typeof(string))); + var secondParam = new ParameterProvider("second_param", $"", new CSharpType(typeof(int)), defaultValue: Snippet.Default); + var currentFoo = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [secondParam, firstParam]), + Snippet.Return(Snippet.Null), + new TestTypeProvider()); + + // The previously-published second_param carried a distinct default representation. + var previousDefault = Snippet.Literal(0); + var previousSignature = new MethodSignature( + "Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", + [ + new ParameterProvider("first_param", $"", new CSharpType(typeof(string))), + new ParameterProvider("second_param", $"", new CSharpType(typeof(int)), defaultValue: previousDefault), + ]); + + var reordered = BackCompatHelper.TryRestorePreviousParameterOrder(currentFoo, previousSignature); + + Assert.IsTrue(reordered); + CollectionAssert.AreEqual( + new[] { "first_param", "second_param" }, + currentFoo.Signature.Parameters.Select(p => p.Name).ToArray()); + + // The current second_param's default was restored from the previous contract. + var restoredSecond = currentFoo.Signature.Parameters.Single(p => p.Name == "second_param"); + Assert.AreSame(previousDefault, restoredSecond.DefaultValue); + } + + // Validates that a new optional parameter with an Unknown location is not treated as a + // non-body parameter, so no back-compat overload is produced for it. + [Test] + public void HasNewOptionalNonBodyParametersOnlyRejectsUnknownLocation() + { + var previousSignature = new MethodSignature( + "GetData", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", + [new ParameterProvider("param1", $"", new CSharpType(typeof(int)))]); + + // A new optional parameter whose location is Unknown must not qualify. + var unknownLocationSignature = new MethodSignature( + "GetData", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", + [ + new ParameterProvider("param1", $"", new CSharpType(typeof(int))), + new ParameterProvider("param2", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default, location: ParameterLocation.Unknown), + ]); + + Assert.IsFalse(BackCompatHelper.HasNewOptionalNonBodyParametersOnly(previousSignature, unknownLocationSignature)); + + // The same shape with an explicit non-body location does qualify. + var queryLocationSignature = new MethodSignature( + "GetData", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", + [ + new ParameterProvider("param1", $"", new CSharpType(typeof(int))), + new ParameterProvider("param2", $"", new CSharpType(typeof(bool)), defaultValue: Snippet.Default, location: ParameterLocation.Query), + ]); + + Assert.IsTrue(BackCompatHelper.HasNewOptionalNonBodyParametersOnly(previousSignature, queryLocationSignature)); + } + + // Validates that the base BuildMethodsForBackCompatibility default automatically restores a + // previously-published parameter name on any derived type — keyed on the parameter's spec + // original name — and that the rename propagates into the already-built method body. + [Test] + public async Task BuildMethodsForBackCompatibilityRestoresPreviousParameterName() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // A spec parameter "oldParam" that the generator renamed to "newParam". + var inputParameter = InputFactory.QueryParameter("oldParam", InputPrimitiveType.String, isRequired: true); + inputParameter.Update(name: "newParam"); + + var parameter = new ParameterProvider(inputParameter); + var fooMethod = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [parameter]), + new MethodBodyStatement[] + { + // Passing the parameter as an argument to another method exercises propagation + // of the restored name into invocation arguments, not just the return. + Snippet.This.Invoke("Validate", parameter).Terminate(), + Snippet.Return(parameter), + }, + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "TestClient", methods: [fooMethod]); + + // The default has no override here; the base restores the previously-published name and + // the rename propagates into the already-built body. + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // Validates that a parameter whose spec name is not in the last contract keeps its current name. + [Test] + public async Task BuildMethodsForBackCompatibilityKeepsUnpublishedParameterName() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // "brandNewParam" has no counterpart in the last contract (TestClient.Foo(oldParam)). + var inputParameter = InputFactory.QueryParameter("brandNewParam", InputPrimitiveType.String, isRequired: true); + var parameter = new ParameterProvider(inputParameter); + var fooMethod = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [parameter]), + Snippet.Return(parameter), + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "TestClient", methods: [fooMethod]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + // When the restored name is used both as an argument (AsArgument -> _asArgument) and + // as a variable (-> _asVariable), materializing both cached expressions before the rename, the + // rename must keep them sharing one declaration. Otherwise the writer renames the two + // declarations of the same name to "oldParam0"/"oldParam1", producing non-compiling code. + [Test] + public async Task BuildMethodsForBackCompatibilityRestoredNameDoesNotSplitDeclarations() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var inputParameter = InputFactory.QueryParameter("oldParam", InputPrimitiveType.String, isRequired: true); + inputParameter.Update(name: "newParam"); + + var parameter = new ParameterProvider(inputParameter); + var fooMethod = new MethodProvider( + new MethodSignature("Foo", $"", MethodSignatureModifiers.Public, new CSharpType(typeof(string)), $"", [parameter]), + new MethodBodyStatement[] + { + Snippet.This.Invoke("Validate", parameter.AsArgument()).Terminate(), + Snippet.Return(parameter), + }, + new TestTypeProvider()); + + var typeProvider = new TestTypeProvider(name: "TestClient", methods: [fooMethod]); + + typeProvider.ProcessTypeForBackCompatibility(); + + var actual = new TypeProviderWriter(typeProvider).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), actual); + } + + [Test] public async Task LastContractViewLoadedForRenamedType() { 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 b50f507ce00..a524a955e4c 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 @@ -92,6 +92,26 @@ public void CountsGenericParametersAsSingleArgument() Assert.IsTrue(baseline.IsMemberSuppressed("Ns.Foo", "Configure", 2)); } + [Test] + public void ParsesMembersMustExistField() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(method: "Baseline"); + + // A removed field/enum member has no parameter list; it is recorded with arity 0. + Assert.IsTrue(baseline.IsMemberSuppressed("Ns.Foo", "LegacyField", 0)); + } + + [Test] + public void ParsesEnumValuesMustMatch() + { + var baseline = Helpers.GetApiCompatBaselineFromFile(method: "Baseline"); + + // An accepted enum value difference suppresses back-compat handling for that member. + Assert.IsTrue(baseline.IsMemberSuppressed("Ns.CapacityLevel", "FiftyThousand", 0)); + // A different member on the same enum must not match. + Assert.IsFalse(baseline.IsMemberSuppressed("Ns.CapacityLevel", "OneHundred", 0)); + } + [Test] public void IgnoresUnknownRulesAndMalformedLines() { 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 f2e71a155c5..52fd15fee3d 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 @@ -6,3 +6,5 @@ MembersMustExist : Member 'public Ns.Foo..ctor(Ns.Kind, System.String)' does not 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. +MembersMustExist : Member 'public Ns.Kind Ns.Foo.LegacyField' does not exist in the implementation but it does exist in the contract. +EnumValuesMustMatch : Enum value 'Ns.CapacityLevel Ns.CapacityLevel.FiftyThousand' is (System.Int32)10 in the implementation but (System.Int32)50000 in the contract. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/TestHelpers/TestTypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/TestHelpers/TestTypeProvider.cs index e142850b002..4b339e7bff4 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/TestHelpers/TestTypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/TestHelpers/TestTypeProvider.cs @@ -13,6 +13,7 @@ internal class TestTypeProvider : TypeProvider private readonly TypeSignatureModifiers? _declarationModifiers; private readonly MethodProvider[] _methods; private readonly PropertyProvider[] _properties; + private readonly ConstructorProvider[] _constructors; private readonly string _name; private readonly string _namespace; protected override string BuildRelativeFilePath() => $"{Name}.cs"; @@ -24,6 +25,8 @@ internal class TestTypeProvider : TypeProvider protected internal override PropertyProvider[] BuildProperties() => _properties; protected internal override MethodProvider[] BuildMethods() => _methods; + + protected internal override ConstructorProvider[] BuildConstructors() => _constructors; protected override TypeProvider[] BuildNestedTypes() => NestedTypesInternal ?? base.BuildNestedTypes(); public static readonly TypeProvider Empty = new TestTypeProvider(); @@ -33,11 +36,13 @@ internal TestTypeProvider( TypeSignatureModifiers? declarationModifiers = null, IEnumerable? methods = null, IEnumerable? properties = null, - string? ns = null) + string? ns = null, + IEnumerable? constructors = null) { _declarationModifiers = declarationModifiers; _methods = methods?.ToArray() ?? []; _properties = properties?.ToArray() ?? []; + _constructors = constructors?.ToArray() ?? []; _name = name ?? "TestName"; _namespace = ns ?? "Test"; } diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index 75a6fe512ee..6ca83258649 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -12,6 +12,10 @@ - [New Property Added Together with a Rename](#scenario-new-property-added-together-with-a-rename) - [Model Properties](#model-properties) - [AdditionalProperties Type Preservation](#additionalproperties-type-preservation) + - [Fixed Enum Members](#fixed-enum-members) + - [Explicit (Non-contiguous) Values Preserved](#scenario-explicit-non-contiguous-values-preserved) + - [Removed Integer Enum Member Re-added](#scenario-removed-integer-enum-member-re-added) + - [Baseline-Accepted Removal Honored](#scenario-baseline-accepted-removal-honored) - [API Version Enum](#api-version-enum) - [Non-abstract Base Models](#non-abstract-base-models) - [Model Constructors](#model-constructors) @@ -42,13 +46,14 @@ When generating code, the generator can optionally receive a compiled assembly f Sometimes a breaking change (such as removing a model or a model factory method) is intentional and has already been reviewed and accepted. In the Azure SDK, such accepted breaking changes are recorded in an [ApiCompat](https://github.com/dotnet/sdk/tree/main/src/Compatibility) baseline (suppression) file, conventionally located at `eng/apicompatbaselines/.txt`. -Without awareness of these files, the back-compat system would resurrect every member present in the last contract — re-introducing the very API that was intentionally removed (and potentially referencing types that no longer exist). To prevent this, the generator discovers the baseline file by walking up from the project directory and parses its `TypesMustExist` and `MembersMustExist` suppressions. The back-compat code then consults the baseline in two ways: +Without awareness of these files, the back-compat system would resurrect every member present in the last contract — re-introducing the very API that was intentionally removed (and potentially referencing types that no longer exist). To prevent this, the generator discovers the baseline file by walking up from the project directory and parses its `TypesMustExist`, `MembersMustExist`, and `EnumValuesMustMatch` suppressions. The back-compat code then consults the baseline in the following ways: - **Skipping resurrected members:** before regenerating a compatibility shim for a removed member, it checks whether the removal has been accepted in the baseline (matched by declaring-type full name, member name, and parameter count) and, if so, skips it. - **Allowing property type changes:** the back-compat system normally preserves a property's previous (last-contract) type to avoid a breaking change. When that previous type — or a type nested within it (e.g. the element type of a collection) — has itself been intentionally removed and accepted in the baseline, preserving it would reference a now-deleted type. In that case the generator allows the property to take its current (spec) type instead. +- **Skipping re-added enum members:** before re-adding a fixed enum member that exists in the last contract but was dropped from the current spec, it checks whether that removal — or an accepted value difference recorded as `EnumValuesMustMatch` — has been accepted in the baseline (matched by the declaring enum's fully-qualified name and the member name) and, if so, omits the member. > [!NOTE] -> Baseline awareness is currently wired into the model factory back-compat path (`ModelFactoryProvider`) and the model property type-preservation path (`ModelProvider`). The other back-compat consumers (`ModelProvider` constructors, the enum providers, `ClientProvider`, and `RestClientProvider`) can be made baseline-aware in the same way as a follow-up. +> Baseline awareness is currently wired into the base method back-compat path in `TypeProvider` (the parameter reorder/removal loop, inherited by `ClientProvider` and also used directly by `ModelFactoryProvider`), the model property type-preservation path (`ModelProvider`), and the fixed enum member path (`FixedEnumProvider`). The remaining back-compat consumers (`ModelProvider` constructors and `RestClientProvider`) can be made baseline-aware in the same way as a follow-up. ## Supported Scenarios @@ -363,6 +368,85 @@ public partial class MyModel - For object types, serialization uses `Utf8JsonWriter.WriteObjectValue()` to handle arbitrary values - Binary compatibility is fully maintained - existing client code continues to work without recompilation +### Fixed Enum Members + +Fixed enums (C# `enum` types) preserve their previously shipped members and values by comparing the current spec against the last contract. Because the underlying value of an integer-backed enum member is part of its public API, the generator keeps those values stable across regenerations, aligns member order to the last contract, and re-adds members that were dropped from the current spec. + +> [!NOTE] +> These behaviors apply to **integer-backed** fixed enums (`int`/`long`). + +#### Scenario: Explicit (Non-contiguous) Values Preserved + +**Description:** When an integer enum's members carry explicit, non-contiguous values, the generator preserves each member's exact value from the last contract instead of reassigning positional ordinals. + +**Example:** + +Previous version (GA surface): + +```csharp +public enum CapacityReservationLevel +{ + OneHundred = 100, + TwoHundred = 200, + FiveHundred = 500, +} +``` + +**Generated Result:** The members keep their exact values (`100`, `200`, `500`) rather than being reassigned to `0`, `1`, `2`. + +#### Scenario: Removed Integer Enum Member Re-added + +**Description:** When an integer enum member present in the last contract is dropped from the current spec, the generator re-adds it — at its original position and with its original explicit value — to avoid removing a previously shipped member. + +**Example:** + +Previous version: + +```csharp +public enum SampleEnum +{ + Alpha = 0, + Beta = 1, + Gamma = 2, +} +``` + +Current TypeSpec removes `Beta`: + +```csharp +public enum SampleEnum +{ + Alpha = 0, + Gamma = 2, +} +``` + +**Generated Result:** `Beta` is re-added at its original position with its original value: + +```csharp +public enum SampleEnum +{ + Alpha = 0, + Beta = 1, + Gamma = 2, +} +``` + +**Key Points:** + +- Members shared between the current spec and the last contract are ordered to match the last contract and keep their last-contract values +- New members introduced by the current spec are appended after the last-contract members, keeping their spec values +- The re-added member's underlying value is read from the previously published assembly's metadata (no debug symbols required) + +#### Scenario: Baseline-Accepted Removal Honored + +**Description:** When the removal of an enum member has been intentionally accepted in the [ApiCompat baseline](#apicompat-baseline-awareness) — recorded as a `MembersMustExist` removal or an `EnumValuesMustMatch` value-difference suppression — the generator honors that decision and does **not** re-add the member. + +**Key Points:** + +- Suppressed members are matched by the declaring enum's fully-qualified name and the member name +- This lets a library intentionally drop a previously shipped enum member once the removal is reviewed and recorded in the baseline + ### API Version Enum Service version enums maintain backward compatibility by preserving version values from previous releases.