chore: clean up TypeProvider attributes

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ModelReaderWriterContextDefinition.cs

@@ -39,16 +39,10 @@ protected override TypeSignatureModifiers BuildDeclarationModifiers()
         // They are rebuilt at write time while non-buildable attributes, including visitor updates, are preserved.
         protected override bool ShouldAnalyzeAttributesInReferenceMap => false;
 
-        protected override IReadOnlyList<MethodBodyStatement> BuildAttributesForWrite()
-        {
-            var visitorAttributes = base.BuildAttributesForWrite().Where(static attribute => !IsBuildableAttribute(attribute));
-            return [.. BuildAttributes(), .. visitorAttributes];
-        }
-
         protected override IReadOnlyList<MethodBodyStatement> BuildAttributes()
         {
             var attributes = new Dictionary<string, MethodBodyStatement>();
-            var customizedBuildableTypes = GetCustomizedBuildableTypes();
+            var customizedBuildableTypes = BuildCustomizedBuildableTypes();
 
             // Add ModelReaderWriterBuildableAttribute for all IPersistableModel types
             (HashSet<CSharpType> buildableTypes, HashSet<TypeProvider> buildableProviders) = CollectBuildableTypes();
@@ -88,12 +82,62 @@ protected override IReadOnlyList<MethodBodyStatement> BuildAttributes()
                     provider.Type.FullyQualifiedName);
             }
 
-            AddLastContractBuildableAttributes(attributes, customizedBuildableTypes);
-
             // Sort by the simple type name (last part after the last dot) instead of the fully qualified name
             return attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value).ToList();
         }
 
+        protected override IReadOnlyList<MethodBodyStatement> BuildAttributesForBackCompatibility(IReadOnlyList<MethodBodyStatement> originalAttributes)
+        {
+            if (LastContractView?.Attributes is not { Count: > 0 })
+            {
+                return originalAttributes;
+            }
+
+            // Re-key the generated buildable attributes so last-contract entries can be deduplicated against
+            // them and the combined set can be re-sorted; any non-buildable attributes are preserved as-is.
+            var attributes = new Dictionary<string, MethodBodyStatement>();
+            var others = new List<MethodBodyStatement>();
+            foreach (var attribute in originalAttributes)
+            {
+                var identity = GetBuildableAttributeIdentity(attribute);
+                if (identity != null)
+                {
+                    attributes[identity] = attribute;
+                }
+                else
+                {
+                    others.Add(attribute);
+                }
+            }
+
+            AddLastContractBuildableAttributes(attributes, BuildCustomizedBuildableTypes());
+
+            return [.. attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value), .. others];
+        }
+
+        private static string? GetBuildableAttributeIdentity(MethodBodyStatement attribute)
+        {
+            var attributeStatement = attribute switch
+            {
+                AttributeStatement direct => direct,
+                SuppressionStatement suppression => suppression.AsStatement<AttributeStatement>(),
+                _ => null
+            };
+
+            if (attributeStatement is null || !string.Equals(
+                attributeStatement.Type.FullyQualifiedName,
+                s_buildableAttributeType.FullyQualifiedName,
+                StringComparison.Ordinal))
+            {
+                return null;
+            }
+
+            var targetType = GetBuildableAttributeTargetType(attributeStatement);
+            return targetType is null
+                ? null
+                : GetTypeIdentity(targetType);
+        }
+
         private void AddLastContractBuildableAttributes(
             Dictionary<string, MethodBodyStatement> attributes,
             HashSet<string> customizedBuildableTypes)
@@ -178,19 +222,7 @@ private void AddLastContractBuildableAttributes(
             return null;
         }
 
