diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs index 3d71670d5a9..0f2f51a6fb7 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 @@ -701,6 +701,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; @@ -735,7 +736,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) { @@ -761,7 +772,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); } } @@ -819,6 +830,56 @@ protected internal virtual IReadOnlyList BuildMethodsForBackComp protected internal virtual IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) => [.. originalConstructors]; + /// + /// Adds any back-compatibility attributes from the last contract that are not already present in + /// (or the custom-code attributes). CodeGen-specific + /// attributes are never restored. The original attributes are returned unchanged when there is + /// nothing new to add. + /// + protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + { + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; + + 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) + { + seen.Add(attribute.ToDisplayString()); + } + foreach (var attribute in CustomCodeView?.Attributes ?? []) + { + seen.Add(attribute.ToDisplayString()); + } + + List? merged = null; + foreach (var attribute in lastContractAttributes) + { + if (ShouldPreserveLastContractAttribute(attribute) && seen.Add(attribute.ToDisplayString())) + { + merged ??= [.. original]; + merged.Add(attribute); + } + } + + return merged ?? original; + } + + private static bool ShouldPreserveLastContractAttribute(AttributeStatement attribute) + { + var attributeName = attribute.Data?.AttributeClass?.Name; + return attributeName is not + (CodeGenAttributes.CodeGenSuppressAttributeName or + CodeGenAttributes.CodeGenMemberAttributeName or + CodeGenAttributes.CodeGenTypeAttributeName or + CodeGenAttributes.CodeGenSerializationAttributeName); + } + private IReadOnlyList? _enumValues; private bool ShouldGenerate(ConstructorProvider constructor) 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..b8ae0486c1c --- /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.ObsoleteAttribute("bc")] + 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..c1a103978e3 --- /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 +{ + [Obsolete("bc")] + 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..b8ae0486c1c --- /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.ObsoleteAttribute("bc")] + 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..c1a103978e3 --- /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 +{ + [Obsolete("bc")] + 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..59a95fbad6e --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract.cs @@ -0,0 +1,15 @@ +// + +#nullable disable + +using System; + +namespace Test +{ + [global::System.ObsoleteAttribute] + [global::System.ObsoleteAttribute] + [global::System.ObsoleteAttribute("This is obsolete")] + public partial class TestName + { + } +} 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/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 9ec0b21c32d..73a27613b3d 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 @@ -24,6 +24,97 @@ public void Setup() MockHelpers.LoadMockGenerator(); } + [Test] + public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract() + { + var provider = new AttributeTestProvider(); + + // With no last contract to restore attributes from, the method only ever adds new back-compat + // attributes, so it returns the original attributes unchanged (without deduplicating them). + var attributes = provider.GetBackCompatibilityAttributes(); + provider.Update(attributes: attributes); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContract() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract declares [Obsolete("bc")] which is not present in the original set, so it + // should be appended to the result. + var attributes = provider.GetBackCompatibilityAttributes([]); + provider.Update(attributes: attributes); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + + // The original set already contains the same [Obsolete("bc")] attribute that the last contract + // declares, so nothing new is added and the original list is returned unchanged. + IReadOnlyList original = + [ + new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("bc")), + ]; + var attributes = provider.GetBackCompatibilityAttributes(original); + + // No new attribute is added, so the original list is returned by reference (no allocation). + Assert.AreSame(original, attributes); + + provider.Update(attributes: attributes); + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract only declares a CodeGen-specific attribute, which is never restored, so the + // original (empty) list is returned unchanged. + var original = Array.Empty(); + var attributes = provider.GetBackCompatibilityAttributes(original); + + // Nothing is restored, so the original list is returned by reference (no allocation). + Assert.AreSame(original, attributes); + + provider.Update(attributes: attributes); + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + private static string Write(TypeProvider provider) => + CodeModelGenerator.Instance.GetWriter(provider).Write().Content; + + private class AttributeTestProvider : TestTypeProvider + { + public AttributeTestProvider(string? name = null) + : base(name: name) + { + } + + protected override TypeSignatureModifiers BuildDeclarationModifiers() => + TypeSignatureModifiers.Public | TypeSignatureModifiers.Class; + + protected override IReadOnlyList BuildAttributes() => + [ + new AttributeStatement(typeof(ObsoleteAttribute)), + new AttributeStatement(typeof(ObsoleteAttribute)), + new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("This is obsolete")), + ]; + + public IReadOnlyList GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(Attributes); + + public IReadOnlyList GetBackCompatibilityAttributes(IEnumerable original) => BuildAttributesForBackCompatibility(original); + } + [Test] public void TestUpdateCanonicalView() {