elnabo / json2object

Type safe Haxe/JSON (de)serializer
MIT License
66 stars 17 forks source link

Schemas should require explicit Null<T> for nullability #41

Closed Gama11 closed 5 years ago

Gama11 commented 5 years ago

It looks like the schema generated for this:

typedef Project = {
    final name:String;
}

currently permits null as a valid value for name. I think this should only be the case if it's actually explicitly made nullable via Null<T> (which @:optional implies).

Gama11 commented 5 years ago

Even the root is permitted to be null, so schema validation would consider null as a valid JSON file.

elnabo commented 5 years ago

And since 7558e69d609f4b74ec8791493241caf3a2e16caf the generated schema are more null safe.

Gama11 commented 5 years ago

Hm.. it still feels a bit strange that null is explicitly suggested for @:optional fields, for instance here:

Of course it's technically correct since the Haxe counterpart var ?includeProjectFile:Bool; is implicitly a Null<T>, but I'd say it's unusual to put an explicit null value into the JSON, the intention with @:optional / ? is usually to omit it entirely in that case.

Perhaps a distinction should be made between Null<T> with and without @:optional?

Gama11 commented 5 years ago

It's actually worse for String fields since null is then the only suggestion:

elnabo commented 5 years ago

{"type":"null"} is no longer proposed for @:optional fields since 55b75880b5bf120a8196421c597f7946ff616dd8.

Gama11 commented 5 years ago

Nice, that makes it behave much more nicely. :)