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

Report on misspelled keys on s/keys & s/merge #62

Closed
ikitommi opened this issue Dec 31, 2017 · 4 comments
Closed

Report on misspelled keys on s/keys & s/merge #62

ikitommi opened this issue Dec 31, 2017 · 4 comments

Comments

@ikitommi
Copy link

Hi.

And thanks for the awesome lib! Would it be possible to have a way to report on extra keys on s/keys or s/merge specs? I have a configuration with mostly all the keys unqualified and optional. There is no guard against misspelled keys at the moment.

Foremost, this would be useful on the development phase, thus looking at expound :)

Added helpers on spec-tools to extract all the keys from both s/keys and s/merge that could be used/copied to get the actual set of defined keys to be checked against.

And my use-case is here.

@ikitommi ikitommi changed the title Report on extra keys on s/keys & s/merge Report on misspelled keys on s/keys & s/merge Dec 31, 2017
@bhb
Copy link
Owner

bhb commented Dec 31, 2017

@ikitommi Thanks very much for the idea!

I'm going to think a bit about whether this is something that fits well into Expound.

One concern is that I'm wondering if there a way to consistently get all keys for Clojure and Clojurescript in the case where there are multi-specs that use keys. There's more detail in this thread. Thank you for linking to the spec-tools code - I haven't yet tried it out for the multi-spec case, but it may solve this issue. I have also created https://dev.clojure.org/jira/browse/CLJ-2294 to ask for support in spec for this use case.

My second concern is how to integrate this into Expound. Currently Expound doesn't alter how spec validation works, it just replaces the printer. I could add options to the expound function, but changing validation in the case of instrumentation, or using explain (which are both supported) would require implementing wrappers around instrument, explain, assert, etc. This isn't necessary a bad thing (I'm considering doing this for Expound beta anyway), but I just want to think about what this would entail.

For the implementation, I think Expound would either need to modify the way spec validates data, or perhaps do a second pass of validation that specifically look for this case.

Anyway, I'll think more about this. I certainly agree it would be useful (I'd use it!) but I'm not sure if it should be in Expound or another library.

@ikitommi
Copy link
Author

ikitommi commented Jan 2, 2018

Hi. I poked with the strictly-specking that figwheel is using. Did a PR to make it work with Clojure 1.9.0. It does all the things I would like to have, but sadly, requires a custom ss/strict-keys to be used to get the errors for extra keys.

If the config maps were only 1-level deep, I could let the users define the spec as vanilla s/keys and convert them behind the scenes to ss/strict-keys and run validation against that. But can't do this with nested maps without mutating the Spec registry. Bummer. Something like local-keys might help here. I might do that, and push it to spec-tools or to strictly-specking.

I think the custom explain you mentioned isn't easy either. For s/keys, the s/explain* recurs to normal s/explain. Things like CLJ-2116 and CLJ-2251 would help here.

Hopefully you figure out a way to do this, maybe expound could use strictly-specking? Or merge the libs? I'll post here if I get something helpful in spec-tools (doesn't do multi-specs yet btw).

@bhb
Copy link
Owner

bhb commented May 29, 2018

@ikitommi @bhauman just released spell-spec. Would this work for your use case here?

@ikitommi
Copy link
Author

This is it. Will start integrating that into spec-tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants