Close #305: Qualify @ ~ ~@ sexpr's under clojure.core (#306)

* Qualify @ ~ ~@ sexpr's under clojure.core z/sexpr should return the same value as clojure.core/read-string as per the documentation. The tests were moved from clojure.tools.reader.edn/read-string to clojure.tools.reader/read-string because the latter implements Clojure's reader. After this commit, the only special forms that return different values are ` and #=. * test & ci: bump sci-test for new tools.reader vars * lib test: patch zprint to disable new failing test See kkinnear/zprint#333 --------- Co-authored-by: lread <lee@dlread.com>
clj-commons · Sep 8, 2024 · 7ddab96 · 7ddab96
1 parent 29e39a2
commit 7ddab96
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 36 deletions.
diff --git a/deps.edn b/deps.edn
@@ -137,7 +137,7 @@
 
            :sci-test {:extra-paths ["target/generated/sci-test/src"]
                       :extra-deps {lread/sci-test {:git/url "https://github.com/lread/sci-test.git"
-                                                   :sha "5adc46b18efa8661c9c86bb79c0876265cf6612a"}}}
+                                                   :sha "27318786f077d491c9cbc0c9d99e5781d9a9ef83"}}}
 
            :native-test {:extra-paths ["target/generated/graal"]}
 

diff --git a/doc/01-user-guide.adoc b/doc/01-user-guide.adoc
@@ -1062,7 +1062,7 @@ a|<unsupported operation>
 
 2+a|*Deref* `:deref`
 a|`@form`
-a|`(deref form)`
+a|`(clojure.core/deref form)`
 
 2+a|*Metadata* `:meta`
 a|`^{:a 1 :b 2} [1 2 3]`
@@ -1098,7 +1098,11 @@ a|`(quote symbol)`
 
 2+a|*Syntax unquote* `:unquote`
 a|`~symbol`
-a|`(unquote symbol)`
+a|`(clojure.core/unquote symbol)`
+
+2+a|*Unquote splicing* `:unquote-spliciing`
+a|`~@symbol`
+a|`(clojure.core/unquote-splicing symbol)`
 
 2+a|*Tagged literal* `:reader-macro`
 a|`#foo/bar [1 2 3]`
@@ -1181,7 +1185,7 @@ a|`'#::my-ns-alias{my-symbol 1}`
 
 ==== Rewrite-clj Default Auto-Resolve Handling
 
-When calling `sepxr` on an auto-resolved keyword or symbol node, rewrite-clj will resolve:
+When calling `sexpr` on an auto-resolved keyword or symbol node, rewrite-clj will resolve:
 
 * the current namespace to `?\_current-ns_?`
 * namespaced alias `x` to `??\_x_??`
@@ -1202,6 +1206,8 @@ To illustrate:
 ;; => #:??_my-alias_??{s1 3, :b 2, :a 1}
 ----
 
+Currently, symbols under syntax quote are never resolved.
+
 [#custom-auto-resolve]
 ==== Custom Auto-Resolve Handling
 

diff --git a/doc/design/namespaced-elements.adoc b/doc/design/namespaced-elements.adoc
@@ -102,6 +102,11 @@ The following rewrite-clj nodes throw an exception for `sexpr` which is sensible
 - whitespace
 - uneval, which is rewrite-clj's term for `#_`
 
+The following nodes have different `sexpr` representations than if you called `read-string`:
+
+- read eval
+- syntax quote
+
 === Sexpr support for namespaced elements in rewrite-clj v0 and rewrite-cljs
 _Auto-resolved_ keywords have been around https://groups.google.com/g/clojure/c/i770QaIFiF0/m/v63cZgrlBwAJ[since at least Clojure 1.0, which was released in May 2009].
 https://github.com/clojure/clojure/blob/master/changes.md#12-support-for-working-with-maps-with-qualified-keys[Namespaced maps were introduced in Clojure 1.9, released in December 2017].

diff --git a/script/test_libs.clj b/script/test_libs.clj
@@ -245,7 +245,16 @@
         (string/replace #"\[lein-zprint \"1.2.4\"\]"
                         "[lein-zprint \"1.2.4.1\"]")
         (->> (spit p))))
