orchard icon indicating copy to clipboard operation
orchard copied to clipboard

Add support for spec-2

Open r0man opened this issue 3 years ago • 6 comments

Hello,

this PR adds support for spec-2 to orchard. spec-2 is supposed to be the successor of clojure.spec.alpha, but is still in an experimental state. The main advantage of spec-2 is that it adds s/schema and s/select which should improve some short comings of s/keys from the original spec. s/schema and s/select are supposed to help with re-using spec declarations. You would use s/schema to define the general shape of a data structure and then use s/select to make concrete sub selections of specs that have been defined with s/schema. It is not recommended to use spec-2 in production, but some people might do.

When Clojure Spec was initially released it lived under the namespace clojure.spec. Some time after the release it was renamed to clojure.spec.alpha. I believe because of this transition phase Orchard supports both namespaces and checks at run time which version of Clojure Spec is available and resolves specs accordingly. This PR adds a third option to the mix.

The PR changes Orchard to support spec-list, spec-form and spec-example NREPL ops in the following way:

For the spec-form and spec-example NREPL ops the Clojure specs are resolved and generated in the following order:

  • spec-alpha-2 (aka clojure.alpha.spec)

  • spec-alpha (aka clojure.spec.alpha, which is shipped with Clojure these days)

  • spec (aka clojure.spec, which is shipped in old Clojure versions?)

There is however a chance of a conflict if a spec is declared in more than one of the registries, and multiple variants of Clojure Spec are loaded at the same time. The most likely scenario is that people who use spec-2 have loaded both spec-alpha-2 (aka clojure.alpha.spec) and spec-alpha (aka clojure.spec.alpha). So spec-2 specs would shadow specs defined in a "previous" spec variant.

For the spec-list NREPL op the available spec registries are merged with spec-2 (clojure.alpha.spec) taking precedence over spec-alpha (aka clojure.spec.alpha) over spec (aka clojure.spec).

This order has been chosen because I believe people using spec-2 would prefer to see those specs instead of specs form older versions, in the unlikely case of a spec defined in multiple registries.

To be honest, I'm not super confident of the "unlikely" case and what happens in the future. This was the most pragmatic approach I could think of to get spec-2 working in Cider. It seems to work in my project and it should not affect people not using spec-2.

An alternative approach could be:

We could index the results of the NREPL operations by their spec variants. So, the spec-list op could return:

{:spec-alpha-2 ["some/spec", "another/spec"] :spec-alpha ["some/spec"] :spec ["some/spec"]}

And something similar for the other ops. The Cider frontend could then display all available Specs at the same time. If we want to go this direction we should probably introduces a new middleware with new ops for this, or add additional keys to the result of the existing ops. This might be the more "clean" approach, to support multiple flavors of Clojure Spec. But I'm not sure if it's worth the effort and I believe it would complicate the frontends a bit. Also I don't know how important it is to still support clojure.spec.

Any other ideas how to best integrate this?

Another open question is:

If we want to add tests for spec-2 we somehow need to bring the dependency in. spec-2 is only available via deps.edn. We could try to add it to orchard via lein-tools-deps or use some script to fetch it and leverage Leinigen's checkout feature to somehow add it to the classpath.

WDYT?

  • [x] The commits are consistent with our contribution guidelines
  • [x] You've added tests to cover your change(s)
  • [x] All tests are passing
  • [x] The new code is not generating reflection warnings
  • [x] You've updated the changelog (if adding/changing user-visible functionality)

r0man avatar Sep 13 '22 20:09 r0man

Here are the changes I made to Cider: https://github.com/r0man/cider/tree/spec2

It adds pretty printing for s/schema and s/select and seems to work fine with the approach of this PR.

r0man avatar Sep 13 '22 20:09 r0man

If we want to add tests for spec-2 we somehow need to bring the dependency in. spec-2 is only available via deps.edn. We could try to add it to orchard via lein-tools-deps or use some script to fetch it and leverage Leinigen's checkout feature to somehow add it to the classpath.

Can you elaborate on this? Why is it hard to have it as a dep with a Lein project?

bbatsov avatar Sep 19 '22 05:09 bbatsov

Overall, I'm fine with the approach you've taken in this PR, but I have to admit that the current situation with the 3 versions of Spec is killing me. Let's hope at the end it will be just Spec 2 and nothing else. I find this to be extremely funny:

spec-alpha-2 (aka clojure.alpha.spec)

spec-alpha (aka clojure.spec.alpha, which is shipped with Clojure these days)

Naming is hard!

bbatsov avatar Sep 19 '22 05:09 bbatsov

We could try to add it to orchard via lein-tools-deps or use some script to fetch it and leverage Leinigen's checkout feature to somehow add it to the classpath.

It's actually super easy - one clones the repo (shallowly, of course) and adds "src/main/clojure" to the Lein :source-paths.

vemv avatar Sep 19 '22 05:09 vemv

Here's the PR for CIDER https://github.com/clojure-emacs/cider/pull/3249

r0man avatar Sep 19 '22 14:09 r0man

If we want to add tests for spec-2 we somehow need to bring the dependency in. spec-2 is only available via deps.edn. We could try to add it to orchard via lein-tools-deps or use some script to fetch it and leverage Leinigen's checkout feature to somehow add it to the classpath.

Can you elaborate on this? Why is it hard to have it as a dep with a Lein project?

I think this is a non-issue. :) I did what @vemv suggested. I'm checking out spec-2 via git in the Makefile and added the source dir to project.clj. I had to exclude those sources from eastwood, since it detected some lint issues in spec-2.

r0man avatar Sep 19 '22 14:09 r0man