Clojure

clojure.walk/postwalk does not preserve MapEntry type objects

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: Release 1.9
  • Fix Version/s: Release 1.10
  • Component/s: None
  • Labels:
  • Patch:
    Code and Test
  • Approval:
    Screened

Description

This came up on Slack. A naïve implementation of "lispify" to turn vectors into lists used this code:

(defn lispify [s]
  (w/postwalk (fn [e] (if (vector? e) (apply list e) e)) s))

But when called like this:

(lispify [:html {:a "b"} ""])

It produces this error: java.lang.ClassCastException: clojure.lang.Keyword cannot be cast to java.util.Map$Entry

My initial reaction was to change the condition to (and (vector? e) (not (map-entry? e))) but that still failed, because while walking the hash map, the MapEntry [:a "b"] was turned into a PersistentVector.

At this point, we can switch to using prewalk and it works as expected:

(defn lispify [s]
  (w/prewalk (fn [e] (if (and (vector? e) (not (map-entry? e))) (apply list e) e)) s))

Now we get the expected result:

boot.user> (lispify [:html {:a "b"} ""])
(:html {:a "b"} "")

This seems unintuitive at best and feels like a bug: postwalk should preserve the MapEntry type rather than converting it to a PersistentVector.

Cause: The problem seems to be this line https://github.com/clojure/clojure/blob/master/src/clj/clojure/walk.clj#L45:

(instance? clojure.lang.IMapEntry form) (outer (vec (map inner form)))

Proposed: Change to:

(instance? clojure.lang.IMapEntry form) (outer (clojure.lang.MapEntry/create (inner (first form)) (inner (second form)))))

This would preserve the type of the subelement.

Patch: clj-2031-w-test-v2.diff
Screened by: Alex Miller

Activity

Hide
Alex Miller added a comment -

seems reasonable

Show
Alex Miller added a comment - seems reasonable
Hide
Jozef Wagner added a comment -

Added patch with test

Show
Jozef Wagner added a comment - Added patch with test
Hide
Alex Miller added a comment -

Instead of the calls to .key and .val you should just call key and val.

Show
Alex Miller added a comment - Instead of the calls to .key and .val you should just call key and val.
Hide
Jozef Wagner added a comment -

Good catch, thanks! Added patch clj-2031-w-test-v2.diff that uses key and val instead.

Show
Jozef Wagner added a comment - Good catch, thanks! Added patch clj-2031-w-test-v2.diff that uses key and val instead.
Hide
Sean Corfield added a comment -

This keeps coming up on Slack – given there's a patch and this is prescreened, can it just get fixed as part of the next Clojure 1.9.0 Alpha/Beta build?

Show
Sean Corfield added a comment - This keeps coming up on Slack – given there's a patch and this is prescreened, can it just get fixed as part of the next Clojure 1.9.0 Alpha/Beta build?
Hide
Alex Miller added a comment -

It's in a list for Rich to look at.

Show
Alex Miller added a comment - It's in a list for Rich to look at.

People

Vote (13)
Watch (8)

Dates

  • Created:
    Updated: