Update XML Doc, add tests, feedback

src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs

@@ -126,6 +126,28 @@ public static bool TryAdd<TKey, TValue>(Dictionary<TKey, TValue> dictionary, in
 #endif
         }
 
+        /// <summary>
+        /// Emulates Dictionary(IEnumerable{KeyValuePair}) on netstandard.
+        /// </summary>
+        public static Dictionary<TKey, TValue> CreateDictionaryFromCollection<TKey, TValue>(
+            IEnumerable<KeyValuePair<TKey, TValue>> collection,
+            IEqualityComparer<TKey> comparer)
+            where TKey : notnull
+        {
+#if NETSTANDARD2_0 || NETFRAMEWORK
+            var dictionary = new Dictionary<TKey, TValue>(comparer);
+
+            foreach (KeyValuePair<TKey, TValue> item in collection)
+            {
+                dictionary.Add(item.Key, item.Value);
+            }
+
+            return dictionary;
+#else
+            return new Dictionary<TKey, TValue>(collection: collection, comparer);
+#endif
+        }
+
         public static bool IsFinite(double value)
         {
 #if BUILDING_INBOX_LIBRARY

src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonNode.cs

@@ -9,6 +9,8 @@ namespace System.Text.Json.Node
     /// <summary>
     ///   The base class that represents a single node within a mutable JSON document.
     /// </summary>
+    /// <seealso cref="JsonSerializerOptions.UnknownTypeHandling"/> to specify how a type
+    /// declared as an <see cref="object"/> can be deserialized as a <see cref="JsonNode"/>.
     public abstract partial class JsonNode
     {
         private JsonNode? _parent;

src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonObject.KeyCollection.cs

@@ -3,7 +3,6 @@
 
 using System.Collections;
 using System.Collections.Generic;
-using System.Diagnostics;
 
 namespace System.Text.Json.Node
 {
@@ -17,9 +16,9 @@ private KeyCollection GetKeyCollection(JsonObject jsonObject)
             return _keyCollection ??= new KeyCollection(jsonObject);
         }
 
-        private class KeyCollection : ICollection<string>
+        private sealed class KeyCollection : ICollection<string>
         {
-            private JsonObject _jObject;
+            private readonly JsonObject _jObject;
 
             public KeyCollection(JsonObject jsonObject)
             {

src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonObject.List.cs

@@ -13,21 +13,14 @@ public partial class JsonObject
 
         private Dictionary<string, JsonNode?>? _dictionary;
         private List<KeyValuePair<string, JsonNode?>>? _list;
+
+        /// We defer creating the comparer as long as possible in case no options were specified during creation.
+        /// In that case if later we are added to a parent with a non-null options, we use the parent options.
         private StringComparer? _stringComparer;
 
         private string? _lastKey;
         private JsonNode? _lastValue;
 
-        /// <summary>
-        /// We defer creating the comparer as long as possible in case no options were specified during creation.
-        /// In that case if later we are added to a parent with a non-null options, we use the parent options.
-        /// </summary>
-        private StringComparer GetStringComparer()
-        {
-            Debug.Assert(_stringComparer != null);
-            return _stringComparer;
-        }
-
         private void CreateStringComparer()
         {
             bool caseInsensitive = Options?.PropertyNameCaseInsensitive == true;
@@ -113,6 +106,8 @@ private int NodeCount
             CreateList();
             Debug.Assert(_list != null);
 
+            CreateDictionaryIfThreshold();
+
             JsonNode? existing = null;
 
             if (_dictionary != null)
@@ -121,7 +116,6 @@ private int NodeCount
                 if (JsonHelpers.TryAdd(_dictionary, propertyName, node))
                 {
                     node?.AssignParent(this);
-                    CreateDictionaryIfThreshold();
                     _list.Add(new KeyValuePair<string, JsonNode?>(propertyName, node));
                     return null;
                 }
@@ -157,13 +151,11 @@ private int NodeCount
                 }
 
                 node?.AssignParent(this);
-                CreateDictionaryIfThreshold();
                 _list[i] = new KeyValuePair<string, JsonNode?>(propertyName, node);
             }
             else
             {
                 node?.AssignParent(this);
-                CreateDictionaryIfThreshold();
                 _dictionary?.Add(propertyName, node);
                 _list.Add(new KeyValuePair<string, JsonNode?>(propertyName, node));
                 Debug.Assert(existing == null);
@@ -178,11 +170,12 @@ private int NodeCount
         private int FindNodeIndex(string propertyName)
         {
             Debug.Assert(_list != null);
+            Debug.Assert(_stringComparer != null);
 
             for (int i = 0; i < _list.Count; i++)
             {
                 KeyValuePair<string, JsonNode?> current = _list[i];
-                if (GetStringComparer().Compare(propertyName, current.Key) == 0)
+                if (_stringComparer.Compare(propertyName, current.Key) == 0)
                 {
                     return i;
                 }
@@ -194,25 +187,20 @@ private int FindNodeIndex(string propertyName)
         private void CreateDictionaryIfThreshold()
         {
             Debug.Assert(_list != null);
+            Debug.Assert(_stringComparer != null);
 
             if (_dictionary == null && _list.Count > ListToDictionaryThreshold)
             {
-                var dict = new Dictionary<string, JsonNode?>(GetStringComparer());
-
-                // 'foreach' has the advantage over a 'for(;;)' loop since it detects modifications.
-                foreach (KeyValuePair<string, JsonNode?> item in _list)
-                {
-                    dict.Add(item.Key, item.Value);
-                }
-
-                _dictionary = dict;
+                _dictionary = JsonHelpers.CreateDictionaryFromCollection(_list, _stringComparer);
             }
         }
 
         private void ClearNodes()
         {
             if (_jsonElement != null)
             {
+                Debug.Assert(_list == null);
+                Debug.Assert(_dictionary == null);
                 _jsonElement = null;
                 return;
             }
@@ -233,11 +221,10 @@ private void ClearNodes()
         /// </summary>
         private bool ContainsNode(KeyValuePair<string, JsonNode?> node)
         {
-            CreateList();
-
             foreach (KeyValuePair<string, JsonNode?> item in this)
             {
-                if (ReferenceEquals(item.Value, node.Value) && GetStringComparer().Compare(item.Key, node.Key) == 0)
+                Debug.Assert(_stringComparer != null);
+                if (ReferenceEquals(item.Value, node.Value) && _stringComparer.Compare(item.Key, node.Key) == 0)
                 {
                     return true;
                 }
@@ -278,16 +265,15 @@ private bool ContainsNode(JsonNode? node)
 
         private bool ContainsNode(string propertyName)
         {
-            CreateList();
-
             if (_dictionary != null)
             {
                 return _dictionary.ContainsKey(propertyName);
             }
 
             foreach (string item in GetKeyCollection(this))
             {
-                if (GetStringComparer().Compare(item, propertyName) == 0)
+                Debug.Assert(_stringComparer != null);
+                if (_stringComparer.Compare(item, propertyName) == 0)
                 {
                     return true;
                 }
@@ -300,6 +286,7 @@ private bool TryRemoveNode(string propertyName, out JsonNode? existing)
         {
             CreateList();
             Debug.Assert(_list != null);
+            Debug.Assert(_stringComparer != null);
 
             if (_dictionary != null)
             {
@@ -315,7 +302,8 @@ private bool TryRemoveNode(string propertyName, out JsonNode? existing)
             for (int i = 0; i < _list.Count; i++)
             {
                 KeyValuePair<string, JsonNode?> current = _list[i];
-                if (GetStringComparer().Compare(propertyName, current.Key) == 0)
+
+                if (_stringComparer.Compare(current.Key, propertyName) == 0)
                 {
                     _list.RemoveAt(i);
                     existing = current.Value;
@@ -332,6 +320,7 @@ private bool TryFindNode(string propertyName, out JsonNode? property)
         {
             CreateList();
             Debug.Assert(_list != null);
+            Debug.Assert(_stringComparer != null);
 
             if (propertyName == _lastKey)
             {
@@ -342,10 +331,9 @@ private bool TryFindNode(string propertyName, out JsonNode? property)
                 return true;
             }
 
-            bool success;
             if (_dictionary != null)
             {
-                success = _dictionary.TryGetValue(propertyName, out property);
+                bool success = _dictionary.TryGetValue(propertyName, out property);
                 if (success)
                 {
                     _lastKey = propertyName;
@@ -358,7 +346,7 @@ private bool TryFindNode(string propertyName, out JsonNode? property)
             {
                 foreach (KeyValuePair<string, JsonNode?> item in _list)
                 {
-                    if (GetStringComparer().Compare(propertyName, item.Key) == 0)
+                    if (_stringComparer.Compare(propertyName, item.Key) == 0)
                     {
                         property = item.Value;
                         _lastKey = propertyName;

src/libraries/System.Text.Json/src/System/Text/Json/Node/JsonObject.ValueCollection.cs

@@ -16,9 +16,9 @@ private ValueCollection GetValueCollection(JsonObject jsonObject)
             return _valueCollection ??= new ValueCollection(jsonObject);
         }
 
-        private class ValueCollection : ICollection<JsonNode?>
+        private sealed class ValueCollection : ICollection<JsonNode?>
         {
-            private JsonObject _jObject;
+            private readonly JsonObject _jObject;
 
             public ValueCollection(JsonObject jsonObject)
             {

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonExtensionDataAttribute.cs

@@ -4,14 +4,20 @@
 namespace System.Text.Json.Serialization
 {
     /// <summary>
-    /// When placed on a property or field of type <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/>, any
-    /// properties that do not have a matching property or field are added to that Dictionary during deserialization and written during serialization.
+    /// When placed on a property or field of type <see cref="System.Text.Json.Node.JsonObject"/> or
+    /// <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/>, any properties that do not have a
+    /// matching property or field are added during deserialization and written during serialization.
     /// </summary>
     /// <remarks>
-    /// The TKey value must be <see cref="string"/> and TValue must be <see cref="JsonElement"/> or <see cref="object"/>.
+    /// When using <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/>, the TKey value must be <see cref="string"/>
+    /// and TValue must be <see cref="JsonElement"/> or <see cref="object"/>.
     ///
-    /// During deserializing, when using <see cref="object"/> a "null" JSON value is treated as a <c>null</c> object reference, and when using
-    /// <see cref="JsonElement"/> a "null" is treated as a JsonElement with <see cref="JsonElement.ValueKind"/> set to <see cref="JsonValueKind.Null"/>.
+    /// During deserializing with a <see cref="System.Collections.Generic.IDictionary{TKey, TValue}"/> extension property with TValue as
+    /// <see cref="object"/>, the type of object created will either be a <see cref="System.Text.Json.Node.JsonNode"/> or a
+    /// <see cref="JsonElement"/> depending on the value of <see cref="System.Text.Json.JsonSerializerOptions.UnknownTypeHandling"/>.
+    ///
+    /// If a <see cref="JsonElement"/> is created, a "null" JSON value is treated as a JsonElement with <see cref="JsonElement.ValueKind"/>
+    /// set to <see cref="JsonValueKind.Null"/>, otherwise a "null" JSON value is treated as a <c>null</c> object reference.
     ///
     /// During serializing, the name of the extension data member is not included in the JSON;
     /// the data contained within the extension data is serialized as properties of the JSON object.

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Node/JsonNodeConverter.cs

@@ -17,9 +17,9 @@ internal sealed class JsonNodeConverter : JsonConverter<object?>
         private JsonValueConverter? _valueConverter;
 
         public static JsonNodeConverter Instance { get; } = new JsonNodeConverter();
-        public JsonArrayConverter ArrayConverter => _arrayConverter ?? (_arrayConverter = new JsonArrayConverter());
-        public JsonObjectConverter ObjectConverter => _objectConverter ?? (_objectConverter = new JsonObjectConverter());
-        public JsonValueConverter ValueConverter => _valueConverter ?? (_valueConverter = new JsonValueConverter());
+        public JsonArrayConverter ArrayConverter => _arrayConverter ??= new JsonArrayConverter();
+        public JsonObjectConverter ObjectConverter => _objectConverter ??= new JsonObjectConverter();
+        public JsonValueConverter ValueConverter => _valueConverter ??= new JsonValueConverter();
 
         public override void Write(Utf8JsonWriter writer, object? value, JsonSerializerOptions options)
         {

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs

@@ -168,7 +168,7 @@ internal bool TryRead(ref Utf8JsonReader reader, Type typeToConvert, JsonSeriali
                 {
                     if (state.Current.NumberHandling != null)
                     {
-                        value = ReadNumberWithCustomHandling(ref reader, state.Current.NumberHandling.Value, JsonSerializerOptions options);
+                        value = ReadNumberWithCustomHandling(ref reader, state.Current.NumberHandling.Value, options);
                     }
                     else
                     {

src/libraries/System.Text.Json/tests/System.Text.Json.Tests/JsonNode/JsonObjectTests.cs

@@ -218,6 +218,41 @@ public static void CaseSensitivity_ReadMode()
             Assert.Equal(42, obj["MYPROPERTY"].GetValue<int>());
         }
 
+        [Fact]
+        public static void CaseInsensitive_Remove()
+        {
+            var options = new JsonSerializerOptions() { PropertyNameCaseInsensitive = true };
+            JsonObject obj = JsonSerializer.Deserialize<JsonObject>("{\"MyProperty\":42}", options);
+
+            Assert.True(obj.ContainsKey("MyProperty"));
+            Assert.True(obj.ContainsKey("myproperty"));
+            Assert.True(obj.ContainsKey("MYPROPERTY"));
+
+            Assert.True(obj.Remove("myproperty"));
+            Assert.False(obj.Remove("myproperty"));
+            Assert.False(obj.Remove("MYPROPERTY"));
+            Assert.False(obj.Remove("MyProperty"));
+        }
+
+        [Fact]
+        public static void CaseSensitivity_Remove()
+        {
+            var options = new JsonSerializerOptions() { PropertyNameCaseInsensitive = false };
+            JsonObject obj = JsonSerializer.Deserialize<JsonObject>("{\"MYPROPERTY\":42,\"myproperty\":43}", options);
+
+            Assert.False(obj.ContainsKey("MyProperty"));
+            Assert.True(obj.ContainsKey("MYPROPERTY"));
+            Assert.True(obj.ContainsKey("myproperty"));
+
+            Assert.False(obj.Remove("MyProperty"));
+
+            Assert.True(obj.Remove("MYPROPERTY"));
+            Assert.False(obj.Remove("MYPROPERTY"));
+
+            Assert.True(obj.Remove("myproperty"));
+            Assert.False(obj.Remove("myproperty"));
+        }
+
         [Fact]
         public static void CaseSensitivity_EditMode()
         {
@@ -664,8 +699,7 @@ public static void ListToDictionaryConversions(JsonObject jObject, int count)
             {
                 string key = i.ToString();
 
-                // Contains which does a reference comparison on the value so it
-                // needs to be done before modifying.
+                // Contains does a reference comparison on JsonNode so it needs to be done before modifying.
                 Assert.True(jObject.Contains(new KeyValuePair<string, JsonNode?>(key, jObject[key])));
 
                 jObject[key] = JsonValue.Create(i);