Centralize C# type attribute back-compat merging in TypeProvider

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

@@ -8,6 +8,7 @@
 using System.IO;
 using System.Linq;
 using System.Threading.Tasks;
+using Microsoft.TypeSpec.Generator.ClientModel.Utilities;
 using Microsoft.TypeSpec.Generator.Primitives;
 using Microsoft.TypeSpec.Generator.Providers;
 using Microsoft.TypeSpec.Generator.Statements;
@@ -20,6 +21,10 @@ public class ModelReaderWriterContextDefinition : TypeProvider
         private const string DefaultObsoleteDiagnosticId = "CS0618";
         private static readonly CSharpTypeNameComparer s_cSharpTypeNameComparer = new CSharpTypeNameComparer();
         private static readonly TypeProviderTypeNameComparer s_typeProviderNameComparer = new TypeProviderTypeNameComparer();
+        private static readonly Lazy<HashSet<string>> s_attributesToIgnore = new(() => new(StringComparer.Ordinal)
+        {
+            nameof(ModelReaderWriterBuildableAttribute),
+        });
 
         internal static readonly string s_name = $"{RemovePeriods(ScmCodeModelGenerator.Instance.TypeFactory.PrimaryNamespace)}Context";
 
@@ -76,6 +81,17 @@ protected override IReadOnlyList<MethodBodyStatement> BuildAttributes()
             return attributes.OrderBy(a => GetSimpleTypeName(a.Key)).Select(kvp => kvp.Value).ToList();
         }
 
+        /// <summary>
+        /// Restores back-compatibility attributes from the last contract, then drops any restored
+        /// <see cref="ModelReaderWriterBuildableAttribute"/> since those are recomputed at write time.
+        /// </summary>
+        protected override IReadOnlyList<AttributeStatement> BuildAttributesForBackCompatibility(IEnumerable<AttributeStatement> originalAttributes)
+        {
+            var original = originalAttributes as IReadOnlyList<AttributeStatement> ?? [.. originalAttributes];
+            var merged = base.BuildAttributesForBackCompatibility(original);
+            return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore);
+        }
+
         /// <summary>
         /// Collects all types that implement IPersistableModel, including all models and their properties
         /// that are also IPersistableModel types, recursively without duplicates.

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<HashSet<string>> s_attributesToIgnore = new(() => new(StringComparer.Ordinal)
+        {
+            nameof(PersistableModelProxyAttribute),
+        });
         private readonly ParameterProvider _utf8JsonWriterParameter = new("writer", $"The JSON writer.", typeof(Utf8JsonWriter));
         private readonly ParameterProvider _utf8JsonReaderParameter = new("reader", $"The JSON reader.", typeof(Utf8JsonReader), isRef: true);
         private readonly ParameterProvider _serializationOptionsParameter =
@@ -152,6 +156,17 @@ protected override IReadOnlyList<AttributeStatement> BuildAttributes()
             return [];
         }
 
+        /// <summary>
+        /// Restores back-compatibility attributes from the last contract, then drops any restored
+        /// <see cref="PersistableModelProxyAttribute"/> since those are recomputed at generation time.
+        /// </summary>
+        protected override IReadOnlyList<AttributeStatement> BuildAttributesForBackCompatibility(IEnumerable<AttributeStatement> originalAttributes)
+        {
+            var original = originalAttributes as IReadOnlyList<AttributeStatement> ?? [.. originalAttributes];
+            var merged = base.BuildAttributesForBackCompatibility(original);
+            return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore);
+        }
+
         private CSharpType GetRootModelType()
         {
             // We need to explicitly use the BaseModelProvider when looking up the root type

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<HashSet<string>> 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;
         }
 
