brandonbloom / fipp

Fast Idiomatic Pretty Printer for Clojure
525 stars 44 forks source link

Printing a Var macro ad verbum may render an unreadable EDN file #88

Closed marksto closed 5 months ago

marksto commented 5 months ago

This issue is a peculiar observation at least and a kind of request for collective wisdom at most. =)

I ran into this in my small private declarative test harness. At some point I decided to introduce a replay feature that writes test suites to an EDN file after they are compiled. Since I have to set up layouts for a test case from time to time, there were a few tests where the mocks were set up with the Var macro (#'), like this:

{...
 :mock {#'clojure.core/rand-int (constantly 42)}
 ...}

Stating that this is "Not strictly Edn...", fipp prints a Var it visits via a simple str call, e.g.:

(str #'clojure.core/rand-int)
=> "#'clojure.core/rand-int"

This results in the following EDN output for my particular case:

{...
 :mock {#'clojure.core/rand-int #object[clojure.core$constantly$fn__5747
                                        "0x4d56f4c0"
                                        "clojure.core$constantly$fn__5747@4d56f4c0"]}
 ...}

Please, ignore the function being printed as a mere object. It doesn't really matter here, since it is technically possible to read it back. What ruins the day though is the ad verbum Var macro. Let me demonstrate.

  1. The clojure.edn won't be able to read this by any means.

    (clojure.edn/read-string "{:mock {#'clojure.core/rand-int (constantly 42)}}")
    Execution error at user/eval27686 (form-init11727032908263634995.clj:1).
    No dispatch macro for: '
    
    (clojure.edn/read-string {:default (fn [_tag value] value)}
                            "{:mock {#'clojure.core/rand-int (constantly 42)}}")
    Execution error at user/eval27845 (form-init11727032908263634995.clj:1).
    No dispatch macro for: '

    Since there is no way to provide a reader for the quote symbol ('), the game is over here.

  2. The clojure.tools.reader.edn does a better job here during the parsing phase.

    (clojure.tools.reader.edn/read-string "{:mock {#'clojure.core/rand-int (constantly 42)}}")
    Execution error (ExceptionInfo) at clojure.tools.reader.impl.errors/throw-ex (errors.clj:34).
    No reader function for tag 'clojure.core/rand-int.
    
    (clojure.tools.reader.edn/read-string {:readers {'clojure.core/rand-int identity}}
                                         "{:mock {#'clojure.core/rand-int (constantly 42)}}")
    Execution error (ExceptionInfo) at clojure.tools.reader.impl.errors/throw-ex (errors.clj:34).
    No reader function for tag 'clojure.core/rand-int.
    
    ;; This is unexpected and a bit unfortunate, but passing a reader for every function is not feasible anyway.
    
    (clojure.tools.reader.edn/read-string {:default (fn [_tag value] value)}
                                         "{:mock {#'clojure.core/rand-int (constantly 42)}}")
    Execution error (ExceptionInfo) at clojure.tools.reader.impl.errors/throw-ex (errors.clj:34).
    The map literal starting with (constantly 42) contains 1 form(s). Map literals must contain an even number of forms.

    So here we are able to reach the point where {(constantly 42)} cannot be legitimately parsed as a map. This is because the Var macro has been stripped off as a regular tag and cannot be reinserted in its place. This is how data-reader functions work and there is nothing you can do about it.

In fact, this leads me to believe that printing Var macro ad verbum is the root of the problem and should therefore be avoided.

We could end up with a better (at least readable) EDN file if fipp would tag the Var symbol itself. Something along the lines of:

(clojure.edn/read-string {:readers {'var #(requiring-resolve %)}}
                         "{:mock {#var clojure.core/rand-int (constantly 42)}}")
=> {:mock {#'clojure.core/rand-int (constantly 42)}}

The question is, though, is it possible to override the default visit-var behaviour (in a nice way)? Leaving the idea of printing Vars (and as well functions) to a file outside the scope of the question.

marksto commented 5 months ago

Heck, that was pretty easy!

(extend-protocol fipp.ednize/IOverride
  clojure.lang.Var)

(extend-protocol fipp.ednize/IEdn
  clojure.lang.Var
  (-edn [x]
    (tagged-literal 'var (symbol x))))

Then fipp produces what one would expect:

(with-out-str (fipp {:mock {#'clojure.core/rand-int (constantly 42)}}))
=>
"{:mock {#var clojure.core/rand-int #object[clojure.core$constantly$fn__5747
                                            \"0x183a328a\"
                                            \"clojure.core$constantly$fn__5747@183a328a\"]}}
 "

And we are able to read it back:

(clojure.edn/read-string {:readers {'var    requiring-resolve
                                    'object (fn [[_ _ object-fn-string]] object-fn-string)}}
                         fipp-output)
=> {:mock {#'clojure.core/rand-int "clojure.core$constantly$fn__5747@183a328a"}}

(clojure.tools.reader.edn/read-string {:readers {'var    requiring-resolve
                                                 'object (fn [[_ _ object-fn-string]] object-fn-string)}}
                                      fipp-output)
=> {:mock {#'clojure.core/rand-int "clojure.core$constantly$fn__5747@183a328a"}}

Nice!

Ah, I wish this behaviour were standard for Vars printing in fipp. But it kinda breaks the backwards compatibility of the lib, requires a custom reader tag, which in turn entails some default implementation (I opted for a simple requiring-resolve fn call, but I can easily imagine that this could be a problem).

brandonbloom commented 5 months ago

Glad you found a workaround for your needs.

I actually debated the right thing to do here long ago when I first wrote this library. On one hand, Fipp makes an effort to "Endize" everything. On the other hand, there is already dedicated syntax for vars. This library is primarily used for printing large data files, but it can also be used to print Clojure source. To me, it seemed better to loosen the guarantee that you always get something readable and instead provide more familiar syntax for the Clojure code printer.