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 a026a17b325..c7de0edd6be 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 @@ -38,6 +38,9 @@ protected override string BuildRelativeFilePath() protected override IReadOnlyList BuildMethodsForBackCompatibility(IEnumerable originalMethods) => [.. originalMethods]; + protected override IReadOnlyList? BuildEnumValuesForBackCompatibility(IReadOnlyList originalEnumValues) + => base.BuildEnumValuesForBackCompatibility(originalEnumValues); + 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/ScmModelProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs index 4914233389c..a3f866dcf93 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmModelProvider.cs @@ -661,6 +661,7 @@ private bool BuildNeedsBackCompatAdditionalProperties() } bool needsBackCompat = LastContractView.Properties.Any(p => + MethodSignatureHelper.IsPublicApi(p.Modifiers) && p.Name == AdditionalPropertiesHelper.DefaultAdditionalPropertiesPropertyName); if (needsBackCompat) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs index 1239e640469..0914920412c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs @@ -7,8 +7,8 @@ using System.Reflection; using System.Threading.Tasks; using Microsoft.CodeAnalysis; -using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.ClientModel.Providers; +using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Input.Extensions; using Microsoft.TypeSpec.Generator.Primitives; @@ -697,6 +697,41 @@ public async Task ParameterNamePreservedFromLastContractView() "When 'oldParam' is preserved, the renamed 'newParam' must not appear."); } + [Test] + public async Task ParameterNamePreservedFromInternalLastContractMethod() + { + var queryParam = InputFactory.QueryParameter("oldParam", InputPrimitiveType.String, isRequired: true); + queryParam.Update(name: "newParam"); + + var operation = InputFactory.Operation("GetSomething", parameters: [queryParam]); + var serviceMethod = InputFactory.BasicServiceMethod("GetSomething", operation); + var client = InputFactory.Client("TestClient", methods: [serviceMethod]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + Assert.IsNotNull(clientProvider!.LastContractView); + + var protocolParams = RestClientProvider.GetMethodParameters(serviceMethod, ScmMethodKind.Protocol, clientProvider!); + + // Preserving a previously-published parameter name only renames an existing parameter (it never + // adds members), so the last contract is respected regardless of the owning method's accessibility. + Assert.IsNotNull( + protocolParams.FirstOrDefault(p => string.Equals(p.Name, "oldParam", StringComparison.Ordinal)), + "Parameter name should be restored from the previously-published name even on internal last-contract methods."); + Assert.IsNull( + protocolParams.FirstOrDefault(p => string.Equals(p.Name, "newParam", StringComparison.Ordinal)), + "When 'oldParam' is preserved, the renamed 'newParam' must not appear."); + + var restClientProvider = new MockClientProvider(client, clientProvider!); + var writer = new TypeProviderWriter(restClientProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(parameters: "Generated"), file.Content); + } + [Test] public void ExactNameMethodParameterPreservedInRestClient() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ParameterNamePreservedFromInternalLastContractMethod(Generated).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ParameterNamePreservedFromInternalLastContractMethod(Generated).cs new file mode 100644 index 00000000000..16994a763ef --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ParameterNamePreservedFromInternalLastContractMethod(Generated).cs @@ -0,0 +1,22 @@ +// + +#nullable disable + +using System.ClientModel.Primitives; + +namespace Sample +{ + public partial class TestClient + { + internal global::System.ClientModel.Primitives.PipelineMessage CreateGetSomethingRequest(string oldParam, global::System.ClientModel.Primitives.RequestOptions options) + { + global::Sample.ClientUriBuilder uri = new global::Sample.ClientUriBuilder(); + uri.Reset(_endpoint); + uri.AppendQuery("oldParam", oldParam, true); + global::System.ClientModel.Primitives.PipelineMessage message = Pipeline.CreateMessage(uri.ToUri(), "GET", PipelineMessageClassifier200); + global::System.ClientModel.Primitives.PipelineRequest request = message.Request; + message.Apply(options); + return message; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ParameterNamePreservedFromInternalLastContractMethod/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ParameterNamePreservedFromInternalLastContractMethod/TestClient.cs new file mode 100644 index 00000000000..b5b5235cd58 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/TestData/RestClientProviderTests/ParameterNamePreservedFromInternalLastContractMethod/TestClient.cs @@ -0,0 +1,14 @@ +#nullable disable + +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading.Tasks; + +namespace Sample +{ + public partial class TestClient + { + internal virtual Task GetSomethingAsync(string oldParam, RequestOptions options = null) { return null; } + internal virtual ClientResult GetSomething(string oldParam, RequestOptions options = null) { return null; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/LiteralExpression.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/LiteralExpression.cs index 0b6f139f1c7..61e86532e1c 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/LiteralExpression.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Expressions/LiteralExpression.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Diagnostics.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; namespace Microsoft.TypeSpec.Generator.Expressions @@ -14,26 +15,43 @@ public sealed record LiteralExpression(object? Literal) : ValueExpression { internal override void Write(CodeWriter writer) { - writer.AppendRaw(Literal switch + writer.AppendRaw(Format(Literal) ?? throw new NotImplementedException()); + } + + /// + /// Creates a when maps to a renderable literal. + /// + internal static bool TryCreate(object? value, [NotNullWhen(true)] out LiteralExpression? literal) + { + if (Format(value) is null) { - null => "null", - string s => SyntaxFactory.Literal(s).ToString(), - int i => SyntaxFactory.Literal(i).ToString(), - uint ui => SyntaxFactory.Literal(ui).ToString(), - long l => SyntaxFactory.Literal(l).ToString(), - ulong ul => SyntaxFactory.Literal(ul).ToString(), - byte b => SyntaxFactory.Literal((int)b).ToString(), - sbyte sb => SyntaxFactory.Literal((int)sb).ToString(), - short s => SyntaxFactory.Literal((int)s).ToString(), - ushort us => SyntaxFactory.Literal((uint)us).ToString(), - decimal d => SyntaxFactory.Literal(d).ToString(), - double d => SyntaxFactory.Literal(d).ToString(), - float f => SyntaxFactory.Literal(f).ToString(), - char c => SyntaxFactory.Literal(c).ToString(), - bool b => b ? "true" : "false", - BinaryData bd => bd.ToArray().Length == 0 ? "new byte[] { }" : SyntaxFactory.Literal(bd.ToString()).ToString(), - _ => throw new NotImplementedException() - }); + literal = null; + return false; + } + + literal = new LiteralExpression(value); + return true; } + + private static string? Format(object? literal) => literal switch + { + null => "null", + string s => SyntaxFactory.Literal(s).ToString(), + int i => SyntaxFactory.Literal(i).ToString(), + uint ui => SyntaxFactory.Literal(ui).ToString(), + long l => SyntaxFactory.Literal(l).ToString(), + ulong ul => SyntaxFactory.Literal(ul).ToString(), + byte b => SyntaxFactory.Literal((int)b).ToString(), + sbyte sb => SyntaxFactory.Literal((int)sb).ToString(), + short s => SyntaxFactory.Literal((int)s).ToString(), + ushort us => SyntaxFactory.Literal((uint)us).ToString(), + decimal d => SyntaxFactory.Literal(d).ToString(), + double d => SyntaxFactory.Literal(d).ToString(), + float f => SyntaxFactory.Literal(f).ToString(), + char c => SyntaxFactory.Literal(c).ToString(), + bool b => b ? "true" : "false", + BinaryData bd => bd.ToArray().Length == 0 ? "new byte[] { }" : SyntaxFactory.Literal(bd.ToString()).ToString(), + _ => null + }; } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs index 03df192714c..9528d3f308f 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/LibraryVisitor.cs @@ -297,7 +297,7 @@ protected internal virtual FinallyExpression VisitFinallyExpression(FinallyExpre /// /// The original . /// Null if it should be removed otherwise the modified version of the . - protected virtual PropertyProvider? VisitProperty(PropertyProvider property) + protected internal virtual PropertyProvider? VisitProperty(PropertyProvider property) { return property; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs index 81ee9fcd288..9d0c903ef53 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/PostProcessing/GeneratedCodeWorkspace.cs @@ -233,7 +233,8 @@ .. _assemblyMetadataReferences.Value.Concat(CodeModelGenerator.Instance.Addition project = project .AddMetadataReferences(metadataReferences) .WithCompilationOptions(new CSharpCompilationOptions( - OutputKind.DynamicallyLinkedLibrary, metadataReferenceResolver: _metadataReferenceResolver.Value, nullableContextOptions: NullableContextOptions.Disable)); + OutputKind.DynamicallyLinkedLibrary, metadataReferenceResolver: _metadataReferenceResolver.Value, nullableContextOptions: NullableContextOptions.Disable) + .WithMetadataImportOptions(MetadataImportOptions.All)); return await project.GetCompilationAsync(); } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/EnumProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/EnumProvider.cs index e8496d8987e..62453e8510b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/EnumProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/EnumProvider.cs @@ -68,7 +68,7 @@ protected override string BuildNamespace() => string.IsNullOrEmpty(_inputType?.N protected static string RemoveUnderscores(string name) => name.Replace("_", string.Empty); private HashSet? _customMemberNames; - private HashSet CustomMemberNames => _customMemberNames ??= new HashSet( + private protected HashSet CustomMemberNames => _customMemberNames ??= new HashSet( GetCustomMemberNames(), StringComparer.OrdinalIgnoreCase); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ExtensibleEnumProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ExtensibleEnumProvider.cs index c849dcf2a1b..59340784de0 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ExtensibleEnumProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/ExtensibleEnumProvider.cs @@ -4,8 +4,10 @@ using System; using System.Collections.Generic; using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; +using Microsoft.TypeSpec.Generator.EmitterRpc; using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.Input; using Microsoft.TypeSpec.Generator.Input.Extensions; @@ -274,5 +276,83 @@ protected override TypeProvider[] BuildSerializationProviders() return CodeModelGenerator.Instance.TypeFactory.CreateSerializations(_inputType, this).ToArray(); } protected override bool GetIsEnum() => true; + + protected internal override IReadOnlyList? BuildEnumValuesForBackCompatibility(IReadOnlyList currentValues) + { + var lastContractProperties = LastContractView?.Properties + .Where(p => MethodSignatureHelper.IsPublicApi(p.Modifiers)); + + if (lastContractProperties == null || !lastContractProperties.Any()) + { + return null; + } + + var currentNames = new HashSet(currentValues.Select(v => v.Name), StringComparer.OrdinalIgnoreCase); + var lastContractValueFields = new Dictionary(StringComparer.Ordinal); + foreach (var field in LastContractView!.Fields) + { + lastContractValueFields.TryAdd(field.Name, field); + } + + List? restoredMembers = null; + foreach (var property in lastContractProperties) + { + // Members that still exist in the current spec or are provided by custom code are left untouched. + if (currentNames.Contains(property.Name) || CustomMemberNames.Contains(property.Name)) + { + continue; + } + + // Honor an intentional removal recorded in the ApiCompat baseline. + if (CodeModelGenerator.Instance.SourceInputModel?.ApiCompatBaseline.IsMemberSuppressed(Type.FullyQualifiedName, property.Name, 0) == true) + { + CodeModelGenerator.Instance.Emitter.Debug( + $"Skipping re-add of enum member '{Name}.{property.Name}'; the removal is accepted in the ApiCompat baseline.", + BackCompatibilityChangeCategory.BaselineAcceptedRemovalSkipped); + continue; + } + + if (TryResurrectRemovedMember(property, lastContractValueFields, out var resurrectedMember)) + { + (restoredMembers ??= []).Add(resurrectedMember); + CodeModelGenerator.Instance.Emitter.Debug( + $"Re-added enum member '{property.Name}' to enum '{Name}' to preserve a member from the last contract.", + BackCompatibilityChangeCategory.EnumMemberAddedFromLastContract); + } + } + + if (restoredMembers == null) + { + return null; + } + + // Preserve the current spec order and append the restored members at the end. + return [.. currentValues, .. restoredMembers]; + } + + private bool TryResurrectRemovedMember( + PropertyProvider lastContractProperty, + IReadOnlyDictionary lastContractValueFields, + [NotNullWhen(true)] out EnumTypeMember? member) + { + member = null; + + // The wire value lives in the private const `Value` field. + var valueFieldName = $"{lastContractProperty.Name}Value"; + if (!lastContractValueFields.TryGetValue(valueFieldName, out var valueField) + || valueField.InitializationValue is not LiteralExpression { Literal: { } literalValue }) + { + return false; + } + + var field = new FieldProvider( + FieldModifiers.Private | FieldModifiers.Const, + EnumUnderlyingType, + valueFieldName, + this, + initializationValue: Literal(literalValue)); + member = new EnumTypeMember(lastContractProperty.Name, field, literalValue); + return true; + } } } 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 eccc6634c80..26a44c27fe8 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 @@ -116,7 +116,8 @@ protected internal sealed override IReadOnlyList BuildMethodsFor foreach (var previousMethod in LastContractView.Methods) { - if (currentMethodSignatures.Contains(previousMethod.Signature)) + if (!MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers) || + currentMethodSignatures.Contains(previousMethod.Signature)) { continue; } 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 4d9c4eb985e..d38a547f37f 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 @@ -98,7 +98,7 @@ _inputModel.BaseModel is not null private IDictionary LastContractPropertiesMap => _lastContractPropertiesMap ??= LastContractView?.Properties - .Where(p => MethodProviderHelpers.IsPublicApi(p.Modifiers)) + .Where(p => MethodSignatureHelper.IsPublicApi(p.Modifiers)) .ToDictionary(p => p.Name, p => p.Type) ?? []; private IDictionary? _lastContractPropertiesMap; @@ -629,9 +629,8 @@ protected internal override PropertyProvider[] BuildProperties() // Apply back-compat type replacement only for properties on the public API // surface: changing the type of an internal/private generated property is not - // a source-breaking change, and the last-contract map already excludes - // non-public-API entries. - if (MethodProviderHelpers.IsPublicApi(outputProperty.Modifiers) && + // a source-breaking change + if (MethodSignatureHelper.IsPublicApi(outputProperty.Modifiers) && LastContractPropertiesMap.TryGetValue(outputProperty.Name, out var lastContractPropertyType) && !lastContractPropertyType.Equals(outputProperty.Type)) { @@ -1380,6 +1379,7 @@ private bool ShouldUseObjectAdditionalProperties() // Check if the property exists in the last contract by name var lastContractProperty = LastContractView.Properties.FirstOrDefault(p => + MethodSignatureHelper.IsPublicApi(p.Modifiers) && p.Name == AdditionalPropertiesHelper.DefaultAdditionalPropertiesPropertyName); if (lastContractProperty == null) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs index 7c8d00a8b82..1a822a16db8 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/NamedTypeSymbolProvider.cs @@ -247,18 +247,13 @@ protected internal override PropertyProvider[] BuildProperties() return null; } - private static ValueExpression? GetFieldInitializer(IFieldSymbol fieldSymbol) + private static LiteralExpression? GetFieldInitializer(IFieldSymbol fieldSymbol) { - if (fieldSymbol.ContainingType?.TypeKind == TypeKind.Enum) - { - if (fieldSymbol.HasConstantValue && fieldSymbol.ConstantValue != null) - { - return Literal(fieldSymbol.ConstantValue); - } - return null; - } - - return null; + return fieldSymbol.HasConstantValue && + fieldSymbol.ConstantValue != null && + LiteralExpression.TryCreate(fieldSymbol.ConstantValue, out var initializer) + ? initializer + : null; } private static string? GetOriginalName(ISymbol symbol) 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 e11f30e08e1..b881b47e65c 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 @@ -825,7 +825,8 @@ internal void ProcessTypeForBackCompatibility() IReadOnlyList? updatedEnumValues = null; IEnumerable? newFields = null; - if (this is EnumProvider) + IEnumerable? newProperties = null; + if (this is EnumProvider enumProvider) { var hasFields = LastContractView?.Fields != null && LastContractView.Fields.Count > 0; if (hasFields) @@ -848,7 +849,39 @@ internal void ProcessTypeForBackCompatibility() updatedEnumValues = newEnumValues; } - newFields = filteredFields; + // Sync the enum values before rebuilding the member collections from them. + if (updatedEnumValues != null) + { + _enumValues = updatedEnumValues; + } + + if (enumProvider.IsExtensible) + { + // Extensible enums carry an extra backing `_value` field and surface members + // as properties, so rebuild both from the updated members. Reuse the + // already-visited field and property instances for members that still exist so + // any visitor mutations are preserved and only the restored members are + // (re)visited below. + var existingFields = new Dictionary(StringComparer.Ordinal); + foreach (var field in Fields) + { + existingFields.TryAdd(field.Name, field); + } + newFields = ApplyCustomizationFilter( + BuildFields().Select(f => existingFields.TryGetValue(f.Name, out var existing) ? existing : f)); + + var existingProperties = new Dictionary(StringComparer.Ordinal); + foreach (var property in Properties) + { + existingProperties.TryAdd(property.Name, property); + } + newProperties = ApplyCustomizationFilter( + BuildProperties().Select(p => existingProperties.TryGetValue(p.Name, out var existing) ? existing : p)); + } + else + { + newFields = filteredFields; + } } } } @@ -856,13 +889,8 @@ internal void ProcessTypeForBackCompatibility() var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null; var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null; - if (newFields != null || newMethods != null || newConstructors != null) + if (newFields != null || newProperties != null || newMethods != null || newConstructors != null) { - if (updatedEnumValues != null) - { - _enumValues = updatedEnumValues; - } - // Back-compatibility processing intentionally runs after the library visitor pass so // that the contract comparison uses the final, post-visitor member signatures (otherwise // we could incorrectly decide whether a back-compat member is needed). As a result, any @@ -877,12 +905,16 @@ internal void ProcessTypeForBackCompatibility() { newConstructors = VisitNewMembers(newConstructors, Constructors, static (member, visitor) => visitor.VisitConstructor(member)); } + if (newProperties != null) + { + newProperties = VisitNewMembers(newProperties, Properties, static (member, visitor) => visitor.VisitProperty(member)); + } if (newFields != null) { newFields = VisitNewMembers(newFields, Fields, static (member, visitor) => visitor.VisitField(member)); } - Update(fields: newFields, methods: newMethods, constructors: newConstructors); + Update(fields: newFields, properties: newProperties, methods: newMethods, constructors: newConstructors); } // Providers whose attributes depend on final generation decisions build their attributes at write @@ -973,7 +1005,7 @@ protected internal virtual IReadOnlyList BuildMethodsForBackComp foreach (var previousMethod in previousMethods) { if (currentMethodSignatures.ContainsKey(previousMethod.Signature) - || !MethodProviderHelpers.IsPublicApi(previousMethod.Signature.Modifiers) + || !MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers) || BackCompatHelper.IsMethodRemovalAcceptedInBaseline(this, previousMethod.Signature)) { continue; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs index 724f345569c..1baac80ce24 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs @@ -15,6 +15,10 @@ namespace Microsoft.TypeSpec.Generator { internal class MethodSignatureHelper { + internal static bool IsPublicApi(MethodSignatureModifiers modifiers) + => (modifiers.HasFlag(MethodSignatureModifiers.Public) || modifiers.HasFlag(MethodSignatureModifiers.Protected)) + && !modifiers.HasFlag(MethodSignatureModifiers.Private); + internal static bool ContainsSameParameters(MethodSignature method1, MethodSignature method2) { var count = method1.Parameters.Count; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs index c87ed9c348a..346b90d6a81 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Utilities/BackCompatHelper.cs @@ -112,10 +112,11 @@ public static bool ParametersMatch(IReadOnlyList params1, IRe return null; } - IEnumerable scopedMethods = lastContractMethods; + IEnumerable scopedMethods = lastContractMethods + .Where(m => MethodSignatureHelper.IsPublicApi(m.Signature.Modifiers)); if (methodName != null) { - scopedMethods = lastContractMethods.Where(m => + scopedMethods = scopedMethods.Where(m => string.Equals(m.Signature.Name, methodName, StringComparison.OrdinalIgnoreCase) || string.Equals(m.Signature.Name, methodName + "Async", StringComparison.OrdinalIgnoreCase)); } @@ -224,6 +225,11 @@ public static void RestorePreviousParameterNames( { foreach (var previousMethod in previousMethods) { + if (!MethodSignatureHelper.IsPublicApi(previousMethod.Signature.Modifiers)) + { + continue; + } + if (MethodSignature.MethodSignatureComparer.Equals(currentSignature, previousMethod.Signature)) { return previousMethod; @@ -355,7 +361,7 @@ public static void AddBackCompatOverloads(TypeProvider enclosingType, List (modifiers.HasFlag(MethodSignatureModifiers.Public) || modifiers.HasFlag(MethodSignatureModifiers.Protected)) - && !modifiers.HasFlag(MethodSignatureModifiers.Private); - private static bool ShouldSkipParameterValidation(MethodSignatureBase signature, TypeProvider enclosingType) { // Skip parameter validation for methods that are not public or protected on a public type. return !enclosingType.DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) - || !IsPublicApi(signature.Modifiers); + || !MethodSignatureHelper.IsPublicApi(signature.Modifiers); } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/OutputLibraryVisitorTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/OutputLibraryVisitorTests.cs index 49b40624443..dd59b12dd58 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/OutputLibraryVisitorTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/OutputLibraryVisitorTests.cs @@ -494,7 +494,7 @@ private class TestFilterVisitor : LibraryVisitor return constructor; } - protected override PropertyProvider? VisitProperty(PropertyProvider property) + protected internal override PropertyProvider? VisitProperty(PropertyProvider property) { if (property.Name == "TestProperty") { 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 eb341de30b5..ee08f1dd501 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 @@ -650,6 +650,203 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.IsNull(fields[1].InitializationValue); } + // Validates that a member removed from an extensible (string-backed) enum is re-added from the + // last contract. Unlike fixed string enums, an extensible enum stores its wire value in a private + // const `Value` field, so the value is recoverable (even from a compiled assembly's + // metadata) and the previously shipped member can be restored to avoid a source-breaking removal. + [Test] + public async Task BackCompat_ExtensibleEnumRemovedValueReadded() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(string), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Last contract: Default, Recover, Third. Current input removes "Third". + var input = InputFactory.StringEnum("mockInputEnum", [ + ("Default", "default"), + ("Recover", "recover"), + ], isExtensible: true); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + // "Third" is re-added (appended after the current members) as a public static property. + var properties = enumType.Properties; + Assert.AreEqual(3, properties.Count); + Assert.AreEqual("Default", properties[0].Name); + Assert.AreEqual("Recover", properties[1].Name); + Assert.AreEqual("Third", properties[2].Name); + + // The corresponding enum value carries the wire value recovered from the last contract. + var thirdMember = enumType.EnumValues.SingleOrDefault(v => v.Name == "Third"); + Assert.IsNotNull(thirdMember); + Assert.AreEqual("third", thirdMember!.Value); + + // Validate the full generated output, including the restored const `Value` field + // and the preserved backing `_value` field. + var content = new TypeProviderWriter(enumType).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // Validates that a removed extensible enum member that was NON-public in the last contract is not + // re-added: only public members are part of the compatibility surface, and the last-contract view + // also surfaces non-public members (e.g. an internal member added via custom code). + [Test] + public async Task BackCompat_ExtensibleEnumNonPublicValueNotReadded() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(string), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // Last contract: Default, Recover (public), Internal (internal). Current input removes "Internal". + var input = InputFactory.StringEnum("mockInputEnum", [ + ("Default", "default"), + ("Recover", "recover"), + ], isExtensible: true); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + // The non-public "Internal" member is not part of the public contract, so it is not re-added. + var properties = enumType.Properties; + Assert.AreEqual(2, properties.Count); + Assert.IsFalse(properties.Any(p => p.Name == "Internal")); + Assert.AreEqual("Default", properties[0].Name); + Assert.AreEqual("Recover", properties[1].Name); + Assert.IsFalse(enumType.Fields.Any(f => f.Name == "InternalValue")); + + // Validate the full generated output; "Internal" (property and its const `InternalValue` field) is absent. + var content = new TypeProviderWriter(enumType).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // Validates that a removed extensible enum member is NOT re-added when its removal is accepted in + // the ApiCompat baseline (recorded as a MembersMustExist suppression), so the generator honors the + // intentional removal instead of resurrecting it. Runs against both the text and XML baseline + // formats to ensure either representation of the accepted removal is honored. + [TestCase(".txt")] + [TestCase(".xml")] + public async Task BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts(string baselineExtension) + { + var baseline = Helpers.GetApiCompatBaselineFromFile(fileExtension: baselineExtension); + + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(string), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(), + apiCompatBaseline: baseline); + + // Last contract: Default, Recover, Third. Current input removes "Third", but the baseline + // accepts that removal, so it must NOT be re-added. + var input = InputFactory.StringEnum("mockInputEnum", [ + ("Default", "default"), + ("Recover", "recover"), + ], isExtensible: true); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + var properties = enumType.Properties; + Assert.AreEqual(2, properties.Count); + Assert.IsFalse(properties.Any(p => p.Name == "Third")); + Assert.AreEqual("Default", properties[0].Name); + Assert.AreEqual("Recover", properties[1].Name); + Assert.IsFalse(enumType.Fields.Any(f => f.Name == "ThirdValue")); + + // Validate the full generated output; "Third" (property and its const `ThirdValue` field) + // must be absent regardless of which baseline format recorded the accepted removal. + var content = new TypeProviderWriter(enumType).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // Validates that when custom code already provides a member that the current spec removed (and + // that the last contract still declares), back-compat does NOT re-add it. The custom code is left + // as the single source of truth for that member so the generated member does not collide with it. + [Test] + public async Task BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(string), + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Custom"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last")); + + // Last contract: Default, Recover, Third. Current spec removes "Third", but custom code + // provides it, so back-compat must NOT re-add "Third". + var input = InputFactory.StringEnum("mockInputEnum", [ + ("Default", "default"), + ("Recover", "recover"), + ], isExtensible: true); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + Assert.IsNotNull(enumType.CustomCodeView); + Assert.IsTrue(enumType.CustomCodeView!.Properties.Any(p => p.Name == "Third")); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + // The generated provider must not re-add the custom-owned "Third" member (property or const + // `ThirdValue` field); only the two current members remain in generated code. + var properties = enumType.Properties; + Assert.AreEqual(2, properties.Count); + Assert.AreEqual("Default", properties[0].Name); + Assert.AreEqual("Recover", properties[1].Name); + Assert.IsFalse(properties.Any(p => p.Name == "Third")); + Assert.IsFalse(enumType.Fields.Any(f => f.Name == "ThirdValue")); + + // Validate the full generated output as well. + var content = new TypeProviderWriter(enumType).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + + // Validates that when custom code re-declares a member that both the current spec and the last + // contract still contain, back-compat leaves the member to the custom code (no duplicate generated + // member) while other removed last-contract members are still restored. + [Test] + public async Task BackCompat_ExtensibleEnumCustomCodeMemberPreservedWhileOtherMemberRestored() + { + await MockHelpers.LoadMockGeneratorAsync( + createCSharpTypeCore: (inputType) => typeof(string), + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Custom"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Last")); + + // Last contract: Default, Recover, Third. Current spec keeps Default (customized) and Recover + // but removes "Third". Custom code owns "Default", so it must not be regenerated, while the + // removed "Third" is restored from the last contract. + var input = InputFactory.StringEnum("mockInputEnum", [ + ("Default", "default"), + ("Recover", "recover"), + ], isExtensible: true); + + var enumType = EnumProvider.Create(input); + Assert.IsFalse(enumType is ApiVersionEnumProvider); + Assert.IsNotNull(enumType.CustomCodeView); + Assert.IsTrue(enumType.CustomCodeView!.Properties.Any(p => p.Name == "Default")); + + enumType.EnsureBuilt(); + enumType.ProcessTypeForBackCompatibility(); + + var properties = enumType.Properties; + // "Default" is owned by custom code so it is filtered out of generated code; "Recover" stays + // and "Third" is restored from the last contract, appended after the current members. + Assert.IsFalse(properties.Any(p => p.Name == "Default")); + Assert.IsTrue(properties.Any(p => p.Name == "Recover")); + Assert.IsTrue(properties.Any(p => p.Name == "Third")); + + var thirdMember = enumType.EnumValues.SingleOrDefault(v => v.Name == "Third"); + Assert.IsNotNull(thirdMember); + Assert.AreEqual("third", thirdMember!.Value); + Assert.IsTrue(enumType.Fields.Any(f => f.Name == "ThirdValue")); + } + // 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. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumCustomCodeMemberPreservedWhileOtherMemberRestored(Custom)/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumCustomCodeMemberPreservedWhileOtherMemberRestored(Custom)/MockInputEnum.cs new file mode 100644 index 00000000000..9044df45e5b --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumCustomCodeMemberPreservedWhileOtherMemberRestored(Custom)/MockInputEnum.cs @@ -0,0 +1,9 @@ +#nullable disable + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum + { + public static MockInputEnum Default { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumCustomCodeMemberPreservedWhileOtherMemberRestored(Last)/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumCustomCodeMemberPreservedWhileOtherMemberRestored(Last)/MockInputEnum.cs new file mode 100644 index 00000000000..aa76d3215f1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumCustomCodeMemberPreservedWhileOtherMemberRestored(Last)/MockInputEnum.cs @@ -0,0 +1,27 @@ +#nullable disable + +using System; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + private const string ThirdValue = "third"; + + public MockInputEnum(string value) + { + _value = value ?? throw new ArgumentNullException(nameof(value)); + } + + public static MockInputEnum Default { get; } = new MockInputEnum(DefaultValue); + + public static MockInputEnum Recover { get; } = new MockInputEnum(RecoverValue); + + public static MockInputEnum Third { get; } = new MockInputEnum(ThirdValue); + + public bool Equals(MockInputEnum other) => string.Equals(_value, other._value, StringComparison.InvariantCultureIgnoreCase); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumNonPublicValueNotReadded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumNonPublicValueNotReadded.cs new file mode 100644 index 00000000000..da774fa2930 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumNonPublicValueNotReadded.cs @@ -0,0 +1,46 @@ +// + +#nullable disable + +using System; +using System.ComponentModel; +using Sample; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : global::System.IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + + public MockInputEnum(string value) + { + global::Sample.Argument.AssertNotNull(value, nameof(value)); + + _value = value; + } + + public static global::Sample.Models.MockInputEnum Default { get; } = new global::Sample.Models.MockInputEnum(DefaultValue); + + public static global::Sample.Models.MockInputEnum Recover { get; } = new global::Sample.Models.MockInputEnum(RecoverValue); + + public static bool operator ==(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => left.Equals(right); + + public static bool operator !=(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => !left.Equals(right); + + public static implicit operator global::Sample.Models.MockInputEnum(string value) => new global::Sample.Models.MockInputEnum(value); + + public static implicit operator global::Sample.Models.MockInputEnum?(string value) => (value == null) ? null : new global::Sample.Models.MockInputEnum(value); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override bool Equals(object obj) => ((obj is global::Sample.Models.MockInputEnum other) && this.Equals(other)); + + public bool Equals(global::Sample.Models.MockInputEnum other) => string.Equals(_value, other._value, global::System.StringComparison.InvariantCultureIgnoreCase); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override int GetHashCode() => (_value != null) ? global::System.StringComparer.InvariantCultureIgnoreCase.GetHashCode(_value) : 0; + + public override string ToString() => _value; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumNonPublicValueNotReadded/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumNonPublicValueNotReadded/MockInputEnum.cs new file mode 100644 index 00000000000..e31ebd19b93 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumNonPublicValueNotReadded/MockInputEnum.cs @@ -0,0 +1,27 @@ +#nullable disable + +using System; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + private const string InternalValue = "internal"; + + public MockInputEnum(string value) + { + _value = value ?? throw new ArgumentNullException(nameof(value)); + } + + public static MockInputEnum Default { get; } = new MockInputEnum(DefaultValue); + + public static MockInputEnum Recover { get; } = new MockInputEnum(RecoverValue); + + internal static MockInputEnum Internal { get; } = new MockInputEnum(InternalValue); + + public bool Equals(MockInputEnum other) => string.Equals(_value, other._value, StringComparison.InvariantCultureIgnoreCase); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.cs new file mode 100644 index 00000000000..da774fa2930 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.cs @@ -0,0 +1,46 @@ +// + +#nullable disable + +using System; +using System.ComponentModel; +using Sample; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : global::System.IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + + public MockInputEnum(string value) + { + global::Sample.Argument.AssertNotNull(value, nameof(value)); + + _value = value; + } + + public static global::Sample.Models.MockInputEnum Default { get; } = new global::Sample.Models.MockInputEnum(DefaultValue); + + public static global::Sample.Models.MockInputEnum Recover { get; } = new global::Sample.Models.MockInputEnum(RecoverValue); + + public static bool operator ==(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => left.Equals(right); + + public static bool operator !=(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => !left.Equals(right); + + public static implicit operator global::Sample.Models.MockInputEnum(string value) => new global::Sample.Models.MockInputEnum(value); + + public static implicit operator global::Sample.Models.MockInputEnum?(string value) => (value == null) ? null : new global::Sample.Models.MockInputEnum(value); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override bool Equals(object obj) => ((obj is global::Sample.Models.MockInputEnum other) && this.Equals(other)); + + public bool Equals(global::Sample.Models.MockInputEnum other) => string.Equals(_value, other._value, global::System.StringComparison.InvariantCultureIgnoreCase); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override int GetHashCode() => (_value != null) ? global::System.StringComparer.InvariantCultureIgnoreCase.GetHashCode(_value) : 0; + + public override string ToString() => _value; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.txt b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.txt new file mode 100644 index 00000000000..5d107c84471 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.txt @@ -0,0 +1,4 @@ +# The Third extensible-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 +# extensible-enum member (a public static property) as a MembersMustExist difference on its getter. +MembersMustExist : Member 'public static Sample.Models.MockInputEnum Sample.Models.MockInputEnum.Third.get()' 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_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.xml b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.xml new file mode 100644 index 00000000000..e1a45af9549 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts.xml @@ -0,0 +1,12 @@ + + + + + CP0002 + M:Sample.Models.MockInputEnum.get_Third + + diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts/MockInputEnum.cs new file mode 100644 index 00000000000..aa76d3215f1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenBaselineAccepts/MockInputEnum.cs @@ -0,0 +1,27 @@ +#nullable disable + +using System; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + private const string ThirdValue = "third"; + + public MockInputEnum(string value) + { + _value = value ?? throw new ArgumentNullException(nameof(value)); + } + + public static MockInputEnum Default { get; } = new MockInputEnum(DefaultValue); + + public static MockInputEnum Recover { get; } = new MockInputEnum(RecoverValue); + + public static MockInputEnum Third { get; } = new MockInputEnum(ThirdValue); + + public bool Equals(MockInputEnum other) => string.Equals(_value, other._value, StringComparison.InvariantCultureIgnoreCase); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode(Custom)/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode(Custom)/MockInputEnum.cs new file mode 100644 index 00000000000..affc8ad51a8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode(Custom)/MockInputEnum.cs @@ -0,0 +1,9 @@ +#nullable disable + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum + { + public static MockInputEnum Third { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode(Last)/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode(Last)/MockInputEnum.cs new file mode 100644 index 00000000000..aa76d3215f1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode(Last)/MockInputEnum.cs @@ -0,0 +1,27 @@ +#nullable disable + +using System; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + private const string ThirdValue = "third"; + + public MockInputEnum(string value) + { + _value = value ?? throw new ArgumentNullException(nameof(value)); + } + + public static MockInputEnum Default { get; } = new MockInputEnum(DefaultValue); + + public static MockInputEnum Recover { get; } = new MockInputEnum(RecoverValue); + + public static MockInputEnum Third { get; } = new MockInputEnum(ThirdValue); + + public bool Equals(MockInputEnum other) => string.Equals(_value, other._value, StringComparison.InvariantCultureIgnoreCase); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode.cs new file mode 100644 index 00000000000..da774fa2930 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueNotReaddedWhenProvidedByCustomCode.cs @@ -0,0 +1,46 @@ +// + +#nullable disable + +using System; +using System.ComponentModel; +using Sample; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : global::System.IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + + public MockInputEnum(string value) + { + global::Sample.Argument.AssertNotNull(value, nameof(value)); + + _value = value; + } + + public static global::Sample.Models.MockInputEnum Default { get; } = new global::Sample.Models.MockInputEnum(DefaultValue); + + public static global::Sample.Models.MockInputEnum Recover { get; } = new global::Sample.Models.MockInputEnum(RecoverValue); + + public static bool operator ==(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => left.Equals(right); + + public static bool operator !=(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => !left.Equals(right); + + public static implicit operator global::Sample.Models.MockInputEnum(string value) => new global::Sample.Models.MockInputEnum(value); + + public static implicit operator global::Sample.Models.MockInputEnum?(string value) => (value == null) ? null : new global::Sample.Models.MockInputEnum(value); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override bool Equals(object obj) => ((obj is global::Sample.Models.MockInputEnum other) && this.Equals(other)); + + public bool Equals(global::Sample.Models.MockInputEnum other) => string.Equals(_value, other._value, global::System.StringComparison.InvariantCultureIgnoreCase); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override int GetHashCode() => (_value != null) ? global::System.StringComparer.InvariantCultureIgnoreCase.GetHashCode(_value) : 0; + + public override string ToString() => _value; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueReadded.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueReadded.cs new file mode 100644 index 00000000000..bee3d70db2e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueReadded.cs @@ -0,0 +1,49 @@ +// + +#nullable disable + +using System; +using System.ComponentModel; +using Sample; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : global::System.IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + private const string ThirdValue = "third"; + + public MockInputEnum(string value) + { + global::Sample.Argument.AssertNotNull(value, nameof(value)); + + _value = value; + } + + public static global::Sample.Models.MockInputEnum Default { get; } = new global::Sample.Models.MockInputEnum(DefaultValue); + + public static global::Sample.Models.MockInputEnum Recover { get; } = new global::Sample.Models.MockInputEnum(RecoverValue); + + public static global::Sample.Models.MockInputEnum Third { get; } = new global::Sample.Models.MockInputEnum(ThirdValue); + + public static bool operator ==(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => left.Equals(right); + + public static bool operator !=(global::Sample.Models.MockInputEnum left, global::Sample.Models.MockInputEnum right) => !left.Equals(right); + + public static implicit operator global::Sample.Models.MockInputEnum(string value) => new global::Sample.Models.MockInputEnum(value); + + public static implicit operator global::Sample.Models.MockInputEnum?(string value) => (value == null) ? null : new global::Sample.Models.MockInputEnum(value); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override bool Equals(object obj) => ((obj is global::Sample.Models.MockInputEnum other) && this.Equals(other)); + + public bool Equals(global::Sample.Models.MockInputEnum other) => string.Equals(_value, other._value, global::System.StringComparison.InvariantCultureIgnoreCase); + + [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Never)] + public override int GetHashCode() => (_value != null) ? global::System.StringComparer.InvariantCultureIgnoreCase.GetHashCode(_value) : 0; + + public override string ToString() => _value; + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueReadded/MockInputEnum.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueReadded/MockInputEnum.cs new file mode 100644 index 00000000000..aa76d3215f1 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/EnumProviders/TestData/EnumProviderTests/BackCompat_ExtensibleEnumRemovedValueReadded/MockInputEnum.cs @@ -0,0 +1,27 @@ +#nullable disable + +using System; + +namespace Sample.Models +{ + public readonly partial struct MockInputEnum : IEquatable + { + private readonly string _value; + private const string DefaultValue = "default"; + private const string RecoverValue = "recover"; + private const string ThirdValue = "third"; + + public MockInputEnum(string value) + { + _value = value ?? throw new ArgumentNullException(nameof(value)); + } + + public static MockInputEnum Default { get; } = new MockInputEnum(DefaultValue); + + public static MockInputEnum Recover { get; } = new MockInputEnum(RecoverValue); + + public static MockInputEnum Third { get; } = new MockInputEnum(ThirdValue); + + public bool Equals(MockInputEnum other) => string.Equals(_value, other._value, StringComparison.InvariantCultureIgnoreCase); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs index a3315b96226..e81606c7135 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/ModelFactoryProviderTests.cs @@ -407,6 +407,30 @@ public async Task BackCompatibility_NoCurrentOverloadFound() result); } + [Test] + public async Task BackCompatibility_SkipsNonPublicPreviousMethod() + { + _instance = (await MockHelpers.LoadMockGeneratorAsync( + inputNamespaceName: "Sample.Namespace", + inputModelTypes: ModelList, + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync())).Object; + + var modelFactory = _instance!.OutputLibrary.ModelFactory.Value; + + // The last contract exposes an internal factory method that no longer exists in the current + // contract. It is surfaced in the last-contract view (metadata import includes non-public members) + // but is not part of the public compatibility surface. + Assert.IsTrue(modelFactory.LastContractView!.Methods.Any(m => + m.Signature.Name == "PublicModel1OldName" + && m.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal))); + + modelFactory.ProcessTypeForBackCompatibility(); + + // No back-compat shim is generated for the internal previous method. + var content = new TypeProviderWriter(modelFactory).Write().Content; + Assert.AreEqual(Helpers.GetExpectedFromFile(), content); + } + [Test] public async Task BackCompatibility_SuppressedByApiCompatBaselineNotRegenerated() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_SkipsNonPublicPreviousMethod.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_SkipsNonPublicPreviousMethod.cs new file mode 100644 index 00000000000..913315d1f11 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_SkipsNonPublicPreviousMethod.cs @@ -0,0 +1,61 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using System.Linq; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + public static global::Sample.Models.PublicModel1 PublicModel1(string stringProp = default, global::Sample.Models.Thing modelProp = default, global::System.Collections.Generic.IEnumerable listProp = default, global::System.Collections.Generic.IDictionary dictProp = default) + { + listProp ??= new global::Sample.Namespace.ChangeTrackingList(); + dictProp ??= new global::Sample.Namespace.ChangeTrackingDictionary(); + + return new global::Sample.Models.PublicModel1(stringProp, modelProp, listProp.ToList(), dictProp, additionalBinaryDataProperties: null); + } + + public static global::Sample.Models.PublicModel2 PublicModel2(string stringProp = default, global::Sample.Models.Thing modelProp = default, global::System.Collections.Generic.IEnumerable listProp = default, global::System.Collections.Generic.IDictionary dictProp = default) + { + listProp ??= new global::Sample.Namespace.ChangeTrackingList(); + dictProp ??= new global::Sample.Namespace.ChangeTrackingDictionary(); + + return new global::Sample.Models.PublicModel2(stringProp, modelProp, listProp.ToList(), dictProp, additionalBinaryDataProperties: null); + } + + public static global::Sample.Models.DerivedModel DerivedModel(string stringProp = default, global::Sample.Models.Thing modelProp = default, global::System.Collections.Generic.IEnumerable listProp = default, global::System.Collections.Generic.IDictionary dictProp = default) + { + listProp ??= new global::Sample.Namespace.ChangeTrackingList(); + dictProp ??= new global::Sample.Namespace.ChangeTrackingDictionary(); + + return new global::Sample.Models.DerivedModel( + additionalBinaryDataProperties: null, + stringProp, + modelProp, + listProp.ToList(), + dictProp, + default); + } + + public static global::Sample.Models.BaseModel BaseModel(string stringProp = default, global::Sample.Models.Thing modelProp = default, global::System.Collections.Generic.IEnumerable listProp = default, global::System.Collections.Generic.IDictionary dictProp = default) + { + listProp ??= new global::Sample.Namespace.ChangeTrackingList(); + dictProp ??= new global::Sample.Namespace.ChangeTrackingDictionary(); + + return new global::Sample.Models.BaseModel(stringProp, modelProp, listProp.ToList(), dictProp, additionalBinaryDataProperties: null); + } + + public static global::Sample.Models.ModelWithUnknownAdditionalProperties ModelWithUnknownAdditionalProperties(string stringProp = default, global::Sample.Models.Thing modelProp = default, global::System.Collections.Generic.IEnumerable listProp = default, global::System.Collections.Generic.IDictionary dictProp = default, global::System.Collections.Generic.IDictionary additionalProperties = default) + { + listProp ??= new global::Sample.Namespace.ChangeTrackingList(); + dictProp ??= new global::Sample.Namespace.ChangeTrackingDictionary(); + additionalProperties ??= new global::Sample.Namespace.ChangeTrackingDictionary(); + + return new global::Sample.Models.ModelWithUnknownAdditionalProperties(stringProp, modelProp, listProp.ToList(), dictProp, additionalProperties); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_SkipsNonPublicPreviousMethod/SampleNamespaceModelFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_SkipsNonPublicPreviousMethod/SampleNamespaceModelFactory.cs new file mode 100644 index 00000000000..86c23f174e6 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelFactories/TestData/ModelFactoryProviderTests/BackCompatibility_SkipsNonPublicPreviousMethod/SampleNamespaceModelFactory.cs @@ -0,0 +1,21 @@ +using SampleTypeSpec; +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using Sample.Models; + +namespace Sample.Namespace +{ + public static partial class SampleNamespaceModelFactory + { + internal static PublicModel1 PublicModel1OldName( + string stringProp = default) + { } + } +} + +namespace Sample.Models +{ + public partial class PublicModel1 + { } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs index 9b7270835f5..48b8d37e0e6 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/ModelProviderTests.cs @@ -2378,6 +2378,34 @@ await MockHelpers.LoadMockGeneratorAsync( Assert.AreEqual(typeof(object), propertyType.Arguments[1].FrameworkType, "Value type should be object for backward compatibility"); } + [Test] + public async Task TestBuildProperties_NonPublicObjectAdditionalPropertiesNotUsedForBackwardCompatibility() + { + var inputModel = InputFactory.Model( + "TestModel", + usage: InputModelTypeUsage.Input, + properties: [InputFactory.Property("Name", InputPrimitiveType.String, isRequired: true)], + additionalProperties: InputPrimitiveType.Any); + + await MockHelpers.LoadMockGeneratorAsync( + inputModelTypes: [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = CodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel); + + Assert.IsNotNull(modelProvider); + + // The last contract's object AdditionalProperties property is non-public, so it is not part of the + // contract and must not force object additional properties; BinaryData is preserved. + var additionalPropertiesProperty = modelProvider!.Properties.FirstOrDefault(p => p.Name == "AdditionalProperties"); + Assert.IsNotNull(additionalPropertiesProperty, "AdditionalProperties property should be generated"); + + var propertyType = additionalPropertiesProperty!.Type; + Assert.IsTrue(propertyType.IsDictionary, "Property should be a dictionary type"); + Assert.AreEqual(typeof(string), propertyType.Arguments[0].FrameworkType, "Key type should be string"); + Assert.AreEqual(typeof(BinaryData), propertyType.Arguments[1].FrameworkType, "Value type should stay BinaryData"); + } + [TestCase(InputModelTypeUsage.Output | InputModelTypeUsage.Xml, false, TestName = "XmlOnly_OutputOnly_NoField")] [TestCase(InputModelTypeUsage.Input | InputModelTypeUsage.Xml, false, TestName = "XmlOnly_Input_NoField")] [TestCase(InputModelTypeUsage.Input | InputModelTypeUsage.Output | InputModelTypeUsage.Xml, false, TestName = "XmlOnly_InputAndOutput_NoField")] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/TestBuildProperties_NonPublicObjectAdditionalPropertiesNotUsedForBackwardCompatibility/TestModel.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/TestBuildProperties_NonPublicObjectAdditionalPropertiesNotUsedForBackwardCompatibility/TestModel.cs new file mode 100644 index 00000000000..7a4d9bcb54a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/ModelProviders/TestData/ModelProviderTests/TestBuildProperties_NonPublicObjectAdditionalPropertiesNotUsedForBackwardCompatibility/TestModel.cs @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; + +namespace Sample.Models +{ + public partial class TestModel + { + internal TestModel(string name, IDictionary additionalProperties) + { + Name = name; + AdditionalProperties = additionalProperties; + } + + public string Name { get; set; } + internal IDictionary AdditionalProperties { get; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs index fabe475c562..2eb817a1613 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/NamedTypeSymbolProviders/NamedTypeSymbolProviderTests.cs @@ -666,6 +666,67 @@ public void ValidateEnumFieldsWithExplicitValues() Assert.AreEqual(3, v74Literal!.Literal); } + // Validates that the constant value of a const field on a non-enum type (here a struct, + // mirroring the private `Value` backing constants an extensible enum uses) is + // recovered as the field's initialization value. This is what lets back-compat processing + // read a previously shipped member's wire value from the last contract's metadata. + [Test] + public void ValidateConstFieldInitializerIsRecovered() + { + var compilation = CSharpCompilation.Create( + "Customization", + [CSharpSyntaxTree.ParseText(""" + namespace Sample.Models + { + public readonly partial struct MockInputEnum + { + private const string RecoverValue = "recover"; + private const int Answer = 42; + } + } + """)], + [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)]); + var symbol = compilation.GetTypeByMetadataName("Sample.Models.MockInputEnum"); + Assert.IsNotNull(symbol); + + var provider = new NamedTypeSymbolProvider(symbol!, compilation); + var fields = provider.Fields.ToDictionary(f => f.Name); + + Assert.IsTrue(fields.ContainsKey("RecoverValue")); + var recoverValue = fields["RecoverValue"]; + Assert.IsInstanceOf(recoverValue.InitializationValue); + Assert.AreEqual("recover", (recoverValue.InitializationValue as LiteralExpression)!.Literal); + + Assert.IsTrue(fields.ContainsKey("Answer")); + var answer = fields["Answer"]; + Assert.IsInstanceOf(answer.InitializationValue); + Assert.AreEqual(42, (answer.InitializationValue as LiteralExpression)!.Literal); + } + + // Validates that a non-const field carries no recovered initialization value. + [Test] + public void ValidateNonConstFieldHasNoInitializer() + { + var compilation = CSharpCompilation.Create( + "Customization", + [CSharpSyntaxTree.ParseText(""" + namespace Sample.Models + { + public readonly partial struct MockInputEnum + { + private readonly string _value; + } + } + """)], + [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)]); + var symbol = compilation.GetTypeByMetadataName("Sample.Models.MockInputEnum"); + Assert.IsNotNull(symbol); + + var provider = new NamedTypeSymbolProvider(symbol!, compilation); + var field = provider.Fields.Single(f => f.Name == "_value"); + Assert.IsNull(field.InitializationValue); + } + public enum SomeEnum { Foo, diff --git a/packages/http-client-csharp/generator/docs/backward-compatibility.md b/packages/http-client-csharp/generator/docs/backward-compatibility.md index fe1b761caf6..80e8dc85ee2 100644 --- a/packages/http-client-csharp/generator/docs/backward-compatibility.md +++ b/packages/http-client-csharp/generator/docs/backward-compatibility.md @@ -16,6 +16,8 @@ - [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) + - [Extensible Enum Members](#extensible-enum-members) + - [Removed Extensible Enum Member Re-added](#scenario-removed-extensible-enum-member-re-added) - [API Version Enum](#api-version-enum) - [Non-abstract Base Models](#non-abstract-base-models) - [Model Constructors](#model-constructors) @@ -449,6 +451,53 @@ public enum SampleEnum - 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 +### Extensible Enum Members + +Extensible enums (C# `readonly partial struct` types) preserve their previously shipped members by comparing the current spec against the last contract. The generator re-adds an extensible enum member that was dropped from the current spec, restoring it with its original wire value to avoid removing a previously shipped member. + +#### Scenario: Removed Extensible Enum Member Re-added + +**Description:** When an extensible enum member present in the last contract is dropped from the current spec, the generator restores it — as a public static property backed by its recovered `const` wire value — to keep the previously shipped API. + +**Example:** + +Previous version: + +```csharp +public readonly partial struct OperationStatusType : IEquatable +{ + private const string CompletedValue = "Completed"; + private const string FailedValue = "Failed"; + private const string RunningValue = "Running"; + + public static OperationStatusType Completed { get; } = new OperationStatusType(CompletedValue); + public static OperationStatusType Failed { get; } = new OperationStatusType(FailedValue); + public static OperationStatusType Running { get; } = new OperationStatusType(RunningValue); + // ... +} +``` + +Current TypeSpec removes `Running`. **Generated Result:** `Running` is re-added (appended after the current members) with its original wire value: + +```csharp +public readonly partial struct OperationStatusType : IEquatable +{ + private const string CompletedValue = "Completed"; + private const string FailedValue = "Failed"; + private const string RunningValue = "Running"; + + public static OperationStatusType Completed { get; } = new OperationStatusType(CompletedValue); + public static OperationStatusType Failed { get; } = new OperationStatusType(FailedValue); + public static OperationStatusType Running { get; } = new OperationStatusType(RunningValue); + // ... +} +``` + +**Key Points:** + +- Members that already exist in the current spec, are provided by custom code, or whose removal is accepted in the [ApiCompat baseline](#apicompat-baseline-awareness) are not re-added +- Restored members are appended after the current spec's members, preserving the current spec's order + ### API Version Enum Service version enums maintain backward compatibility by preserving version values from previous releases.