Add issues to the TODOs in S.T.Json source for better tracking and minor clean up...
authorAhson Khan <ahson_ahmedk@yahoo.com>
Sat, 15 Feb 2020 09:13:11 +0000 (01:13 -0800)
committerGitHub <noreply@github.com>
Sat, 15 Feb 2020 09:13:10 +0000 (01:13 -0800)
* Remove redundant check in condition (&& true) since it doesn't change
the behavior.

* Add issues to the TODOs for better tracking and fix some that were easy.

* Update ActiveIssue and enable tests that are now fixed.

16 files changed:
src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.Date.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/Value/KeyValuePairConverter.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleMetadata.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.Stream.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/WriteStackFrame.cs
src/libraries/System.Text.Json/tests/JsonTestHelper.cs
src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs
src/libraries/System.Text.Json/tests/Serialization/CustomConverterTests.Callback.cs
src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs
src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs
src/libraries/System.Text.Json/tests/Serialization/Object.WriteTests.cs
src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs
src/libraries/System.Text.Json/tests/Serialization/Value.WriteTests.cs
src/libraries/System.Text.Json/tests/Utf8JsonWriterTests.cs

index af0d996..15da840 100644 (file)
@@ -421,8 +421,7 @@ namespace System.Text.Json
                 case JsonConstants.Plus:
                 case JsonConstants.Hyphen:
                     parseData.OffsetToken = curByte;
-                    return ParseOffset(ref parseData, source.Slice(sourceIndex))
-                        && true;
+                    return ParseOffset(ref parseData, source.Slice(sourceIndex));
                 default:
                     return false;
             }
index a9e386e..fc5035d 100644 (file)
@@ -11,11 +11,13 @@ namespace System.Text.Json.Serialization.Converters
         private const string KeyName = "Key";
         private const string ValueName = "Value";
 
-        // todo: move these to JsonSerializerOptions and use the proper encoding.
+        // todo: https://github.com/dotnet/runtime/issues/1197
+        // move these to JsonSerializerOptions and use the proper encoding.
         private static readonly JsonEncodedText _keyName = JsonEncodedText.Encode(KeyName, encoder: null);
         private static readonly JsonEncodedText _valueName = JsonEncodedText.Encode(ValueName, encoder: null);
 
-        // todo: it is possible to cache the underlying converters since this is an internal converter and
+        // todo: https://github.com/dotnet/runtime/issues/32352
+        // it is possible to cache the underlying converters since this is an internal converter and
         // an instance is created only once for each JsonSerializerOptions instance.
 
         internal override bool OnTryRead(
index 206de8d..3f23379 100644 (file)
@@ -108,7 +108,7 @@ namespace System.Text.Json
 
                 string key = reader.GetString()!;
 
-                // todo: verify value is converter.TypeToConvert and throw JsonException? (currently no test)
+                // todo: https://github.com/dotnet/runtime/issues/32354
                 state.Current.ReturnValue = state.ReferenceResolver.ResolveReferenceOnDeserialize(key);
                 state.Current.ObjectState = StackFrameObjectState.MetadataRefPropertyEndObject;
             }
index 3f2234a..2e426a1 100644 (file)
@@ -92,7 +92,7 @@ namespace System.Text.Json
 
             var readerState = new JsonReaderState(options.GetReaderOptions());
 
-            // todo: switch to ArrayBuffer implementation to handle and simplify the allocs?
+            // todo: https://github.com/dotnet/runtime/issues/32355
             int utf8BomLength = JsonConstants.Utf8Bom.Length;
             byte[] buffer = ArrayPool<byte>.Shared.Rent(Math.Max(options.DefaultBufferSize, utf8BomLength));
             int bytesInBuffer = 0;
