Merge branch 'main' into MultipleScenarios

microsoft · Oct 24, 2024 · 43c78aa · 43c78aa
2 parents 6723071 + ffef479
commit 43c78aa
Show file tree

Hide file tree

Showing 24 changed files with 91 additions and 76 deletions.
diff --git a/...or/Microsoft.Generator.CSharp.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs b/...or/Microsoft.Generator.CSharp.ClientModel/src/Providers/MrwSerializationTypeDefinition.cs
@@ -1065,6 +1065,7 @@ private MethodBodyStatement[] DeserializeProperty(
             ScopedApi<JsonProperty> jsonProperty,
             IEnumerable<AttributeData> serializationAttributes)
         {
+            bool useCustomDeserializationHook = false;
             var serializationFormat = wireInfo.SerializationFormat;
             var propertyVarReference = variableExpression;
             var deserializationStatements = new MethodBodyStatement[2]
@@ -1088,13 +1089,16 @@ private MethodBodyStatement[] DeserializeProperty(
                             deserializationHook,
                             jsonProperty,
                             ByRef(propertyVarReference)).Terminate()];
+                    useCustomDeserializationHook = true;
                     break;
                 }
             }
 
             return
             [
-                DeserializationPropertyNullCheckStatement(propertyType, wireInfo, jsonProperty, propertyVarReference),
+                useCustomDeserializationHook
+                    ? MethodBodyStatement.Empty
+                    : DeserializationPropertyNullCheckStatement(propertyType, wireInfo, jsonProperty, propertyVarReference),
                 deserializationStatements,
                 Continue
             ];
@@ -1117,7 +1121,7 @@ private static MethodBodyStatement DeserializationPropertyNullCheckStatement(
             CSharpType serializedType = propertyType;
             var propertyIsRequired = wireInfo.IsRequired;
 
-            if (serializedType.IsNullable || !propertyIsRequired)
+            if ((serializedType.IsNullable || !serializedType.IsValueType) && wireInfo.IsNullable)
             {
                 if (!serializedType.IsCollection)
                 {

diff --git a/...CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/DeserializationTests.cs b/...CSharp.ClientModel/test/Providers/MrwSerializationTypeDefinitions/DeserializationTests.cs
@@ -1,6 +1,8 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
+using System;
+using System.Collections.Generic;
 using System.Linq;
 using Microsoft.Generator.CSharp.ClientModel.Providers;
 using Microsoft.Generator.CSharp.Input;
@@ -46,5 +48,50 @@ public void DeserializeStruct()
             var file = writer.Write();
             Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
         }
+
+        [TestCaseSource(nameof(TestDeserializationStatementTestCases))]
+        public void TestDeserializationStatement(InputModelProperty prop, bool hasNullCheck, bool hasNullAssignment)
+        {
+            var inputModel = InputFactory.Model("TestModel", properties: [prop]);
+
+            var mrwProvider = new ModelProvider(inputModel).SerializationProviders.FirstOrDefault();
+            Assert.IsNotNull(mrwProvider);
+
+            var deserializationMethod = mrwProvider!.Methods.Where(m => m.Signature.Name.StartsWith("Deserialize")).FirstOrDefault();
+            Assert.IsNotNull(deserializationMethod);
+
+            var deserializationStatements = deserializationMethod!.BodyStatements;
+
+            Assert.AreEqual(hasNullCheck, deserializationStatements!.ToDisplayString().Contains(
+               "if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))"));
+            Assert.AreEqual(hasNullAssignment, deserializationStatements!.ToDisplayString().Contains("prop1 = null;"));
+        }
+
+        public static IEnumerable<TestCaseData> TestDeserializationStatementTestCases
+        {
+            get
+            {
+                // non-nullable string property
+                yield return new TestCaseData(
+                    InputFactory.Property("prop1", InputPrimitiveType.String),
+                    false,
+                    false);
+                // nullable string property
+                yield return new TestCaseData(
+                    InputFactory.Property("prop1", new InputNullableType(InputPrimitiveType.String)),
+                    true,
+                    true);
+                // required string property
+                yield return new TestCaseData(
+                    InputFactory.Property("prop1", InputPrimitiveType.String, isRequired: true),
+                    false,
+                    false);
+                // reference type property
+                yield return new TestCaseData(
+                    InputFactory.Property("prop1", InputFactory.Model("TestModel2")),
+                    true,
+                    false);
+            }
+        }
     }
 }
