Fix some nullable annotations / null handling in System.Text.Json (#8…

…6077) The `JsonConverter<T>`s for reference type `T`s need to be able to deal with null. Fix the nullable annotations accordingly. This then led to highlghting that the implementation can null ref if a dictionary returns a null key, so adding appropriate validation of that, too.
dotnet · May 11, 2023 · a23e797 · a23e797
1 parent e853102
commit a23e797
Show file tree

Hide file tree

Showing 17 changed files with 113 additions and 59 deletions.
diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs
@@ -913,7 +913,7 @@ public abstract void Write(
             T value,
 #nullable restore
             System.Text.Json.JsonSerializerOptions options);
-        public virtual void WriteAsPropertyName(System.Text.Json.Utf8JsonWriter writer, T value, System.Text.Json.JsonSerializerOptions options) { }
+        public virtual void WriteAsPropertyName(System.Text.Json.Utf8JsonWriter writer, [System.Diagnostics.CodeAnalysis.DisallowNullAttribute] T value, System.Text.Json.JsonSerializerOptions options) { }
     }
     [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Interface, AllowMultiple=true, Inherited=false)]
     public partial class JsonDerivedTypeAttribute : System.Text.Json.Serialization.JsonAttribute
@@ -1128,7 +1128,7 @@ public readonly partial struct JsonDerivedType
     public static partial class JsonMetadataServices
     {
         public static System.Text.Json.Serialization.JsonConverter<bool> BooleanConverter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<byte[]> ByteArrayConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<byte[]?> ByteArrayConverter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<byte> ByteConverter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<char> CharConverter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<System.DateTime> DateTimeConverter { get { throw null; } }
@@ -1139,26 +1139,26 @@ public static partial class JsonMetadataServices
         public static System.Text.Json.Serialization.JsonConverter<short> Int16Converter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<int> Int32Converter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<long> Int64Converter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonArray> JsonArrayConverter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.JsonDocument> JsonDocumentConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonArray?> JsonArrayConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.JsonDocument?> JsonDocumentConverter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.JsonElement> JsonElementConverter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonNode> JsonNodeConverter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonObject> JsonObjectConverter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonValue> JsonValueConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonNode?> JsonNodeConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonObject?> JsonObjectConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<System.Text.Json.Nodes.JsonValue?> JsonValueConverter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<object?> ObjectConverter { get { throw null; } }
         [System.CLSCompliantAttribute(false)]
         public static System.Text.Json.Serialization.JsonConverter<sbyte> SByteConverter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<float> SingleConverter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<string> StringConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<string?> StringConverter { get { throw null; } }
         public static System.Text.Json.Serialization.JsonConverter<System.TimeSpan> TimeSpanConverter { get { throw null; } }
         [System.CLSCompliantAttribute(false)]
         public static System.Text.Json.Serialization.JsonConverter<ushort> UInt16Converter { get { throw null; } }
         [System.CLSCompliantAttribute(false)]
         public static System.Text.Json.Serialization.JsonConverter<uint> UInt32Converter { get { throw null; } }
         [System.CLSCompliantAttribute(false)]
         public static System.Text.Json.Serialization.JsonConverter<ulong> UInt64Converter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<System.Uri> UriConverter { get { throw null; } }
-        public static System.Text.Json.Serialization.JsonConverter<System.Version> VersionConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<System.Uri?> UriConverter { get { throw null; } }
+        public static System.Text.Json.Serialization.JsonConverter<System.Version?> VersionConverter { get { throw null; } }
         public static System.Text.Json.Serialization.Metadata.JsonTypeInfo<TElement[]> CreateArrayInfo<TElement>(System.Text.Json.JsonSerializerOptions options, System.Text.Json.Serialization.Metadata.JsonCollectionInfoValues<TElement[]> collectionInfo) { throw null; }
         public static System.Text.Json.Serialization.Metadata.JsonTypeInfo<TCollection> CreateConcurrentQueueInfo<TCollection, TElement>(System.Text.Json.JsonSerializerOptions options, System.Text.Json.Serialization.Metadata.JsonCollectionInfoValues<TCollection> collectionInfo) where TCollection : System.Collections.Concurrent.ConcurrentQueue<TElement> { throw null; }
         public static System.Text.Json.Serialization.Metadata.JsonTypeInfo<TCollection> CreateConcurrentStackInfo<TCollection, TElement>(System.Text.Json.JsonSerializerOptions options, System.Text.Json.Serialization.Metadata.JsonCollectionInfoValues<TCollection> collectionInfo) where TCollection : System.Collections.Concurrent.ConcurrentStack<TElement> { throw null; }

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.CreateOverloads.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Nodes/JsonValue.CreateOverloads.cs
@@ -225,7 +225,7 @@ public partial class JsonValue
         /// <param name="options">Options to control the behavior.</param>
         /// <returns>The new instance of the <see cref="JsonValue"/> class that contains the specified value.</returns>
         [return: NotNullIfNotNull(nameof(value))]
-        public static JsonValue? Create(string? value, JsonNodeOptions? options = null) => value != null ? new JsonValueTrimmable<string>(value, JsonMetadataServices.StringConverter) : null;
+        public static JsonValue? Create(string? value, JsonNodeOptions? options = null) => value != null ? new JsonValueTrimmable<string?>(value, JsonMetadataServices.StringConverter) : null;
 
         /// <summary>
         ///   Initializes a new instance of the <see cref="JsonValue"/> class that contains the specified value.

diff --git a/...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs b/...raries/System.Text.Json/src/System/Text/Json/Serialization/Converters/CastingConverter.cs
@@ -2,7 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
-using System.Runtime.CompilerServices;
+using System.Diagnostics.CodeAnalysis;
 using System.Text.Json.Reflection;
 
 namespace System.Text.Json.Serialization.Converters
@@ -60,7 +60,7 @@ public override T ReadAsPropertyName(ref Utf8JsonReader reader, Type typeToConve
         internal override T ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
             => JsonSerializer.UnboxOnRead<T>(_sourceConverter.ReadAsPropertyNameCoreAsObject(ref reader, typeToConvert, options))!;
 
-        public override void WriteAsPropertyName(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
+        public override void WriteAsPropertyName(Utf8JsonWriter writer, [DisallowNull] T value, JsonSerializerOptions options)
             => _sourceConverter.WriteAsPropertyNameAsObject(writer, value, options);
 
         internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, T value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)

diff --git a/...System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonArrayConverter.cs b/...System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonArrayConverter.cs
@@ -6,9 +6,9 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class JsonArrayConverter : JsonConverter<JsonArray>
+    internal sealed class JsonArrayConverter : JsonConverter<JsonArray?>
     {
-        public override void Write(Utf8JsonWriter writer, JsonArray value, JsonSerializerOptions options)
+        public override void Write(Utf8JsonWriter writer, JsonArray? value, JsonSerializerOptions options)
         {
             if (value is null)
             {

diff --git a/.../System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs b/.../System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs
@@ -11,7 +11,7 @@ namespace System.Text.Json.Serialization.Converters
     /// Converter for JsonNode-derived types. The {T} value must be Object and not JsonNode
     /// since we allow Object-declared members\variables to deserialize as {JsonNode}.
     /// </summary>
-    internal sealed class JsonNodeConverter : JsonConverter<JsonNode>
+    internal sealed class JsonNodeConverter : JsonConverter<JsonNode?>
     {
         private static JsonNodeConverter? s_nodeConverter;
         private static JsonArrayConverter? s_arrayConverter;

diff --git a/...ystem.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs b/...ystem.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonObjectConverter.cs
@@ -7,7 +7,7 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class JsonObjectConverter : JsonConverter<JsonObject>
+    internal sealed class JsonObjectConverter : JsonConverter<JsonObject?>
     {
         internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
         {
@@ -33,7 +33,7 @@ internal override void ReadElementAndSetProperty(
             jObject[propertyName] = jNodeValue;
         }
 
-        public override void Write(Utf8JsonWriter writer, JsonObject value, JsonSerializerOptions options)
+        public override void Write(Utf8JsonWriter writer, JsonObject? value, JsonSerializerOptions options)
         {
             if (value is null)
             {

diff --git a/...System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs b/...System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonValueConverter.cs
@@ -6,9 +6,9 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class JsonValueConverter : JsonConverter<JsonValue>
+    internal sealed class JsonValueConverter : JsonConverter<JsonValue?>
     {
-        public override void Write(Utf8JsonWriter writer, JsonValue value, JsonSerializerOptions options)
+        public override void Write(Utf8JsonWriter writer, JsonValue? value, JsonSerializerOptions options)
         {
             if (value is null)
             {

diff --git a/.../System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs b/.../System.Text.Json/src/System/Text/Json/Serialization/Converters/Object/ObjectConverter.cs
@@ -91,20 +91,17 @@ internal override object ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type
             return null!;
         }
 
-        public override void WriteAsPropertyName(Utf8JsonWriter writer, object? value, JsonSerializerOptions options)
+        public override void WriteAsPropertyName(Utf8JsonWriter writer, object value, JsonSerializerOptions options)
         {
-            if (value is null)
-            {
-                ThrowHelper.ThrowArgumentNullException(nameof(value));
-            }
-
             WriteAsPropertyNameCore(writer, value, options, isWritingExtensionDataProperty: false);
         }
 
-        internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, object? value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
+        internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, object value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
         {
-            // This converter does not handle nulls.
-            Debug.Assert(value != null);
+            if (value is null)
+            {
+                ThrowHelper.ThrowArgumentNullException(nameof(value));
+            }
 
             Type runtimeType = value.GetType();
             JsonConverter runtimeConverter = options.GetConverterInternal(runtimeType);

diff --git a/...ystem.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ByteArrayConverter.cs b/...ystem.Text.Json/src/System/Text/Json/Serialization/Converters/Value/ByteArrayConverter.cs
@@ -3,7 +3,7 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class ByteArrayConverter : JsonConverter<byte[]>
+    internal sealed class ByteArrayConverter : JsonConverter<byte[]?>
     {
         public override byte[]? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {

diff --git a/...em.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonDocumentConverter.cs b/...em.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonDocumentConverter.cs
@@ -3,14 +3,14 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class JsonDocumentConverter : JsonConverter<JsonDocument>
+    internal sealed class JsonDocumentConverter : JsonConverter<JsonDocument?>
     {
         public override JsonDocument Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
             return JsonDocument.ParseValue(ref reader);
         }
 
-        public override void Write(Utf8JsonWriter writer, JsonDocument value, JsonSerializerOptions options)
+        public override void Write(Utf8JsonWriter writer, JsonDocument? value, JsonSerializerOptions options)
         {
             if (value is null)
             {

diff --git a/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonPrimitiveConverter.cs b/...m.Text.Json/src/System/Text/Json/Serialization/Converters/Value/JsonPrimitiveConverter.cs
@@ -1,14 +1,16 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.CodeAnalysis;
+
 namespace System.Text.Json.Serialization.Converters
 {
     /// <summary>
     /// Inherited by built-in converters serializing types as JSON primitives that support property name serialization.
     /// </summary>
     internal abstract class JsonPrimitiveConverter<T> : JsonConverter<T>
     {
-        public sealed override void WriteAsPropertyName(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
+        public sealed override void WriteAsPropertyName(Utf8JsonWriter writer, [DisallowNull] T value, JsonSerializerOptions options)
         {
             if (value is null)
             {

diff --git a/...s/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/StringConverter.cs b/...s/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/StringConverter.cs
@@ -5,7 +5,7 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class StringConverter : JsonPrimitiveConverter<string>
+    internal sealed class StringConverter : JsonPrimitiveConverter<string?>
     {
         public override string? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
@@ -33,6 +33,11 @@ internal override string ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type
 
         internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, string value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
         {
+            if (value is null)
+            {
+                ThrowHelper.ThrowArgumentNullException(nameof(value));
+            }
+
             if (options.DictionaryKeyPolicy != null && !isWritingExtensionDataProperty)
             {
                 value = options.DictionaryKeyPolicy.ConvertName(value);

diff --git a/...ries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UriConverter.cs b/...ries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/UriConverter.cs
@@ -5,14 +5,14 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class UriConverter : JsonPrimitiveConverter<Uri>
+    internal sealed class UriConverter : JsonPrimitiveConverter<Uri?>
     {
         public override Uri? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
             return reader.TokenType is JsonTokenType.Null ? null : ReadCore(ref reader);
         }
 
-        public override void Write(Utf8JsonWriter writer, Uri value, JsonSerializerOptions options)
+        public override void Write(Utf8JsonWriter writer, Uri? value, JsonSerializerOptions options)
         {
             if (value is null)
             {
@@ -43,6 +43,11 @@ private static Uri ReadCore(ref Utf8JsonReader reader)
 
         internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, Uri value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
         {
+            if (value is null)
+            {
+                ThrowHelper.ThrowArgumentNullException(nameof(value));
+            }
+
             writer.WritePropertyName(value.OriginalString);
         }
     }

diff --git a/.../System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs b/.../System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/VersionConverter.cs
@@ -5,7 +5,7 @@
 
 namespace System.Text.Json.Serialization.Converters
 {
-    internal sealed class VersionConverter : JsonPrimitiveConverter<Version>
+    internal sealed class VersionConverter : JsonPrimitiveConverter<Version?>
     {
 #if NETCOREAPP
         private const int MinimumVersionLength = 3; // 0.0
@@ -76,7 +76,7 @@ private static Version ReadCore(ref Utf8JsonReader reader)
             return null;
         }
 
-        public override void Write(Utf8JsonWriter writer, Version value, JsonSerializerOptions options)
+        public override void Write(Utf8JsonWriter writer, Version? value, JsonSerializerOptions options)
         {
             if (value is null)
             {
@@ -101,6 +101,11 @@ internal override Version ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type
 
         internal override void WriteAsPropertyNameCore(Utf8JsonWriter writer, Version value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
         {
+            if (value is null)
+            {
+                ThrowHelper.ThrowArgumentNullException(nameof(value));
+            }
+
 #if NETCOREAPP
             Span<char> span = stackalloc char[MaximumVersionLength];
             bool formattedSuccessfully = value.TryFormat(span, out int charsWritten);

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
@@ -655,7 +655,7 @@ internal virtual T ReadAsPropertyNameCore(ref Utf8JsonReader reader, Type typeTo
         /// <param name="value">The value to convert. Note that the value of <seealso cref="HandleNull"/> determines if the converter handles <see langword="null" /> values.</param>
         /// <param name="options">The <see cref="JsonSerializerOptions"/> being used.</param>
         /// <remarks>Method should be overridden in custom converters of types used in serialized dictionary keys.</remarks>
-        public virtual void WriteAsPropertyName(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
+        public virtual void WriteAsPropertyName(Utf8JsonWriter writer, [DisallowNull] T value, JsonSerializerOptions options)
         {
             // .NET 5 backward compatibility: hardcode the default converter for primitive key serialization.
             JsonConverter<T>? fallbackConverter = GetFallbackConverterForPropertyNameSerialization(options);
@@ -667,8 +667,13 @@ public virtual void WriteAsPropertyName(Utf8JsonWriter writer, T value, JsonSeri
             fallbackConverter.WriteAsPropertyNameCore(writer, value, options, isWritingExtensionDataProperty: false);
         }
 
-        internal virtual void WriteAsPropertyNameCore(Utf8JsonWriter writer, T value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
+        internal virtual void WriteAsPropertyNameCore(Utf8JsonWriter writer, [DisallowNull] T value, JsonSerializerOptions options, bool isWritingExtensionDataProperty)
         {
+            if (value is null)
+            {
+                ThrowHelper.ThrowArgumentNullException(nameof(value));
+            }
+
             if (isWritingExtensionDataProperty)
             {
                 // Extension data is meant as mechanism to gather unused JSON properties;