Add clojure.set/union, intersection, difference to core.spec.alpha

Question

Several tickets have been created suggesting that several functions in the clojure.set namespace might throw an exception if given non-sets as arguments, because in some cases they return unexpected values. This list is a sample, not intended to be complete: CLJ-810, CLJ-1682, CLJ-1953, CLJ-1954

Now that clojure.spec exists, documenting the expected argument types of these functions can be done more precisely. The specs could be used to dynamically detect incorrect argument types passed to these functions during testing, and via any other ways built on top of clojure.spec in the future.

Alex Miller suggested in a Slack discussion that perhaps it would be helpful if contributors would help implement such specs for functions within Clojure.

The approach taken with the proposed patch is simply to spec that the :args of clojure.set/subset?, superset?, union, intersection, and difference must all be sets, and spec the :ret types of the first two as boolean, and the last three as set. This seems to match all known comments given when declining previous tickets proposing changes to the behavior of these functions.

Patch: CLJ-2287-add-clojure-set-specs-v1.patch

An alternate approach not taken with the patch above would be to also allow nil as arguments and return values, but it is not clear whether that is part of the intended contract of these functions, or an incidental aspect of their current implementations.

I (Andy Fingerhut) looked through all of Clojure and contrib projects for occurrences of clojure.set/union, difference, intersection, superset?, and subset?, and there is only one such call that I can quickly determine does not pass sets to them. It is the one in clojure.data/diff for which there is a patch that can correct this (assuming it is considered a bug) on this ticket: CLJ-1087

Many such calls I could quickly determine always passed sets (e.g. because the arguments were wrapped in (set ...) calls). On a small fraction of them, I gave up trying to figure it out after a minute, since determining the answer fully from code inspection would require going back up the call tree a ways.

Please suggest any other ways that these changes could be evaluated in order to increase your confidence that they are correct.

Answer 1

_Comment made by: alexmiller_

For the moment I’ll say sure. :) I think this particular example is a good one.

I’m not sure that all the specs here are as obvious as you expect, or at least that’s been my experience elsewhere.

I’d expect these specs to be in clojure.set.specs namespace btw.

Answer 2

Comment made by: jafingerhut

Attached CLJ-2287-add-clojure-set-specs-v1.patch dated 2017-Dec-13. These are my first Clojure specs ever, so feel free to go crazy on suggesting improvements.

I used the namespace clojure.set.specs.alpha rather than clojure.set.specs as you suggested, but I can easily change that if you really did want it without the .alpha

Answer 3

Comment made by: jafingerhut

If there is some way I can exercise the :args specs with a large suite of tests, let me know how and I can try it.

And clearly I am putting in at least for :args and :ret pretty obvious choices of specs that I believe are correct, given issues raised against those functions in the past. I don't see how they could be any more subtle than that for these functions, but let me know if I'm missing anything, e.g. the Clojure core team has decided to support non-set arguments for these functions.

I agree that some of the other functions in the clojure.set namespace might be a bit trickier to spec the args of.

Answer 4

Comment made by: jafingerhut

I looked through all of Clojure and contrib projects for occurrences of clojure.set/union, difference, intersection, superset?, and subset?, and there is only one such call that I can quickly determine does not pass sets to them. It is the one in clojure.data/diff for which there is a patch that can 'correct' this (assuming it is considered a bug) on this ticket: https://dev.clojure.org/jira/browse/CLJ-1087

Many such calls I could quickly determine always passed sets (e.g. because the arguments were wrapped in (set ...) calls). On a small fraction of them, I gave up trying to figure it out after a minute, since determining the answer fully from code inspection would require going back up the call tree a ways.

Answer 5

Comment made by: jafingerhut

If you want an off-the-shelf compatible replacement for clojure.set functions that are identical in behavior, except they perform run-time type checks of the arguments you provide to them, and throw an exception if they have the wrong types (e.g. not sets for union, intersection, difference, subset?, and superset?), consider using the fungible library: https://github.com/jafingerhut/funjible

Answer 6

Comment made by: jafingerhut

This Github repository implements some ideas for specs of Clojure core functions: https://github.com/slipset/speculative

There is some discussion on this issue for what kinds of violations of some clojure.set function specs have been seen in some collections of tests. I have not seen a brief summary of all of the data yet. Just wanted to have a link to it from this issue for reference: https://github.com/slipset/speculative/issues/161

Answer 7

Reference: https://clojure.atlassian.net/browse/CLJ-2287 (reported by jafingerhut)