Make nullable-related checks consistent and faster (#42401)
authorSteve Harter <steveharter@users.noreply.github.com>
Wed, 23 Sep 2020 14:52:41 +0000 (09:52 -0500)
committerGitHub <noreply@github.com>
Wed, 23 Sep 2020 14:52:41 +0000 (09:52 -0500)
src/libraries/System.Text.Json/src/System.Text.Json.csproj
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/NullableConverterFactory.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/GenericMethodHolder.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoOfT.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs
src/libraries/System.Text.Json/src/System/Text/Json/TypeExtensions.cs [deleted file]
src/libraries/System.Text.Json/src/System/TypeExtensions.cs [new file with mode: 0644]

index 7542db8..f1ddddb 100644 (file)
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <TargetFrameworks>$(NetCoreAppCurrent);netstandard2.0;netcoreapp3.0;net461</TargetFrameworks>
     <Compile Include="System\Text\Json\Serialization\WriteStackFrame.cs" />
     <Compile Include="System\Text\Json\ThrowHelper.cs" />
     <Compile Include="System\Text\Json\ThrowHelper.Serialization.cs" />
-    <Compile Include="System\Text\Json\TypeExtensions.cs" />
     <Compile Include="System\Text\Json\Writer\JsonWriterHelper.cs" />
     <Compile Include="System\Text\Json\Writer\JsonWriterHelper.Date.cs" />
     <Compile Include="System\Text\Json\Writer\JsonWriterHelper.Escaping.cs" />
     <Compile Include="System\Text\Json\Writer\Utf8JsonWriter.WriteValues.SignedNumber.cs" />
     <Compile Include="System\Text\Json\Writer\Utf8JsonWriter.WriteValues.String.cs" />
     <Compile Include="System\Text\Json\Writer\Utf8JsonWriter.WriteValues.UnsignedNumber.cs" />
+    <Compile Include="System\TypeExtensions.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)'">
     <Compile Include="$(CoreLibSharedDir)System\Diagnostics\CodeAnalysis\DynamicDependencyAttribute.cs" />
index 1967933..dfab566 100644 (file)
@@ -10,7 +10,7 @@ namespace System.Text.Json.Serialization.Converters
     {
         public override bool CanConvert(Type typeToConvert)
         {
-            return Nullable.GetUnderlyingType(typeToConvert) != null;
+            return typeToConvert.IsNullableOfT();
         }
 
         public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
index ef8e362..d321ab2 100644 (file)
@@ -26,7 +26,7 @@ namespace System.Text.Json.Serialization
         {
             // For performance, we only want to call this method for non-nullable value types.
             // Nullable types should be checked againt 'null' before calling this method.
-            Debug.Assert(Nullable.GetUnderlyingType(value.GetType()) == null);
+            Debug.Assert(!value.GetType().CanBeNull());
 
             return EqualityComparer<T>.Default.Equals(default, (T)value);
         }
index 38bd9cb..348c558 100644 (file)
@@ -21,7 +21,7 @@ namespace System.Text.Json.Serialization
             // In the future, this will be check for !IsSealed (and excluding value types).
             CanBePolymorphic = TypeToConvert == JsonClassInfo.ObjectType;
             IsValueType = TypeToConvert.IsValueType;
-            CanBeNull = !IsValueType || Nullable.GetUnderlyingType(TypeToConvert) != null;
+            CanBeNull = !IsValueType || TypeToConvert.IsNullableOfT();
             IsInternalConverter = GetType().Assembly == typeof(JsonConverter).Assembly;
 
             if (HandleNull)
index f132019..4d7d9f4 100644 (file)
@@ -12,7 +12,6 @@ namespace System.Text.Json
     internal abstract class JsonPropertyInfo
     {
         public static readonly JsonPropertyInfo s_missingProperty = GetPropertyPlaceholder();
-        public static readonly Type s_NullableType = typeof(Nullable<>);
 
         private JsonClassInfo? _runtimeClassInfo;
 
index 612db8c..114858d 100644 (file)
@@ -105,8 +105,7 @@ namespace System.Text.Json
             }
 
             _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && DeclaredPropertyType != converter.TypeToConvert;
-            _propertyTypeCanBeNull = !DeclaredPropertyType.IsValueType ||
-                (DeclaredPropertyType.IsGenericType && DeclaredPropertyType.GetGenericTypeDefinition() == s_NullableType);
+            _propertyTypeCanBeNull = DeclaredPropertyType.CanBeNull();
             _propertyTypeEqualsTypeToConvert = typeof(T) == DeclaredPropertyType;
 
             GetPolicies(ignoreCondition, parentTypeNumberHandling, defaultValueIsNull: _propertyTypeCanBeNull);
@@ -285,7 +284,7 @@ namespace System.Text.Json
                                     ThrowHelper.ThrowInvalidCastException_DeserializeUnableToAssignValue(typeOfValue, DeclaredPropertyType);
                                 }
                             }
