Generate raw data field as readonly (#4808)

Fixes #4350
microsoft · Oct 21, 2024 · 48d7bc0 · 48d7bc0
1 parent 1b266e4
commit 48d7bc0
Show file tree

Hide file tree

Showing 12 changed files with 29 additions and 20 deletions.
diff --git a/...MrwSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanChangePropertyName.cs b/...MrwSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanChangePropertyName.cs
@@ -11,7 +11,7 @@ namespace Sample.Models
     public partial class MockInputModel
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
+        private protected readonly global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
 
         internal MockInputModel()
         {

diff --git a/...izationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizePropertyUsingField.cs b/...izationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizePropertyUsingField.cs
@@ -11,7 +11,7 @@ namespace Sample.Models
     public partial class MockInputModel
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
+        private protected readonly global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
 
         internal MockInputModel()
         {

diff --git a/...es/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs b/...es/http-client-csharp/generator/Microsoft.Generator.CSharp/src/Providers/ModelProvider.cs
@@ -773,10 +773,11 @@ private ValueExpression GetConversion(PropertyProvider? property = default, Fiel
             }
 
             var modifiers = FieldModifiers.Private;
-            if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Sealed))
+            if (!DeclarationModifiers.HasFlag(TypeSignatureModifiers.Sealed) && !DeclarationModifiers.HasFlag(TypeSignatureModifiers.Struct))
             {
                 modifiers |= FieldModifiers.Protected;
             }
+            modifiers |= FieldModifiers.ReadOnly;
 
             var rawDataField = new FieldProvider(
                 modifiers: modifiers,

diff --git a/.../generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelProviderTests.cs b/.../generator/Microsoft.Generator.CSharp/test/Providers/ModelProviders/ModelProviderTests.cs
@@ -378,14 +378,16 @@ public void BuildModelAsStruct()
             Assert.AreEqual(TypeSignatureModifiers.Public | TypeSignatureModifiers.Struct | TypeSignatureModifiers.Partial | TypeSignatureModifiers.ReadOnly, modelTypeProvider.DeclarationModifiers);
         }
 
-        [TestCase(true)]
-        [TestCase(false)]
-        public void TestBuildFields(bool containsMixedAdditionalProperties)
+        [TestCase(true, true)]
+        [TestCase(true, false)]
+        [TestCase(false, true)]
+        [TestCase(false, false)]
+        public void TestBuildFields(bool containsMixedAdditionalProperties, bool modelAsStruct)
         {
             InputType? additionalProperties = containsMixedAdditionalProperties
                 ? InputFactory.Union([InputPrimitiveType.Float64, InputPrimitiveType.Int64, InputPrimitiveType.String])
                 : null;
-            var inputModel = InputFactory.Model("TestModel", properties: [], additionalProperties: additionalProperties);
+            var inputModel = InputFactory.Model("TestModel", properties: [], additionalProperties: additionalProperties, modelAsStruct: modelAsStruct);
             var modelTypeProvider = new ModelProvider(inputModel);
             var fields = modelTypeProvider.Fields;
 
@@ -394,8 +396,7 @@ public void TestBuildFields(bool containsMixedAdditionalProperties)
             if (containsMixedAdditionalProperties)
             {
                 Assert.AreEqual(4, fields.Count);
-                Assert.AreEqual("_additionalBinaryDataProperties", fields[0].Name);
-                Assert.AreEqual(new CSharpType(typeof(IDictionary<string, BinaryData>)), fields[0].Type);
+                ValidateAdditionalPropertiesField(fields[0], modelAsStruct);
                 Assert.AreEqual("_additionalDoubleProperties", fields[1].Name);
                 Assert.AreEqual(new CSharpType(typeof(IDictionary<string, double>)), fields[1].Type);
                 Assert.AreEqual("_additionalInt64Properties", fields[2].Name);
@@ -405,12 +406,19 @@ public void TestBuildFields(bool containsMixedAdditionalProperties)
             }
             else
             {
-                Assert.AreEqual(1, fields.Count);
-                Assert.AreEqual("_additionalBinaryDataProperties", fields[0].Name);
-                Assert.AreEqual(new CSharpType(typeof(IDictionary<string, BinaryData>)), fields[0].Type);
+                ValidateAdditionalPropertiesField(fields[0], modelAsStruct);
             }
         }
 
+        private static void ValidateAdditionalPropertiesField(FieldProvider field, bool isStruct)
+        {
+            Assert.AreEqual("_additionalBinaryDataProperties", field.Name);
+            Assert.IsTrue(field.Modifiers.HasFlag(FieldModifiers.Private));
+            Assert.AreEqual(!isStruct, field.Modifiers.HasFlag(FieldModifiers.Protected));
+            Assert.IsTrue(field.Modifiers.HasFlag(FieldModifiers.ReadOnly));
+            Assert.AreEqual(new CSharpType(typeof(IDictionary<string, BinaryData>)), field.Type);
+        }
+
         [TestCaseSource(nameof(BuildAdditionalPropertiesTestCases))]
         public void TestBuildAdditionalProperties(
             InputModelType inputModel,

diff --git a/...tor.CSharp/test/Writers/TestData/TypeProviderWriterTests/TypeProviderWriter_WriteModel.cs b/...tor.CSharp/test/Writers/TestData/TypeProviderWriterTests/TypeProviderWriter_WriteModel.cs
@@ -12,7 +12,7 @@ namespace Sample.Models
     public partial class TestModel
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
+        private protected readonly global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
 
         /// <summary> Initializes a new instance of <see cref="global::Sample.Models.TestModel"/>. </summary>
         /// <param name="requiredString"> Description for requiredString. </param>

diff --git a/...rp/test/Writers/TestData/TypeProviderWriterTests/TypeProviderWriter_WriteModelAsStruct.cs b/...rp/test/Writers/TestData/TypeProviderWriterTests/TypeProviderWriter_WriteModelAsStruct.cs
@@ -12,7 +12,7 @@ namespace Sample.Models
     public readonly partial struct TestModel
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
+        private readonly global::System.Collections.Generic.IDictionary<string, global::System.BinaryData> _additionalBinaryDataProperties;
 
         /// <summary> Initializes a new instance of <see cref="global::Sample.Models.TestModel"/>. </summary>
         /// <param name="requiredString"> Description for requiredString. </param>

diff --git a/...ent-csharp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/Friend.cs b/...ent-csharp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/Friend.cs
@@ -11,7 +11,7 @@ namespace UnbrandedTypeSpec.Models
     public partial class Friend
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected IDictionary<string, BinaryData> _additionalBinaryDataProperties;
+        private protected readonly IDictionary<string, BinaryData> _additionalBinaryDataProperties;
 
         internal Friend(string name)
         {

diff --git a/...ects/Local/Unbranded-TypeSpec/src/Generated/Models/ModelWithRequiredNullableProperties.cs b/...ects/Local/Unbranded-TypeSpec/src/Generated/Models/ModelWithRequiredNullableProperties.cs
@@ -11,7 +11,7 @@ namespace UnbrandedTypeSpec.Models
     public partial class ModelWithRequiredNullableProperties
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected IDictionary<string, BinaryData> _additionalBinaryDataProperties;
+        private protected readonly IDictionary<string, BinaryData> _additionalBinaryDataProperties;
 
         /// <summary> Initializes a new instance of <see cref="ModelWithRequiredNullableProperties"/>. </summary>
         /// <param name="requiredNullablePrimitive"> required nullable primitive type. </param>

diff --git a/...rp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/ProjectedModel.cs b/...rp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/ProjectedModel.cs
@@ -11,7 +11,7 @@ namespace UnbrandedTypeSpec.Models
     public partial class ProjectedModel
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected IDictionary<string, BinaryData> _additionalBinaryDataProperties;
+        private protected readonly IDictionary<string, BinaryData> _additionalBinaryDataProperties;
 
         internal ProjectedModel(string name)
         {

diff --git a/...stProjects/Local/Unbranded-TypeSpec/src/Generated/Models/ReturnsAnonymousModelResponse.cs b/...stProjects/Local/Unbranded-TypeSpec/src/Generated/Models/ReturnsAnonymousModelResponse.cs
@@ -11,7 +11,7 @@ namespace UnbrandedTypeSpec.Models
     public partial class ReturnsAnonymousModelResponse
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected IDictionary<string, BinaryData> _additionalBinaryDataProperties;
+        private protected readonly IDictionary<string, BinaryData> _additionalBinaryDataProperties;
 
         internal ReturnsAnonymousModelResponse()
         {

diff --git a/...rp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/RoundTripModel.cs b/...rp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/RoundTripModel.cs
@@ -14,7 +14,7 @@ namespace UnbrandedTypeSpec.Models
     public partial class RoundTripModel
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected IDictionary<string, BinaryData> _additionalBinaryDataProperties;
+        private protected readonly IDictionary<string, BinaryData> _additionalBinaryDataProperties;
 
         /// <summary> Initializes a new instance of <see cref="RoundTripModel"/>. </summary>
         /// <param name="requiredString"> Required string, illustrating a reference type property. </param>

diff --git a/...ient-csharp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/Thing.cs b/...ient-csharp/generator/TestProjects/Local/Unbranded-TypeSpec/src/Generated/Models/Thing.cs
@@ -14,7 +14,7 @@ namespace UnbrandedTypeSpec.Models
     public partial class Thing
     {
         /// <summary> Keeps track of any properties unknown to the library. </summary>
-        private protected IDictionary<string, BinaryData> _additionalBinaryDataProperties;
+        private protected readonly IDictionary<string, BinaryData> _additionalBinaryDataProperties;
 
         /// <summary> Initializes a new instance of <see cref="Thing"/>. </summary>
         /// <param name="name"> name of the Thing. </param>