S-expr-ed tagged literal results in unqualified reference to read-string

[borkdude]

Sexpr-ing a tagged literal results in an expression like (read-string "#foo/bar 1) .
Imo this should be fully qualified to prevent clashes with referred vars from elsewhere.

PR incoming.

[lread]

I think I get the rationale, but I need more words to explore/verify.
Lemme know if I'm on the right track here!

Background

Rewrite-clj emits (read-string ...) for sexpr when it feels it cannot do better for a given node. This includes tagged literals.

(require '[rewrite-clj.node :as n]
         '[rewrite-clj.parser :as p])

(-> "#inst \"2018-03-28T10:48:00.000\""
    p/parse-string-all
    n/sexpr)
;; => (read-string "#inst \"2018-03-28T10:48:00.000\"")

Problem

The implication is that the user might, in turn, eval this (read-string ...).

(-> "#inst \"2018-03-28T10:48:00.000\""
    p/parse-string-all
    n/sexpr
    eval
    ((juxt type str)))
;; => [java.util.Date "Wed Mar 28 06:48:00 EDT 2018"]

And that because read-string is unqualified, the eval might use an unintended read-string.
Also clojure.core/read-string is not safe.

Snag

But read-string varies by platform.
It is in clojure.core for clojure but not in core for cljs.

So we probably want to emit a qualifier that makes sense across platforms.

Ideas:

1. One idea from Fix #331: sexpr on tagged literal should be fully qualified #332 (comment): clojure.edn/read-string. This is safe and available across platforms. The ns clojure.edn ns does not technically need to be required to be used (oh, that maybe is not true for cljs).

2. Continue to emit read-string by default, but allow user to override through some option. Details TBD.

3. If the same read-string does not work for all platforms, emit a platform-specific read-string. This is not inconsistent with sexpr and differences in clojure platforms.

Related

Not scoped in this issue but worth mentioning:

sexpr also emits read-string for reader conditionals

(-> "#?(:clj x :cljs y)"
    p/parse-string-all
    n/sexpr)
;; => (read-string "#?(:clj x :cljs y)")

Not sure how to handle this one. Would need :read-cond to actually eval, .i.e, from a clj jvm repl:

user=> (clojure.core/read-string {:read-cond :allow} "#?(:clj x :cljs y)")
x

Maybe handle reader conditionals better/differently with sexpr?

[borkdude]

Yeah I think clojure.edn/read-string makes more sense!

[lread]

Ok, I think that sounds good. It won't deal with reader conditionals, but that would have to be a separate issue.