-  (show-patch-diff lib))
+  ;; zprint 1.2.9 has a single failing test for https://github.com/clj-commons/rewrite-clj/pull/306
+  ;; Have raised this with over at zprint https://github.com/kkinnear/zprint/issues/333
+  ;; and have agreement that it is a zprint issue.
+  ;; Disable the failing test which starts on line 2538
+  (status/line :detail "Patching for failing test in v1.2.9")
+  (let [p (str (fs/file home-dir "test/zprint/guide_test.cljc"))
+        lines (-> p slurp string/split-lines)
+        new-lines (update lines 2537 #(str "#_" %))]
+    (->> (string/join "\n" new-lines)
+         (spit p))))
 
 (defn- zprint-prep [{:keys [home-dir]}]
   (status/line :detail "=> Building uberjar for uberjar tests")
@@ -449,7 +458,7 @@
             :test-cmds ["lein test"]}
            {:name "zprint"
             :version "1.2.9"
-            :note "1) planck cljs tests disabled for now: https://github.com/planck-repl/planck/issues/1088"
+            :note "1) planck cljs tests disabled for now: https://github.com/planck-repl/planck/issues/1088 2) failing v1.2.9 test disabled"
             :platforms [:clj :cljs]
             :github-release {:repo "kkinnear/zprint"}
             :patch-fn zprint-patch

diff --git a/src/rewrite_clj/node/quote.cljc b/src/rewrite_clj/node/quote.cljc
@@ -110,7 +110,7 @@
   [children]
   (if (sequential? children)
     (->node
-      :unquote "~" 'unquote
+      :unquote "~" 'clojure.core/unquote
       children)
     (recur [children])))
 
@@ -135,6 +135,6 @@
   [children]
   (if (sequential? children)
     (->node
-      :unquote-splicing "~@" 'unquote-splicing
+      :unquote-splicing "~@" 'clojure.core/unquote-splicing
       children)
     (recur [children])))