diff --git a/...ntModel/test/Providers/MrwSerializationTypeDefinitions/SerializationCustomizationTests.cs b/...ntModel/test/Providers/MrwSerializationTypeDefinitions/SerializationCustomizationTests.cs
@@ -359,5 +359,33 @@ public async Task CanCustomizePropertyUsingField(bool redefineProperty)
             Assert.IsTrue(fullCtor.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Internal));
             Assert.AreEqual(2, fullCtor.Signature.Parameters.Count);
         }
+
+        [Test]
+        public async Task CanChangeToNonNullableProp()
+        {
+            var props = new[]
+            {
+                InputFactory.Property("Prop1", new InputNullableType(InputPrimitiveType.String))
+            };
+
+            var inputModel = InputFactory.Model("Model", properties: props, usage: InputModelTypeUsage.Json);
+            var plugin = await MockHelpers.LoadMockPluginAsync(
+                inputModels: () => [inputModel],
+                compilation: async () => await Helpers.GetCompilationFromDirectoryAsync());
+
+            var modelProvider = plugin.Object.OutputLibrary.TypeProviders.Single(t => t is ModelProvider);
+            var serializationProvider = modelProvider.SerializationProviders.Single(t => t is MrwSerializationTypeDefinition);
+            Assert.IsNotNull(serializationProvider);
+
+            var deserializationMethod = serializationProvider!.Methods.Where(m => m.Signature.Name.StartsWith("Deserialize")).FirstOrDefault();
+            Assert.IsNotNull(deserializationMethod);
+
+            var deserializationStatements = deserializationMethod!.BodyStatements;
+
+            Assert.IsTrue(deserializationStatements!.ToDisplayString().Contains(
+                "if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))"));
+            // since the customized property is non-nullable, the assignment to null should not be present
+            Assert.IsFalse(deserializationStatements!.ToDisplayString().Contains("prop1 = null;"));
+        }
     }
 }
diff --git a/...viders/MrwSerializationTypeDefinitions/TestData/DeserializationTests/DeserializeStruct.cs b/...viders/MrwSerializationTypeDefinitions/TestData/DeserializationTests/DeserializeStruct.cs
@@ -25,11 +25,6 @@ namespace Sample.Models
             {
                 if (prop.NameEquals("stringProperty"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        stringProperty = null;
-                        continue;
-                    }
                     stringProperty = prop.Value.GetString();
                     continue;
                 }

diff --git a/...nTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeEnumToFieldFrameworkType.cs b/...nTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeEnumToFieldFrameworkType.cs
@@ -81,7 +81,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = prop.Value.GetObject();

diff --git a/...zationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeEnumToFrameworkType.cs b/...zationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeEnumToFrameworkType.cs
@@ -81,7 +81,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = prop.Value.GetObject();

diff --git a/...ializationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeFixedEnumString.cs b/...ializationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeFixedEnumString.cs
@@ -78,7 +78,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = prop.Value.GetString().ToMockInputEnum();

diff --git a/...TypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeNullableStringToFixedEnum.cs b/...TypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeNullableStringToFixedEnum.cs
@@ -85,7 +85,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = prop.Value.GetString().ToMockInputEnum();

diff --git a/...lizationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeStringToFixedEnum.cs b/...lizationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeStringToFixedEnum.cs
@@ -78,7 +78,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = prop.Value.GetString().ToMockInputEnum();

diff --git a/...wSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeUriProperty.cs b/...wSerializationTypeDefinitions/TestData/ModelCustomizationTests/CanCustomizeUriProperty.cs
@@ -81,7 +81,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = new global::System.Uri(prop.Value.GetString());

diff --git a/...peDefinitions/TestData/SerializationCustomizationTests/CanChangeDictionaryToBinaryData.cs b/...peDefinitions/TestData/SerializationCustomizationTests/CanChangeDictionaryToBinaryData.cs
@@ -88,7 +88,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = global::System.BinaryData.FromString(prop.Value.GetRawText());

diff --git a/...lizationTypeDefinitions/TestData/SerializationCustomizationTests/CanChangePropertyName.cs b/...lizationTypeDefinitions/TestData/SerializationCustomizationTests/CanChangePropertyName.cs
@@ -91,7 +91,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop2 = null;
                         continue;
                     }
                     global::System.Collections.Generic.List<string> array = new global::System.Collections.Generic.List<string>();

