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

"Strict mode" to disallow additional fields? #15

Open
Gama11 opened this issue Feb 26, 2017 · 4 comments
Open

"Strict mode" to disallow additional fields? #15

Gama11 opened this issue Feb 26, 2017 · 4 comments

Comments

@Gama11
Copy link
Contributor

Gama11 commented Feb 26, 2017

Would it be possible to have a "strict mode" that errors when the parser encounters fields that are not part of the specified type? The "Reading" section in the Readme states:

Notice how fields not mentioned in the expected type do not show up.

So it seems additional fields are just ignored right now.

In a "strict mode", the parser should error with Unexpected field 'f' in this example:

class Main {
    public static function main() {
        var o:Struct = tink.Json.parse('{"e":"foo", "f":"bar"}');
        trace(o);
    }
}

typedef Struct = {
    var e:String;
}
@back2dos
Copy link
Member

back2dos commented Mar 1, 2017

It would indeed be possible. I'm not 100% sure you'd want strict parsing for the whole document though, but rather be more granular about it:

typedef Strict<T> = T;
typedef FlatStrict<T> = T; //strict about T, but not recursively
typedef Lenient<T> = T;

Or maybe there are better ways, I don't really know. My main motivation for tink_json was to consume data from external sources, so I did want proper errors if some fields were changed / removed, but skip any additions - or data that doesn't interest me to start with. So I don't really have any opinion on this. What do you think about more granularity?

@Gama11
Copy link
Contributor Author

Gama11 commented Mar 1, 2017

More granularity sounds good in principle, and that typedef-approach seems flexible enough. Lenient would be the default, and override a Strict that has been specified at a higher level in the hierarchy?

A more general note: I was looking into tink_json for parsing haxe-formatter config files (very much WIP btw, not really usable yet :D). However, it seems to be more performance-oriented, and I don't care about performance so much for parsing a single json file, just about having error messages that are as nice as possible. I think json2object will be a better fit for that (once it supports abstracts). :)

I like how json2object handles additional fields btw: it puts a warning into parser.warnings; which you can check if you want, or otherwise just ignore.

@back2dos
Copy link
Member

back2dos commented Mar 1, 2017

Well, concerning your use case, I would caution against warning user about superfluous fields in tooling configs. It is very common practice for tools that are build on top to add fields into the basic config (just consider what some tools do to npm's package.json).

As for the rest, tink_json happens to be fast (in fact according to @nadako on Haxe 3.4 it's faster on JS than native JSON parsing), but performance is merely a secondary goal. If you look closer, you will discover that this library does far more than json2object. But I really don't want to waste your time with weird attempts to sell it to you. If there's a lib that tackles the subset you care about in a manner that suits you more, then that's awesome. I just wish they hadn't built it entirely from scratch ... but where would that leave all the fun, right? ^^

@Gama11
Copy link
Contributor Author

Gama11 commented Mar 1, 2017

Hm... I think it's more important to catch typos and the like, improving the user experience (which affects every user), than to support tools built on-top (which only affects some, and only in case such tools are made). Those tools should be able to manage by only passing what's needed to the formatter, or if needed, there could be a --quiet mode.

Anyway, this is getting a bit off-topic. Thanks for the advice. :)

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

2 participants