rewrite-clj

https://github.com/clj-commons/rewrite-clj
lread 2020-11-28T17:10:38.142300Z

I am wondering about namespaced maps and sexpr behaviour on affected map keys. Given:

#:my-prefix{:x 1}
When we perform an sexpr on the map-node we would expect:
{:my-prefix/x 1}
But when we perform an sexpr on the individual contained keyword-node for :x, I’m guessing that a caller might expect the context to be taken into account and see a return of:
:my-prefix/x
Rather than what I have rewrite-cljc (WIP) currently return:
:x
Before I rabbit-hole too much, wondering what other folks think here. Perhaps it is acceptable to document a caveat, that if you want map keys to be affected by their namespaced map, to sexpr at the map-node rather than the key-node level?

lread 2020-11-29T16:08:30.143400Z

Yeah, good point. About those read-string variants, I guess if the caller wants, they can then eval the read-string expression? You might assume I should know, but as I don’t have access to the original author of rewrite-clj, I make guesses, and ponder with others in this channel. :man-shrugging:

lread 2020-11-29T16:38:05.143600Z

@dave.dixon I forgot I had raised an issue to document read-string returns. Added your example. https://github.com/lread/rewrite-cljc-playground/issues/53

1
lread 2020-11-29T17:03:50.144700Z

As for the original discussion about losing namespaced map context when sexpr-ing on an individual key in the map… There might be something I can do, but I think I’ll just raise a git issue to maybe (or maybe not) address later… and document the limitation. @borkdude would this work for you too?

borkdude 2020-11-29T17:05:20.144900Z

@lee I'm not sure what to do here. I think supporting this will maybe complicate things since the nodes are no longer context-free?

borkdude 2020-11-29T17:06:12.145200Z

An alternative might be to make the keys namespaced as well and store the namespaced map as such

borkdude 2020-11-29T17:09:55.145600Z

@lee I think that may not be a bad idea. I'm not sure what the consequences are for restoring the code to string from such a change

lread 2020-11-29T17:10:14.145800Z

The issue is only for sepxr. So kind of a special case. Are you suggesting the keyword would optionally also carry the namespaced map prefix and auto-resolved? values?

lread 2020-11-29T17:13:51.146Z

The zipper is aware of the tree and knows the parent. I suppose it could optionally pass in the parent node info when sexpr ing a keyword-node that is in a namespaced map. But that might be a bit gross.

lread 2020-11-29T17:17:59.146200Z

Alternatively the parser could add the namespaced map context to the keyword node. But then we’d have that same data in two places, perhaps making node update confusing.

borkdude 2020-11-29T17:20:13.146500Z

@lee I was suggesting that the keyword node would just be literally :foo/bar represented as a normal rewrite-clj keyword

borkdude 2020-11-29T17:20:58.146700Z

sexpr-ing that would then automatically work

borkdude 2020-11-29T17:21:16.146900Z

and also these nodes are more consistent to look at outside the context of the map

lread 2020-11-29T17:23:47.147100Z

How would that work for folks who just want to reformat source code?

#::myalias  {:x 1 :y 2}
We’d still need to have keyword-node represent :x as :x no?

borkdude 2020-11-29T17:26:48.147400Z

@lee I think this would actually simplify things. What if you would add an un-namespaced key? Then you would have to print the map differently anyways. So when printing a namespaced map, one should check if the map is still namespaced and if all the children are still namespaced accordingly.

lread 2020-11-29T17:36:45.147600Z

I’m not following… yet! :simple_smile: A tool like cljfmt only wants whitespace affected. Non-whitespace code remains as written by the author. If the author wrote:

#:booya {:x 1 :booya/y 2}
This is equivalent to, but not as originally written:
#:booya{:x 1 :y 2}
Would your idea support that preserving the originally written code? (Forgive me if I’ve totally misunderstood your idea).

borkdude 2020-11-29T17:37:40.147900Z

I understand that this is the case for cljfmt, but not for refactoring tools

borkdude 2020-11-29T17:37:54.148200Z

e.g. the editor that @dave.dixon is going to make, might manipulate keys of a map

sparkofreason 2020-11-29T17:38:09.148400Z

This seems no different than editing clojure code. If you had the map #::myalias {:x 1 :y 2} and wanted to add un-namespaced :z, you'd have to edit the code to be {::myalias/x 1 :myalias/y 2 :z 3}. Those two things should parse to different AST's. How about a function to transform between the two AST node types?

borkdude 2020-11-29T17:39:07.148600Z

@dave.dixon I'm talking about the case where you first parse the first example, then manipulate the keys code-wise and then write out the map. I'm not even sure if that works correctly today.

borkdude 2020-11-29T17:40:28.148800Z

So if you store the keys always in a normalized fashion, you don't have to bother looking at the properties of the parent map. The printing of keys should always work independently from the parent map. And when printing the parent map, one can do a simple check: 1) the map was namespaced and 2) the children are all still namepaced, then write the prefix form, else write a normal map with individual keys

lread 2020-11-29T17:45:17.149Z

So, would rewrite-cljc rewrite this:

{:foo/x 1 :foo/y 2}
to this?:
#:foo{:x 1, :y 2}

borkdude 2020-11-29T17:47:36.149300Z

no, because the parent map of the keys is not namespaced

borkdude 2020-11-29T17:47:56.149500Z

you would only write the second if the original map node was already namespaced

sparkofreason 2020-11-29T17:48:36.149700Z

(-> (z/of-string "#:foo{:doink 1}")
     z/down
     z/right
     (z/append-child (n/keyword-node :bar))
     (z/append-child (n/value 2))
     (z/print-root))
=>
#:foo{:doink 1 :bar 1}

