OrderedSet/hashCode throws an NPE if it contains nil

[NoahTheDuke]

Repo:

(.hashCode (os/ordered-set nil :a :b :c))

Stack trace:

#error {
 :cause nil
 :via
 [{:type java.lang.NullPointerException
   :message nil
   :at [flatland.ordered.set.OrderedSet$fn__4371 invoke "set.clj" 59]}]
 :trace
 [[flatland.ordered.set.OrderedSet$fn__4371 invoke "set.clj" 59]
  [clojure.core$map$fn__5935 invoke "core.clj" 2770]
  [clojure.lang.LazySeq sval "LazySeq.java" 42]
  [clojure.lang.LazySeq seq "LazySeq.java" 51]
  [clojure.lang.RT seq "RT.java" 535]
  [clojure.core$seq__5467 invokeStatic "core.clj" 139]
  [clojure.core.protocols$seq_reduce invokeStatic "protocols.clj" 24]
  [clojure.core.protocols$fn__8236 invokeStatic "protocols.clj" 75]
  [clojure.core.protocols$fn__8236 invoke "protocols.clj" 75]
  [clojure.core.protocols$fn__8178$G__8173__8191 invoke "protocols.clj" 13]
  [clojure.core$reduce invokeStatic "core.clj" 6882]
  [clojure.core$reduce invoke "core.clj" 6868]
  [flatland.ordered.set.OrderedSet hashCode "set.clj" 59]
...

Comes from unconditionally calling .hashCode on all of the contained items:

ordered/src/flatland/ordered/set.clj

Line 59 in 8cc133d

(reduce + (map #(.hashCode ^Object %) (.seq this))))

I believe this could be solved by either using clojure.core/hash or by changing to a keep (which is fine because the hash of nil is 0):

(reduce + (keep #(when % (.hashCode ^Object %)) (.seq this)))) 

[borkdude]

Sounds good to me

[jafingerhut]

Sorry if I am belaboring the obvious, but definitely good to add a test case that compares .hashCode of an ordered set against .hashCode of Clojure's built-in set type, both containing a nil element (and a few other chosen elements) to verify that they are the same with whatever code changes you make.

[NoahTheDuke]

Seems there was an incorrectly written test to check for this very thing. Oops!

[borkdude]

I don't think this issue is fixed since the hashcode for set with nil will be the same as hash-set with false now.

[NoahTheDuke]

Ah dang, right you are.