Skip to content

Commit

Permalink
fix the nullable value type (#3567)
Browse files Browse the repository at this point in the history
Fixes Azure/autorest.csharp#4678

We are only checking if the input type is a generic type, and then get
its generic argument if it is.
We are not checking if it is this special generic type
`System.Nullable<T>` which appears to be a generic type but we actually
want it to be non-generic but nullable.

This PR changes the final constructor that would be invoke for all cases
that a ctor of CSharpType from a `System.Type` to add a check if it is
`System.Nullable<T>`, we will use `T` as `_type`, and the arguments of
`T` as the real arguments.
  • Loading branch information
ArcturusZhang committed Jun 12, 2024
1 parent 1f82d65 commit 913aa03
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ public class CSharpType
/// <param name="isNullable">Optional flag to determine if the constructed type should be nullable. Defaults to <c>false</c>.</param>
public CSharpType(Type type, bool isNullable = false) : this(
type,
isNullable,
type.IsGenericType ? type.GetGenericArguments().Select(p => new CSharpType(p)).ToArray() : Array.Empty<CSharpType>())
type.IsGenericType ? type.GetGenericArguments().Select(p => new CSharpType(p)).ToArray() : Array.Empty<CSharpType>(),
isNullable)
{ }

/// <summary>
Expand Down Expand Up @@ -97,6 +97,16 @@ public CSharpType(Type type, IReadOnlyList<CSharpType> 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<T>`
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<CSharpType>();
isNullable = true;
}

_type = type.IsGenericType ? type.GetGenericTypeDefinition() : type;
ValidateArguments(_type, arguments);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +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 Moq;
using NUnit.Framework;

namespace Microsoft.Generator.CSharp.Tests
{
Expand Down Expand Up @@ -394,5 +394,71 @@ public void TestToString(Type type, string expectedString)

Assert.AreEqual(expected, actual);
}

[TestCaseSource(nameof(ValidateNullableTypesData))]
public void ValidateNullableTypes(Type type, IReadOnlyList<CSharpType> expectedArguments, bool expectedIsNullable)
{
var csharpType = new CSharpType(type);

CollectionAssert.AreEqual(expectedArguments, csharpType.Arguments);
Assert.AreEqual(expectedIsNullable, csharpType.IsNullable);
}

private static object[] ValidateNullableTypesData = [
new object[]
{
typeof(int), Array.Empty<CSharpType>(), false
},
new object[]
{
typeof(int?), Array.Empty<CSharpType>(), true
},
new object[]
{
typeof(Uri), Array.Empty<CSharpType>(), false
},
new object[]
{
typeof(Guid), Array.Empty<CSharpType>(), false
},
new object[]
{
typeof(Guid?), Array.Empty<CSharpType>(), true
},
new object[]
{
typeof(TestStruct<int>), new CSharpType[] { typeof(int) }, false
},
new object[]
{
typeof(TestStruct<int>?), new CSharpType[] { typeof(int) }, true
},
new object[]
{
typeof(TestStruct<int?>), new CSharpType[] { typeof(int?) }, false
},
new object[]
{
typeof(TestStruct<int?>?), new CSharpType[] { typeof(int?) }, true
},
new object[]
{
typeof(TestStruct<TestStruct<int>>), new CSharpType[] { typeof(TestStruct<int>) }, false
},
new object[]
{
typeof(TestStruct<TestStruct<int>>?), new CSharpType[] { typeof(TestStruct<int>) }, true
},
new object[]
{
typeof(TestStruct<TestStruct<int>?>), new CSharpType[] { typeof(TestStruct<int>?) }, false
},
new object[]
{
typeof(TestStruct<TestStruct<int>?>?), new CSharpType[] { typeof(TestStruct<int>?) }, true
},
];

internal struct TestStruct<T> { }
}
}

0 comments on commit 913aa03

Please sign in to comment.