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

Use clojure.spec.alpha #403

Open
alex-dixon opened this issue Jul 21, 2018 · 4 comments
Open

Use clojure.spec.alpha #403

alex-dixon opened this issue Jul 21, 2018 · 4 comments

Comments

@alex-dixon
Copy link
Contributor

This has been mentioned elsewhere but doesn't seem to have a dedicated issue.

Continuing from @rbrush's comment:

Yes, I agree that we should include this in Clara proper and remove the old Prismatic schema. [...]
we need to get on the evolutionary trunk of this stuff in Clojure.
#367 (comment)

1.10 alphas are out now. I have some specs in various places. I'm going to start gathering them into a branch with this issue number.

I could use some help on maintaining support for 1.7 and 1.8.

@cfleming on this:

this seems to be easy. As an example it's what clojure.java.jdbc does. In fact, the specs could even be kept in a totally separate artifact if required.
#367 (comment)

It looks like they only define specs in a single namespace and that this namespace is only required by a test:
https://github.com/clojure/java.jdbc/blob/d8a35b9531b76a772467ca24b2d866b7bc29b6c6/src/test/clojure/clojure/java/jdbc_test.clj#L29
They use s/fdefs, so when the test is run with > 1.9 they turn on instrumentation, which throws spec errors for function vars the s/fdefs are defined for.

I don't know whether or how users get these errors, since the namespace with the fdefs doesn't appear to be required by anything other than this test.

This may be minor, but they don't require spec in any of their namespaces either. Without it, you can't use s/valid, s/assert, s/conform, or s/def inline with source code, which is a more common pattern than having separate spec-only namespaces, and provides more flexibility than s/fdef alone.

So potentially we could use s/fdefs only and conditionally require the namespaces with them based on *clojure-version*.

Thoughts?

@metacritical
Copy link

metacritical commented Aug 28, 2018

This parser would need to be written in parts for each spec type and slowly replaced. Is there a road map/steps to implement this requirement.

@WilliamParker
Copy link
Collaborator

@alex-dixon

So potentially we could use s/fdefs only and conditionally require the namespaces with them based on *clojure-version*.

That sounds reasonable - would it be better to detect the spec namespaces on the classpath themselves though, or perhaps the combination of the two?

@metacritical

This parser would need to be written in parts for each spec type and slowly replaced. Is there a road map/steps to implement this requirement.

Is this proposal to replace the existing logic in the DSL namespace with something from spec? https://github.com/cerner/clara-rules/blob/0.19.0/src/main/clojure/clara/rules/dsl.clj
To be honest, I've explored it a bit, but I haven't dug deeply into spec's APIs yet, just haven't had the time, and applications to Clara's parsing isn't something I've thought about. Do you have thoughts on how this could be done and what the benefits would be? I think the answer to "is there a road map" is that there isn't one, as far as I'm aware.

@metacritical
Copy link

metacritical commented Oct 5, 2018

@WilliamParker A benefit of 'clojure.spec' would be destructing of data is automatic based on the shape of data. So you can write your walker/parser code using a multi method dispatch, which is quite readable and thus maintanable, compared to case condition where we trying to read symbols, keywords and destructuring manually.

@alex-dixon
Copy link
Contributor Author

you can write your walker/parser code using a multi method dispatch, which is quite readable and thus maintanable, compared to case condition where we trying to read symbols, keywords and destructuring manually.

The second way might be better if you’re relying on s/conform to parse syntax for you that may have specs registered for it. spec ends up conforming everything recursively, so it wouldn’t necessarily just conform using Clara’s defined specs — could be any registered spec. I’m not sure whether this could break existing users. May depend on when user defined specs are loaded relative to when Clara’s are.

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

3 participants