From 6a13a5b0ec81959a19be33a79e6300336c610255 Mon Sep 17 00:00:00 2001 From: Arcturus Zhang Date: Wed, 12 Jun 2024 10:29:22 +0800 Subject: [PATCH 1/5] fix the nullable value type --- .../src/OutputTypes/CSharpType.cs | 28 +++++++++++++++++-- .../test/CSharpTypeTests.cs | 13 +++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs index 9d251e69253..88c2e4d4ad1 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs @@ -65,11 +65,33 @@ public class CSharpType /// The base system type. /// Optional flag to determine if the constructed type should be nullable. Defaults to false. public CSharpType(Type type, bool isNullable = false) : this( - type, - isNullable, - type.IsGenericType ? type.GetGenericArguments().Select(p => new CSharpType(p)).ToArray() : Array.Empty()) + GetRealType(type), + GetRealArguments(type), + GetRealIsNullable(type, isNullable)) { } + private static Type GetRealType(Type type) + { + return Nullable.GetUnderlyingType(type) ?? type; + } + + private static IReadOnlyList GetRealArguments(Type type) + { + var underlyingType = Nullable.GetUnderlyingType(type); + if (underlyingType != null) + { + // if we are a System.Nullable type, we put nothing in the arguments because we have promoted the first argument to the position of type + return Array.Empty(); + } + + return type.IsGenericType ? type.GetGenericArguments().Select(p => new CSharpType(p)).ToArray() : Array.Empty(); + } + + private static bool GetRealIsNullable(Type type, bool isNullable) + { + return Nullable.GetUnderlyingType(type) != null ? true : isNullable; + } + /// /// Constructs a non-nullable from a with arguments /// diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs index 67d715c3bff..b9c91660ce0 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs @@ -9,6 +9,7 @@ using Moq; using System.IO; using System.Text; +using System.Net; namespace Microsoft.Generator.CSharp.Tests { @@ -394,5 +395,17 @@ public void TestToString(Type type, string expectedString) Assert.AreEqual(expected, actual); } + + [TestCase(typeof(int), false)] + [TestCase(typeof(int?), true)] + [TestCase(typeof(Uri), false)] + [TestCase(typeof(Guid), false)] + [TestCase(typeof(Guid?), true)] + public void ValidateNullableTypes(Type type, bool expectedIsNullable) + { + var csharpType = new CSharpType(type); + + Assert.AreEqual(expectedIsNullable, csharpType.IsNullable); + } } } From 5c6200d2c747e7fe52760eee2a5ddeb6aa097b1d Mon Sep 17 00:00:00 2001 From: Arcturus Zhang Date: Wed, 12 Jun 2024 10:39:51 +0800 Subject: [PATCH 2/5] implement in a more sufficiant way --- .../src/OutputTypes/CSharpType.cs | 16 +++++++++++++--- .../test/CSharpTypeTests.cs | 4 ++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs index 88c2e4d4ad1..c5706ddf9d8 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs @@ -65,9 +65,9 @@ public class CSharpType /// The base system type. /// Optional flag to determine if the constructed type should be nullable. Defaults to false. public CSharpType(Type type, bool isNullable = false) : this( - GetRealType(type), - GetRealArguments(type), - GetRealIsNullable(type, isNullable)) + type, + type.IsGenericType ? type.GetGenericArguments().Select(p => new CSharpType(p)).ToArray() : Array.Empty(), + isNullable) { } private static Type GetRealType(Type type) @@ -119,6 +119,16 @@ public CSharpType(Type type, IReadOnlyList arguments, bool isNullabl { Debug.Assert(type.Namespace != null, "type.Namespace != null"); + // handle nullable value types explicitly because they are implemented using generic type `System.Nullable` + var underlyingValueType = Nullable.GetUnderlyingType(type); + if (underlyingValueType != null) + { + // in this block, we are converting input like `typeof(int?)` into the way as if they input: `typeof(int), isNullable: true` + type = underlyingValueType; + arguments = type.IsGenericType ? type.GetGenericArguments().Select(p => new CSharpType(p)).ToArray() : Array.Empty(); + isNullable = true; + } + _type = type.IsGenericType ? type.GetGenericTypeDefinition() : type; ValidateArguments(_type, arguments); diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs index b9c91660ce0..ea0b7c3461c 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs @@ -401,11 +401,15 @@ public void TestToString(Type type, string expectedString) [TestCase(typeof(Uri), false)] [TestCase(typeof(Guid), false)] [TestCase(typeof(Guid?), true)] + [TestCase(typeof(TestStruct), false)] + [TestCase(typeof(TestStruct?), true)] public void ValidateNullableTypes(Type type, bool expectedIsNullable) { var csharpType = new CSharpType(type); Assert.AreEqual(expectedIsNullable, csharpType.IsNullable); } + + internal struct TestStruct where T : struct { } } } From 65a7d12108f555659d386f110fdad2afb4865e21 Mon Sep 17 00:00:00 2001 From: Arcturus Zhang Date: Wed, 12 Jun 2024 10:43:31 +0800 Subject: [PATCH 3/5] clean up --- .../src/OutputTypes/CSharpType.cs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs index c5706ddf9d8..bab9c08fcb5 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/src/OutputTypes/CSharpType.cs @@ -70,28 +70,6 @@ public CSharpType(Type type, bool isNullable = false) : this( isNullable) { } - private static Type GetRealType(Type type) - { - return Nullable.GetUnderlyingType(type) ?? type; - } - - private static IReadOnlyList GetRealArguments(Type type) - { - var underlyingType = Nullable.GetUnderlyingType(type); - if (underlyingType != null) - { - // if we are a System.Nullable type, we put nothing in the arguments because we have promoted the first argument to the position of type - return Array.Empty(); - } - - return type.IsGenericType ? type.GetGenericArguments().Select(p => new CSharpType(p)).ToArray() : Array.Empty(); - } - - private static bool GetRealIsNullable(Type type, bool isNullable) - { - return Nullable.GetUnderlyingType(type) != null ? true : isNullable; - } - /// /// Constructs a non-nullable from a with arguments /// From 52ea43db0bf0a5cb9264135ee1d2f6cac2bbe037 Mon Sep 17 00:00:00 2001 From: Arcturus Zhang Date: Wed, 12 Jun 2024 10:56:54 +0800 Subject: [PATCH 4/5] refine the test cases --- .../test/CSharpTypeTests.cs | 68 ++++++++++++++++--- 1 file changed, 59 insertions(+), 9 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs index ea0b7c3461c..3848b7c7692 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs @@ -396,20 +396,70 @@ public void TestToString(Type type, string expectedString) Assert.AreEqual(expected, actual); } - [TestCase(typeof(int), false)] - [TestCase(typeof(int?), true)] - [TestCase(typeof(Uri), false)] - [TestCase(typeof(Guid), false)] - [TestCase(typeof(Guid?), true)] - [TestCase(typeof(TestStruct), false)] - [TestCase(typeof(TestStruct?), true)] - public void ValidateNullableTypes(Type type, bool expectedIsNullable) + [TestCaseSource(nameof(ValidateNullableTypesData))] + public void ValidateNullableTypes(Type type, IReadOnlyList expectedArguments, bool expectedIsNullable) { var csharpType = new CSharpType(type); + CollectionAssert.AreEqual(expectedArguments, csharpType.Arguments); Assert.AreEqual(expectedIsNullable, csharpType.IsNullable); } - internal struct TestStruct where T : struct { } + private static object[] ValidateNullableTypesData = [ + new object[] + { + typeof(int), Array.Empty(), false + }, + new object[] + { + typeof(int?), Array.Empty(), true + }, + new object[] + { + typeof(Uri), Array.Empty(), false + }, + new object[] + { + typeof(Guid), Array.Empty(), false + }, + new object[] + { + typeof(Guid?), Array.Empty(), true + }, + new object[] + { + typeof(TestStruct), new CSharpType[] { typeof(int) }, false + }, + new object[] + { + typeof(TestStruct?), new CSharpType[] { typeof(int) }, true + }, + new object[] + { + typeof(TestStruct), new CSharpType[] { typeof(int?) }, false + }, + new object[] + { + typeof(TestStruct?), new CSharpType[] { typeof(int?) }, true + }, + new object[] + { + typeof(TestStruct>), new CSharpType[] { typeof(TestStruct) }, false + }, + new object[] + { + typeof(TestStruct>?), new CSharpType[] { typeof(TestStruct) }, true + }, + new object[] + { + typeof(TestStruct?>), new CSharpType[] { typeof(TestStruct?) }, false + }, + new object[] + { + typeof(TestStruct?>?), new CSharpType[] { typeof(TestStruct?) }, true + }, + ]; + + internal struct TestStruct { } } } From 1637dbe95353ebaea4e44ee19494eade48325351 Mon Sep 17 00:00:00 2001 From: Arcturus Zhang Date: Wed, 12 Jun 2024 10:57:19 +0800 Subject: [PATCH 5/5] clean up --- .../Microsoft.Generator.CSharp/test/CSharpTypeTests.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs index 3848b7c7692..e5cf8aed3e1 100644 --- a/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.Generator.CSharp/test/CSharpTypeTests.cs @@ -1,15 +1,14 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -using System.Collections.Generic; using System; -using NUnit.Framework; -using System.Linq; +using System.Collections.Generic; using System.Collections.Immutable; -using Moq; using System.IO; +using System.Linq; using System.Text; -using System.Net; +using Moq; +using NUnit.Framework; namespace Microsoft.Generator.CSharp.Tests {