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 01/30] 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 02/30] 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 03/30] 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 04/30] 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 05/30] 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 06/30] 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 07/30] 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 08/30] 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)), From f74deb3d24cdf7983ae41ff96149949350a4a93b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:43:19 +0000 Subject: [PATCH 09/30] Restore attributes unless known non-restorable; MRW skips buildable Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 40 +++++++++++++++++++ ...ModelReaderWriterContextDefinitionTests.cs | 27 +++++++++++++ .../SampleContext.cs | 11 +++++ .../src/Providers/TypeProvider.cs | 25 ++++++++---- ...patibilitySkipsEditorBrowsableAttribute.cs | 10 +++++ .../BackCompatAttributeType.cs | 9 +++++ ...CompatibilitySkipsExperimentalAttribute.cs | 10 +++++ .../BackCompatAttributeType.cs | 9 +++++ .../test/Providers/TypeProviderTests.cs | 34 ++++++++++++++++ 9 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute/SampleContext.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute/BackCompatAttributeType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsExperimentalAttribute/BackCompatAttributeType.cs 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 7d5cc8976e6..7a45ef5b134 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 @@ -98,6 +98,46 @@ 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 + /// that would be restored. The buildable + /// attributes are recomputed at write time from the final provider selection, so restoring stale + /// copies from the last contract would be incorrect. + /// + protected override IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + { + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; + var merged = base.BuildAttributesForBackCompatibility(original); + + // base returns the original list unchanged when nothing was restored. + if (ReferenceEquals(merged, original)) + { + return merged; + } + + // base only appends attributes that are new relative to the original set, so keep the + // originally-generated buildable attributes while dropping any restored from the last contract. + var originalDisplayStrings = new HashSet(StringComparer.Ordinal); + foreach (var attribute in original) + { + originalDisplayStrings.Add(attribute.ToDisplayString()); + } + + var result = new List(merged.Count); + foreach (var attribute in merged) + { + if (attribute.Type.Equals(typeof(ModelReaderWriterBuildableAttribute)) + && !originalDisplayStrings.Contains(attribute.ToDisplayString())) + { + continue; + } + + result.Add(attribute); + } + + return result; + } + private static bool IsBuildableAttribute(MethodBodyStatement statement) { var attribute = statement switch 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 d55537cc0d0..8e881a66f3d 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 @@ -1856,6 +1856,33 @@ 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 TestableModelReaderWriterContextDefinition(); + + // The last contract declares a ModelReaderWriterBuildable attribute (owned by generation and + // rebuilt at write time) alongside an Obsolete attribute. Only the non-buildable Obsolete + // attribute should be restored. + var result = contextDefinition.BuildAttributesForBackCompatibilityPublic([]); + + Assert.IsFalse( + result.Any(a => a.Type.Equals(typeof(ModelReaderWriterBuildableAttribute))), + "ModelReaderWriterBuildable attributes must not be restored from the last contract."); + Assert.IsTrue( + result.Any(a => a.Type.Equals(typeof(ObsoleteAttribute))), + "Non-buildable back-compat attributes should still be restored."); + } + + private class TestableModelReaderWriterContextDefinition : ModelReaderWriterContextDefinition + { + public IReadOnlyList BuildAttributesForBackCompatibilityPublic(IEnumerable originalAttributes) + => BuildAttributesForBackCompatibility(originalAttributes); + } + 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/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..81d2f20b6e5 --- /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; +using System.ClientModel.Primitives; + +namespace Sample +{ + [ModelReaderWriterBuildable(typeof(object))] + [Obsolete("bc")] + public partial class SampleContext : ModelReaderWriterContext + { + } +} 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 1b5669bcffb..4ebd47b8037 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 @@ -1032,11 +1032,24 @@ protected internal virtual IReadOnlyList BuildConstructorsF return [.. constructors]; } + // Attributes that should never be restored from the last contract. These are either CodeGen + // customization markers or attributes that generation (re)applies based on the current inputs, + // so restoring a stale copy from the last contract would be incorrect. + private static readonly HashSet s_attributesNotToRestore = new(StringComparer.Ordinal) + { + CodeGenAttributes.CodeGenSuppressAttributeName, + CodeGenAttributes.CodeGenMemberAttributeName, + CodeGenAttributes.CodeGenTypeAttributeName, + CodeGenAttributes.CodeGenSerializationAttributeName, + nameof(System.ComponentModel.EditorBrowsableAttribute), + nameof(System.Diagnostics.CodeAnalysis.ExperimentalAttribute), + }; + /// /// 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. + /// (or the custom-code attributes). Attributes that + /// generation owns (see ) are never restored. The original + /// attributes are returned unchanged when there is nothing new to add. /// protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) { @@ -1075,11 +1088,7 @@ protected internal virtual IReadOnlyList BuildAttributesForB 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); + return attributeName is null || !s_attributesNotToRestore.Contains(attributeName); } private IReadOnlyList? _enumValues; 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 01245e4df95..a4ae4ba1a17 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 @@ -91,6 +91,40 @@ public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract only declares an EditorBrowsable attribute, which generation owns and is + // never restored, so the original (empty) list is returned unchanged. + var original = Array.Empty(); + var attributes = provider.GetBackCompatibilityAttributes(original); + + Assert.AreSame(original, attributes); + + provider.Update(attributes: attributes); + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + + [Test] + public async Task BuildAttributesForBackCompatibilitySkipsExperimentalAttribute() + { + await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract only declares an Experimental attribute, which generation owns and is never + // restored, so the original (empty) list is returned unchanged. + var original = Array.Empty(); + var attributes = provider.GetBackCompatibilityAttributes(original); + + 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; From 2d19520db60bded037dcf5f69a3f52c16bbe04c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 18:12:52 +0000 Subject: [PATCH 10/30] Address review: virtual predicate for back-compat attrs, add Obsolete to ignore list, fix tests Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 47 ++++--------------- ...ModelReaderWriterContextDefinitionTests.cs | 4 +- .../SampleContext.cs | 4 +- .../src/Providers/TypeProvider.cs | 29 +++++++----- ...patibilityAddsAttributeFromLastContract.cs | 2 +- .../BackCompatAttributeType.cs | 2 +- ...bilityDoesNotDuplicateExistingAttribute.cs | 2 +- .../BackCompatAttributeType.cs | 2 +- ...nsGeneratedAttributesWhenNoLastContract.cs | 3 +- .../BackCompatAttributeType.cs | 9 ++++ ...ilitySkipsAttributesAlreadyInCustomCode.cs | 13 +++++ .../BackCompatAttributeType.cs | 10 ++++ .../test/Providers/TypeProviderTests.cs | 32 +++++++++---- 13 files changed, 92 insertions(+), 67 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode(Custom)/BackCompatAttributeType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode/BackCompatAttributeType.cs 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 7a45ef5b134..d903b6b0626 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 @@ -98,45 +98,18 @@ 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 - /// that would be restored. The buildable - /// attributes are recomputed at write time from the final provider selection, so restoring stale - /// copies from the last contract would be incorrect. - /// - protected override IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) { - var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; - var merged = base.BuildAttributesForBackCompatibility(original); - - // base returns the original list unchanged when nothing was restored. - if (ReferenceEquals(merged, original)) - { - return merged; - } + nameof(ModelReaderWriterBuildableAttribute), + }); - // base only appends attributes that are new relative to the original set, so keep the - // originally-generated buildable attributes while dropping any restored from the last contract. - var originalDisplayStrings = new HashSet(StringComparer.Ordinal); - foreach (var attribute in original) - { - originalDisplayStrings.Add(attribute.ToDisplayString()); - } - - var result = new List(merged.Count); - foreach (var attribute in merged) - { - if (attribute.Type.Equals(typeof(ModelReaderWriterBuildableAttribute)) - && !originalDisplayStrings.Contains(attribute.ToDisplayString())) - { - continue; - } - - result.Add(attribute); - } - - return result; - } + /// + /// Restores back-compatibility attributes from the last contract, excluding any + /// . + /// + protected override bool ShouldRestoreLastContractAttribute(AttributeStatement attribute) + => base.ShouldRestoreLastContractAttribute(attribute) + && !s_attributesToIgnore.Value.Contains(attribute.Type.Name); private static bool IsBuildableAttribute(MethodBodyStatement statement) { 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 8e881a66f3d..dedccc313ec 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 @@ -1865,7 +1865,7 @@ await MockHelpers.LoadMockGeneratorAsync( var contextDefinition = new TestableModelReaderWriterContextDefinition(); // The last contract declares a ModelReaderWriterBuildable attribute (owned by generation and - // rebuilt at write time) alongside an Obsolete attribute. Only the non-buildable Obsolete + // rebuilt at write time) alongside a Description attribute. Only the non-buildable Description // attribute should be restored. var result = contextDefinition.BuildAttributesForBackCompatibilityPublic([]); @@ -1873,7 +1873,7 @@ await MockHelpers.LoadMockGeneratorAsync( result.Any(a => a.Type.Equals(typeof(ModelReaderWriterBuildableAttribute))), "ModelReaderWriterBuildable attributes must not be restored from the last contract."); Assert.IsTrue( - result.Any(a => a.Type.Equals(typeof(ObsoleteAttribute))), + result.Any(a => a.Type.Equals(typeof(System.ComponentModel.DescriptionAttribute))), "Non-buildable back-compat attributes should still be restored."); } 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 index 81d2f20b6e5..59e2f4bbab2 100644 --- 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 @@ -1,10 +1,10 @@ -using System; +using System.ComponentModel; using System.ClientModel.Primitives; namespace Sample { [ModelReaderWriterBuildable(typeof(object))] - [Obsolete("bc")] + [Description("bc")] public partial class SampleContext : ModelReaderWriterContext { } 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 4ebd47b8037..45fa71986e5 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,6 +3,8 @@ using System; using System.Collections.Generic; +using System.ComponentModel; +using System.Diagnostics.CodeAnalysis; using System.Linq; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -1032,24 +1034,22 @@ protected internal virtual IReadOnlyList BuildConstructorsF return [.. constructors]; } - // Attributes that should never be restored from the last contract. These are either CodeGen - // customization markers or attributes that generation (re)applies based on the current inputs, - // so restoring a stale copy from the last contract would be incorrect. - private static readonly HashSet s_attributesNotToRestore = new(StringComparer.Ordinal) + private static readonly Lazy> s_nonRestorableAttributeNames = new(() => new(StringComparer.Ordinal) { CodeGenAttributes.CodeGenSuppressAttributeName, CodeGenAttributes.CodeGenMemberAttributeName, CodeGenAttributes.CodeGenTypeAttributeName, CodeGenAttributes.CodeGenSerializationAttributeName, - nameof(System.ComponentModel.EditorBrowsableAttribute), - nameof(System.Diagnostics.CodeAnalysis.ExperimentalAttribute), - }; + nameof(EditorBrowsableAttribute), + nameof(ExperimentalAttribute), + nameof(ObsoleteAttribute), + }); /// /// Adds any back-compatibility attributes from the last contract that are not already present in /// (or the custom-code attributes). Attributes that - /// generation owns (see ) are never restored. The original - /// attributes are returned unchanged when there is nothing new to add. + /// generation owns (see ) are never restored. The + /// original attributes are returned unchanged when there is nothing new to add. /// protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) { @@ -1075,7 +1075,7 @@ protected internal virtual IReadOnlyList BuildAttributesForB List? merged = null; foreach (var attribute in lastContractAttributes) { - if (ShouldPreserveLastContractAttribute(attribute) && seen.Add(attribute.ToDisplayString())) + if (ShouldRestoreLastContractAttribute(attribute) && seen.Add(attribute.ToDisplayString())) { merged ??= [.. original]; merged.Add(attribute); @@ -1085,10 +1085,15 @@ protected internal virtual IReadOnlyList BuildAttributesForB return merged ?? original; } - private static bool ShouldPreserveLastContractAttribute(AttributeStatement attribute) + /// + /// Determines whether an attribute declared on the last contract should be restored onto the + /// current generation. Attributes that generation owns (see ) + /// are never restored. Derived types can override this to exclude additional attributes. + /// + protected virtual bool ShouldRestoreLastContractAttribute(AttributeStatement attribute) { var attributeName = attribute.Data?.AttributeClass?.Name; - return attributeName is null || !s_attributesNotToRestore.Contains(attributeName); + return attributeName is null || !s_nonRestorableAttributeNames.Value.Contains(attributeName); } private IReadOnlyList? _enumValues; 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 index b8ae0486c1c..56d4291a2e8 100644 --- 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 @@ -6,7 +6,7 @@ namespace Test { - [global::System.ObsoleteAttribute("bc")] + [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 index c1a103978e3..be4634aaa7e 100644 --- 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 @@ -2,7 +2,7 @@ namespace Test { - [Obsolete("bc")] + [CLSCompliant(true)] 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 index b8ae0486c1c..56d4291a2e8 100644 --- 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 @@ -6,7 +6,7 @@ namespace Test { - [global::System.ObsoleteAttribute("bc")] + [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 index c1a103978e3..be4634aaa7e 100644 --- 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 @@ -2,7 +2,7 @@ namespace Test { - [Obsolete("bc")] + [CLSCompliant(true)] 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 index 59a95fbad6e..8240d63b051 100644 --- 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 @@ -6,9 +6,8 @@ namespace Test { - [global::System.ObsoleteAttribute] - [global::System.ObsoleteAttribute] [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/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..c64efdd2fa2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode.cs @@ -0,0 +1,13 @@ +// + +#nullable disable + +using System; + +namespace Test +{ + [global::System.SerializableAttribute] + 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..afd84d66669 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCustomCode/BackCompatAttributeType.cs @@ -0,0 +1,10 @@ +using System; + +namespace Test +{ + [CLSCompliant(true)] + [Serializable] + 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 a4ae4ba1a17..39cec3b0ca2 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 @@ -30,8 +30,8 @@ public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoL { 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). + // With no last contract to restore attributes from, the method returns the generated + // attributes unchanged. var attributes = provider.GetBackCompatibilityAttributes(); provider.Update(attributes: attributes); @@ -44,7 +44,7 @@ public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContra 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 + // The last contract declares [CLSCompliant(true)] which is not present in the original set, so it // should be appended to the result. var attributes = provider.GetBackCompatibilityAttributes([]); provider.Update(attributes: attributes); @@ -58,11 +58,11 @@ public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAtt 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. + // The original set already contains the same [CLSCompliant(true)] 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")), + new AttributeStatement(typeof(CLSCompliantAttribute), Snippet.Literal(true)), ]; var attributes = provider.GetBackCompatibilityAttributes(original); @@ -73,6 +73,23 @@ public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAtt 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 = new AttributeTestProvider(name: "BackCompatAttributeType"); + + // The last contract declares a mix of attributes: [CLSCompliant(true)] which is also present in + // the custom code, and [Serializable] which is new. Only the [Serializable] attribute should be + // restored because the [CLSCompliant(true)] attribute already exists in the custom code. + var attributes = provider.GetBackCompatibilityAttributes([]); + provider.Update(attributes: attributes); + + Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); + } + [Test] public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() { @@ -140,9 +157,8 @@ protected override TypeSignatureModifiers BuildDeclarationModifiers() => protected override IReadOnlyList BuildAttributes() => [ - new AttributeStatement(typeof(ObsoleteAttribute)), - new AttributeStatement(typeof(ObsoleteAttribute)), new AttributeStatement(typeof(ObsoleteAttribute), Snippet.Literal("This is obsolete")), + new AttributeStatement(typeof(SerializableAttribute)), ]; public IReadOnlyList GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(Attributes); From 64f6b163ff85dcaced26089308e7b152cb5e382a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:26:02 +0000 Subject: [PATCH 11/30] Address review: remove virtual predicate, centralize back-compat attr merge in shared overload Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 7 ++-- .../src/Providers/TypeProvider.cs | 33 ++++++++++++------- 2 files changed, 24 insertions(+), 16 deletions(-) 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 d903b6b0626..b93523a3889 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 @@ -105,11 +105,10 @@ protected override IReadOnlyList BuildAttributes() /// /// Restores back-compatibility attributes from the last contract, excluding any - /// . + /// since those are recomputed at write time. /// - protected override bool ShouldRestoreLastContractAttribute(AttributeStatement attribute) - => base.ShouldRestoreLastContractAttribute(attribute) - && !s_attributesToIgnore.Value.Contains(attribute.Type.Name); + protected override IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + => BuildAttributesForBackCompatibility(originalAttributes, s_attributesToIgnore.Value); private static bool IsBuildableAttribute(MethodBodyStatement statement) { 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 45fa71986e5..d55459ea176 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 @@ -1052,6 +1052,19 @@ protected internal virtual IReadOnlyList BuildConstructorsF /// original attributes are returned unchanged when there is nothing new to add. /// protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) + => BuildAttributesForBackCompatibility(originalAttributes, additionalNonRestorableAttributeNames: null); + + /// + /// Adds any back-compatibility attributes from the last contract that are not already present in + /// (or the custom-code attributes), skipping the attributes + /// that generation owns (see ) plus any listed in + /// . Derived types call this overload to + /// exclude additional attributes without re-implementing the merge/dedup logic. The original + /// attributes are returned unchanged when there is nothing new to add. + /// + protected IReadOnlyList BuildAttributesForBackCompatibility( + IEnumerable originalAttributes, + IReadOnlyCollection? additionalNonRestorableAttributeNames) { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; @@ -1075,7 +1088,14 @@ protected internal virtual IReadOnlyList BuildAttributesForB List? merged = null; foreach (var attribute in lastContractAttributes) { - if (ShouldRestoreLastContractAttribute(attribute) && seen.Add(attribute.ToDisplayString())) + var attributeName = attribute.Type.Name; + if (s_nonRestorableAttributeNames.Value.Contains(attributeName) + || additionalNonRestorableAttributeNames?.Contains(attributeName) == true) + { + continue; + } + + if (seen.Add(attribute.ToDisplayString())) { merged ??= [.. original]; merged.Add(attribute); @@ -1085,17 +1105,6 @@ protected internal virtual IReadOnlyList BuildAttributesForB return merged ?? original; } - /// - /// Determines whether an attribute declared on the last contract should be restored onto the - /// current generation. Attributes that generation owns (see ) - /// are never restored. Derived types can override this to exclude additional attributes. - /// - protected virtual bool ShouldRestoreLastContractAttribute(AttributeStatement attribute) - { - var attributeName = attribute.Data?.AttributeClass?.Name; - return attributeName is null || !s_nonRestorableAttributeNames.Value.Contains(attributeName); - } - private IReadOnlyList? _enumValues; private bool ShouldGenerate(ConstructorProvider constructor) From 523bc677eb19e6203149fe3a029325e31f21132f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:47:47 +0000 Subject: [PATCH 12/30] Remove new protected overload; MRW override post-filters buildable attrs Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 41 +++++++++++++++---- .../src/Providers/TypeProvider.cs | 17 +------- 2 files changed, 35 insertions(+), 23 deletions(-) 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 b93523a3889..c4fe0c49357 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 @@ -98,17 +98,44 @@ protected override IReadOnlyList BuildAttributes() return attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value).ToList(); } - private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) - { - nameof(ModelReaderWriterBuildableAttribute), - }); - /// - /// Restores back-compatibility attributes from the last contract, excluding any + /// 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) - => BuildAttributesForBackCompatibility(originalAttributes, s_attributesToIgnore.Value); + { + var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; + var merged = base.BuildAttributesForBackCompatibility(original); + + // base returns the original list unchanged when nothing was restored. + if (ReferenceEquals(merged, original)) + { + return merged; + } + + // base only appends attributes that are new relative to the original set, so keep the + // originally-generated attributes while dropping any buildable ones restored from the last + // contract. + var originalDisplayStrings = new HashSet(StringComparer.Ordinal); + foreach (var attribute in original) + { + originalDisplayStrings.Add(attribute.ToDisplayString()); + } + + var result = new List(merged.Count); + foreach (var attribute in merged) + { + if (attribute.Type.Equals(typeof(ModelReaderWriterBuildableAttribute)) + && !originalDisplayStrings.Contains(attribute.ToDisplayString())) + { + continue; + } + + result.Add(attribute); + } + + return result; + } private static bool IsBuildableAttribute(MethodBodyStatement statement) { 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 d55459ea176..732d9f19ae1 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 @@ -1052,19 +1052,6 @@ protected internal virtual IReadOnlyList BuildConstructorsF /// original attributes are returned unchanged when there is nothing new to add. /// protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) - => BuildAttributesForBackCompatibility(originalAttributes, additionalNonRestorableAttributeNames: null); - - /// - /// Adds any back-compatibility attributes from the last contract that are not already present in - /// (or the custom-code attributes), skipping the attributes - /// that generation owns (see ) plus any listed in - /// . Derived types call this overload to - /// exclude additional attributes without re-implementing the merge/dedup logic. The original - /// attributes are returned unchanged when there is nothing new to add. - /// - protected IReadOnlyList BuildAttributesForBackCompatibility( - IEnumerable originalAttributes, - IReadOnlyCollection? additionalNonRestorableAttributeNames) { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; @@ -1088,9 +1075,7 @@ protected IReadOnlyList BuildAttributesForBackCompatibility( List? merged = null; foreach (var attribute in lastContractAttributes) { - var attributeName = attribute.Type.Name; - if (s_nonRestorableAttributeNames.Value.Contains(attributeName) - || additionalNonRestorableAttributeNames?.Contains(attributeName) == true) + if (s_nonRestorableAttributeNames.Value.Contains(attribute.Type.Name)) { continue; } From 55a499bced40cb8c5523a735d01bdc6d2da9d56e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 20:43:51 +0000 Subject: [PATCH 13/30] Address review: hashset-based buildable filter, Serializable/json ignore list, TestData assertions Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 10 ++-- ...ModelReaderWriterContextDefinitionTests.cs | 12 ++--- ...ibilityDoesNotRestoreBuildableAttribute.cs | 14 ++++++ .../src/Providers/TypeProvider.cs | 18 +++++++ ...ilitySkipsAttributesAlreadyInCustomCode.cs | 4 +- .../BackCompatAttributeType.cs | 6 ++- .../test/Providers/TypeProviderTests.cs | 47 ++++++++----------- .../test/TestHelpers/TestTypeProvider.cs | 12 ++++- 8 files changed, 80 insertions(+), 43 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute.cs 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 c4fe0c49357..da371b2cca7 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 @@ -114,8 +114,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa } // base only appends attributes that are new relative to the original set, so keep the - // originally-generated attributes while dropping any buildable ones restored from the last - // contract. + // originally-generated attributes while dropping any restored attributes that generation owns. var originalDisplayStrings = new HashSet(StringComparer.Ordinal); foreach (var attribute in original) { @@ -125,7 +124,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa var result = new List(merged.Count); foreach (var attribute in merged) { - if (attribute.Type.Equals(typeof(ModelReaderWriterBuildableAttribute)) + if (s_attributesToIgnore.Value.Contains(attribute.Type.Name) && !originalDisplayStrings.Contains(attribute.ToDisplayString())) { continue; @@ -137,6 +136,11 @@ protected override IReadOnlyList BuildAttributesForBackCompa return result; } + private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) + { + nameof(ModelReaderWriterBuildableAttribute), + }); + private static bool IsBuildableAttribute(MethodBodyStatement statement) { var attribute = statement switch 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 dedccc313ec..b61d871e03e 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 @@ -1867,14 +1867,12 @@ await MockHelpers.LoadMockGeneratorAsync( // The last contract declares a ModelReaderWriterBuildable attribute (owned by generation and // rebuilt at write time) alongside a Description attribute. Only the non-buildable Description // attribute should be restored. - var result = contextDefinition.BuildAttributesForBackCompatibilityPublic([]); + var attributes = contextDefinition.BuildAttributesForBackCompatibilityPublic([]); + contextDefinition.Update(attributes: attributes); - Assert.IsFalse( - result.Any(a => a.Type.Equals(typeof(ModelReaderWriterBuildableAttribute))), - "ModelReaderWriterBuildable attributes must not be restored from the last contract."); - Assert.IsTrue( - result.Any(a => a.Type.Equals(typeof(System.ComponentModel.DescriptionAttribute))), - "Non-buildable back-compat attributes should still be restored."); + var writer = new TypeProviderWriter(contextDefinition); + var file = writer.Write(); + Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } private class TestableModelReaderWriterContextDefinition : ModelReaderWriterContextDefinition 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/src/Providers/TypeProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs index 732d9f19ae1..2571aac3f6f 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 @@ -6,6 +6,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Linq; +using System.Text.Json.Serialization; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.TypeSpec.Generator.EmitterRpc; @@ -1043,6 +1044,23 @@ protected internal virtual IReadOnlyList BuildConstructorsF nameof(EditorBrowsableAttribute), nameof(ExperimentalAttribute), nameof(ObsoleteAttribute), + nameof(SerializableAttribute), + 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), }); /// 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 index c64efdd2fa2..65523f6f0ab 100644 --- 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 @@ -2,11 +2,9 @@ #nullable disable -using System; - namespace Test { - [global::System.SerializableAttribute] + [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 index afd84d66669..a13ba7eb8b6 100644 --- 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 @@ -2,8 +2,12 @@ namespace Test { + public class RestorableAttribute : Attribute + { + } + [CLSCompliant(true)] - [Serializable] + [Restorable] 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 39cec3b0ca2..c39f9f1efdf 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,7 +28,11 @@ public void Setup() [Test] public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoLastContract() { - var provider = new AttributeTestProvider(); + 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, the method returns the generated // attributes unchanged. @@ -42,7 +46,7 @@ public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoL public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContract() { await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The last contract declares [CLSCompliant(true)] which is not present in the original set, so it // should be appended to the result. @@ -56,7 +60,7 @@ public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContra public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute() { await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The original set already contains the same [CLSCompliant(true)] attribute that the last // contract declares, so nothing new is added and the original list is returned unchanged. @@ -79,10 +83,10 @@ public async Task BuildAttributesForBackCompatibilitySkipsAttributesAlreadyInCus await MockHelpers.LoadMockGeneratorAsync( compilation: async () => await Helpers.GetCompilationFromDirectoryAsync(parameters: "Custom"), lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The last contract declares a mix of attributes: [CLSCompliant(true)] which is also present in - // the custom code, and [Serializable] which is new. Only the [Serializable] attribute should be + // 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. var attributes = provider.GetBackCompatibilityAttributes([]); provider.Update(attributes: attributes); @@ -94,7 +98,7 @@ await MockHelpers.LoadMockGeneratorAsync( public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() { await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The last contract only declares a CodeGen-specific attribute, which is never restored, so the // original (empty) list is returned unchanged. @@ -112,7 +116,7 @@ public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() public async Task BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribute() { await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The last contract only declares an EditorBrowsable attribute, which generation owns and is // never restored, so the original (empty) list is returned unchanged. @@ -129,7 +133,7 @@ public async Task BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribu public async Task BuildAttributesForBackCompatibilitySkipsExperimentalAttribute() { await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var provider = new AttributeTestProvider(name: "BackCompatAttributeType"); + var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The last contract only declares an Experimental attribute, which generation owns and is never // restored, so the original (empty) list is returned unchanged. @@ -145,26 +149,13 @@ public async Task BuildAttributesForBackCompatibilitySkipsExperimentalAttribute( 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), Snippet.Literal("This is obsolete")), - new AttributeStatement(typeof(SerializableAttribute)), - ]; - - public IReadOnlyList GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(Attributes); - - public IReadOnlyList GetBackCompatibilityAttributes(IEnumerable original) => BuildAttributesForBackCompatibility(original); - } + private static TestTypeProvider CreateAttributeTestProvider( + string? name = null, + IEnumerable? attributes = null) => + new TestTypeProvider( + name: name ?? "TestName", + declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + attributes: attributes); [Test] public void TestUpdateCanonicalView() 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..170a71aa661 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"; } @@ -50,5 +56,9 @@ internal TestTypeProvider( internal TypeProvider[]? NestedTypesInternal { get; set; } protected override TypeSignatureModifiers BuildDeclarationModifiers() => _declarationModifiers ?? base.BuildDeclarationModifiers(); + + internal IReadOnlyList GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(Attributes); + + internal IReadOnlyList GetBackCompatibilityAttributes(IEnumerable original) => BuildAttributesForBackCompatibility(original); } } From fe3157664c262cf04e9b42205c196bb76cca29b1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 20:56:51 +0000 Subject: [PATCH 14/30] test: drive attribute back-compat tests through ProcessTypeForBackCompatibility Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../test/Providers/TypeProviderTests.cs | 54 ++++++------------- .../test/TestHelpers/TestTypeProvider.cs | 4 -- 2 files changed, 17 insertions(+), 41 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 c39f9f1efdf..1a2301388aa 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 @@ -34,10 +34,9 @@ public void BuildAttributesForBackCompatibilityReturnsGeneratedAttributesWhenNoL new AttributeStatement(typeof(SerializableAttribute)), ]); - // With no last contract to restore attributes from, the method returns the generated + // With no last contract to restore attributes from, back-compat processing leaves the generated // attributes unchanged. - var attributes = provider.GetBackCompatibilityAttributes(); - provider.Update(attributes: attributes); + provider.ProcessTypeForBackCompatibility(); Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } @@ -48,10 +47,9 @@ public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContra 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 original set, so it - // should be appended to the result. - var attributes = provider.GetBackCompatibilityAttributes([]); - provider.Update(attributes: attributes); + // 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)); } @@ -60,20 +58,16 @@ public async Task BuildAttributesForBackCompatibilityAddsAttributeFromLastContra public async Task BuildAttributesForBackCompatibilityDoesNotDuplicateExistingAttribute() { await MockHelpers.LoadMockGeneratorAsync(lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); - // The original set already contains the same [CLSCompliant(true)] attribute that the last - // contract declares, so nothing new is added and the original list is returned unchanged. - IReadOnlyList original = + // 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)), - ]; - 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.ProcessTypeForBackCompatibility(); - provider.Update(attributes: attributes); Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } @@ -88,8 +82,7 @@ await MockHelpers.LoadMockGeneratorAsync( // 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. - var attributes = provider.GetBackCompatibilityAttributes([]); - provider.Update(attributes: attributes); + provider.ProcessTypeForBackCompatibility(); Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } @@ -101,14 +94,9 @@ public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() var provider = CreateAttributeTestProvider(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); + // generated (empty) set is left unchanged. + provider.ProcessTypeForBackCompatibility(); - provider.Update(attributes: attributes); Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } @@ -119,13 +107,9 @@ public async Task BuildAttributesForBackCompatibilitySkipsEditorBrowsableAttribu var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The last contract only declares an EditorBrowsable attribute, which generation owns and is - // never restored, so the original (empty) list is returned unchanged. - var original = Array.Empty(); - var attributes = provider.GetBackCompatibilityAttributes(original); + // never restored, so the generated (empty) set is left unchanged. + provider.ProcessTypeForBackCompatibility(); - Assert.AreSame(original, attributes); - - provider.Update(attributes: attributes); Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } @@ -136,13 +120,9 @@ public async Task BuildAttributesForBackCompatibilitySkipsExperimentalAttribute( var provider = CreateAttributeTestProvider(name: "BackCompatAttributeType"); // The last contract only declares an Experimental attribute, which generation owns and is never - // restored, so the original (empty) list is returned unchanged. - var original = Array.Empty(); - var attributes = provider.GetBackCompatibilityAttributes(original); - - Assert.AreSame(original, attributes); + // restored, so the generated (empty) set is left unchanged. + provider.ProcessTypeForBackCompatibility(); - provider.Update(attributes: attributes); Assert.AreEqual(Helpers.GetExpectedFromFile(), Write(provider)); } 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 170a71aa661..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 @@ -56,9 +56,5 @@ internal TestTypeProvider( internal TypeProvider[]? NestedTypesInternal { get; set; } protected override TypeSignatureModifiers BuildDeclarationModifiers() => _declarationModifiers ?? base.BuildDeclarationModifiers(); - - internal IReadOnlyList GetBackCompatibilityAttributes() => BuildAttributesForBackCompatibility(Attributes); - - internal IReadOnlyList GetBackCompatibilityAttributes(IEnumerable original) => BuildAttributesForBackCompatibility(original); } } From 705fde44ae177f6a6a91df21ec91fd5b098d0935 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 21:14:55 +0000 Subject: [PATCH 15/30] test: drive MRW back-compat test through ProcessTypeForBackCompatibility Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinitionTests.cs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) 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 b61d871e03e..cc25294077d 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 @@ -1862,25 +1862,18 @@ public async Task BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttr await MockHelpers.LoadMockGeneratorAsync( lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); - var contextDefinition = new TestableModelReaderWriterContextDefinition(); + var contextDefinition = new ModelReaderWriterContextDefinition(); // The last contract declares a ModelReaderWriterBuildable attribute (owned by generation and - // rebuilt at write time) alongside a Description attribute. Only the non-buildable Description - // attribute should be restored. - var attributes = contextDefinition.BuildAttributesForBackCompatibilityPublic([]); - contextDefinition.Update(attributes: attributes); + // 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 TestableModelReaderWriterContextDefinition : ModelReaderWriterContextDefinition - { - public IReadOnlyList BuildAttributesForBackCompatibilityPublic(IEnumerable originalAttributes) - => BuildAttributesForBackCompatibility(originalAttributes); - } - private class CustomSerializationProvider : TypeProvider { private readonly bool _usePersistableModel; From a386cef9407321b1c6b8d9b9e21b155488a134d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 21:56:27 +0000 Subject: [PATCH 16/30] feat: shared Scm back-compat attribute helper; MRW serialization skips proxy attr Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 30 +---- .../MrwSerializationTypeDefinition.cs | 16 +++ .../Utilities/ScmBackCompatibilityHelpers.cs | 51 ++++++++ .../BackCompatibilityTests.cs | 42 ++++++ ...mpatibilityDoesNotRestoreProxyAttribute.cs | 123 ++++++++++++++++++ .../Pet.cs | 11 ++ 6 files changed, 245 insertions(+), 28 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Utilities/ScmBackCompatibilityHelpers.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs 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 da371b2cca7..93135577c1a 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.Expressions; using Microsoft.TypeSpec.Generator.Primitives; using Microsoft.TypeSpec.Generator.Providers; @@ -106,34 +107,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; var merged = base.BuildAttributesForBackCompatibility(original); - - // base returns the original list unchanged when nothing was restored. - if (ReferenceEquals(merged, original)) - { - return merged; - } - - // base only appends attributes that are new relative to the original set, so keep the - // originally-generated attributes while dropping any restored attributes that generation owns. - var originalDisplayStrings = new HashSet(StringComparer.Ordinal); - foreach (var attribute in original) - { - originalDisplayStrings.Add(attribute.ToDisplayString()); - } - - var result = new List(merged.Count); - foreach (var attribute in merged) - { - if (s_attributesToIgnore.Value.Contains(attribute.Type.Name) - && !originalDisplayStrings.Contains(attribute.ToDisplayString())) - { - continue; - } - - result.Add(attribute); - } - - return result; + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore.Value); } private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) 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 adbb3026dda..805eb93a75a 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 @@ -150,6 +150,22 @@ protected override IReadOnlyList BuildAttributes() return []; } + private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) + { + nameof(PersistableModelProxyAttribute), + }); + + /// + /// 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.Value); + } + 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/Utilities/ScmBackCompatibilityHelpers.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Utilities/ScmBackCompatibilityHelpers.cs new file mode 100644 index 00000000000..44445c6a140 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Utilities/ScmBackCompatibilityHelpers.cs @@ -0,0 +1,51 @@ +// 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. + /// + public static IReadOnlyList FilterRestoredAttributes( + IReadOnlyList originalAttributes, + IReadOnlyList restoredAttributes, + HashSet generatorOwnedAttributeNames) + { + // The base returns the original list unchanged when nothing was restored. + if (ReferenceEquals(restoredAttributes, originalAttributes)) + { + return restoredAttributes; + } + + 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 (generatorOwnedAttributeNames.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/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs new file mode 100644 index 00000000000..0320763285a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs @@ -0,0 +1,42 @@ +// 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 + { + [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], + 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..6e946614dcb --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs @@ -0,0 +1,123 @@ +// + +#nullable disable + +using System; +using System.ClientModel.Primitives; +using System.Collections.Generic; +using System.ComponentModel; +using System.Text.Json; +using Sample; + +namespace Sample.Models +{ + [global::System.ComponentModel.DescriptionAttribute("bc")] + public partial class Pet : global::System.ClientModel.Primitives.IJsonModel + { + internal Pet() + { + } + + protected virtual global::Sample.Models.Pet PersistableModelCreateCore(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + switch (format) + { + case "J": + using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(data, global::Sample.ModelSerializationExtensions.JsonDocumentOptions)) + { + return global::Sample.Models.Pet.DeserializePet(document.RootElement, options); + } + default: + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support reading '{options.Format}' format."); + } + } + + protected virtual global::System.BinaryData PersistableModelWriteCore(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + switch (format) + { + case "J": + return global::System.ClientModel.Primitives.ModelReaderWriter.Write(this, options, global::Sample.SampleContext.Default); + default: + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support writing '{options.Format}' format."); + } + } + + global::System.BinaryData global::System.ClientModel.Primitives.IPersistableModel.Write(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelWriteCore(options); + + global::Sample.Models.Pet global::System.ClientModel.Primitives.IPersistableModel.Create(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelCreateCore(data, options); + + string global::System.ClientModel.Primitives.IPersistableModel.GetFormatFromOptions(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => "J"; + + void global::System.ClientModel.Primitives.IJsonModel.Write(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + writer.WriteStartObject(); + this.JsonModelWriteCore(writer, options); + writer.WriteEndObject(); + } + + protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + if ((format != "J")) + { + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support writing '{format}' format."); + } + writer.WritePropertyName("name"u8); + writer.WriteStringValue(Name); + if (((options.Format != "W") && (_additionalBinaryDataProperties != null))) + { + foreach (var item in _additionalBinaryDataProperties) + { + writer.WritePropertyName(item.Key); +#if NET6_0_OR_GREATER + writer.WriteRawValue(item.Value); +#else + using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(item.Value)) + { + global::System.Text.Json.JsonSerializer.Serialize(writer, document.RootElement); + } +#endif + } + } + } + + global::Sample.Models.Pet global::System.ClientModel.Primitives.IJsonModel.Create(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.JsonModelCreateCore(ref reader, options); + + protected virtual global::Sample.Models.Pet JsonModelCreateCore(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; + if ((format != "J")) + { + throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support reading '{format}' format."); + } + using global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.ParseValue(ref reader); + return global::Sample.Models.Pet.DeserializePet(document.RootElement, options); + } + + internal static global::Sample.Models.Pet DeserializePet(global::System.Text.Json.JsonElement element, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) + { + if ((element.ValueKind == global::System.Text.Json.JsonValueKind.Null)) + { + return null; + } + string name = default; + global::System.Collections.Generic.IDictionary additionalBinaryDataProperties = new global::Sample.ChangeTrackingDictionary(); + foreach (var prop in element.EnumerateObject()) + { + if (prop.NameEquals("name"u8)) + { + name = prop.Value.GetString(); + continue; + } + if ((options.Format != "W")) + { + additionalBinaryDataProperties.Add(prop.Name, global::System.BinaryData.FromString(prop.Value.GetRawText())); + } + } + return new global::Sample.Models.Pet(name, additionalBinaryDataProperties); + } + } +} 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 + { + } +} From 7c1fb8db4e01fc26a42371124def3cff389f5f06 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 21:57:44 +0000 Subject: [PATCH 17/30] refactor: accept IReadOnlySet in shared back-compat attribute helper Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Utilities/ScmBackCompatibilityHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 44445c6a140..61c0f8a56db 100644 --- 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 @@ -19,7 +19,7 @@ internal static class ScmBackCompatibilityHelpers public static IReadOnlyList FilterRestoredAttributes( IReadOnlyList originalAttributes, IReadOnlyList restoredAttributes, - HashSet generatorOwnedAttributeNames) + IReadOnlySet generatorOwnedAttributeNames) { // The base returns the original list unchanged when nothing was restored. if (ReferenceEquals(restoredAttributes, originalAttributes)) From 6e4b68b5cdc1eaf98adf8e3884c4c8049571d985 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 22:10:19 +0000 Subject: [PATCH 18/30] refactor: move back-compat attribute fields to top; filter MRW proxy test data Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 9 +- .../MrwSerializationTypeDefinition.cs | 9 +- .../BackCompatibilityTests.cs | 14 +++ ...mpatibilityDoesNotRestoreProxyAttribute.cs | 109 ------------------ .../test/TestHelpers/MockHelpers.cs | 2 + .../src/Providers/TypeProvider.cs | 56 ++++----- 6 files changed, 52 insertions(+), 147 deletions(-) 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 93135577c1a..93a95216986 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 @@ -22,6 +22,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"; @@ -110,11 +114,6 @@ protected override IReadOnlyList BuildAttributesForBackCompa return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore.Value); } - private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) - { - nameof(ModelReaderWriterBuildableAttribute), - }); - private static bool IsBuildableAttribute(MethodBodyStatement statement) { var attribute = statement switch 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 805eb93a75a..7bae936a9e1 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 = @@ -150,11 +154,6 @@ protected override IReadOnlyList BuildAttributes() return []; } - private static readonly Lazy> s_attributesToIgnore = new(() => new(StringComparer.Ordinal) - { - nameof(PersistableModelProxyAttribute), - }); - /// /// Restores back-compatibility attributes from the last contract, then drops any restored /// since those are recomputed at generation time. 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 index 0320763285a..b7d11a2a4ff 100644 --- 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 @@ -14,6 +14,18 @@ namespace Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.MrwSerializat { 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() { @@ -24,6 +36,8 @@ public async Task BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribut 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); 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 index 6e946614dcb..1a3aa8f1a61 100644 --- 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 @@ -2,122 +2,13 @@ #nullable disable -using System; using System.ClientModel.Primitives; -using System.Collections.Generic; using System.ComponentModel; -using System.Text.Json; -using Sample; namespace Sample.Models { [global::System.ComponentModel.DescriptionAttribute("bc")] public partial class Pet : global::System.ClientModel.Primitives.IJsonModel { - internal Pet() - { - } - - protected virtual global::Sample.Models.Pet PersistableModelCreateCore(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) - { - string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; - switch (format) - { - case "J": - using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(data, global::Sample.ModelSerializationExtensions.JsonDocumentOptions)) - { - return global::Sample.Models.Pet.DeserializePet(document.RootElement, options); - } - default: - throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support reading '{options.Format}' format."); - } - } - - protected virtual global::System.BinaryData PersistableModelWriteCore(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) - { - string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; - switch (format) - { - case "J": - return global::System.ClientModel.Primitives.ModelReaderWriter.Write(this, options, global::Sample.SampleContext.Default); - default: - throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support writing '{options.Format}' format."); - } - } - - global::System.BinaryData global::System.ClientModel.Primitives.IPersistableModel.Write(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelWriteCore(options); - - global::Sample.Models.Pet global::System.ClientModel.Primitives.IPersistableModel.Create(global::System.BinaryData data, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.PersistableModelCreateCore(data, options); - - string global::System.ClientModel.Primitives.IPersistableModel.GetFormatFromOptions(global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => "J"; - - void global::System.ClientModel.Primitives.IJsonModel.Write(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) - { - writer.WriteStartObject(); - this.JsonModelWriteCore(writer, options); - writer.WriteEndObject(); - } - - protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWriter writer, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) - { - string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; - if ((format != "J")) - { - throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support writing '{format}' format."); - } - writer.WritePropertyName("name"u8); - writer.WriteStringValue(Name); - if (((options.Format != "W") && (_additionalBinaryDataProperties != null))) - { - foreach (var item in _additionalBinaryDataProperties) - { - writer.WritePropertyName(item.Key); -#if NET6_0_OR_GREATER - writer.WriteRawValue(item.Value); -#else - using (global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.Parse(item.Value)) - { - global::System.Text.Json.JsonSerializer.Serialize(writer, document.RootElement); - } -#endif - } - } - } - - global::Sample.Models.Pet global::System.ClientModel.Primitives.IJsonModel.Create(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) => this.JsonModelCreateCore(ref reader, options); - - protected virtual global::Sample.Models.Pet JsonModelCreateCore(ref global::System.Text.Json.Utf8JsonReader reader, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) - { - string format = (options.Format == "W") ? ((global::System.ClientModel.Primitives.IPersistableModel)this).GetFormatFromOptions(options) : options.Format; - if ((format != "J")) - { - throw new global::System.FormatException($"The model {nameof(global::Sample.Models.Pet)} does not support reading '{format}' format."); - } - using global::System.Text.Json.JsonDocument document = global::System.Text.Json.JsonDocument.ParseValue(ref reader); - return global::Sample.Models.Pet.DeserializePet(document.RootElement, options); - } - - internal static global::Sample.Models.Pet DeserializePet(global::System.Text.Json.JsonElement element, global::System.ClientModel.Primitives.ModelReaderWriterOptions options) - { - if ((element.ValueKind == global::System.Text.Json.JsonValueKind.Null)) - { - return null; - } - string name = default; - global::System.Collections.Generic.IDictionary additionalBinaryDataProperties = new global::Sample.ChangeTrackingDictionary(); - foreach (var prop in element.EnumerateObject()) - { - if (prop.NameEquals("name"u8)) - { - name = prop.Value.GetString(); - continue; - } - if ((options.Format != "W")) - { - additionalBinaryDataProperties.Add(prop.Name, global::System.BinaryData.FromString(prop.Value.GetRawText())); - } - } - return new global::Sample.Models.Pet(name, additionalBinaryDataProperties); - } } } 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 6c6e743ad89..c6fefa217ba 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 @@ -35,6 +35,7 @@ public static async Task> LoadMockGeneratorAsync( string? configuration = null, Func? createCSharpTypeCore = null, Func? createCSharpTypeCoreFallback = null, + Func>? createSerializationsCore = null, string? outputPath = null) { var mockGenerator = LoadMockGenerator( @@ -46,6 +47,7 @@ public static async Task> LoadMockGeneratorAsync( configuration: configuration, createCSharpTypeCore: createCSharpTypeCore, createCSharpTypeCoreFallback: createCSharpTypeCoreFallback, + createSerializationsCore: createSerializationsCore, outputPath: outputPath); var compilationResult = compilation == null ? null : await compilation(); 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 2571aac3f6f..614485e6825 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 @@ -28,6 +28,34 @@ public abstract class TypeProvider private Lazy _declaringTypeName; private readonly InputType? _inputType; + private static readonly Lazy> s_nonRestorableAttributeNames = new(() => new(StringComparer.Ordinal) + { + CodeGenAttributes.CodeGenSuppressAttributeName, + CodeGenAttributes.CodeGenMemberAttributeName, + CodeGenAttributes.CodeGenTypeAttributeName, + CodeGenAttributes.CodeGenSerializationAttributeName, + nameof(EditorBrowsableAttribute), + nameof(ExperimentalAttribute), + nameof(ObsoleteAttribute), + nameof(SerializableAttribute), + 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()); @@ -1035,34 +1063,6 @@ protected internal virtual IReadOnlyList BuildConstructorsF return [.. constructors]; } - private static readonly Lazy> s_nonRestorableAttributeNames = new(() => new(StringComparer.Ordinal) - { - CodeGenAttributes.CodeGenSuppressAttributeName, - CodeGenAttributes.CodeGenMemberAttributeName, - CodeGenAttributes.CodeGenTypeAttributeName, - CodeGenAttributes.CodeGenSerializationAttributeName, - nameof(EditorBrowsableAttribute), - nameof(ExperimentalAttribute), - nameof(ObsoleteAttribute), - nameof(SerializableAttribute), - 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), - }); - /// /// Adds any back-compatibility attributes from the last contract that are not already present in /// (or the custom-code attributes). Attributes that From 79feacff421aed94f93c6b411f1d0470c473defe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 23:11:55 +0000 Subject: [PATCH 19/30] Skip restoring any CodeGen-prefixed attribute in back-compat merge Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 12 +++++------- .../src/SourceInput/CodeGenAttributes.cs | 2 ++ 2 files changed, 7 insertions(+), 7 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 614485e6825..40c51718b2c 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 @@ -30,10 +30,6 @@ public abstract class TypeProvider private static readonly Lazy> s_nonRestorableAttributeNames = new(() => new(StringComparer.Ordinal) { - CodeGenAttributes.CodeGenSuppressAttributeName, - CodeGenAttributes.CodeGenMemberAttributeName, - CodeGenAttributes.CodeGenTypeAttributeName, - CodeGenAttributes.CodeGenSerializationAttributeName, nameof(EditorBrowsableAttribute), nameof(ExperimentalAttribute), nameof(ObsoleteAttribute), @@ -1066,8 +1062,9 @@ protected internal virtual IReadOnlyList BuildConstructorsF /// /// Adds any back-compatibility attributes from the last contract that are not already present in /// (or the custom-code attributes). Attributes that - /// generation owns (see ) are never restored. The - /// original attributes are returned unchanged when there is nothing new to add. + /// generation owns (any CodeGen-prefixed attribute or one listed in + /// ) are never restored. The original attributes are + /// returned unchanged when there is nothing new to add. /// protected internal virtual IReadOnlyList BuildAttributesForBackCompatibility(IEnumerable originalAttributes) { @@ -1093,7 +1090,8 @@ protected internal virtual IReadOnlyList BuildAttributesForB List? merged = null; foreach (var attribute in lastContractAttributes) { - if (s_nonRestorableAttributeNames.Value.Contains(attribute.Type.Name)) + if (attribute.Type.Name.Contains(CodeGenAttributes.CodeGenAttributePrefix, StringComparison.Ordinal) + || s_nonRestorableAttributeNames.Value.Contains(attribute.Type.Name)) { continue; } 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..08bd90f00af 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 { + public const string CodeGenAttributePrefix = "CodeGen"; + public const string CodeGenSuppressAttributeName = "CodeGenSuppressAttribute"; public const string CodeGenMemberAttributeName = "CodeGenMemberAttribute"; From cadf5bf81fa5fc1ae1cdbea9f3bc4ec6e50e012e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:33:27 +0000 Subject: [PATCH 20/30] Make CodeGenAttributePrefix internal; check hashset first with case-insensitive CodeGen match Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 4 ++-- .../src/SourceInput/CodeGenAttributes.cs | 2 +- 2 files changed, 3 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 40c51718b2c..e18880e283c 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 @@ -1090,8 +1090,8 @@ protected internal virtual IReadOnlyList BuildAttributesForB List? merged = null; foreach (var attribute in lastContractAttributes) { - if (attribute.Type.Name.Contains(CodeGenAttributes.CodeGenAttributePrefix, StringComparison.Ordinal) - || s_nonRestorableAttributeNames.Value.Contains(attribute.Type.Name)) + if (s_nonRestorableAttributeNames.Value.Contains(attribute.Type.Name) + || attribute.Type.Name.Contains(CodeGenAttributes.CodeGenAttributePrefix, StringComparison.OrdinalIgnoreCase)) { continue; } 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 08bd90f00af..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,7 +11,7 @@ namespace Microsoft.TypeSpec.Generator.SourceInput { public static class CodeGenAttributes { - public const string CodeGenAttributePrefix = "CodeGen"; + internal const string CodeGenAttributePrefix = "CodeGen"; public const string CodeGenSuppressAttributeName = "CodeGenSuppressAttribute"; From da7bb35f41d23e604d3064452b9b141dfa641bea Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:38:38 +0000 Subject: [PATCH 21/30] test: cover skipping all CodeGen-prefixed attributes in back-compat merge Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- ...bilitySkipsAllCodeGenPrefixedAttributes.cs | 11 ++++++++++ .../BackCompatAttributeType.cs | 21 +++++++++++++++++++ .../test/Providers/TypeProviderTests.cs | 14 +++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsAllCodeGenPrefixedAttributes/BackCompatAttributeType.cs 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/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 1a2301388aa..ec32c578abe 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 @@ -100,6 +100,20 @@ public async Task BuildAttributesForBackCompatibilitySkipsCodeGenAttributes() 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() { From 840298284210639533b9e26d02c1a9445279b158 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 15:47:50 +0000 Subject: [PATCH 22/30] fix: ScmModelProvider skips restoring PersistableModelProxy in back-compat merge Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ScmModelProvider.cs | 17 ++++++++ .../BackCompatibilityTests.cs | 41 +++++++++++++++++++ ...mpatibilityDoesNotRestoreProxyAttribute.cs | 32 +++++++++++++++ .../Pet.cs | 11 +++++ 4 files changed, 101 insertions(+) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs 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..0352338785b 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.Value); + } + protected override ConstructorProvider[] BuildConstructors() { List constructors = [.. base.BuildConstructors()]; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs new file mode 100644 index 00000000000..e0b41c1c1d2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Linq; +using System.Threading.Tasks; +using Microsoft.TypeSpec.Generator.Input; +using Microsoft.TypeSpec.Generator.Primitives; +using Microsoft.TypeSpec.Generator.Providers; +using Microsoft.TypeSpec.Generator.Tests.Common; +using NUnit.Framework; +using ScmModel = Microsoft.TypeSpec.Generator.ClientModel.Providers.ScmModelProvider; + +namespace Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.ScmModelProvider +{ + public class BackCompatibilityTests + { + [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); + } + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/BackCompatibilityTests/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/BackCompatibilityTests/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/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/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/ScmModelProvider/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 + { + } +} From 177c99a707c5be023f379574884d026f5bfee527 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:00:09 +0000 Subject: [PATCH 23/30] test: move ScmModelProvider back-compat test into ScmModelProviderTests suite Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../BackCompatibilityTests.cs | 41 ------------------- .../ScmModelProvider/ScmModelProviderTests.cs | 24 +++++++++++ ...mpatibilityDoesNotRestoreProxyAttribute.cs | 0 .../Pet.cs | 0 4 files changed, 24 insertions(+), 41 deletions(-) delete mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/{BackCompatibilityTests => ScmModelProviderTests}/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs (100%) rename packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/{BackCompatibilityTests => ScmModelProviderTests}/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs (100%) diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs deleted file mode 100644 index e0b41c1c1d2..00000000000 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/BackCompatibilityTests.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System.Linq; -using System.Threading.Tasks; -using Microsoft.TypeSpec.Generator.Input; -using Microsoft.TypeSpec.Generator.Primitives; -using Microsoft.TypeSpec.Generator.Providers; -using Microsoft.TypeSpec.Generator.Tests.Common; -using NUnit.Framework; -using ScmModel = Microsoft.TypeSpec.Generator.ClientModel.Providers.ScmModelProvider; - -namespace Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.ScmModelProvider -{ - public class BackCompatibilityTests - { - [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); - } - } -} 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/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs similarity index 100% rename from packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs rename to packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs From 9a5f87c445371ebae83796cfe12d39ce775ac3fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:23:00 +0000 Subject: [PATCH 24/30] Skip restoring DefaultMember attribute in back-compat merge; add unit test Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 2 ++ ...ackCompatibilitySkipsDefaultMemberAttribute.cs | 11 +++++++++++ .../BackCompatAttributeType.cs | 15 +++++++++++++++ .../test/Providers/TypeProviderTests.cs | 14 ++++++++++++++ 4 files changed, 42 insertions(+) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute/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 e18880e283c..0700a729eba 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 @@ -6,6 +6,7 @@ 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; @@ -34,6 +35,7 @@ public abstract class TypeProvider nameof(ExperimentalAttribute), nameof(ObsoleteAttribute), nameof(SerializableAttribute), + nameof(DefaultMemberAttribute), nameof(JsonConstructorAttribute), nameof(JsonConverterAttribute), nameof(JsonDerivedTypeAttribute), 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/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index ec32c578abe..e6a7b246939 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 @@ -140,6 +140,20 @@ public async Task BuildAttributesForBackCompatibilitySkipsExperimentalAttribute( 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)); + } + private static string Write(TypeProvider provider) => CodeModelGenerator.Instance.GetWriter(provider).Write().Content; From b8f7634e6c04059c2019d2b2c5fb156256d21d8c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:29:28 +0000 Subject: [PATCH 25/30] fix(csharp): prevent emitter crash when restoring last-contract attributes with integral literal args Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Expressions/LiteralExpression.cs | 8 ++++- .../src/Providers/TypeProvider.cs | 33 +++++++++++++++++-- ...esAttributeWithIntegralLiteralArguments.cs | 11 +++++++ .../BackCompatAttributeType.cs | 16 +++++++++ .../test/Providers/TypeProviderTests.cs | 14 ++++++++ .../Statements/AttributeStatementTests.cs | 17 ++++++++++ ...AttributeStatementWithIntegralArguments.cs | 1 + 7 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityRestoresAttributeWithIntegralLiteralArguments/BackCompatAttributeType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Statements/TestData/AttributeStatementTests/AttributeStatementWithIntegralArguments.cs 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 69a9e4855cf..e8360cce7ce 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 @@ -1001,11 +1001,17 @@ protected internal virtual IReadOnlyList BuildAttributesForB var seen = new HashSet(); foreach (var attribute in original) { - seen.Add(attribute.ToDisplayString()); + if (TryGetAttributeDisplayString(attribute, out var display)) + { + seen.Add(display); + } } foreach (var attribute in CustomCodeView?.Attributes ?? []) { - seen.Add(attribute.ToDisplayString()); + if (TryGetAttributeDisplayString(attribute, out var display)) + { + seen.Add(display); + } } List? merged = null; @@ -1017,7 +1023,10 @@ protected internal virtual IReadOnlyList BuildAttributesForB continue; } - if (seen.Add(attribute.ToDisplayString())) + // 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); @@ -1027,6 +1036,24 @@ protected internal virtual IReadOnlyList BuildAttributesForB return merged ?? original; } + // Renders an attribute to its display string used as the de-duplication key. Attributes read + // from the last contract (or custom code) may reference argument literals or types that the + // generator cannot render; in that case rendering throws and we treat the attribute as + // non-restorable instead of crashing the entire generation. + 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/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/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index dd7527065d0..519a807eb88 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 @@ -154,6 +154,20 @@ public async Task BuildAttributesForBackCompatibilitySkipsDefaultMemberAttribute 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; 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 From d101de61db91095f650384daf7829a423aa9f7e4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:30:45 +0000 Subject: [PATCH 26/30] refactor(csharp): narrow back-compat attribute render catch to specific exception types Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../Microsoft.TypeSpec.Generator/src/Providers/TypeProvider.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 e8360cce7ce..e80164903cd 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 @@ -1047,7 +1047,8 @@ private static bool TryGetAttributeDisplayString(AttributeStatement attribute, [ displayString = attribute.ToDisplayString(); return true; } - catch (Exception) + catch (Exception ex) when ( + ex is NotImplementedException or NotSupportedException or InvalidOperationException or NullReferenceException) { displayString = null; return false; From edc3ee3b3b94e38a7c3117bd734ceaf7c6a965c5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:37:05 +0000 Subject: [PATCH 27/30] fix(csharp): restrict back-compat attribute restoration to public/protected types Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 19 ++++++++++++----- ...patibilityDoesNotRestoreForInternalType.cs | 10 +++++++++ .../BackCompatAttributeType.cs | 9 ++++++++ .../test/Providers/TypeProviderTests.cs | 21 +++++++++++++++++-- 4 files changed, 52 insertions(+), 7 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreForInternalType/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 e80164903cd..ea547f37837 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 @@ -981,16 +981,25 @@ protected internal virtual IReadOnlyList BuildConstructorsF } /// - /// Adds any back-compatibility attributes from the last contract that are not already present in - /// (or the custom-code attributes). Attributes that - /// generation owns (any CodeGen-prefixed attribute or one listed in - /// ) are never restored. The original attributes are - /// returned unchanged when there is nothing new to add. + /// 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]; + // Only externally visible (public or protected) types can break a consumer contract, so we + // only restore attributes for those types. + if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) + && !DeclarationModifiers.HasFlag(TypeSignatureModifiers.Protected)) + { + return original; + } + if (LastContractView?.Attributes is not { Count: > 0 } lastContractAttributes) { return original; 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/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 519a807eb88..70a38c21772 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 @@ -171,12 +171,29 @@ public async Task BuildAttributesForBackCompatibilityRestoresAttributeWithIntegr 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)); + } + private static TestTypeProvider CreateAttributeTestProvider( string? name = null, - IEnumerable? attributes = null) => + IEnumerable? attributes = null, + TypeSignatureModifiers? declarationModifiers = null) => new TestTypeProvider( name: name ?? "TestName", - declarationModifiers: TypeSignatureModifiers.Public | TypeSignatureModifiers.Class, + declarationModifiers: declarationModifiers ?? (TypeSignatureModifiers.Public | TypeSignatureModifiers.Class), attributes: attributes); [Test] From 4a9b6cc444a9b6af111d3e6e9a88ed25faf4f59f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 19:41:39 +0000 Subject: [PATCH 28/30] refactor(csharp): catch any exception when rendering back-compat attribute display string Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/TypeProvider.cs | 7 +------ 1 file changed, 1 insertion(+), 6 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 ea547f37837..d942337f597 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 @@ -1045,10 +1045,6 @@ protected internal virtual IReadOnlyList BuildAttributesForB return merged ?? original; } - // Renders an attribute to its display string used as the de-duplication key. Attributes read - // from the last contract (or custom code) may reference argument literals or types that the - // generator cannot render; in that case rendering throws and we treat the attribute as - // non-restorable instead of crashing the entire generation. private static bool TryGetAttributeDisplayString(AttributeStatement attribute, [NotNullWhen(true)] out string? displayString) { try @@ -1056,8 +1052,7 @@ private static bool TryGetAttributeDisplayString(AttributeStatement attribute, [ displayString = attribute.ToDisplayString(); return true; } - catch (Exception ex) when ( - ex is NotImplementedException or NotSupportedException or InvalidOperationException or NullReferenceException) + catch (Exception) { displayString = null; return false; From 7f8188f5b9244f5875d7fe60030ac8ea50c4feca Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 20:05:25 +0000 Subject: [PATCH 29/30] test/refactor(csharp): dedup back-compat test; lazy owned-attrs param; drop comment Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../ModelReaderWriterContextDefinition.cs | 2 +- .../MrwSerializationTypeDefinition.cs | 2 +- .../src/Providers/ScmModelProvider.cs | 2 +- .../Utilities/ScmBackCompatibilityHelpers.cs | 9 +++++-- .../src/Providers/TypeProvider.cs | 2 -- .../BackCompatAttributeType.cs | 9 +++++++ ...tesAcrossGeneratedCustomAndLastContract.cs | 14 +++++++++++ .../BackCompatAttributeType.cs | 15 ++++++++++++ .../test/Providers/TypeProviderTests.cs | 24 +++++++++++++++++++ 9 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract(Custom)/BackCompatAttributeType.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract.cs create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TestData/TypeProviderTests/BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContract/BackCompatAttributeType.cs 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 69a9762ad93..13d50ab00c2 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 @@ -89,7 +89,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; var merged = base.BuildAttributesForBackCompatibility(original); - return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore.Value); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, static () => s_attributesToIgnore.Value); } /// 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 e2d708b0b13..87ef7cd4fe4 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 @@ -164,7 +164,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; var merged = base.BuildAttributesForBackCompatibility(original); - return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore.Value); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, static () => s_attributesToIgnore.Value); } private CSharpType GetRootModelType() 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 0352338785b..5d02ae26cbc 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 @@ -140,7 +140,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; var merged = base.BuildAttributesForBackCompatibility(original); - return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore.Value); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, static () => s_attributesToIgnore.Value); } protected override ConstructorProvider[] 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 index 61c0f8a56db..384de827cfb 100644 --- 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 @@ -15,11 +15,14 @@ internal static class ScmBackCompatibilityHelpers /// 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, - IReadOnlySet generatorOwnedAttributeNames) + Func> generatorOwnedAttributeNames) { // The base returns the original list unchanged when nothing was restored. if (ReferenceEquals(restoredAttributes, originalAttributes)) @@ -27,6 +30,8 @@ public static IReadOnlyList FilterRestoredAttributes( return restoredAttributes; } + var ownedNames = generatorOwnedAttributeNames(); + var originalDisplayStrings = new HashSet(StringComparer.Ordinal); foreach (var attribute in originalAttributes) { @@ -36,7 +41,7 @@ public static IReadOnlyList FilterRestoredAttributes( var result = new List(restoredAttributes.Count); foreach (var attribute in restoredAttributes) { - if (generatorOwnedAttributeNames.Contains(attribute.Type.Name) + if (ownedNames.Contains(attribute.Type.Name) && !originalDisplayStrings.Contains(attribute.ToDisplayString())) { continue; 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 d942337f597..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 @@ -992,8 +992,6 @@ protected internal virtual IReadOnlyList BuildAttributesForB { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; - // Only externally visible (public or protected) types can break a consumer contract, so we - // only restore attributes for those types. if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Public) && !DeclarationModifiers.HasFlag(TypeSignatureModifiers.Protected)) { 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/TypeProviderTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/Providers/TypeProviderTests.cs index 70a38c21772..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 @@ -187,6 +187,30 @@ public async Task BuildAttributesForBackCompatibilityDoesNotRestoreForInternalTy 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, From 68a9a9b8127334b52d25a0b4dad939994b92a9da Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 20:13:10 +0000 Subject: [PATCH 30/30] Pass lazy attribute-ignore set directly to FilterRestoredAttributes Co-authored-by: jorgerangel-msft <102122018+jorgerangel-msft@users.noreply.github.com> --- .../src/Providers/ModelReaderWriterContextDefinition.cs | 2 +- .../src/Providers/MrwSerializationTypeDefinition.cs | 2 +- .../src/Providers/ScmModelProvider.cs | 2 +- .../src/Utilities/ScmBackCompatibilityHelpers.cs | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) 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 13d50ab00c2..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 @@ -89,7 +89,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; var merged = base.BuildAttributesForBackCompatibility(original); - return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, static () => s_attributesToIgnore.Value); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore); } /// 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 87ef7cd4fe4..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 @@ -164,7 +164,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; var merged = base.BuildAttributesForBackCompatibility(original); - return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, static () => s_attributesToIgnore.Value); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore); } private CSharpType GetRootModelType() 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 5d02ae26cbc..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 @@ -140,7 +140,7 @@ protected override IReadOnlyList BuildAttributesForBackCompa { var original = originalAttributes as IReadOnlyList ?? [.. originalAttributes]; var merged = base.BuildAttributesForBackCompatibility(original); - return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, static () => s_attributesToIgnore.Value); + return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore); } protected override ConstructorProvider[] 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 index 384de827cfb..93ec02c9c56 100644 --- 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 @@ -22,7 +22,7 @@ internal static class ScmBackCompatibilityHelpers public static IReadOnlyList FilterRestoredAttributes( IReadOnlyList originalAttributes, IReadOnlyList restoredAttributes, - Func> generatorOwnedAttributeNames) + Lazy> generatorOwnedAttributeNames) { // The base returns the original list unchanged when nothing was restored. if (ReferenceEquals(restoredAttributes, originalAttributes)) @@ -30,7 +30,7 @@ public static IReadOnlyList FilterRestoredAttributes( return restoredAttributes; } - var ownedNames = generatorOwnedAttributeNames(); + var ownedNames = generatorOwnedAttributeNames.Value; var originalDisplayStrings = new HashSet(StringComparer.Ordinal); foreach (var attribute in originalAttributes)