index fb42a2b..917e092 100644 (file)
@@ -77,7 +77,9 @@ namespace System.Text.Json
 
                 do
                 {
-                    state.FlushThreshold = (int)(bufferWriter.Capacity * .9); //todo: determine best value here
+                    // todo: determine best value here
+                    // https://github.com/dotnet/runtime/issues/32356
+                    state.FlushThreshold = (int)(bufferWriter.Capacity * .9);
                     isFinalBlock = WriteCore(
                         writer,
                         value,
index 9497f30..512d40f 100644 (file)
@@ -334,6 +334,7 @@ namespace System.Text.Json
             _haveTypesBeenCreated = true;
 
             // todo: for performance and reduced instances, consider using the converters and JsonClassInfo from s_defaultOptions by cloning (or reference directly if no changes).
+            // https://github.com/dotnet/runtime/issues/32357
             if (!_classes.TryGetValue(classType, out JsonClassInfo? result))
             {
                 result = _classes.GetOrAdd(classType, new JsonClassInfo(classType, this));
index 01292e9..7261392 100644 (file)
@@ -99,6 +99,7 @@ namespace System.Text.Json
             JsonClassInfo newClassInfo = options.GetOrAddClass(type);
 
             // todo: check if type==newtype and skip below?
+            // https://github.com/dotnet/runtime/issues/32358
 
             // Set for exception handling calculation of JsonPath.
             JsonPropertyNameAsString = propertyName;
index 5af9d78..06ab14a 100644 (file)
@@ -719,7 +719,7 @@ namespace System.Text.Json
                     );
 
             // Temporary hack until we can use the same escape algorithm on both sides and make sure we want uppercase hex.
-            // Todo: create new AssertContentsAgainJsonNet to avoid calling NormalizeToJsonNetFormat when not necessary.
+            // Todo: https://github.com/dotnet/runtime/issues/32351
             Assert.Equal(expectedValue.NormalizeToJsonNetFormat(), value.NormalizeToJsonNetFormat());
         }
 
@@ -741,7 +741,7 @@ namespace System.Text.Json
                     );
 
             // Temporary hack until we can use the same escape algorithm on both sides and make sure we want uppercase hex.
-            // Todo: create new AssertContentsNotEqualAgainJsonNet to avoid calling NormalizeToJsonNetFormat when not necessary.
+            // Todo: https://github.com/dotnet/runtime/issues/32351
             Assert.NotEqual(expectedValue.NormalizeToJsonNetFormat(), value.NormalizeToJsonNetFormat());
         }
 
index 45b4d2c..8acb146 100644 (file)
@@ -617,7 +617,7 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Equal(0, obj.MyImmutableList.Count);
             TestRoundTrip(obj);
 
-            // Skip ImmutableArray due to https://github.com/dotnet/corefx/issues/42399.
+            // TODO: Skip ImmutableArray due to https://github.com/dotnet/runtime/issues/1037.
             const string inputJsonWithNullCollections =
                 @"{
                     ""Array"":null,
index aa158a1..d0f00a3 100644 (file)
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Diagnostics;
 using Xunit;
 
 namespace System.Text.Json.Serialization.Tests
@@ -23,14 +24,22 @@ namespace System.Text.Json.Serialization.Tests
                 // The options are not passed here as that would cause an infinite loop.
                 Customer value = JsonSerializer.Deserialize<Customer>(ref reader);
 
-                value.Name = value.Name + "Hello!";
+                value.Name += "Hello!";
                 return value;
             }
 
             public override void Write(Utf8JsonWriter writer, Customer value, JsonSerializerOptions options)
             {
-                // todo: there is no WriteValue yet.
-                throw new NotSupportedException();
+                writer.WriteStartArray();
+
+                long bytesWrittenSoFar = writer.BytesCommitted + writer.BytesPending;
+
+                JsonSerializer.Serialize(writer, value);
+
+                Debug.Assert(writer.BytesPending == 0);
+                long payloadLength =  writer.BytesCommitted - bytesWrittenSoFar;
+                writer.WriteNumberValue(payloadLength);
+                writer.WriteEndArray();
             }
         }
 
@@ -44,6 +53,10 @@ namespace System.Text.Json.Serialization.Tests
 
             Customer customer = JsonSerializer.Deserialize<Customer>(json, options);
             Assert.Equal("MyNameHello!", customer.Name);