diff --git a/...peDefinitions/TestData/SerializationCustomizationTests/CanChangePropertySerializedName.cs b/...peDefinitions/TestData/SerializationCustomizationTests/CanChangePropertySerializedName.cs
@@ -91,31 +91,16 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
             {
                 if (prop.NameEquals("customName"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        name = null;
-                        continue;
-                    }
                     name = prop.Value.GetString();
                     continue;
                 }
                 if (prop.NameEquals("flavor"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        flavor = null;
-                        continue;
-                    }
                     flavor = prop.Value.GetString();
                     continue;
                 }
                 if (prop.NameEquals("customColor2"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        customColor = null;
-                        continue;
-                    }
                     customColor = prop.Value.GetString();
                     continue;
                 }

diff --git a/...eDefinitions/TestData/SerializationCustomizationTests/CanChangeToNonNullableProp/Model.cs b/...eDefinitions/TestData/SerializationCustomizationTests/CanChangeToNonNullableProp/Model.cs
@@ -0,0 +1,10 @@
+
+using Microsoft.Generator.CSharp.Customization;
+
+namespace Sample.Models
+{
+    public partial class Model
+    {
+        public int Prop1 { get; set; }
+    }
+}
diff --git a/...Definitions/TestData/SerializationCustomizationTests/CanCustomizeExtensibleEnum(int32).cs b/...Definitions/TestData/SerializationCustomizationTests/CanCustomizeExtensibleEnum(int32).cs
@@ -78,7 +78,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = new global::Sample.Models.EnumType(prop.Value.GetInt32());

diff --git a/...efinitions/TestData/SerializationCustomizationTests/CanCustomizeExtensibleEnum(string).cs b/...efinitions/TestData/SerializationCustomizationTests/CanCustomizeExtensibleEnum(string).cs
@@ -78,7 +78,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = new global::Sample.Models.EnumType(prop.Value.GetString());

diff --git a/...ns/TestData/SerializationCustomizationTests/CanCustomizeLiteralExtensibleEnum(int32,1).cs b/...ns/TestData/SerializationCustomizationTests/CanCustomizeLiteralExtensibleEnum(int32,1).cs
@@ -78,7 +78,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = new global::Sample.Models.EnumType(prop.Value.GetInt32());

diff --git a/...TestData/SerializationCustomizationTests/CanCustomizeLiteralExtensibleEnum(string,foo).cs b/...TestData/SerializationCustomizationTests/CanCustomizeLiteralExtensibleEnum(string,foo).cs
@@ -78,7 +78,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
                 {
                     if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
                     {
-                        prop1 = null;
                         continue;
                     }
                     prop1 = new global::Sample.Models.EnumType(prop.Value.GetString());

diff --git a/...ypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizePropertyUsingField.cs b/...ypeDefinitions/TestData/SerializationCustomizationTests/CanCustomizePropertyUsingField.cs
@@ -79,11 +79,6 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
             {
                 if (prop.NameEquals("prop1"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        prop1 = null;
-                        continue;
-                    }
                     prop1 = prop.Value.GetString();
                     continue;
                 }

diff --git a/...peDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethod.cs b/...peDefinitions/TestData/SerializationCustomizationTests/CanCustomizeSerializationMethod.cs
@@ -92,21 +92,11 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
             {
                 if (prop.NameEquals("prop1"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        prop1 = null;
-                        continue;
-                    }
                     DeserializationMethod(prop, ref prop1);
                     continue;
                 }
                 if (prop.NameEquals("prop2"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        prop2 = null;
-                        continue;
-                    }
                     DeserializationMethod(prop, ref prop2);
                     continue;
                 }

diff --git a/...Data/SerializationCustomizationTests/CanCustomizeSerializationMethodForRenamedProperty.cs b/...Data/SerializationCustomizationTests/CanCustomizeSerializationMethodForRenamedProperty.cs
@@ -92,21 +92,11 @@ protected virtual void JsonModelWriteCore(global::System.Text.Json.Utf8JsonWrite
             {
                 if (prop.NameEquals("prop1"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        prop1 = null;
-                        continue;
-                    }
                     DeserializationMethod(prop, ref prop1);
                     continue;
                 }
                 if (prop.NameEquals("prop2"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        prop3 = null;
-                        continue;
-                    }
                     DeserializationMethod(prop, ref prop3);
                     continue;
                 }

diff --git a/...TypeDefinitions/TestData/SerializationCustomizationTests/CanReplaceSerializationMethod.cs b/...TypeDefinitions/TestData/SerializationCustomizationTests/CanReplaceSerializationMethod.cs
@@ -49,11 +49,6 @@ public partial class MockInputModel : global::System.ClientModel.Primitives.IJso
             {
                 if (prop.NameEquals("prop1"u8))
                 {
-                    if ((prop.Value.ValueKind == global::System.Text.Json.JsonValueKind.Null))
-                    {
-                        prop1 = null;
-                        continue;
-                    }
                     prop1 = prop.Value.GetString();
                     continue;
                 }