This is a breaking change to the JSON API, that intends to fix the issues with nullabillity of optional arguments.
The main issue is that the API was designed ~10 years ago, and made use of the very flexible notation supported by Resharper for [ContractAnnotations], which allowed for APIs similar to Get<T>(string key, bool required = false) that would describe a method that could not return null if required was true. A lot of methods made use of this pattern (ie: by default optional, but becomes required if the second parameter is true).
Unfortunately, the native nullability syntax of modern C# does not support this particular pattern (where nullability is based on the value of a boolean argument), which creates a lot of false positives, and forces one to add ! at the end of the statement, like this:
// OLD API
string id = json.Get<string>("id", required: true); // produces a warning because the compiler thinks it can return null
string name = json.Get<string>("name", required: true)!; // the '!' works around the issue but could be a minefield for future refactorings
string? description = json.Get<string>("description"); // optional value can be nullable
The goal of this PR is to change the APIs to remove the "required" parameter, and use different conventions for optional vs required.
For methods that return JsonValue (or derived), they will by default use the "required" policy, and will have another variant with the suffix OrDefault which will use the "optional" policy.
For ex: GetObjectstring) (required) vs GetObjectOrDefault(string) (optional).
For generic methods that casts values into managed types, the overload with an argument for the default value will be "optional", while the overload without this argument will be required.
For ex: Get<T>(string) (required) vs Get<T>(string, T) (optional).
// NEW API
string id = json.Get<string>("id"); // required, throws is id is null or missing
string name = json.Get<string>("name"); // no '!' needed any more
string? description = json.Get<string?>("description", null); // needs to specify "<string?>" explicitly
string role = json.Get<string>("role", "user"); // with default value is null or missing
int role = json.Get<int>("level", 1); // no need to fiddle with 'int?'
SomeEnum mode = json.Get<SomeEnum>("mode", SomeEnum.Default); // allows specifying custom defaults for enums
The main problem is that a lot of code used the previous API pattern, and needed a strategy to refactor it to the new API with breaking the code. The strategy used was to introduced the new API with all methods prefixed with _ (ie: add new _Get<T>(...) methods, but keep the old Get<T>(....) ), mark old methods as [Obsolete], and then painstakingly update the legacy code to use the new _Get overloads. Once completed, remove the old methods, ensure that everything compiles/run. And then finally use a simple rename refactoring to remove the _ prefix of the new API. All this work was done in this branch.
During this work, a lot of "bad patterns" where identified, and the new API attempts to correct it as well.
A few minor changes:
indexing an array with "optional" policy will return the JsonNull.Error singleton, instead of throwing an exception. This singleton can be recognized and will throw the appropriate exception if coerced into a value with the "required" policy). This allows fluent code like obj["someArray"][42]["id"] to null-propagate, similar to how obj["someObject"]["foo"]["bar"] would if foo was missing
the "null or missing" semantic is now implemented identically everywhere. A value is "null or missing" if it is either explicitly null, if the key is not present in the parent object, of the index is out of range of the parent array. In both { } and { "foo": null }, the foo field is "null or missing". The default value of types are not null or missing, so for example false, 0, "", [ ] and { } are NOT null or missing.
Direct deserialization from files/streams/buffers into CLR types will use the "required" policy by default. In 99.9%+ of use cases, the code was not expecting to deal with the case of a file or buffer of size 4 with content null (as in a file with 4 characters null. In most cases (config files, data dump), either the file would be missing/empty or would contain an empty object or array, not a null literal. If this cases must be supported, you should instead Parse the file into a DOM (which will return the JsonNull.Null singleton) and then deal with it accordingly.
The myriad of Parse/ParseObject/ParseArray had too many possible combinations and was simplified. It can support multiple types of inputes (string, Slice, byte[], ReadOnlySpan, ....), it can return a value, object or array, and can also be readonly or mutable. Only the "Parse" variants (that return a JsonValue) are implemented. If you need a JsonObject or JsonArray, you have to call Parse(...).AsObject() or Parse(...).AsArray().
A big chunk of bat pattern was also present in unit tests, whenever one had to check the content of a JSON Object (read from a database, from a remote API, etc..). Changes where made to allow a more "fluent" way to test the content, tailored to this use case (inside a unit test, which is a lot more forgiving than in production). Unfortunately, some native assertions in NUnit are not customizable enough (such as Is.Null, Is.True and Is.False) so another helper type IsJson had to be introduced.
Assert.That(obj["hello"], IsJson.EqualTo("world")); // test that `hello` is equal to the string "world"
Assert.That(obj["enabled"], IsJson.True); // test that `enabled` is equal to the boolean true
Assert.That(obj["not_found"], IsJson.Null); // test that `not_found` is null or missing (not present, or explicit null)
Assert.That(obj["foo"], IsJson.JsonObject); // test that `foo` is present and is an object
Assert.That(obj["bar"], IsJson.Not.Empty); // test that `bar` is either an object or array or string that is not empty
Assert.That(obj["bar"], IsJson.Array.And.Not.Empty); // test that `bar` is an array and is not empty
Assert.That(obj["foo"]["bar"][1]["baz"], IsJson.EqualTo(42)); // test that `foo.bar[1].baz` is present and is equal to 42
This is a breaking change to the JSON API, that intends to fix the issues with nullabillity of optional arguments.
The main issue is that the API was designed ~10 years ago, and made use of the very flexible notation supported by Resharper for
[ContractAnnotations], which allowed for APIs similar toGet<T>(string key, bool required = false)that would describe a method that could not returnnullifrequiredwastrue. A lot of methods made use of this pattern (ie: by default optional, but becomes required if the second parameter istrue).Unfortunately, the native nullability syntax of modern C# does not support this particular pattern (where nullability is based on the value of a boolean argument), which creates a lot of false positives, and forces one to add
!at the end of the statement, like this:The goal of this PR is to change the APIs to remove the "required" parameter, and use different conventions for optional vs required.
OrDefaultwhich will use the "optional" policy.GetObjectstring)(required) vsGetObjectOrDefault(string)(optional).Get<T>(string)(required) vsGet<T>(string, T)(optional).The main problem is that a lot of code used the previous API pattern, and needed a strategy to refactor it to the new API with breaking the code. The strategy used was to introduced the new API with all methods prefixed with
_(ie: add new_Get<T>(...)methods, but keep the oldGet<T>(....)), mark old methods as[Obsolete], and then painstakingly update the legacy code to use the new_Getoverloads. Once completed, remove the old methods, ensure that everything compiles/run. And then finally use a simple rename refactoring to remove the_prefix of the new API. All this work was done in this branch.During this work, a lot of "bad patterns" where identified, and the new API attempts to correct it as well.
A few minor changes:
JsonNull.Errorsingleton, instead of throwing an exception. This singleton can be recognized and will throw the appropriate exception if coerced into a value with the "required" policy). This allows fluent code likeobj["someArray"][42]["id"]to null-propagate, similar to howobj["someObject"]["foo"]["bar"]would iffoowas missing{ }and{ "foo": null }, thefoofield is "null or missing". The default value of types are not null or missing, so for examplefalse,0,"",[ ]and{ }are NOT null or missing.null(as in a file with 4 charactersnull. In most cases (config files, data dump), either the file would be missing/empty or would contain an empty object or array, not a null literal. If this cases must be supported, you should insteadParsethe file into a DOM (which will return theJsonNull.Nullsingleton) and then deal with it accordingly.Parse(...).AsObject()orParse(...).AsArray().A big chunk of bat pattern was also present in unit tests, whenever one had to check the content of a JSON Object (read from a database, from a remote API, etc..). Changes where made to allow a more "fluent" way to test the content, tailored to this use case (inside a unit test, which is a lot more forgiving than in production). Unfortunately, some native assertions in NUnit are not customizable enough (such as Is.Null, Is.True and Is.False) so another helper type
IsJsonhad to be introduced.