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

Description

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

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

[java] ERROR in (test-walk-bean) (:-1)
[java] expected: (clojure.walk/prewalk identity b)
[java] actual: java.lang.UnsupportedOperationException: empty
[java] at clojure.core.proxy$clojure.lang.APersistentMap$ff19274a.empty (:-1)
[java] clojure.core$empty.invokeStatic (core.clj:5202)
[java] clojure.walk$walk.invokeStatic (walk.clj:49)
[java] clojure.walk$prewalk.invokeStatic (walk.clj:64)
[java] clojure.walk$prewalk.invoke (walk.clj:60)
[java] clojure.lang.AFn.applyToHelper (AFn.java:156)
[java] clojure.lang.AFn.applyTo (AFn.java:144)
[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.

Environment

None

Activity

Show:
Alex Miller
August 31, 2018, 4:00 PM

Thanks, I've given you edit rights

Alex Miller
August 31, 2018, 4:08 PM

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.

Ning Sun
August 31, 2018, 4:33 PM

update description and add reproduce

Ning Sun
September 1, 2018, 3:08 AM

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.

Alex Miller
September 1, 2018, 4:29 PM

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.

Assignee

Unassigned

Reporter

Ning Sun

Labels

Approval

None

Patch

Code and Test

Affects versions

Priority

Major