Skip to content
Prev Previous commit
Next Next commit
Addressing feedback
  • Loading branch information
buyaa-n committed Nov 21, 2019
commit 8459413ac724b8ad62bf01b8a4b508b5d1063e90
24 changes: 17 additions & 7 deletions src/libraries/System.Text.Json/ref/System.Text.Json.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ public void Dispose() { }
public bool MoveNext() { throw null; }
void System.Collections.IEnumerator.Reset() { }
}
public sealed partial class JsonBoolean : System.Text.Json.JsonNode, System.IEquatable<System.Text.Json.JsonBoolean>
public sealed partial class JsonBoolean : System.Text.Json.JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
System.IEquatable<System.Text.Json.JsonBoolean>
#nullable restore
{
public JsonBoolean() { }
public JsonBoolean(bool value) { }
Expand Down Expand Up @@ -220,9 +223,7 @@ public JsonException(string? message, string? path, long? lineNumber, long? byte
public JsonException(string? message, string? path, long? lineNumber, long? bytePositionInLine, System.Exception? innerException) { }
public long? BytePositionInLine { get { throw null; } }
public long? LineNumber { get { throw null; } }
#pragma warning disable 8609
public override string? Message { get { throw null; } }
#pragma warning restore 8609
public override string Message { get { throw null; } }
public string? Path { get { throw null; } }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto; should we make the path in the ctor be non-nullable, and then make this non-nullable, too?

@ahsonkhan, @steveharter, can you help review this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make the path in the ctor be non-nullable, and then make this non-nullable, too?

Changing the (string) and (string, Exception) ctors to assign "" instead of null?

Copy link
Contributor Author

@buyaa-n buyaa-n Dec 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So am gonna change it non null and set empty string wherever it setting null. There is also assert checking it for null but seems useless will, delete it

public override void GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { }
}
Expand Down Expand Up @@ -274,7 +275,10 @@ public partial struct JsonNodeOptions
public System.Text.Json.DuplicatePropertyNameHandlingStrategy DuplicatePropertyNameHandling { readonly get { throw null; } set { } }
public int MaxDepth { readonly get { throw null; } set { } }
}
public sealed partial class JsonNull : System.Text.Json.JsonNode, System.IEquatable<System.Text.Json.JsonNull>
public sealed partial class JsonNull : System.Text.Json.JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
System.IEquatable<System.Text.Json.JsonNull>
#nullable restore
{
public JsonNull() { }
public override System.Text.Json.JsonValueKind ValueKind { get { throw null; } }
Expand All @@ -286,7 +290,10 @@ public JsonNull() { }
public static bool operator !=(System.Text.Json.JsonNull? left, System.Text.Json.JsonNull? right) { throw null; }
public override string ToString() { throw null; }
}
public sealed partial class JsonNumber : System.Text.Json.JsonNode, System.IEquatable<System.Text.Json.JsonNumber>
public sealed partial class JsonNumber : System.Text.Json.JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
System.IEquatable<System.Text.Json.JsonNumber>
#nullable restore
{
public JsonNumber() { }
public JsonNumber(byte value) { }
Expand Down Expand Up @@ -466,7 +473,10 @@ public JsonSerializerOptions() { }
public bool WriteIndented { get { throw null; } set { } }
public System.Text.Json.Serialization.JsonConverter? GetConverter(System.Type typeToConvert) { throw null; }
}
public sealed partial class JsonString : System.Text.Json.JsonNode, System.IEquatable<System.Text.Json.JsonString>
public sealed partial class JsonString : System.Text.Json.JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
System.IEquatable<System.Text.Json.JsonString>
#nullable restore
{
public JsonString() { }
public JsonString(System.DateTime value) { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,12 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont
/// <summary>
/// Gets a message that describes the current exception.
/// </summary>
public override string? Message
public override string Message
{
#pragma warning disable 8609
get
{
return _message;
return _message ?? base.Message;
}
#pragma warning restore 8609
}

internal void SetMessage(string? message)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ namespace System.Text.Json
/// <summary>
/// Represents a mutable boolean JSON value.
/// </summary>
public sealed class JsonBoolean : JsonNode, IEquatable<JsonBoolean>
public sealed class JsonBoolean : JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
IEquatable<JsonBoolean>
#nullable restore
{
/// <summary>
/// Initializes a new instance of the <see cref="JsonBoolean"/> class representing the value <see langword="false"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ namespace System.Text.Json
/// <summary>
/// Represents the null JSON value.
/// </summary>
public sealed class JsonNull : JsonNode, IEquatable<JsonNull>
public sealed class JsonNull : JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
IEquatable<JsonNull>
#nullable restore
{
/// <summary>
/// Initializes a new instance of the <see cref="JsonNull"/> class representing the value <see langword="null"/>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ namespace System.Text.Json
/// <summary>
/// Represents a mutable numeric JSON value.
/// </summary>
public sealed class JsonNumber : JsonNode, IEquatable<JsonNumber>
public sealed class JsonNumber : JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
IEquatable<JsonNumber>
#nullable restore
{
private string _value = null!;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ namespace System.Text.Json
/// <summary>
/// Represents a mutable text JSON value.
/// </summary>
public sealed class JsonString : JsonNode, IEquatable<JsonString>
public sealed class JsonString : JsonNode,
#nullable disable // to enable use with both T and T? for reference types due to IEquatable<T> being invariant
IEquatable<JsonString>
#nullable restore
{
private string _value = null!;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ public override bool CanConvert(Type type)
[PreserveDependency(
".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions)",
"System.Text.Json.Serialization.Converters.JsonConverterEnum`1")]
public override JsonConverter? CreateConverter(Type type, JsonSerializerOptions? options)
public override JsonConverter CreateConverter(Type type, JsonSerializerOptions? options)
{
JsonConverter? converter = (JsonConverter?)Activator.CreateInstance(
JsonConverter converter = (JsonConverter)Activator.CreateInstance(
typeof(JsonConverterEnum<>).MakeGenericType(type),
BindingFlags.Instance | BindingFlags.Public,
binder: null,
new object[] { EnumConverterOptions.AllowNumbers },
culture: null);
culture: null)!;

return converter;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ public override bool CanConvert(Type typeToConvert)
}

[PreserveDependency(".ctor()", "System.Text.Json.Serialization.Converters.JsonKeyValuePairConverter`2")]
public override JsonConverter? CreateConverter(Type type, JsonSerializerOptions? options)
public override JsonConverter CreateConverter(Type type, JsonSerializerOptions? options)
{
Type keyType = type.GetGenericArguments()[0];
Type valueType = type.GetGenericArguments()[1];

JsonConverter? converter = (JsonConverter?)Activator.CreateInstance(
JsonConverter converter = (JsonConverter)Activator.CreateInstance(
typeof(JsonKeyValuePairConverter<,>).MakeGenericType(new Type[] { keyType, valueType }),
BindingFlags.Instance | BindingFlags.Public,
binder: null,
args: null,
culture: null);
culture: null)!;

return converter;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public abstract class JsonConverterFactory : JsonConverter
/// </summary>
protected JsonConverterFactory() { }

internal JsonConverter? GetConverterInternal(Type typeToConvert, JsonSerializerOptions? options)
internal JsonConverter GetConverterInternal(Type typeToConvert, JsonSerializerOptions? options)
{
Debug.Assert(CanConvert(typeToConvert));
return CreateConverter(typeToConvert, options);
Expand All @@ -33,6 +33,6 @@ protected JsonConverterFactory() { }
/// <returns>
/// An instance of a <see cref="JsonConverter{T}"/> where T is compatible with <paramref name="typeToConvert"/>.
/// </returns>
public abstract JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions? options);
public abstract JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions? options);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ public override bool CanConvert(Type typeToConvert)
[PreserveDependency(
".ctor(System.Text.Json.Serialization.Converters.EnumConverterOptions, System.Text.Json.JsonNamingPolicy)",
"System.Text.Json.Serialization.Converters.JsonConverterEnum`1")]
public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions? options)
public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions? options)
{
JsonConverter? converter = (JsonConverter?)Activator.CreateInstance(
JsonConverter converter = (JsonConverter)Activator.CreateInstance(
typeof(JsonConverterEnum<>).MakeGenericType(typeToConvert),
BindingFlags.Instance | BindingFlags.Public,
binder: null,
new object?[] { _converterOptions, _namingPolicy },
culture: null);
culture: null)!;

return converter;
}
Expand Down