naomijub / edn-rs

[DEPRECATED]: Crate to parse and emit EDN
https://crates.io/crates/edn-rs
MIT License
81 stars 14 forks source link

Re-think `Tagged` #148

Closed Grinkers closed 5 months ago

Grinkers commented 8 months ago

Original discussion in #141. I'm open to ideas. This will come after #95 and #141 for sure. I will let this sit in the back of my mind for some weeks/months. This seems really deep, like it might end up needing to almost reimplement clojure.

Off-spec

https://github.com/edn-format/edn?tab=readme-ov-file#tagged-elements I think the current implementation doesn't follow the spec. I believe we shouldn't have a Tagged type at all (and certainly not a (String, Edn)). An interesting thing to note is that I believe this is lossy. In my #foo and #bar examples, after the read-string, the tag is actually lost

reading a tag and tagged element yields one value.

https://github.com/edn-rs/edn-rs/blob/d64f20c3079268372970398349ae5bf5b7b40f41/tests/deserialize.rs#L716 This should throw an error without any custom readers.

(clojure.edn/read-string "#keyword :4")
java.lang.RuntimeException: No reader function for tag keyword [at <repl>:1:1]

Idea 1 (seems bad)

Clojure has a couple ways to define custom readers. One can be seen https://clojuredocs.org/clojure.core/data-readers Rust doesn't have a great way to dynamically do this. That would require creating Edn instances, mutating/registering them, pointers/lifetimes, etc. It would be a horrible mess to use.

Idea 2

The other way can be seen in this example

(require '[clojure.edn :as edn])

(defn foo [i] (if (= i 41) 42 "foobar"))
(defn bar [i] (str i "bar"))

;; prints "foobar"
(println (edn/read-string {:readers {'foo foo 'bar bar}} "#foo bar"))
;; prints "foobar"
(println (edn/read-string {:readers {'foo foo 'bar bar}} "#bar foo"))
;; prints "42"
(println (edn/read-string {:readers {'foo foo 'bar bar}} "#foo 41"))

I think we could do something like this in rust

fn foo(edn: (str, Edn)) -> T1 {
  match edn.0 {
    Edn::UInt(41) => 42,
    _ => "foobar".to_owned(),
  }
}

fn uui(edn: (str, Edn)) -> T2 {
   // some custom library/impl/whatever
   uuid::parse(edn.1)
}

fn custom_parse(): -> F 
where
  F: FnOnce(str) -> Result<GeneratedSumTypeForTs>, 
{
  edn_rs::some_new_fancy_macro!(foo, bar, more, readers)
}

// users can register a custom fn and then use it
let custom_reader_from_str = custom_parse();
let edn = custom_reader_from_str("#foo 41");

Idea 3

Agree this is insane and just keep the current implementation and leave it as an exercise for the user. Alternatively, we can feature gate Tagged, so people who need it to not be lossy can opt-in, but mark it as unstable/off-spec.

Grinkers commented 8 months ago

Wow, TIL clojure's read-string is actually quite powerful. It really is fully Extensible.

(require '[clojure.edn :as edn]
         '[clojure.string :as str]
         '[cheshire.core :as json])

(defn parse-rust-nums [s]
  (as-> s a
    (str/split a #"_")
    (apply str a)
    (Integer/parseInt a)))
(def parsed-data
  (edn/read-string
   {:readers {'rust parse-rust-nums 'json #(json/parse-string %) 'keyword #(keyword %)}}
   "[#json \"{\\\"cat\\\": 42}\" #rust \"123_456_221\" #keyword \"keyword with spaces\"]"))

;; (clojure.lang.PersistentArrayMap java.lang.Integer clojure.lang.Keyword)
(prn (map type parsed-data))
;; [{"cat" 42} 123456221 :keyword with spaces]. 
(prn parsed-data)

note that clojure actually supports keywords with spaces in them, even though it's not normally possible to type them in a repl directly with : or edn-able with prn-str (this is a particular clojure pet peeve of mine). A little custom reader can add support for this.

naomijub commented 8 months ago

Will try to read with more attention this weekend

Grinkers commented 5 months ago

https://github.com/edn-rs/edn-rs/issues/158