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

Enforce Constructor Call or Add Nullable Returns #1002

Open
AeonSake opened this issue Oct 28, 2024 · 2 comments
Open

Enforce Constructor Call or Add Nullable Returns #1002

AeonSake opened this issue Oct 28, 2024 · 2 comments

Comments

@AeonSake
Copy link

Is your feature request related to a problem? Please describe.
Currently, the IDeserializer.Deserialize<T> methods return null if the parsed string is empty (or whitespaces). However, because the interface description declares the return type as T and not T? it is not immediately obvious that null can even be a valid return value.

Describe the solution you'd like
I would like the deserializer to always enforce the type construction (optionally via a DeserializerBuilder option like WithEnforceConstructorCall() ?), or change the interface return type to T? to make it clear to the user, that null is a possibility.

Describe alternatives you've considered
Override/ignore compiler info/warning and do a null-check on the returned instance regardless.

Additional context
There may be built-in deserializer options I did not see and this might also be possible with custom type converters, object factories, etc. If that is the case, please let me know.

@EdwardCooke
Copy link
Collaborator

Can you not do something like this deserialize.Deserialize<SomeclassName?>(yamltext);

Or are you saying that you would just expect the interface to already have the nullable attribute by default? I think that could be a breaking change, albeit just compiler warnings but it would be annoying for existing consumers to suddenly need nullable checks or directives to get rid of them.

@AeonSake
Copy link
Author

In my opinion, the interface should always fully inform the user about the type to expect. If null is a possibility, the interface should reflect that. Alternatively, an exception would also be ok I guess. But as it stands now, the interface tells the user that null is never a valid result, therefore also producing compiler warnings if you do a null-check regardless (depending on your settings).
So either have the type always be reflected as T?, throw an exception, or make it a compiler setting to at least construct the empty object instead of returning the unexpected null. As I understand it, an empty string is technically valid YAML, but contains no valid documents. I would still like the interface to reflect that behavior.

Just for the sake of comparison, Newtonsoft.Json and System.Text.Json both have T? as the expected return type (though System.Text.Json also throws an exception since an empty string is technically not valid JSON).

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

4 participants
@AeonSake @EdwardCooke and others