diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs index 947302cbe17..4f696af106d 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs @@ -8,6 +8,7 @@ using System.IO; using System.Linq; using System.Threading.Tasks; +using Microsoft.TypeSpec.Generator.ClientModel.Utilities; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; using Microsoft.TypeSpec.Generator.Statements; @@ -20,6 +21,10 @@ public class ModelReaderWriterContextDefinition : TypeProvider private const string DefaultObsoleteDiagnosticId = "CS0618"; private static readonly CSharpTypeNameComparer s_cSharpTypeNameComparer = new CSharpTypeNameComparer(); private static readonly TypeProviderTypeNameComparer s_typeProviderNameComparer = new TypeProviderTypeNameComparer(); + private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) + { + nameof(ModelReaderWriterBuildableAttribute), + }); internal static readonly string s_name = $"{RemovePeriods(ScmCodeModelGenerator.Instance.TypeFactory.PrimaryNamespace)}Context"; @@ -76,6 +81,17 @@ protected override IReadOnlyList BuildAttributes() return attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value).ToList(); } + /// + /// Restores back-compatibility attributes from the last contract, then drops any restored + /// since those are recomputed at write time. + /// + protected override IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + { + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; + var merged = base.BuildAttributesForBackCompatibility(original); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore); + } + /// /// Collects all types that implement IPersistableModel, including all models and their properties /// that are also IPersistableModel types, recursively without duplicates. 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 16ac19e14e9..aff0d45a21f 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 @@ -42,6 +42,10 @@ public partial class MrwSerializationTypeDefinition : TypeProvider private const string DeserializationMethodNamePrefix = "Deserialize"; private const string WriteAction = "writing"; private const string ReadAction = "reading"; + private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) + { + nameof(PersistableModelProxyAttribute), + }); private readonly ParameterProvider _utf8JsonWriterParameter = new("writer", $"The JSON writer.", typeof(Utf8JsonWriter)); private readonly ParameterProvider _utf8JsonReaderParameter = new("reader", $"The JSON reader.", typeof(Utf8JsonReader), isRef: true); private readonly ParameterProvider _serializationOptionsParameter = @@ -152,6 +156,17 @@ protected override IReadOnlyList BuildAttributes() return []; } + /// + /// Restores back-compatibility attributes from the last contract, then drops any restored + /// since those are recomputed at generation time. + /// + protected override IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + { + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; + var merged = base.BuildAttributesForBackCompatibility(original); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore); + } + private CSharpType GetRootModelType() { // We need to explicitly use the BaseModelProvider when looking up the root type 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..b31acc3d234 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 @@ -15,6 +15,7 @@ using System.Text.Json.Serialization; using Microsoft.TypeSpec.Generator.ClientModel.Primitives; using Microsoft.TypeSpec.Generator.ClientModel.Snippets; +using Microsoft.TypeSpec.Generator.ClientModel.Utilities; using Microsoft.TypeSpec.Generator.EmitterRpc; using Microsoft.TypeSpec.Generator.Expressions; using Microsoft.TypeSpec.Generator.Input; @@ -43,6 +44,10 @@ public sealed class ScmModelProvider : ModelProvider private static AttributeStatement _fileBinaryContentExpAttribute = new( typeof(ExperimentalAttribute), [Literal(FileBinaryContentDiagnosticId)]); + private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) + { + nameof(PersistableModelProxyAttribute), + }); internal const string ScmEvaluationTypeDiagnosticId = "SCME0001"; internal const string FileBinaryContentDiagnosticId = "SCME0004"; @@ -126,6 +131,18 @@ protected override PropertyProvider[] BuildProperties() return properties; } + /// + /// Restores back-compatibility attributes from the last contract, then drops any restored + /// since those are recomputed at generation time by the + /// serialization provider. + /// + protected override IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + { + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; + var merged = base.BuildAttributesForBackCompatibility(original); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore); + } + protected override ConstructorProvider[] BuildConstructors() { List constructors = [.. base.BuildConstructors()]; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Utilities/ScmBackCompatibilityHelpers.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Utilities/ScmBackCompatibilityHelpers.cs new file mode 100644 index 00000000000..93ec02c9c56 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Utilities/ScmBackCompatibilityHelpers.cs @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using Microsoft.TypeSpec.Generator.Statements; + +namespace Microsoft.TypeSpec.Generator.ClientModel.Utilities +{ + internal static class ScmBackCompatibilityHelpers + { + /// + /// Drops any generator-owned attributes that were newly restored from the last contract. + /// is the result of the base + /// BuildAttributesForBackCompatibility call; any attribute whose type name is in + /// and was not already present in + /// is removed, since generation recomputes it. + /// is only evaluated when there is restored + /// content to filter, so callers can pass a lazily-initialized set without paying for it when + /// nothing was restored. + /// + public static IReadOnlyList FilterRestoredAttributes( + IReadOnlyList originalAttributes, + IReadOnlyList restoredAttributes, + Lazy> generatorOwnedAttributeNames) + { + // The base returns the original list unchanged when nothing was restored. + if (ReferenceEquals(restoredAttributes, originalAttributes)) + { + return restoredAttributes; + } + + var ownedNames = generatorOwnedAttributeNames.Value; + + var originalDisplayStrings = new HashSet(StringComparer.Ordinal); + foreach (var attribute in originalAttributes) + { + originalDisplayStrings.Add(attribute.ToDisplayString()); + } + + var result = new List(restoredAttributes.Count); + foreach (var attribute in restoredAttributes) + { + if (ownedNames.Contains(attribute.Type.Name) + && !originalDisplayStrings.Contains(attribute.ToDisplayString())) + { + continue; + } + + result.Add(attribute); + } + + return result; + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs index 79b77b510c9..2977dd36020 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs @@ -1622,6 +1622,24 @@ public void ValidateTypeWithCustomSerializationProviderImplementingIPersistableM "ModelWithCustomSerialization should be included because it has a serialization provider implementing IJsonModel"); } + [Test] + public async Task BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute() + { + await MockHelpers.LoadMockGeneratorAsync( + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var contextDefinition = new ModelReaderWriterContextDefinition(); + + // The last contract declares a ModelReaderWriterBuildable attribute (owned by generation and + // rebuilt at write time) alongside a Description attribute. Back-compat processing should only + // restore the non-buildable Description attribute. + contextDefinition.ProcessTypeForBackCompatibility(); + + var writer = new TypeProviderWriter(contextDefinition); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + private class CustomSerializationProvider : TypeProvider { private readonly bool _usePersistableModel; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute.cs new file mode 100644 index 00000000000..7aa673a8a7f --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +using System.ClientModel.Primitives; +using System.ComponentModel; + +namespace Sample +{ + [global::System.ComponentModel.DescriptionAttribute("bc")] + public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute/SampleContext.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute/SampleContext.cs new file mode 100644 index 00000000000..59e2f4bbab2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute/SampleContext.cs @@ -0,0 +1,11 @@ +using System.ComponentModel; +using System.ClientModel.Primitives; + +namespace Sample +{ + [ModelReaderWriterBuildable(typeof(object))] + [Description("bc")] + public partial class SampleContext : ModelReaderWriterContext + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs new file mode 100644 index 00000000000..b7d11a2a4ff --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Linq; +using System.Threading.Tasks; +using Microsoft.TypeSpec.Generator.ClientModel.Providers; +using Microsoft.TypeSpec.Generator.Input; +using Microsoft.TypeSpec.Generator.Primitives; +using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Tests.Common; +using NUnit.Framework; + +namespace Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.MrwSerializationTypeDefinitions +{ + internal class BackCompatibilityTests + { + private class MockMrwProvider : MrwSerializationTypeDefinition + { + public MockMrwProvider(InputModelType inputModel, ModelProvider modelProvider) + : base(inputModel, modelProvider) + { + } + + protected override MethodProvider[] BuildMethods() => []; + protected internal override FieldProvider[] BuildFields() => []; + protected override ConstructorProvider[] BuildConstructors() => []; + } + + [Test] + public async Task BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute() + { + var inputModel = InputFactory.Model("pet", properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true) + ]); + + var mockGenerator = await MockHelpers.LoadMockGeneratorAsync( + inputModels: () => [inputModel], + createSerializationsCore: (inputType, typeProvider) + => inputType is InputModelType modelType ? [new MockMrwProvider(modelType, (typeProvider as ModelProvider)!)] : [], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = mockGenerator.Object.OutputLibrary.TypeProviders.Single(t => t is ModelProvider); + var serializationProvider = modelProvider.SerializationProviders.Single(t => t is MrwSerializationTypeDefinition); + + // The last contract declares a PersistableModelProxy attribute (owned by generation and + // recomputed at generation time) alongside a Description attribute. Back-compat processing + // should only restore the non-proxy Description attribute. + serializationProvider.ProcessTypeForBackCompatibility(); + + var writer = new TypeProviderWriter(serializationProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs new file mode 100644 index 00000000000..1a3aa8f1a61 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +using System.ClientModel.Primitives; +using System.ComponentModel; + +namespace Sample.Models +{ + [global::System.ComponentModel.DescriptionAttribute("bc")] + public partial class Pet : global::System.ClientModel.Primitives.IJsonModel + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs new file mode 100644 index 00000000000..adca8a49c2e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs @@ -0,0 +1,11 @@ +using System.ComponentModel; +using System.ClientModel.Primitives; + +namespace Sample.Models +{ + [PersistableModelProxy(typeof(object))] + [Description("bc")] + public partial class Pet + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs index eff1d0ac65b..509f3f6642a 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/ScmModelProviderTests.cs @@ -22,6 +22,30 @@ public void SetUp() MockHelpers.LoadMockGenerator(); } + [Test] + public async Task BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute() + { + var inputModel = InputFactory.Model("pet", properties: + [ + InputFactory.Property("name", InputPrimitiveType.String, isRequired: true) + ]); + + await MockHelpers.LoadMockGeneratorAsync( + inputModels: () => [inputModel], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var modelProvider = (ScmModel)ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel)!; + + // The last contract declares a PersistableModelProxy attribute (owned by generation and + // recomputed at generation time by the serialization provider) alongside a Description + // attribute. Back-compat processing should only restore the non-proxy Description attribute. + modelProvider.ProcessTypeForBackCompatibility(); + + var writer = new TypeProviderWriter(modelProvider); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); + } + [Test] public void TestSimpleDynamicModel() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs new file mode 100644 index 00000000000..2b052e2b926 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs @@ -0,0 +1,32 @@ +// + +#nullable disable + +using System; +using System.Collections.Generic; +using System.ComponentModel; +using Sample; + +namespace Sample.Models +{ + [global::System.ComponentModel.DescriptionAttribute("bc")] + public partial class Pet + { + private protected readonly global::System.Collections.Generic.IDictionary _additionalBinaryDataProperties; + + public Pet(string name) + { + global::Sample.Argument.AssertNotNull(name, nameof(name)); + + Name = name; + } + + internal Pet(string name, global::System.Collections.Generic.IDictionary additionalBinaryDataProperties) + { + Name = name; + _additionalBinaryDataProperties = additionalBinaryDataProperties; + } + + public string Name { get; set; } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs new file mode 100644 index 00000000000..adca8a49c2e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs @@ -0,0 +1,11 @@ +using System.ComponentModel; +using System.ClientModel.Primitives; + +namespace Sample.Models +{ + [PersistableModelProxy(typeof(object))] + [Description("bc")] + public partial class Pet + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/TestHelpers/MockHelpers.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/TestHelpers/MockHelpers.cs index 9148f659e43..fba0522e78f 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/TestHelpers/MockHelpers.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/TestHelpers/MockHelpers.cs @@ -34,7 +34,8 @@ public static async Task> LoadMockGeneratorAsync( Func>? apiVersions = null, string? configuration = null, Func? createCSharpTypeCore = null, - Func? createCSharpTypeCoreFallback = null) + Func? createCSharpTypeCoreFallback = null, + Func>? createSerializationsCore = null) { var mockGenerator = LoadMockGenerator( inputLiterals: inputLiterals, @@ -44,7 +45,8 @@ public static async Task> LoadMockGeneratorAsync( apiVersions: apiVersions, configuration: configuration, createCSharpTypeCore: createCSharpTypeCore, - createCSharpTypeCoreFallback: createCSharpTypeCoreFallback); + createCSharpTypeCoreFallback: createCSharpTypeCoreFallback, + createSerializationsCore: createSerializationsCore); var compilationResult = compilation == null ? null : await compilation(); var lastContractCompilationResult = lastContractCompilation == null ? null : await lastContractCompilation(); 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 bb67f001301..a105f2f4a4c 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 @@ -18,13 +18,19 @@ internal override void Write(CodeWriter writer) { null => "null", string s => SyntaxFactory.Literal(s).ToString(), + byte b => SyntaxFactory.Literal((int)b).ToString(), + sbyte sb => SyntaxFactory.Literal((int)sb).ToString(), + short sh => SyntaxFactory.Literal((int)sh).ToString(), + ushort us => SyntaxFactory.Literal((int)us).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(), 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", + bool bo => bo ? "true" : "false", BinaryData bd => bd.ToArray().Length == 0 ? "new byte[] { }" : SyntaxFactory.Literal(bd.ToString()).ToString(), _ => throw new NotImplementedException() }); 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 83380c4299b..6d34a2f4726 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 @@ -3,7 +3,11 @@ using System; using System.Collections.Generic; +using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Reflection; +using System.Text.Json.Serialization; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.TypeSpec.Generator.EmitterRpc; @@ -24,6 +28,31 @@ public abstract class TypeProvider private Lazy _specView; private readonly InputType? _inputType; + private static readonly Lazy> s_nonRestorableAttributeNames = new(() => new(StringComparer.Ordinal) + { + nameof(EditorBrowsableAttribute), + nameof(ExperimentalAttribute), + nameof(ObsoleteAttribute), + nameof(SerializableAttribute), + nameof(DefaultMemberAttribute), + nameof(JsonConstructorAttribute), + nameof(JsonConverterAttribute), + nameof(JsonDerivedTypeAttribute), + nameof(JsonExtensionDataAttribute), + nameof(JsonIgnoreAttribute), + nameof(JsonIncludeAttribute), + nameof(JsonNumberHandlingAttribute), + nameof(JsonObjectCreationHandlingAttribute), + nameof(JsonPolymorphicAttribute), + nameof(JsonPropertyNameAttribute), + nameof(JsonPropertyOrderAttribute), + nameof(JsonRequiredAttribute), + nameof(JsonSerializableAttribute), + nameof(JsonSourceGenerationOptionsAttribute), + nameof(JsonStringEnumMemberNameAttribute), + nameof(JsonUnmappedMemberHandlingAttribute), + }); + protected TypeProvider(InputType? inputType = default) { _customCodeView = new(() => BuildCustomCodeView()); @@ -711,6 +740,7 @@ internal void ProcessTypeForBackCompatibility() { var hasMethods = LastContractView?.Methods != null && LastContractView.Methods.Count > 0; var hasConstructors = LastContractView?.Constructors != null && LastContractView.Constructors.Count > 0; + var hasAttributes = LastContractView?.Attributes is { Count: > 0 }; IReadOnlyList? updatedEnumValues = null; IEnumerable? newFields = null; @@ -745,7 +775,17 @@ internal void ProcessTypeForBackCompatibility() var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null; var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null; - if (newFields != null || newMethods != null || newConstructors != null) + IReadOnlyList? newAttributes = null; + if (hasAttributes) + { + var backCompatAttributes = BuildAttributesForBackCompatibility(Attributes); + if (backCompatAttributes.Count != Attributes.Count) + { + newAttributes = backCompatAttributes; + } + } + + if (newFields != null || newMethods != null || newConstructors != null || newAttributes != null) { if (updatedEnumValues != null) { @@ -771,7 +811,7 @@ internal void ProcessTypeForBackCompatibility() newFields = VisitNewMembers(newFields, Fields, static (member, visitor) => visitor.VisitField(member)); } - Update(fields: newFields, methods: newMethods, constructors: newConstructors); + Update(fields: newFields, methods: newMethods, constructors: newConstructors, attributes: newAttributes); } } @@ -940,6 +980,83 @@ protected internal virtual IReadOnlyList BuildConstructorsF return [.. constructors]; } + /// + /// Merges the generated () and custom-code attributes with + /// the attributes from the last contract, restoring any last-contract attribute that is not already + /// present so that removing it does not break a consumer contract. Restoration only applies to types + /// that are externally visible (public or protected); attributes that generation owns (any attribute + /// whose name contains CodeGen or one listed in ) + /// are never restored. The original attributes are returned unchanged when there is nothing to add. + /// + protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + { + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; + + if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) + && !DeclarationModifiers.HasFlag(TypeSignatureModifiers.Protected)) + { + return original; + } + + if (LastContractView?.Attributes is not { Count: > 0 } lastContractAttributes) + { + return original; + } + + // Track the original (generated + custom) attributes so we only add back-compat attributes + // that don't already exist. + var seen = new HashSet(); + foreach (var attribute in original) + { + if (TryGetAttributeDisplayString(attribute, out var display)) + { + seen.Add(display); + } + } + foreach (var attribute in CustomCodeView?.Attributes ?? []) + { + if (TryGetAttributeDisplayString(attribute, out var display)) + { + seen.Add(display); + } + } + + List? merged = null; + foreach (var attribute in lastContractAttributes) + { + if (s_nonRestorableAttributeNames.Value.Contains(attribute.Type.Name) + || attribute.Type.Name.Contains(CodeGenAttributes.CodeGenAttributePrefix, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + // An attribute from the last contract may reference argument literals or types that the + // generator cannot render. Such an attribute cannot be safely restored, so skip it rather + // than crashing the entire generation. + if (TryGetAttributeDisplayString(attribute, out var display) && seen.Add(display)) + { + merged ??= [.. original]; + merged.Add(attribute); + } + } + + return merged ?? original; + } + + private static bool TryGetAttributeDisplayString(AttributeStatement attribute, [NotNullWhen(true)] out string? displayString) + { + try + { + displayString = attribute.ToDisplayString(); + return true; + } + catch (Exception) + { + displayString = null; + return false; + } + } + private IReadOnlyList? _enumValues; private bool ShouldGenerate(ConstructorProvider constructor) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/CodeGenAttributes.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/CodeGenAttributes.cs index bcf1938517e..fb48e3b6ecc 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/CodeGenAttributes.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/SourceInput/CodeGenAttributes.cs @@ -11,6 +11,8 @@ namespace Microsoft.TypeSpec.Generator.SourceInput { public static class CodeGenAttributes { + internal const string CodeGenAttributePrefix = "CodeGen"; + public const string CodeGenSuppressAttributeName = "CodeGenSuppressAttribute"; public const string CodeGenMemberAttributeName = "CodeGenMemberAttribute"; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract.cs new file mode 100644 index 00000000000..56d4291a2e8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract.cs @@ -0,0 +1,13 @@ +// + +#nullable disable + +using System; + +namespace Test +{ + [global::System.CLSCompliantAttribute(true)] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract/BackCompatAttributeType.cs new file mode 100644 index 00000000000..be4634aaa7e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract/BackCompatAttributeType.cs @@ -0,0 +1,9 @@ +using System; + +namespace Test +{ + [CLSCompliant(true)] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract(Custom)/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract(Custom)/BackCompatAttributeType.cs new file mode 100644 index 00000000000..ecf8c5a1971 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract(Custom)/BackCompatAttributeType.cs @@ -0,0 +1,9 @@ +using System; + +namespace Test +{ + [CLSCompliant(true)] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract.cs new file mode 100644 index 00000000000..012b7edb962 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +using System; + +namespace Test +{ + [global::System.ObsoleteAttribute("This is obsolete")] + [global::Test.RestorableAttribute] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract/BackCompatAttributeType.cs new file mode 100644 index 00000000000..903d1075e93 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract/BackCompatAttributeType.cs @@ -0,0 +1,15 @@ +using System; + +namespace Test +{ + public class RestorableAttribute : Attribute + { + } + + [Obsolete("This is obsolete")] + [CLSCompliant(true)] + [Restorable] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute.cs new file mode 100644 index 00000000000..56d4291a2e8 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute.cs @@ -0,0 +1,13 @@ +// + +#nullable disable + +using System; + +namespace Test +{ + [global::System.CLSCompliantAttribute(true)] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute/BackCompatAttributeType.cs new file mode 100644 index 00000000000..be4634aaa7e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute/BackCompatAttributeType.cs @@ -0,0 +1,9 @@ +using System; + +namespace Test +{ + [CLSCompliant(true)] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType.cs new file mode 100644 index 00000000000..3d2c3d968f2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType.cs @@ -0,0 +1,10 @@ +// + +#nullable disable + +namespace Test +{ + internal partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType/BackCompatAttributeType.cs new file mode 100644 index 00000000000..be4634aaa7e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType/BackCompatAttributeType.cs @@ -0,0 +1,9 @@ +using System; + +namespace Test +{ + [CLSCompliant(true)] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments.cs new file mode 100644 index 00000000000..1a6081a77ca --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments.cs @@ -0,0 +1,11 @@ +// + +#nullable disable + +namespace Test +{ + [global::Test.NumericAttribute(1, 2, 3, 4, 5U, 6UL)] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments/BackCompatAttributeType.cs new file mode 100644 index 00000000000..93583a6d06e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments/BackCompatAttributeType.cs @@ -0,0 +1,16 @@ +using System; + +namespace Test +{ + public class NumericAttribute : Attribute + { + public NumericAttribute(byte byteValue, sbyte sbyteValue, short shortValue, ushort ushortValue, uint uintValue, ulong ulongValue) + { + } + } + + [Numeric(1, 2, 3, 4, 5, 6)] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract.cs new file mode 100644 index 00000000000..8240d63b051 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract.cs @@ -0,0 +1,14 @@ +// + +#nullable disable + +using System; + +namespace Test +{ + [global::System.ObsoleteAttribute("This is obsolete")] + [global::System.SerializableAttribute] + public partial class TestName + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes.cs new file mode 100644 index 00000000000..65523f6f0ab --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes.cs @@ -0,0 +1,11 @@ +// + +#nullable disable + +namespace Test +{ + [global::Test.RestorableAttribute] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes/BackCompatAttributeType.cs new file mode 100644 index 00000000000..faef82335ab --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes/BackCompatAttributeType.cs @@ -0,0 +1,21 @@ +using System; + +namespace Test +{ + public class CodeGenModelAttribute : Attribute + { + public CodeGenModelAttribute(string name) + { + } + } + + public class RestorableAttribute : Attribute + { + } + + [CodeGenModel("Something")] + [Restorable] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode(Custom)/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode(Custom)/BackCompatAttributeType.cs new file mode 100644 index 00000000000..ecf8c5a1971 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode(Custom)/BackCompatAttributeType.cs @@ -0,0 +1,9 @@ +using System; + +namespace Test +{ + [CLSCompliant(true)] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode.cs new file mode 100644 index 00000000000..65523f6f0ab --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode.cs @@ -0,0 +1,11 @@ +// + +#nullable disable + +namespace Test +{ + [global::Test.RestorableAttribute] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode/BackCompatAttributeType.cs new file mode 100644 index 00000000000..a13ba7eb8b6 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode/BackCompatAttributeType.cs @@ -0,0 +1,14 @@ +using System; + +namespace Test +{ + public class RestorableAttribute : Attribute + { + } + + [CLSCompliant(true)] + [Restorable] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes.cs new file mode 100644 index 00000000000..00d03a5061a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes.cs @@ -0,0 +1,10 @@ +// + +#nullable disable + +namespace Test +{ + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes/BackCompatAttributeType.cs new file mode 100644 index 00000000000..700a1ea9756 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes/BackCompatAttributeType.cs @@ -0,0 +1,10 @@ +using System; +using Microsoft.TypeSpec.Generator.Customizations; + +namespace Test +{ + [CodeGenSuppress("Foo")] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute.cs new file mode 100644 index 00000000000..65523f6f0ab --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute.cs @@ -0,0 +1,11 @@ +// + +#nullable disable + +namespace Test +{ + [global::Test.RestorableAttribute] + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute/BackCompatAttributeType.cs new file mode 100644 index 00000000000..ffc8d56b04c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute/BackCompatAttributeType.cs @@ -0,0 +1,15 @@ +using System; +using System.Reflection; + +namespace Test +{ + public class RestorableAttribute : Attribute + { + } + + [DefaultMember("Something")] + [Restorable] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute.cs new file mode 100644 index 00000000000..00d03a5061a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute.cs @@ -0,0 +1,10 @@ +// + +#nullable disable + +namespace Test +{ + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute/BackCompatAttributeType.cs new file mode 100644 index 00000000000..2b424b62126 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute/BackCompatAttributeType.cs @@ -0,0 +1,9 @@ +using System.ComponentModel; + +namespace Test +{ + [EditorBrowsable(EditorBrowsableState.Never)] + public class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute.cs new file mode 100644 index 00000000000..00d03a5061a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute.cs @@ -0,0 +1,10 @@ +// + +#nullable disable + +namespace Test +{ + public partial class BackCompatAttributeType + { + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute/BackCompatAttributeType.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute/BackCompatAttributeType.cs new file mode 100644 index 00000000000..1a8e3ad72e2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute/BackCompatAttributeType.cs @@ -0,0 +1,9 @@ +using System.Diagnostics.CodeAnalysis; + +namespace Test +{ + [Experimental("TEST001")] + public class BackCompatAttributeType + { + } +} 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 34e119f008a..3fb505cbddb 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 @@ -25,6 +25,201 @@ public void Setup() MockHelpers.LoadMockGenerator(); } + [Test] + public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract() + { + var provider = CreateAttributeTestProvider(attributes: + [ + new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("This is obsolete")), + new AttributeStatement(typeof(SerializableAttribute)), + ]); + + // With no last contract to restore attributes from, back-compat processing leaves the generated + // attributes unchanged. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContract() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract declares [CLSCompliant(true)] which is not present in the generated set, so it + // should be appended by back-compat processing. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // The generated set already contains the same [CLSCompliant(true)] attribute that the last + // contract declares, so nothing new is added. + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType", attributes: + [ + new AttributeStatement(typeof(CLSCompliantAttribute), Snippet.Literal(true)), + ]); + + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode() + { + await MockHelpers.LoadMockGeneratorAsync( + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Custom"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract declares a mix of attributes: [CLSCompliant(true)] which is also present in + // the custom code, and [Restorable] which is new. Only the [Restorable] attribute should be + // restored because the [CLSCompliant(true)] attribute already exists in the custom code. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract only declares a CodeGen-specific attribute, which is never restored, so the + // generated (empty) set is left unchanged. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract declares a mix: a CodeGen-prefixed attribute ([CodeGenModel("Something")]) + // that is not one of the explicitly-known CodeGen attributes, and a [Restorable] attribute. Only + // [Restorable] should be restored because any CodeGen-prefixed attribute is never restored. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract only declares an EditorBrowsable attribute, which generation owns and is + // never restored, so the generated (empty) set is left unchanged. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsExperimentalAttribute() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract only declares an Experimental attribute, which generation owns and is never + // restored, so the generated (empty) set is left unchanged. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract declares a mix: a [DefaultMember] attribute (which indicates specific + // runtime behavior and is never restored) and a [Restorable] attribute. Only [Restorable] + // should be restored. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract declares an attribute whose arguments use every integral literal kind + // (byte/sbyte/short/ushort/uint/ulong). These must be rendered without throwing so the + // attribute can be restored rather than crashing generation. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + private static string Write(TypeProvider provider) => + CodeModelGenerator.Instance.GetWriter(provider).Write().Content; + + [Test] + public async Task BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = CreateAttributeTestProvider( + name: "BackCompatAttributeType", + declarationModifiers: TypeSignatureModifiers.Internal | TypeSignatureModifiers.Class); + + // The last contract declares [CLSCompliant(true)], but attribute restoration only applies to + // externally visible (public or protected) types. Since the generated type is internal, nothing + // is restored. + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract() + { + await MockHelpers.LoadMockGeneratorAsync( + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Custom"), + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + // The type has an attribute in each of the three sources: + // - generated: [Obsolete("This is obsolete")] + // - custom code: [CLSCompliant(true)] + // - last contract: [Obsolete("This is obsolete")], [CLSCompliant(true)], [Restorable] + // Back-compat merging should not produce duplicates: the generated [Obsolete] and the + // custom-code [CLSCompliant] already cover two of the last-contract attributes, so only the + // new [Restorable] attribute is restored. + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType", attributes: + [ + new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("This is obsolete")), + ]); + + provider.ProcessTypeForBackCompatibility(); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + private static TestTypeProvider CreateAttributeTestProvider( + string? name = null, + IEnumerable? attributes = null, + TypeSignatureModifiers? declarationModifiers = null) => + new TestTypeProvider( + name: name ?? "TestName", + declarationModifiers: declarationModifiers ?? (TypeSignatureModifiers.Public | TypeSignatureModifiers.Class), + attributes: attributes); + [Test] public void TestUpdateCanonicalView() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/AttributeStatementTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/AttributeStatementTests.cs index 1643c492e14..b88c42dbb46 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/AttributeStatementTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/AttributeStatementTests.cs @@ -94,5 +94,22 @@ public void AttributeStatementWithArgumentsAndNamedArguments() Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false)); } + + [Test] + public void AttributeStatementWithIntegralArguments() + { + var attributeStatement = new AttributeStatement(typeof(CLSCompliantAttribute), + new LiteralExpression((byte)1), + new LiteralExpression((sbyte)2), + new LiteralExpression((short)3), + new LiteralExpression((ushort)4), + new LiteralExpression((uint)5), + new LiteralExpression((ulong)6)); + + using var writer = new CodeWriter(); + attributeStatement.Write(writer); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), writer.ToString(false)); + } } } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithIntegralArguments.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithIntegralArguments.cs new file mode 100644 index 00000000000..8df11821b34 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithIntegralArguments.cs @@ -0,0 +1 @@ +[global::System.CLSCompliantAttribute(1, 2, 3, 4, 5U, 6UL)] \ No newline at end of file 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 4b339e7bff4..cb4982aa5c6 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 @@ -5,6 +5,7 @@ using System.Linq; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Statements; namespace Microsoft.TypeSpec.Generator.Tests { @@ -14,6 +15,7 @@ internal class TestTypeProvider : TypeProvider private readonly MethodProvider[] _methods; private readonly PropertyProvider[] _properties; private readonly ConstructorProvider[] _constructors; + private readonly MethodBodyStatement[] _attributes; private readonly string _name; private readonly string _namespace; protected override string BuildRelativeFilePath() => $"{Name}.cs"; @@ -29,6 +31,8 @@ internal class TestTypeProvider : TypeProvider protected internal override ConstructorProvider[] BuildConstructors() => _constructors; protected override TypeProvider[] BuildNestedTypes() => NestedTypesInternal ?? base.BuildNestedTypes(); + protected override IReadOnlyList BuildAttributes() => _attributes; + public static readonly TypeProvider Empty = new TestTypeProvider(); internal TestTypeProvider( @@ -37,12 +41,14 @@ internal TestTypeProvider( IEnumerable? methods = null, IEnumerable? properties = null, string? ns = null, - IEnumerable? constructors = null) + IEnumerable? constructors = null, + IEnumerable? attributes = null) { _declarationModifiers = declarationModifiers; _methods = methods?.ToArray() ?? []; _properties = properties?.ToArray() ?? []; _constructors = constructors?.ToArray() ?? []; + _attributes = attributes?.ToArray() ?? []; _name = name ?? "TestName"; _namespace = ns ?? "Test"; }