borkdude 2020-11-29T17:48:50.150Z

yeah, that's wrong

borkdude 2020-11-29T17:49:21.150200Z

also getting the key :doink from the children and calling sexpr on it would be wrong right now

sparkofreason 2020-11-29T17:51:34.150400Z

It's wrong in the sense of how the Clojure compiler would interpret it, right in the sense of being a literal translation of what was typed. For example, the clojure compiler would not accept the form ` ` (p/parse-string "{:foo 1 :bar}")

borkdude 2020-11-29T17:52:14.150600Z

sexpr should return not what is typed, but what is semantically the correct representation in clojure

borkdude 2020-11-29T17:52:51.150800Z

to return what is typed you can already use str

sparkofreason 2020-11-29T17:54:44.151Z

Yes, I agree that sexpr should handle things differently. And in general, it may be useful to have functions that transform nodes from a literal representation of the text to something closer to what the compiler actually sees after the reader does its thing. sexpr could probably leverage that to be more consistent.

borkdude 2020-11-29T17:59:30.151200Z

Does (sexpr namespaced-map) give the right result right now?

lread 2020-11-29T18:41:53.151400Z

Sorry, had to run an errand… This is a great discussion, thanks to you both! Forgetting about sexpr or a moment, I think it is important that what rewrite-cljc writes out what was written by the original author of the parsed code (with the exception of newlines which might be converted). I don’t mean semantically equivalent, I mean what the author typed in at the keyboard. This should, ideally, return true for any source we throw at rewrite-cljc:

(def source (slurp "<https://raw.githubusercontent.com/borkdude/clj-kondo/master/src/pod/borkdude/clj_kondo.clj>"))
(= source (-&gt; source z/of-string z/root-string))
@borkdude, will this hold true with your idea?

borkdude 2020-11-29T18:55:30.151600Z

Yes, it should work

lread 2020-11-29T18:57:37.151800Z

Cool, thanks @borkdude, that helps me to understand your angle better. I’ll dig in and explore your idea more deeply.

lread 2020-11-29T19:04:36.152Z

@borkdude, with your idea, would this rewrite as it was written?

#:foo {:x 1 :foo/y 2}
if not, I suppose having rewrite-cljc remember that y was qualified with foo when parsed would work.

borkdude 2020-11-29T19:10:33.152200Z

@lee that's an edge case I haven't thought about. This is another edge case I didn't know was possible:

#:foo {:x 1 :bar/y 2}
{:foo/x 1, :bar/y 2}

lread 2020-11-29T19:13:22.152400Z

There’s a really good code snippit in https://clojure.atlassian.net/browse/CLJ-1910 that describes namespaced map behaviours.

lread 2020-11-29T19:14:54.152600Z

Until I read its examples, I did not know about the _ prefix:

;; effects on keywords with ns, without ns, with _ ns, and non-kw
user=&gt; #:foo{:kw 1, :n/kw 2, :_/bare 3, 0 4}
{:foo/kw 1, :n/kw 2, :bare 3, 0 4}

lread 2020-11-29T19:15:48.152800Z

afk for a bit…

borkdude 2020-11-29T19:16:05.153Z

Huh... wow

borkdude 2020-11-29T19:16:17.153200Z

that seems like a major hack ;)

borkdude 2020-11-29T19:23:19.153400Z

@lee I'm happy to learn edamame parses this correctly:

user=&gt; (e/parse-string "#:foo{:kw 1, :_/bar 3}")
{:bar 3, :foo/kw 1}
because it relies on tools.reader for this 😅

lread 2020-11-29T20:33:55.153600Z

Ha! I guess it is not a horrible way to support unqualified keys in a namespaced map?

lread 2020-11-29T20:34:07.153800Z

Just probably not well known

borkdude 2020-11-29T20:35:25.154Z

I don't think namespaced maps are really that widely used anyways

lread 2020-11-29T21:20:39.154700Z

Anyhoo, I’ll sleep on all this and take a look again tomorrow. If nothing obvious and good comes to mind, I’ll just defer the work to a git issue.

lread 2020-11-29T21:25:49.155100Z

Just so I can get some momentum of some sort! :simple_smile:

borkdude 2020-11-29T21:42:08.155300Z

Most of those hits seem to show either tests for clojure itself or related tooling, or Datomic-related stuff

borkdude 2020-11-29T21:42:43.155600Z

Have a good night

sparkofreason 2020-11-28T17:18:56.142400Z

I'd vote caveat. For me, at least, the utility of rewrite-cljc is to manipulate code as written, rather than as evaluated.

lread 2020-11-28T17:27:02.142600Z

Thanks @dave.dixon, one concern is that rewrite-clj uses sexpr internally in zip API’s find-value, find-next-value and get. I suppose I could caveat for those too.

sparkofreason 2020-11-28T17:38:07.142800Z

Huh, the use of sexpr there is interesting, since it gives you something more like the evaluated result, after reader dispatch etc. has been applied.

lread 2020-11-28T17:55:56.143Z

Yeah, rewrite-clj’s sexpr, as I see it, is about convenience. I think the idea was that it is often more convenient to work with Clojure forms than rewrite-clj nodes. In the other direction we have convenient coercion from Clojure forms to rewrite-clj nodes.

sparkofreason 2020-11-28T19:00:11.143200Z

Functions like find-value are definitely a convenience and a nice abstraction, and covers a lot of cases. There's just going to be some corner cases like this where the user will have to be aware of the differences, because not all clojure syntax has a direct representation as an s-expression. I notice, for example, that (z/sexpr (z/of-string "#inst \"2020-01-01\"")) gives (read-string "#inst \"2020-01-01\"").