+        /// <summary>
+        /// Restores back-compatibility attributes from the last contract, then drops any restored
+        /// <see cref="PersistableModelProxyAttribute"/> since those are recomputed at generation time by the
+        /// serialization provider.
+        /// </summary>
+        protected override IReadOnlyList<AttributeStatement> BuildAttributesForBackCompatibility(IEnumerable<AttributeStatement> originalAttributes)
+        {
+            var original = originalAttributes as IReadOnlyList<AttributeStatement> ?? [.. originalAttributes];
+            var merged = base.BuildAttributesForBackCompatibility(original);
+            return ScmBackCompatibilityHelpers.FilterRestoredAttributes(original, merged, s_attributesToIgnore);
+        }
+
         protected override ConstructorProvider[] BuildConstructors()
         {
             List<ConstructorProvider> constructors = [.. base.BuildConstructors()];

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Utilities/ScmBackCompatibilityHelpers.cs

@@ -0,0 +1,56 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using Microsoft.TypeSpec.Generator.Statements;
+
+namespace Microsoft.TypeSpec.Generator.ClientModel.Utilities
+{
+    internal static class ScmBackCompatibilityHelpers
+    {
+        /// <summary>
+        /// Drops any generator-owned attributes that were newly restored from the last contract.
+        /// <paramref name="restoredAttributes"/> is the result of the base
+        /// <c>BuildAttributesForBackCompatibility</c> call; any attribute whose type name is in
+        /// <paramref name="generatorOwnedAttributeNames"/> and was not already present in
+        /// <paramref name="originalAttributes"/> is removed, since generation recomputes it.
+        /// <paramref name="generatorOwnedAttributeNames"/> 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.
+        /// </summary>
+        public static IReadOnlyList<AttributeStatement> FilterRestoredAttributes(
+            IReadOnlyList<AttributeStatement> originalAttributes,
+            IReadOnlyList<AttributeStatement> restoredAttributes,
+            Lazy<HashSet<string>> generatorOwnedAttributeNames)
+        {
+            // The base returns the original list unchanged when nothing was restored.
+            if (ReferenceEquals(restoredAttributes, originalAttributes))
+            {
+                return restoredAttributes;
+            }
+
+            var ownedNames = generatorOwnedAttributeNames.Value;
+
+            var originalDisplayStrings = new HashSet<string>(StringComparer.Ordinal);
+            foreach (var attribute in originalAttributes)
+            {
+                originalDisplayStrings.Add(attribute.ToDisplayString());
+            }
+
+            var result = new List<AttributeStatement>(restoredAttributes.Count);
+            foreach (var attribute in restoredAttributes)
+            {
+                if (ownedNames.Contains(attribute.Type.Name)
+                    && !originalDisplayStrings.Contains(attribute.ToDisplayString()))
+                {
+                    continue;
+                }
+
+                result.Add(attribute);
+            }
+
+            return result;
+        }
+    }
+}

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

@@ -1622,6 +1622,24 @@ public void ValidateTypeWithCustomSerializationProviderImplementingIPersistableM
                 "ModelWithCustomSerialization should be included because it has a serialization provider implementing IJsonModel");
         }
 