-                            else if (DeclaredPropertyType.IsValueType && !DeclaredPropertyType.IsNullableValueType())
+                            else if (!_propertyTypeCanBeNull)
                             {
                                 ThrowHelper.ThrowInvalidOperationException_DeserializeUnableToAssignNull(DeclaredPropertyType);
                             }
index 8d0c4f1..87049b4 100644 (file)
@@ -185,7 +185,7 @@ namespace System.Text.Json
             // We also throw to avoid passing an invalid argument to setters for nullable struct properties,
             // which would cause an InvalidProgramException when the generated IL is invoked.
             // This is not an issue of the converter is wrapped in NullableConverter<T>.
-            if (runtimePropertyType.IsNullableType() && !converter.TypeToConvert.IsNullableType())
+            if (runtimePropertyType.CanBeNull() && !converter.TypeToConvert.CanBeNull())
             {
                 ThrowHelper.ThrowInvalidOperationException_ConverterCanConvertNullableRedundant(runtimePropertyType, converter);
             }
diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/TypeExtensions.cs b/src/libraries/System.Text.Json/src/System/Text/Json/TypeExtensions.cs
deleted file mode 100644 (file)
index 8abbf6d..0000000
+++ /dev/null
@@ -1,42 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-
-using System;
-
-namespace System.Text.Json
-{
-    internal static class TypeExtensions
-    {
-        /// <summary>
-        /// Returns <see langword="true" /> when the given type is of type <see cref="Nullable{T}"/>.
-        /// </summary>
-        public static bool IsNullableValueType(this Type type)
-        {
-            return Nullable.GetUnderlyingType(type) != null;
-        }
-
-        /// <summary>
-        /// Returns <see langword="true" /> when the given type is either a reference type or of type <see cref="Nullable{T}"/>.
-        /// </summary>
-        public static bool IsNullableType(this Type type)
-        {
-            return !type.IsValueType || IsNullableValueType(type);
-        }
-
-        /// <summary>
-        /// Returns <see langword="true" /> when the given type is assignable from <paramref name="from"/>.
-        /// </summary>
-        /// <remarks>
-        /// Other than <see cref="Type.IsAssignableFrom(Type)"/> also returns <see langword="true" /> when <paramref name="type"/> is of type <see cref="Nullable{T}"/> where <see langword="T" /> : <see langword="IInterface" /> and <paramref name="from"/> is of type <see langword="IInterface" />.
-        /// </remarks>
-        public static bool IsAssignableFromInternal(this Type type, Type from)
-        {
-            if (IsNullableValueType(from) && type.IsInterface)
-            {
-                return type.IsAssignableFrom(from.GetGenericArguments()[0]);
-            }
-
-            return type.IsAssignableFrom(from);
-        }
-    }
-}
diff --git a/src/libraries/System.Text.Json/src/System/TypeExtensions.cs b/src/libraries/System.Text.Json/src/System/TypeExtensions.cs
new file mode 100644 (file)
index 0000000..1e83527
--- /dev/null
@@ -0,0 +1,42 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Runtime.CompilerServices;
+
+namespace System.Text.Json
+{
+    internal static class TypeExtensions
+    {
+        private static readonly Type s_nullableType = typeof(Nullable<>);
+
+        /// <summary>
+        /// Returns <see langword="true" /> when the given type is of type <see cref="Nullable{T}"/>.
+        /// </summary>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static bool IsNullableOfT(this Type type) =>
+            type.IsGenericType && type.GetGenericTypeDefinition() == s_nullableType;
+
+        /// <summary>
+        /// Returns <see langword="true" /> when the given type is either a reference type or of type <see cref="Nullable{T}"/>.
+        /// </summary>
+        /// <remarks>This calls <see cref="Type.IsValueType"/> which is slow. If knowledge already exists
+        /// that the type is a value type, call <see cref="IsNullableOfT"/> instead.</remarks>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public static bool CanBeNull(this Type type) =>
+            !type.IsValueType || type.IsNullableOfT();
+
+        /// <summary>
+        /// Returns <see langword="true" /> when the given type is assignable from <paramref name="from"/> including support
+        /// when <paramref name="from"/> is <see cref="Nullable{T}"/> by using the {T} generic parameter for <paramref name="from"/>.
+        /// </summary>
+        public static bool IsAssignableFromInternal(this Type type, Type from)
+        {
+            if (IsNullableOfT(from) && type.IsInterface)
+            {
+                return type.IsAssignableFrom(from.GetGenericArguments()[0]);
+            }
+
+            return type.IsAssignableFrom(from);
+        }
+    }
+}