clj-commons / rewrite-clj

Rewrite Clojure code and edn
https://cljdoc.org/d/rewrite-clj
MIT License
586 stars 53 forks source link

Adapt as necessary to any new Clojure 1.12 syntax/conventions #279

Closed lread closed 1 day ago

lread commented 1 week ago

Clojure 1.12 is now in beta.

Probably best to add some explicit rewrite-clj tests for any new syntax it introduces. Since we've had no complaints, things are probably fine, but this gives me a chance to list and understand 1.12 newnesses.

Method Values

I don't think there is anything to confuse rewrite-clj here:

Before 1.12

(map #(Long/toBinaryString %) (range 8))
;; => ("0" "1" "10" "11" "100" "101" "110" "111")

In Clojure 1.12 can be expressed as:

(map Long/toBinaryString (range 8))
;; => ("0" "1" "10" "11" "100" "101" "110" "111")

Qualified Methods

Again, nothing weird expected for rewrite-clj here:

Before 1.12

(map #(new Long %) ["1" "2"])
;; => (1 2)

;; static method
(map #(Long/decode %) ["0xf" "0xa"])
;; => (15 10)

;; instance method
(map #(.toUpperCase %) ["hey" "ho"])
;; =>  ("HEY" "HO")

New in 1.12

(map Long/new ["1" "2"])
;; => (1 2)

;; static method
(map Long/decode ["0xf" "0xa"])
;; => (15 10)

;; instance method
(map String/.toUpperCase ["hey" "ho"])
;; => ("HEY" "HO")

Param tags

I'll verify param tags, they should read like regular metadata. I'll compare with how we currently handle ^SomeType and its long-form ^{:tag SomeType}.

;; short form
(^[double] String/valueOf 3)
;; => "3.0"

;; long form
(^{:param-tags [double]} String/valueOf 3)
;; => "3.0"

;; arity selection example
(map ^[_ _] clojure.lang.Tuple/create [1 2 3] [4 5 6])
;; => ([1 4] [2 5] [3 6])

Array class syntax

I don't expect issues here.

Did not come up with a meaningful example, but... this contrived code in 1.11

(String. ^"[B" (byte-array [72 73]))
;; => "HI"

Can be expressed in 1.12 like so:

(String. ^byte/1 (byte-array [72 73]))
;; => "HI"
lread commented 1 week ago

Ok. Interesting. Currently barfing on the new array class syntax.

(z/of-string "(String. ^byte/1 (byte-array [72 73]))")
;; => Execution error (ExceptionInfo) at clojure.tools.reader.impl.errors/throw-ex (errors.clj:34).
;;    Invalid symbol: byte/1.

And with clojure tools reader:

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

(edn/read-string "(String. ^byte/1 (byte-array [72 73]))")
;; => Execution error (ExceptionInfo) at clojure.tools.reader.impl.errors/throw-ex (errors.clj:34).
;;    Invalid symbol: byte/1.

Aha: TLDR-73

lread commented 1 week ago

Options for supporting Array class tokens

Decision1: How to support new syntax

  1. Wait for new version of tools.reader, and update our dependency PRO: Easy for us. CON: If user overrides tools.reader in their deps with older version rewrite-clj will fail for 1.12 array class token syntax. CON: We have to wait for updated tools reader to parse array class syntax

  2. Support new syntax using current version of tools.reader PRO: Can support 1.12 array class token syntax now PRO: If user is overriding with older version of tools.reader rewrite-clj will still be able to parse 1.12 syntax CON: Means customizing tools.reader behaviour, but we've already done this, so there is a precedent

I'm leaning toward option 2.

Decision2: When to support new syntax

  1. Optionally [rejected] Allow the user to consider array class token syntax as invalid through some rewrite-clj option. PRO: I don't really see any big pro here. Rewrite-clj wants to parse valid Clojure code, but has never explicitly been a validator of specific versions of Clojure code. CON: Waste of effort, won't be used.

  2. Always [chosen] No option to consider new array class token syntax as invalid. PRO: Simple. Matches current spirit of rewrite-clj.

borkdude commented 1 week ago

Option 1.3: use edamame, already supports it ;)

lread commented 1 week ago

Also interesting: tools.reader barfs on shorthand syntax for metadata (see TRDR-70) but rewrite-clj happens to handle it.

(edn/read-string "(^[double] String/valueOf 3)")
;; => Execution error (ExceptionInfo) at clojure.tools.reader.impl.errors/throw-ex (errors.clj:34).
;;    Metadata cannot be [double]. Metadata must be a Symbol, Keyword, String or Map.
(-> "(^[double] String/valueOf 3)"
    z/of-string
    z/string)
;; => "(^[double] String/valueOf 3)"
lread commented 1 week ago

Option 1.3: use edamame, already supports it ;)

Ha! Of course it does!

borkdude commented 1 week ago

Wait, maybe I spoke to soon. What does tools.reader currently not support? I assumed it was about the ^[bool] metadata, which edamame does already support, but nothing else new (which maybe already works with tools.reader as well).

lread commented 1 week ago

Oh @borkdude, I think, even if half-joking, you mean have rewrite-clj read Clojure with edamame instead of tools.reader! I initially thought you were suggesting folks use edamame instead of rewrite-clj. I think we chatted about this idea ages ago.

borkdude commented 1 week ago

The first, not the latter. It was half-joking yes.

lread commented 1 week ago

Wait, maybe I spoke to soon. What does tools.reader currently not support? I assumed it was about the ^[bool] metadata, which edamame does already support, but nothing else new (which maybe already works with tools.reader as well).

The new array class token syntax (ex. byte/1). See https://github.com/clj-commons/rewrite-clj/issues/279#issuecomment-2176499743

lread commented 1 week ago

Tools.reader also does not support the new ^[bool] syntax yet but rewrite-clj happens to support it fine.

borkdude commented 1 week ago

Same in edamame, but I could/should probably fix that

lread commented 1 week ago

It looks like rewrite-clj will need changes for vector metadata after all.

Currently we have:

(def n (p/parse-string "^[double] foo"))

(meta (node/sexpr n))
;; => {[double] true}

So vector metadata ^[double] is reader sugar that should always be returned as metadata {:param-tags [double]} from sexpr. But ^[double] should always, of course, be preserved in nodes for rewrite-clj rewriting.