Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Empty YAML Documents #4

Closed
AeonSake opened this issue Nov 19, 2024 · 1 comment
Closed

Handle Empty YAML Documents #4

AeonSake opened this issue Nov 19, 2024 · 1 comment

Comments

@AeonSake
Copy link

Empty strings are technically valid YAML but will not return a valid type instance. Despite the interface definition, IDeserializer will return null if given an empty string (or anything that cannot be deserialized into the given type). It seems this library assumes that IDeserializer will always return a valid JToken instance before calling ToObject(). Since Newtonsoft.Json doesn't throw an exception on empty documents, but returns null instead, I would suggest changing the return type to T? (and object? for the non-generic methods) and check the IDeserializer result for null before calling ToObject(). Might also be a good idea to support nullability in the library as a whole.

https://github.com/tomlm/YamlConvert/blob/8fe35eed2b8b4d324e63d09c02fa6d6e9355a3fe/YamlConvert/YamlConvert.cs#L80C1-L85C10

With null-check:

public static T DeserializeObject<T>(string yaml, JsonSerializerSettings jsonSettings, IDeserializer deserializer = null)
{
    deserializer = deserializer ?? YamlConvert.DefaultDeserializer;
    return deserializer.Deserialize<JToken>(yaml)?.ToObject<T>(JsonSerializer.Create(jsonSettings));
}

With proper return type and null-check (assuming nullability is enabled on the library):

public static T? DeserializeObject<T>(string yaml, JsonSerializerSettings jsonSettings, IDeserializer? deserializer = null)
{
    deserializer = deserializer ?? YamlConvert.DefaultDeserializer;
    return deserializer.Deserialize<JToken>(yaml)?.ToObject<T>(JsonSerializer.Create(jsonSettings));
}

I have also created an issue on the YamlDotNet repository regarding the incorrect return types in their interface.

My current workaround is now checking the input string for valid, non-empty YAML document content which seems a bit redundant since the underlying converters do that already.

Regardless of this issue, great work on this library. It helped me a ton to avoid rewriting a large number of JSON converters just to support YAML.

@tomlm
Copy link
Owner

tomlm commented Nov 21, 2024

I have just published v3.1 which supports this scenario.
YamlConvert.DeserializeObject<T>("") => default(T);
so

YamlConvert.Deserialize<int>("") => 0
YamlConvert.DerializeObject<int?>("") => null

@tomlm tomlm closed this as completed Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@tomlm @AeonSake and others