diff --git a/src/rewrite_clj/node/reader_macro.cljc b/src/rewrite_clj/node/reader_macro.cljc
@@ -68,7 +68,7 @@
   (node-type [_node] :deref)
   (printable-only?[_node] false)
   (sexpr* [_node opts]
-    (list* 'deref (node/sexprs children opts)))
+    (list* 'clojure.core/deref (node/sexprs children opts)))
   (length [_node]
     (inc (node/sum-lengths children)))
   (string [_node]

diff --git a/test/rewrite_clj/parser_test.cljc b/test/rewrite_clj/parser_test.cljc
@@ -1,10 +1,9 @@
 (ns ^{:doc "Tests for EDN parser."
       :author "Yannick Scherer"}
  rewrite-clj.parser-test
-  (:refer-clojure :exclude [read-string])
   (:require [clojure.string :as string]
-            [clojure.test :refer [deftest is]]
-            [clojure.tools.reader.edn :refer [read-string]]
+            [clojure.test :refer [deftest is testing]]
+            [clojure.tools.reader :as rdr]
             [rewrite-clj.node :as node]
             [rewrite-clj.parser :as p])
   #?(:clj (:import [clojure.lang ExceptionInfo]
@@ -130,29 +129,40 @@
        :default (is (Double/isNaN e)))))
 
 (deftest t-parsing-reader-prefixed-data
-  (doseq [[s t ws sexpr]
-          [["@sym"        :deref            []              '(deref sym)]
-           ["@  sym"      :deref            [:whitespace]   '(deref sym)]
-           ["'sym"        :quote            []              '(quote sym)]
-           ["'  sym"      :quote            [:whitespace]   '(quote sym)]
-           ["`sym"        :syntax-quote     []              '(quote sym)]
-           ["`  sym"      :syntax-quote     [:whitespace]   '(quote sym)]
-           ["~sym"        :unquote          []              '(unquote sym)]
-           ["~  sym"      :unquote          [:whitespace]   '(unquote sym)]
-           ["~@sym"       :unquote-splicing []              '(unquote-splicing sym)]
-           ["~@  sym"     :unquote-splicing [:whitespace]   '(unquote-splicing sym)]
-           ["#=sym"       :eval             []              '(eval 'sym)]
-           ["#=  sym"     :eval             [:whitespace]   '(eval 'sym)]
-           ["#'sym"       :var              []              '(var sym)]
-           ["#'\nsym"     :var              [:newline]      '(var sym)]]]
-    (let [n (p/parse-string s)
-          children (node/children n)
-          c (map node/tag children)]
-      (is (= t (node/tag n)))
-      (is (= :token (last c)))
-      (is (= sexpr (node/sexpr n)))
-      (is (= 'sym (node/sexpr (last children))))
-      (is (= ws (vec (butlast c)))))))
+  (doseq [[ s         t                 ws            sexpr        ltag   lcld]
+          [["@sym"    :deref            []            '@sym        :token 'sym]
+           ["@  sym"  :deref            [:whitespace] '@sym        :token 'sym]
+           ["'sym"    :quote            []            ''sym        :token 'sym]
+           ["'  sym"  :quote            [:whitespace] ''sym        :token 'sym]
+           ["`sym"    :syntax-quote     []            ''sym        :token 'sym]
+           ["`  sym"  :syntax-quote     [:whitespace] ''sym        :token 'sym]
+           ["~sym"    :unquote          []            '~sym        :token 'sym]
+           ["~  sym"  :unquote          [:whitespace] '~sym        :token 'sym]
+           ["~@sym"   :unquote-splicing []            '~@sym       :token 'sym]
+           ["~@  sym" :unquote-splicing [:whitespace] '~@sym       :token 'sym]
+           ["~ @sym"  :unquote          [:whitespace] '~ @sym      :deref '@sym]
+           ["#=sym"   :eval             []            '(eval 'sym) :token 'sym]
+           ["#=  sym" :eval             [:whitespace] '(eval 'sym) :token 'sym]
+           ["#'sym"   :var              []            '#'sym       :token 'sym]
+           ["#'\nsym" :var              [:newline]    '#'sym       :token 'sym]]]
+    (testing (pr-str s)
+      (let [n (p/parse-string s)
+            children (node/children n)
+            c (map node/tag children)]
+        (is (= t (node/tag n)) "tag")
+        (is (= ltag (last c)) "ltag")
+        (is (= sexpr (node/sexpr n)) "sexpr")
+        (is (= s (node/string n)) "string")
+        ;; ` and #= return different sexpr's than via clojure.core/read-string
+        (when-not (#{:syntax-quote :eval} t)
+          (is (= sexpr
+                 #?(:cljs (rdr/read-string s)
+                    :default (binding [rdr/*read-eval* false] (rdr/read-string s)))
+                 #?@(:cljs []
+                     :default [(binding [*read-eval* false] (read-string s))]))
+              "read-string"))
+        (is (= lcld (node/sexpr (last children))) "lcld")
+        (is (= ws (vec (butlast c))) "ws")))))
 
 (deftest t-eval
   (let [n (p/parse-string "#=(+ 1 2)")]
@@ -223,7 +233,8 @@
              fq (frequencies (map node/tag children))]
          (is (= t (node/tag n)))
          (is (= (string/trim s) (node/string n)))
-         (is (= (node/sexpr n) (read-string s)))
+         (is (= (node/sexpr n) #?(:cljs (rdr/read-string s)
+                                  :default (binding [rdr/*read-eval* false] (rdr/read-string s)))))
          (is (= w (:whitespace fq 0)))
          (is (= c (:token fq 0))))))