-        private static bool IsBuildableAttribute(MethodBodyStatement statement)
-        {
-            var attribute = statement switch
-            {
-                AttributeStatement directAttribute => directAttribute,
-                SuppressionStatement suppression => suppression.AsStatement<AttributeStatement>(),
-                _ => null
-            };
-
-            return attribute?.Type.Equals(s_buildableAttributeType) == true;
-        }
-
-        private HashSet<string> GetCustomizedBuildableTypes()
+        private HashSet<string> BuildCustomizedBuildableTypes()
         {
             var customizedTypes = new HashSet<string>(StringComparer.Ordinal);
             foreach (var attribute in CustomCodeView?.Attributes ?? [])

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/ModelReaderWriterContextDefinitionTests.cs

@@ -13,6 +13,7 @@
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.TypeSpec.Generator.ClientModel.Providers;
+using Microsoft.TypeSpec.Generator.Expressions;
 using Microsoft.TypeSpec.Generator.Input;
 using Microsoft.TypeSpec.Generator.Primitives;
 using Microsoft.TypeSpec.Generator.Providers;
@@ -1871,6 +1872,34 @@ public async Task CustomizedBuildableAttributesAreNotRegenerated()
                 "Buildable attributes supplied by a customized context should not be regenerated");
         }
 
+        [Test]
+        public async Task ResetRecalculatesCustomizedBuildableTypes()
+        {
+            var clientProvider = new TestClientProviderWithResponseErrorReturnType();
+            var outputLibrary = new TestOutputLibrary([clientProvider]);
+            var mockGenerator = MockHelpers.LoadMockGenerator(createOutputLibrary: () => outputLibrary);
+
+            // The initial customization declares a buildable attribute for Azure.ResponseError, so the generated
+            // buildable attribute for that type is suppressed.
+            var suppressedCompilation = await Helpers.GetCompilationFromDirectoryAsync("Suppressed");
+            mockGenerator.SetupProperty(p => p.SourceInputModel, new SourceInputModel(suppressedCompilation, null));
+
+            var contextDefinition = new ModelReaderWriterContextDefinition();
+            var suppressedContent = new TypeProviderWriter(contextDefinition).Write().Content;
+            Assert.AreEqual(Helpers.GetExpectedFromFile("Suppressed"), suppressedContent);
+
+            // Change the customization view so it no longer suppresses the buildable attribute, then reset the
+            // provider. Reset must clear the customized buildable types so they are recalculated from the current
+            // customization view instead of reusing the stale suppression.
+            var emptyCompilation = await Helpers.GetCompilationFromDirectoryAsync("Empty");
+            mockGenerator.Object.SourceInputModel = new SourceInputModel(emptyCompilation, null);
+
+            contextDefinition.Reset();
+
+            var recalculatedContent = new TypeProviderWriter(contextDefinition).Write().Content;
+            Assert.AreEqual(Helpers.GetExpectedFromFile("Empty"), recalculatedContent);
+        }
+
         [Test]
         public async Task LastContractBuildableAttributesAreRestoredWhenMissing()
         {
@@ -1911,6 +1940,48 @@ await MockHelpers.LoadMockGeneratorAsync(
             Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
         }
 
+        [Test]
+        public async Task VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute()
+        {
+            // The last contract declares a buildable attribute for RegularModel (so back-compat re-keying runs),
+            // RegularModel is emitted by the current generation, and a visitor appends a non-buildable attribute
+            // that carries a typeof(RegularModel) argument. Back-compat re-keying must key by the buildable
+            // attribute's target type only, so the visitor attribute cannot displace the generated buildable entry
+            // and is instead preserved alongside it in the final generated context.
+            var regularModel = InputFactory.Model("RegularModel", properties:
+            [
+                InputFactory.Property("Property1", InputPrimitiveType.String)
+            ]);
+
+            await MockHelpers.LoadMockGeneratorAsync(
+                inputModels: () => [regularModel],
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+
+            var contextDefinition = new ModelReaderWriterContextDefinition();
+
+            // Reuse the exact CSharpType from the generated buildable attribute so the visitor attribute shares the
+            // same type identity as the buildable attribute it must not overwrite.
+            var modelType = contextDefinition.Attributes
+                .Where(a => string.Equals(
+                    a.Type.FullyQualifiedName,
+                    typeof(ModelReaderWriterBuildableAttribute).FullName,
+                    StringComparison.Ordinal))
+                .SelectMany(a => a.Arguments.OfType<TypeOfExpression>())
+                .Select(argument => argument.Type)
+                .First(t => t.Name == "RegularModel");
+
+            // Simulate a library visitor appending [TypeConverter(typeof(RegularModel))] after the generated
+            // attributes, mirroring how visitor additions are attached on top of the generated set.
+            var visitorAttribute = new AttributeStatement(
+                new CSharpType(typeof(System.ComponentModel.TypeConverterAttribute)),
+                Snippet.TypeOf(modelType));
+            contextDefinition.Update(attributes: [.. contextDefinition.Attributes, visitorAttribute]);
+
+            var writer = new TypeProviderWriter(contextDefinition);
+            var file = writer.Write();
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
+        }
+
         [Test]
         public async Task BuildAttributesForBackCompatibilityDeduplicatesAcrossGeneratedCustomAndLastContractBuildableAttributes()
         {
@@ -2172,12 +2243,15 @@ await MockHelpers.LoadMockGeneratorAsync(
         // Buildable attributes restored from the last contract are symbol-based (IsFrameworkType == false), so
         // match by fully qualified name to cover both generated and restored entries.
         private static List<AttributeStatement> GetBuildableAttributes(ModelReaderWriterContextDefinition contextDefinition)
-            => contextDefinition.Attributes
+        {
+            contextDefinition.ProcessTypeForBackCompatibility();
+            return contextDefinition.Attributes
                 .Where(a => string.Equals(
                     a.Type.FullyQualifiedName,
                     typeof(ModelReaderWriterBuildableAttribute).FullName,
                     StringComparison.Ordinal))
                 .ToList();
+        }
 
         [Test]
         public async Task CustomProjectionPropertiesDoNotAddBuildableTypes()

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty).cs

@@ -0,0 +1,14 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System.ClientModel.Primitives;
+using Azure;
+
+namespace Sample
+{
+    [global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Azure.ResponseError))]
+    public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
+    {
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Empty)/SampleContext.cs

@@ -0,0 +1,6 @@
+namespace Sample
+{
+    public partial class SampleContext
+    {
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed).cs

@@ -0,0 +1,12 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System.ClientModel.Primitives;
+
+namespace Sample
+{
+    public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
+    {
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/ResetRecalculatesCustomizedBuildableTypes(Suppressed)/SampleContext.cs

@@ -0,0 +1,9 @@
+using System.ClientModel.Primitives;
+
+namespace Sample
+{
+    [ModelReaderWriterBuildable(typeof(Azure.ResponseError))]
+    public partial class SampleContext
+    {
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute.cs

@@ -0,0 +1,16 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System.ClientModel.Primitives;
+using System.ComponentModel;
+using Sample.Models;
+
+namespace Sample
+{
+    [global::System.ClientModel.Primitives.ModelReaderWriterBuildableAttribute(typeof(global::Sample.Models.RegularModel))]
+    [global::System.ComponentModel.TypeConverterAttribute(typeof(global::Sample.Models.RegularModel))]
+    public partial class SampleContext : global::System.ClientModel.Primitives.ModelReaderWriterContext
+    {
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/Definitions/TestData/ModelReaderWriterContextDefinitionTests/VisitorTypeValuedAttributeDoesNotOverwriteBuildableAttribute/SampleContext.cs

@@ -0,0 +1,16 @@
+using System.ClientModel.Primitives;
+
+namespace Sample
+{
+    [ModelReaderWriterBuildable(typeof(Sample.Models.RegularModel))]
+    public partial class SampleContext
+    {
+    }
+}
+
+namespace Sample.Models
+{
+    public partial class RegularModel
+    {
+    }
+}