Proxy generated from (bean) doesn't work with clojure.walk

Question

Currently if you run clojure.walk functions on a proxy generated from (bean), it throws exception:

(clojure.walk/prewalk identity (bean (java.util.Date.)))

 (link: java) ERROR in (test-walk-bean) (:-1)
 (link: java) expected: (clojure.walk/prewalk identity b)
 (link: java)   actual: java.lang.UnsupportedOperationException: empty
 (link: java)  at clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)
 (link: java)     clojure.core$empty.invokeStatic (core.clj:5202)
 (link: java)     clojure.walk$walk.invokeStatic (walk.clj:49)
 (link: java)     clojure.walk$prewalk.invokeStatic (walk.clj:64)
 (link: java)     clojure.walk$prewalk.invoke (walk.clj:60)
 (link: java)     clojure.lang.AFn.applyToHelper (AFn.java:156)
 (link: java)     clojure.lang.AFn.applyTo (AFn.java:144)
 (link: java)     clojure.core$apply.invokeStatic (core.clj:657)

Because the proxy doesn't implement empty, which is required by clojure.walk. This patch added a test to reproduce and a fix.

Answer 1

Comment made by: alexmiller

Can you add repro in description?

And if you have not signed a Contributors Agreement, please do so or we can't consider the patch. https://clojure.org/community/contributing

Thanks!

Answer 2

Comment made by: sunng

Seems the description isn't editable.

I have included a testcase in the patch to reproduce this issue:

(clojure.walk/postwalk identity (bean (java.util.Date.)))

I have already signed agreement as (Ning Sun, classicning@gmail.com, github:sunng87)

Answer 3

Comment made by: alexmiller

Thanks, I've given you edit rights

Answer 4

Comment made by: alexmiller

Looking at this a little more closely, I do not think it makes sense to pursue the solution you're suggesting. bean presents a (read-only) map view of a Java object. empty is a mutating operation and something we don't support for things like records which have a similar purpose.

I'm not objecting to the problem statement - I think that's a reasonable use case. But I think the fix needs to be in walk, not in bean.

Answer 5

Comment made by: sunng

update description and add reproduce

Answer 6

Comment made by: sunng

Thanks, Alex. I just checked again that doc of clojure.core/empty says it, which delegates to .empty of IPersistentCollection, is to return an empty collection of the same category. And the implementation of PersistentArrayMap just returned a cached empty instance. Neither of them mutated the original collection. So I think it's reasonable to return an empty map here.

Answer 7

Comment made by: alexmiller

These are all immutable collections so I'm not talking about actual mutation, but about change from one immutable structure to another. There are several existing cases where we have immutable collections that don't support empty because it doesn't make conceptual sense, like record instances and map entries.

What I'm saying is that I believe this is one of those cases, regardless of whether the particular path of interfaces and impls allows the method to be called. Conceptually, bean creates an instance that is a read-only view of a Java object. It makes no sense to "empty" an object view - objects always have fields (this is the same argument with records, which have fixed fields, and map entries, which have key/value). I believe in ClojureScript there is actually a protocol that marks whether a collection can be emptied and perhaps that would make sense here as well.

Answer 8

Reference: https://clojure.atlassian.net/browse/CLJ-2399 (reported by sunng)

Answer 9

I think adding a special case for APersistentMap to simply use (into {} ... inside of walk instead would fix this issue.