From 08d2fc3d07a2249c801097b371db19f3cec40f46 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 20:08:14 +0000 Subject: [PATCH 1/8] Initial plan From 6a86f1b35ffcaa342c7f158bf3e98998f6701fad Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 20:17:17 +0000 Subject: [PATCH 2/8] Centralize C# type attribute back-compat merging in TypeProvider Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/CanonicalTypeProvider.cs | 2 +- .../src/Providers/TypeProvider.cs | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs index b2c2915f915..a445d7adad2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs @@ -48,7 +48,7 @@ public CanonicalTypeProvider(TypeProvider generatedTypeProvider, InputType? inpu protected override IReadOnlyList BuildAttributes() { - return [.. _generatedTypeProvider.Attributes, .. _generatedTypeProvider.CustomCodeView?.Attributes ?? []]; + return _generatedTypeProvider.BuildAttributesForBackCompatibility(); } private protected override CanonicalTypeProvider BuildCanonicalView() => this; 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..0b69e489616 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 @@ -819,6 +819,52 @@ protected internal virtual IReadOnlyList BuildMethodsForBackComp protected internal virtual IReadOnlyList BuildConstructorsForBackCompatibility(IEnumerable originalConstructors) => [.. originalConstructors]; + /// + /// Builds the set of type-level attributes to emit, preserving back-compatibility by merging the + /// generated attributes with the compatible attributes restored from the last contract and the + /// custom-code attributes. Attributes restored from the last contract are limited to non-public + /// types and exclude CodeGen-specific attributes that should not be preserved. The merged result is + /// deduplicated so identical attributes are only emitted once. + /// + protected internal IReadOnlyList BuildAttributesForBackCompatibility() + { + // Only non-public types can be internalized for back-compatibility, so attributes from the last + // contract are only restored for them. + var lastContractAttributes = DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) + ? null + : LastContractView?.Attributes.Where(ShouldPreserveLastContractAttribute); + + return DeduplicateAttributes( + Attributes, + lastContractAttributes, + CustomCodeView?.Attributes); + } + + private static IReadOnlyList DeduplicateAttributes(params IEnumerable?[] attributeSets) + { + var seen = new HashSet(); + var attributes = new List(); + foreach (var attribute in attributeSets.SelectMany(static attributeSet => attributeSet ?? [])) + { + if (seen.Add(attribute.ToDisplayString())) + { + attributes.Add(attribute); + } + } + + return attributes; + } + + 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) From 83afc6f0a3fee43d693ad45ec72de17f7afd2f00 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 20:37:04 +0000 Subject: [PATCH 3/8] Add unit test for BuildAttributesForBackCompatibility dedup Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../test/Providers/TypeProviderTests.cs | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) 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..32af56056f6 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,34 @@ public void Setup() MockHelpers.LoadMockGenerator(); } + [Test] + public void BuildAttributesForBackCompatibilityDeduplicatesAttributes() + { + var provider = new AttributeTestProvider(); + var attributes = provider.GetBackCompatibilityAttributes(); + + // The three generated attributes contain a duplicate ObsoleteAttribute which should be collapsed. + Assert.AreEqual(2, attributes.Count); + var rendered = attributes.Select(a => a.ToDisplayString()).ToList(); + Assert.AreEqual(rendered.Count, rendered.Distinct().Count()); + } + + private class AttributeTestProvider : TestTypeProvider + { + internal AttributeTestProvider() : base() + { + } + + 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(); + } + [Test] public void TestUpdateCanonicalView() { From bbb6960ecd199c913767281436845272d35d288b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 20:38:23 +0000 Subject: [PATCH 4/8] Remove redundant test constructor per review feedback Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../test/Providers/TypeProviderTests.cs | 4 ---- 1 file changed, 4 deletions(-) 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 32af56056f6..f82059ee506 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 @@ -38,10 +38,6 @@ public void BuildAttributesForBackCompatibilityDeduplicatesAttributes() private class AttributeTestProvider : TestTypeProvider { - internal AttributeTestProvider() : base() - { - } - protected override IReadOnlyList BuildAttributes() => [ new AttributeStatement(typeof(ObsoleteAttribute)), From e5769dbe4bb5d269e4fa2770fa527f27550ebe6b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 21:03:05 +0000 Subject: [PATCH 5/8] Address review: revert canonical delegation, only add new back-compat attributes, perf Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/CanonicalTypeProvider.cs | 2 +- .../src/Providers/TypeProvider.cs | 68 ++++++++++++------- .../test/Providers/TypeProviderTests.cs | 9 ++- 3 files changed, 50 insertions(+), 29 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs index a445d7adad2..b2c2915f915 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/CanonicalTypeProvider.cs @@ -48,7 +48,7 @@ public CanonicalTypeProvider(TypeProvider generatedTypeProvider, InputType? inpu protected override IReadOnlyList BuildAttributes() { - return _generatedTypeProvider.BuildAttributesForBackCompatibility(); + return [.. _generatedTypeProvider.Attributes, .. _generatedTypeProvider.CustomCodeView?.Attributes ?? []]; } private protected override CanonicalTypeProvider BuildCanonicalView() => this; 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 0b69e489616..552d924c4ad 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 != null && LastContractView.Attributes.Count > 0; IReadOnlyList? updatedEnumValues = null; IEnumerable? newFields = null; @@ -735,7 +736,20 @@ internal void ProcessTypeForBackCompatibility() var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null; var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null; - if (newFields != null || newMethods != null || newConstructors != null) + // Restore any back-compat attributes from the last contract that are not already present in the + // generated or custom-code attributes. Because attributes are only ever added (never removed), + // a larger count than the current attributes indicates new attributes were restored. + IReadOnlyList? newAttributes = null; + if (hasAttributes) + { + var backCompatAttributes = BuildAttributesForBackCompatibility(); + if (backCompatAttributes.Count != Attributes.Count) + { + newAttributes = backCompatAttributes; + } + } + + if (newFields != null || newMethods != null || newConstructors != null || newAttributes != null) { if (updatedEnumValues != null) { @@ -761,7 +775,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); } } @@ -820,39 +834,47 @@ protected internal virtual IReadOnlyList BuildConstructorsF => [.. originalConstructors]; /// - /// Builds the set of type-level attributes to emit, preserving back-compatibility by merging the - /// generated attributes with the compatible attributes restored from the last contract and the - /// custom-code attributes. Attributes restored from the last contract are limited to non-public - /// types and exclude CodeGen-specific attributes that should not be preserved. The merged result is - /// deduplicated so identical attributes are only emitted once. + /// Builds the set of type-level attributes to emit, adding any back-compatibility attributes + /// restored from the last contract that are not already present in the original set of generated + /// and custom-code attributes. Attributes are only restored for non-public types (which are the + /// only ones that can be internalized) and CodeGen-specific attributes are never restored. The + /// original attributes are returned unchanged when there is nothing new to add. /// protected internal IReadOnlyList BuildAttributesForBackCompatibility() { + var originalAttributes = Attributes; + // Only non-public types can be internalized for back-compatibility, so attributes from the last // contract are only restored for them. - var lastContractAttributes = DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) - ? null - : LastContractView?.Attributes.Where(ShouldPreserveLastContractAttribute); - - return DeduplicateAttributes( - Attributes, - lastContractAttributes, - CustomCodeView?.Attributes); - } + if (DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) || + LastContractView?.Attributes is not { Count: > 0 } lastContractAttributes) + { + return originalAttributes; + } - private static IReadOnlyList DeduplicateAttributes(params IEnumerable?[] attributeSets) - { + // Track the original (generated + custom) attributes so we only add back-compat attributes + // that don't already exist. var seen = new HashSet(); - var attributes = new List(); - foreach (var attribute in attributeSets.SelectMany(static attributeSet => attributeSet ?? [])) + foreach (var attribute in originalAttributes) + { + seen.Add(attribute.ToDisplayString()); + } + foreach (var attribute in CustomCodeView?.Attributes ?? []) + { + seen.Add(attribute.ToDisplayString()); + } + + List? merged = null; + foreach (var attribute in lastContractAttributes) { - if (seen.Add(attribute.ToDisplayString())) + if (ShouldPreserveLastContractAttribute(attribute) && seen.Add(attribute.ToDisplayString())) { - attributes.Add(attribute); + merged ??= [.. originalAttributes]; + merged.Add(attribute); } } - return attributes; + return merged ?? originalAttributes; } private static bool ShouldPreserveLastContractAttribute(AttributeStatement attribute) 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 f82059ee506..abc6444d1ac 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,15 +25,14 @@ public void Setup() } [Test] - public void BuildAttributesForBackCompatibilityDeduplicatesAttributes() + public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract() { var provider = new AttributeTestProvider(); var attributes = provider.GetBackCompatibilityAttributes(); - // The three generated attributes contain a duplicate ObsoleteAttribute which should be collapsed. - Assert.AreEqual(2, attributes.Count); - var rendered = attributes.Select(a => a.ToDisplayString()).ToList(); - Assert.AreEqual(rendered.Count, rendered.Distinct().Count()); + // With no last contract to restore attributes from, the method only ever adds new back-compat + // attributes, so it returns the generated attributes unchanged (without deduplicating them). + Assert.AreEqual(3, attributes.Count); } private class AttributeTestProvider : TestTypeProvider From 4f3265acd5b0e4ee300f6112034eb5fe61bcd11d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 21:04:44 +0000 Subject: [PATCH 6/8] Refine back-compat attribute comment and skip work for public types Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 552d924c4ad..7f56663733c 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,7 +701,10 @@ 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 != null && LastContractView.Attributes.Count > 0; + // Only non-public types can be internalized for back-compatibility, so there is no attribute + // work to do for public types even when the last contract declares attributes. + var hasAttributes = !DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) && + LastContractView?.Attributes is { Count: > 0 }; IReadOnlyList? updatedEnumValues = null; IEnumerable? newFields = null; @@ -737,8 +740,9 @@ internal void ProcessTypeForBackCompatibility() var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null; // Restore any back-compat attributes from the last contract that are not already present in the - // generated or custom-code attributes. Because attributes are only ever added (never removed), - // a larger count than the current attributes indicates new attributes were restored. + // generated or custom-code attributes. BuildAttributesForBackCompatibility returns either the + // current attributes unchanged or a superset with the restored attributes appended, so a larger + // count reliably signals that new attributes were added. IReadOnlyList? newAttributes = null; if (hasAttributes) { From 921c0ecb5e43c098a4e6eb10c2cc87c6fb8aaa48 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 21:37:34 +0000 Subject: [PATCH 7/8] Address review: parameterize BuildAttributesForBackCompatibility, drop internalize gating, add tests Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 33 +++++------ .../BackCompatAttributeType.cs | 9 +++ .../BackCompatAttributeType.cs | 9 +++ .../BackCompatAttributeType.cs | 10 ++++ .../test/Providers/TypeProviderTests.cs | 58 ++++++++++++++++++- 5 files changed, 97 insertions(+), 22 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract/BackCompatAttributeType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute/BackCompatAttributeType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes/BackCompatAttributeType.cs 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 7f56663733c..618e6a23c21 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,10 +701,7 @@ internal void ProcessTypeForBackCompatibility() { var hasMethods = LastContractView?.Methods != null && LastContractView.Methods.Count > 0; var hasConstructors = LastContractView?.Constructors != null && LastContractView.Constructors.Count > 0; - // Only non-public types can be internalized for back-compatibility, so there is no attribute - // work to do for public types even when the last contract declares attributes. - var hasAttributes = !DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) && - LastContractView?.Attributes is { Count: > 0 }; + var hasAttributes = LastContractView?.Attributes is { Count: > 0 }; IReadOnlyList? updatedEnumValues = null; IEnumerable? newFields = null; @@ -746,7 +743,7 @@ internal void ProcessTypeForBackCompatibility() IReadOnlyList? newAttributes = null; if (hasAttributes) { - var backCompatAttributes = BuildAttributesForBackCompatibility(); + var backCompatAttributes = BuildAttributesForBackCompatibility(Attributes); if (backCompatAttributes.Count != Attributes.Count) { newAttributes = backCompatAttributes; @@ -838,28 +835,24 @@ protected internal virtual IReadOnlyList BuildConstructorsF => [.. originalConstructors]; /// - /// Builds the set of type-level attributes to emit, adding any back-compatibility attributes - /// restored from the last contract that are not already present in the original set of generated - /// and custom-code attributes. Attributes are only restored for non-public types (which are the - /// only ones that can be internalized) and CodeGen-specific attributes are never restored. The - /// original attributes are returned unchanged when there is nothing new to add. + /// 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 IReadOnlyList BuildAttributesForBackCompatibility() + protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) { - var originalAttributes = Attributes; + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; - // Only non-public types can be internalized for back-compatibility, so attributes from the last - // contract are only restored for them. - if (DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) || - LastContractView?.Attributes is not { Count: > 0 } lastContractAttributes) + if (LastContractView?.Attributes is not { Count: > 0 } lastContractAttributes) { - return originalAttributes; + 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 originalAttributes) + foreach (var attribute in original) { seen.Add(attribute.ToDisplayString()); } @@ -873,12 +866,12 @@ protected internal IReadOnlyList BuildAttributesForBackCompa { if (ShouldPreserveLastContractAttribute(attribute) && seen.Add(attribute.ToDisplayString())) { - merged ??= [.. originalAttributes]; + merged ??= [.. original]; merged.Add(attribute); } } - return merged ?? originalAttributes; + return merged ?? original; } private static bool ShouldPreserveLastContractAttribute(AttributeStatement attribute) 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/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/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 abc6444d1ac..1591654fcfd 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 @@ -31,12 +31,64 @@ public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoL var attributes = provider.GetBackCompatibilityAttributes(); // With no last contract to restore attributes from, the method only ever adds new back-compat - // attributes, so it returns the generated attributes unchanged (without deduplicating them). + // attributes, so it returns the original attributes unchanged (without deduplicating them). Assert.AreEqual(3, attributes.Count); } + [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([]); + + Assert.AreEqual(1, attributes.Count); + Assert.AreEqual("ObsoleteAttribute", attributes[0].Type.Name); + } + + [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); + + Assert.AreEqual(1, attributes.Count); + Assert.AreSame(original, attributes); + } + + [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); + + Assert.AreEqual(0, attributes.Count); + Assert.AreSame(original, attributes); + } + private class AttributeTestProvider : TestTypeProvider { + public AttributeTestProvider(string? name = null) + : base(name: name) + { + } + protected override IReadOnlyList BuildAttributes() => [ new AttributeStatement(typeof(ObsoleteAttribute)), @@ -44,7 +96,9 @@ protected override IReadOnlyList BuildAttributes() => new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("This is obsolete")), ]; - public IReadOnlyList GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(); + public IReadOnlyList GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(Attributes); + + public IReadOnlyList GetBackCompatibilityAttributes(IEnumerable original) => BuildAttributesForBackCompatibility(original); } [Test] From ff61d50e12b983368047da1057055a46fda85093 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Jul 2026 22:39:01 +0000 Subject: [PATCH 8/8] Address review: remove comment, use TestData for generated-code assertions Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 4 --- ...patibilityAddsAttributeFromLastContract.cs | 13 ++++++++++ ...bilityDoesNotDuplicateExistingAttribute.cs | 13 ++++++++++ ...nsGeneratedAttributesWhenNoLastContract.cs | 15 +++++++++++ ...BackCompatibilitySkipsCodeGenAttributes.cs | 10 +++++++ .../test/Providers/TypeProviderTests.cs | 26 ++++++++++++++----- 6 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityAddsAttributeFromLastContract.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsCodeGenAttributes.cs 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 618e6a23c21..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 @@ -736,10 +736,6 @@ internal void ProcessTypeForBackCompatibility() var newMethods = hasMethods ? BuildMethodsForBackCompatibility(Methods) : null; var newConstructors = hasConstructors ? BuildConstructorsForBackCompatibility(Constructors) : null; - // Restore any back-compat attributes from the last contract that are not already present in the - // generated or custom-code attributes. BuildAttributesForBackCompatibility returns either the - // current attributes unchanged or a superset with the restored attributes appended, so a larger - // count reliably signals that new attributes were added. IReadOnlyList? newAttributes = null; if (hasAttributes) { 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/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/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/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 1591654fcfd..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 @@ -28,11 +28,13 @@ public void Setup() public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract() { var provider = new AttributeTestProvider(); - var attributes = provider.GetBackCompatibilityAttributes(); // 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). - Assert.AreEqual(3, attributes.Count); + var attributes = provider.GetBackCompatibilityAttributes(); + provider.Update(attributes: attributes); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } [Test] @@ -44,9 +46,9 @@ public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContra // 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(1, attributes.Count); - Assert.AreEqual("ObsoleteAttribute", attributes[0].Type.Name); + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } [Test] @@ -63,8 +65,11 @@ public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAtt ]; var attributes = provider.GetBackCompatibilityAttributes(original); - Assert.AreEqual(1, attributes.Count); + // 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] @@ -78,10 +83,16 @@ public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() var original = Array.Empty(); var attributes = provider.GetBackCompatibilityAttributes(original); - Assert.AreEqual(0, attributes.Count); + // 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) @@ -89,6 +100,9 @@ public AttributeTestProvider(string? name = null) { } + protected override TypeSignatureModifiers BuildDeclarationModifiers() => + TypeSignatureModifiers.Public | TypeSignatureModifiers.Class; + protected override IReadOnlyList BuildAttributes() => [ new AttributeStatement(typeof(ObsoleteAttribute)),