+        [Test]
+        public async Task BuildAttributesForBackCompatibilityDoesNotRestoreBuildableAttribute()
+        {
+            await MockHelpers.LoadMockGeneratorAsync(
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+
+            var contextDefinition = new ModelReaderWriterContextDefinition();
+
+            // The last contract declares a ModelReaderWriterBuildable attribute (owned by generation and
+            // rebuilt at write time) alongside a Description attribute. Back-compat processing should only
+            // restore the non-buildable Description attribute.
+            contextDefinition.ProcessTypeForBackCompatibility();
+
+            var writer = new TypeProviderWriter(contextDefinition);
+            var file = writer.Write();
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
+        }
+
         private class CustomSerializationProvider : TypeProvider
         {
             private readonly bool _usePersistableModel;

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

@@ -0,0 +1,14 @@
+// <auto-generated/>
+
+#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
+    {
+    }
+}

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

@@ -0,0 +1,11 @@
+using System.ComponentModel;
+using System.ClientModel.Primitives;
+
+namespace Sample
+{
+    [ModelReaderWriterBuildable(typeof(object))]
+    [Description("bc")]
+    public partial class SampleContext : ModelReaderWriterContext
+    {
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/BackCompatibilityTests.cs

@@ -0,0 +1,56 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.TypeSpec.Generator.ClientModel.Providers;
+using Microsoft.TypeSpec.Generator.Input;
+using Microsoft.TypeSpec.Generator.Primitives;
+using Microsoft.TypeSpec.Generator.Providers;
+using Microsoft.TypeSpec.Generator.Tests.Common;
+using NUnit.Framework;
+
+namespace Microsoft.TypeSpec.Generator.ClientModel.Tests.Providers.MrwSerializationTypeDefinitions
+{
+    internal class BackCompatibilityTests
+    {
+        private class MockMrwProvider : MrwSerializationTypeDefinition
+        {
+            public MockMrwProvider(InputModelType inputModel, ModelProvider modelProvider)
+                : base(inputModel, modelProvider)
+            {
+            }
+
+            protected override MethodProvider[] BuildMethods() => [];
+            protected internal override FieldProvider[] BuildFields() => [];
+            protected override ConstructorProvider[] BuildConstructors() => [];
+        }
+
+        [Test]
+        public async Task BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute()
+        {
+            var inputModel = InputFactory.Model("pet", properties:
+            [
+                InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)
+            ]);
+
+            var mockGenerator = await MockHelpers.LoadMockGeneratorAsync(
+                inputModels: () => [inputModel],
+                createSerializationsCore: (inputType, typeProvider)
+                    => inputType is InputModelType modelType ? [new MockMrwProvider(modelType, (typeProvider as ModelProvider)!)] : [],
+                lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+
+            var modelProvider = mockGenerator.Object.OutputLibrary.TypeProviders.Single(t => t is ModelProvider);
+            var serializationProvider = modelProvider.SerializationProviders.Single(t => t is MrwSerializationTypeDefinition);
+
+            // The last contract declares a PersistableModelProxy attribute (owned by generation and
+            // recomputed at generation time) alongside a Description attribute. Back-compat processing
+            // should only restore the non-proxy Description attribute.
+            serializationProvider.ProcessTypeForBackCompatibility();
+
+            var writer = new TypeProviderWriter(serializationProvider);
+            var file = writer.Write();
+            Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
+        }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/BackCompatibilityTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs

@@ -0,0 +1,14 @@
+// <auto-generated/>
+
+#nullable disable
+
+using System.ClientModel.Primitives;
+using System.ComponentModel;
+
+namespace Sample.Models
+{
+    [global::System.ComponentModel.DescriptionAttribute("bc")]
+    public partial class Pet : global::System.ClientModel.Primitives.IJsonModel<global::Sample.Models.Pet>
+    {
+    }
+}

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
+    {
+    }
+}

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()
         {

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute.cs

@@ -0,0 +1,32 @@
+// <auto-generated/>
+
+#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<string, global::System.BinaryData> _additionalBinaryDataProperties;
+
+        public Pet(string name)
+        {
+            global::Sample.Argument.AssertNotNull(name, nameof(name));
+
+            Name = name;
+        }
+
+        internal Pet(string name, global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> additionalBinaryDataProperties)
+        {
+            Name = name;
+            _additionalBinaryDataProperties = additionalBinaryDataProperties;
+        }
+
+        public string Name { get; set; }
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmModelProvider/TestData/ScmModelProviderTests/BuildAttributesForBackCompatibilityDoesNotRestoreProxyAttribute/Pet.cs

@@ -0,0 +1,11 @@
+using System.ComponentModel;
+using System.ClientModel.Primitives;
+
+namespace Sample.Models
+{
+    [PersistableModelProxy(typeof(object))]
+    [Description("bc")]
+    public partial class Pet
+    {
+    }
+}

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/TestHelpers/MockHelpers.cs

@@ -34,7 +34,8 @@ public static async Task<Mock<ScmCodeModelGenerator>> LoadMockGeneratorAsync(
             Func<IReadOnlyList<string>>? apiVersions = null,
             string? configuration = null,
             Func<InputType, CSharpType>? createCSharpTypeCore = null,
-            Func<InputType, bool>? createCSharpTypeCoreFallback = null)
+            Func<InputType, bool>? createCSharpTypeCoreFallback = null,
+            Func<InputType, TypeProvider, IReadOnlyList<TypeProvider>>? createSerializationsCore = null)
         {
             var mockGenerator = LoadMockGenerator(
                 inputLiterals: inputLiterals,
@@ -44,7 +45,8 @@ public static async Task<Mock<ScmCodeModelGenerator>> LoadMockGeneratorAsync(
                 apiVersions: apiVersions,
                 configuration: configuration,
                 createCSharpTypeCore: createCSharpTypeCore,
-                createCSharpTypeCoreFallback: createCSharpTypeCoreFallback);
+                createCSharpTypeCoreFallback: createCSharpTypeCoreFallback,
+                createSerializationsCore: createSerializationsCore);
 
             var compilationResult = compilation == null ? null : await compilation();
             var lastContractCompilationResult = lastContractCompilation == null ? null : await lastContractCompilation();