+
+            string result = JsonSerializer.Serialize(customer, options);
+            int expectedLength = JsonSerializer.Serialize(customer).Length;
+            Assert.Equal(@"[{""CreditLimit"":0,""Name"":""MyNameHello!"",""Address"":{""City"":null}}," + $"{expectedLength}]", result);
         }
     }
 }
index 5397692..6868ffd 100644 (file)
@@ -1263,11 +1263,14 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<Dictionary<string, string>>(json));
         }
 
-        [Fact, ActiveIssue("JsonElement fails since it is a struct.")]
+        [Fact]
         public static void ObjectToJsonElement()
         {
             string json = @"{""MyDictionary"":{""Key"":""Value""}}";
-            JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(json);
+            Dictionary<string, JsonElement> result = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(json);
+            JsonElement element = result["MyDictionary"];
+            Assert.Equal(JsonValueKind.Object, element.ValueKind);
+            Assert.Equal("Value", element.GetProperty("Key").GetString());
         }
 
         [Fact]
index d374f7c..32a1192 100644 (file)
@@ -364,7 +364,7 @@ namespace System.Text.Json.Serialization.Tests
         }
 
         [Fact]
-        [ActiveIssue("JsonElement needs to support Path")]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/32359")]
         public static void ExtensionPropertyRoundTripFails()
         {
             try
index a8f1a65..34f96cc 100644 (file)
@@ -3,6 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
+using System.Text.Encodings.Web;
 using Xunit;
 
 namespace System.Text.Json.Serialization.Tests
@@ -125,5 +126,17 @@ namespace System.Text.Json.Serialization.Tests
             [JsonPropertyName("p_3")]
             public object P3 => "";
         }
+
+        // https://github.com/dotnet/corefx/issues/40979
+        [Fact]
+        public static void EscapingShouldntStackOverflow_40979()
+        {
+            var test = new { Name = "\u6D4B\u8A6611" };
+
+            var options = new JsonSerializerOptions { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, PropertyNamingPolicy = JsonNamingPolicy.CamelCase };
+            string result = JsonSerializer.Serialize(test, options);
+
+            Assert.Equal("{\"name\":\"\u6D4B\u8A6611\"}", result);
+        }
     }
 }
index 1fe8fcb..7686582 100644 (file)
@@ -271,7 +271,7 @@ namespace System.Text.Json.Serialization.Tests
             public ClassWithIgnoredUnsupportedBigInteger MyClass { get; set; }
         }
 
-        // Todo: add tests with missing object property and missing collection property.
+        // Todo: https://github.com/dotnet/runtime/issues/32348
 
         public class ClassWithPrivateSetterAndGetter
         {
index f2a0ba6..6cb8de0 100644 (file)
@@ -22,19 +22,6 @@ namespace System.Text.Json.Serialization.Tests
             Assert.NotEqual(expected, JsonSerializer.Serialize(inputString));
         }
 
-        // todo: move this to object tests; it is not a value test.
-        // https://github.com/dotnet/corefx/issues/40979
-        [Fact]
-        public static void EscapingShouldntStackOverflow_40979()
-        {
-            var test = new { Name = "\u6D4B\u8A6611" };
-
-            var options = new JsonSerializerOptions { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping, PropertyNamingPolicy = JsonNamingPolicy.CamelCase };
-            string result = JsonSerializer.Serialize(test, options);
-
-            Assert.Equal("{\"name\":\"\u6D4B\u8A6611\"}", result);
-        }
-
         [Fact]
         public static void WritePrimitives()
         {
index d24cb8e..96a05b6 100644 (file)
@@ -5554,7 +5554,7 @@ namespace System.Text.Json.Tests
                 jsonUtf8.WriteEndObject();
                 jsonUtf8.Flush();
 
-                // TODO: The output doesn't match what JSON.NET does (different rounding/e-notation).
+                // TODO: https://github.com/dotnet/runtime/issues/32350
                 // JsonTestHelper.AssertContents(expectedStr, output);
             }
         }