Skip to content

Commit 75d57ca

Browse files
committed
Work-around tools reader newline behaviour
I've reported what, seems to me, curious behaviour in clojure tools reader. See https://clojure.atlassian.net/browse/TRDR-65 Instead of waiting for a resolution from the tools reader project, I have taken a crack at a work-around. In short: 1. For Clojure: insert a custom newline normalizing reader into the reader chain. 2. For ClojueScript: compensate for the fact tools reader peek-char returns \r instead of \n for \r\n. As part of this work, now only calling clojure.tools.reader APIs from rewrite-clj.reader namespace. Localizing access to clojure.tools.reader through a single point ensures that any fixes/customizations we have atop the clojure.tools.reader get used. Includes: - moving customized version of read-keyword fn from rewrite-clj.keyword.parser to rewrite-clj.reader namespace. - rewrite-clj.parser.util fns were mostly duplicated in rewrite-clj.reader. Turf rewrite-clj.parser and have parser namespaces not be shy to access rewrite-clj.reader namespace instead. Logged as potentially breaking in change log. Closes #93
1 parent 780d69f commit 75d57ca

File tree

7 files changed

+76
-120
lines changed

7 files changed

+76
-120
lines changed

CHANGELOG.adoc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ Rewrite-cljs users migrating to rewrite-clj v1 are now at, and will remain at, f
5858
* Many updates to docs and docstrings
5959

6060
==== Fixes
61+
* end of line variants in source now normalized consistently to `\newline` https://github.com/clj-commons/rewrite-clj/issues/93[#93]
6162
* postwalk on larger source file no longer throws StackOverflow https://github.com/clj-commons/rewrite-clj/issues/69[#69]
6263
* postwalk now walks in post order https://github.com/clj-commons/rewrite-clj/issues/123[#123]
6364
* we now preserve newline at end of file https://github.com/clj-commons/rewrite-clj/issues/121[#121]
@@ -129,6 +130,8 @@ Rewrite-cljs users migrating to rewrite-clj v1 are now at, and will remain at, f
129130
** Switched to `clojure.tools.reader.edn`
130131
** Some rewrite-cljs optimizations were dropped in favor of a single code base
131132
** Deleted unused `rewrite-clj.node.indent` https://github.com/clj-commons/rewrite-clj/issues/116[#116]
133+
** Deleted redundent `rewrite-clj.parser.util` as part of https://github.com/clj-commons/rewrite-clj/issues/93[#93].
134+
If you were using this internal namespace you can opt to switch to, the also internal, `rewrite-clj.reader` namespace.
132135

133136
== rewrite-clj v0
134137

src/rewrite_clj/parser/keyword.cljc

Lines changed: 8 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,18 @@
11
(ns ^:no-doc rewrite-clj.parser.keyword
2-
(:require [clojure.tools.reader.edn :as edn]
3-
;; internal tools reader namespaces to support read-keyword override work
4-
[clojure.tools.reader.impl.commons :as reader-impl-commons]
5-
[clojure.tools.reader.impl.errors :as reader-impl-errors]
6-
[clojure.tools.reader.impl.utils :as reader-impl-utils]
7-
[clojure.tools.reader.reader-types :as r]
8-
[rewrite-clj.node.keyword :as nkeyword]
9-
[rewrite-clj.parser.utils :as u] ))
2+
(:require [rewrite-clj.node.keyword :as nkeyword]
3+
[rewrite-clj.reader :as reader]))
104

115
#?(:clj (set! *warn-on-reflection* true))
126

13-
(defn- read-keyword
14-
"This customized version of clojure.tools.reader.edn's read-keyword allows for
15-
an embedded `::` in a keyword to to support [garden-style keywords](https://github.com/noprompt/garden)
16-
like `:&::before`. This function was transcribed from clj-kondo."
17-
[reader]
18-
(let [ch (r/read-char reader)]
19-
(if-not (reader-impl-utils/whitespace? ch)
20-
(let [#?(:clj ^String token :default token) (#'edn/read-token reader :keyword ch)
21-
s (reader-impl-commons/parse-symbol token)]
22-
(if (and s
23-
;; (== -1 (.indexOf token "::")) becomes:
24-
(not (zero? (.indexOf token "::"))))
25-
(let [#?(:clj ^String ns :default ns) (s 0)
26-
#?(:clj ^String name :default name) (s 1)]
27-
(if (identical? \: (nth token 0))
28-
(reader-impl-errors/throw-invalid reader :keyword token) ; No ::kw in edn.
29-
(keyword ns name)))
30-
(reader-impl-errors/throw-invalid reader :keyword token)))
31-
(reader-impl-errors/throw-single-colon reader))))
32-
337
(defn parse-keyword
348
[#?(:cljs ^not-native reader :default reader)]
35-
(u/ignore reader)
36-
(if-let [c (r/peek-char reader)]
9+
(reader/ignore reader)
10+
(if-let [c (reader/peek reader)]
3711
(if (= c \:)
3812
(do
39-
(r/read-char reader)
13+
(reader/next reader)
4014
(nkeyword/keyword-node
41-
(read-keyword reader)
15+
(reader/read-keyword reader)
4216
true))
43-
(nkeyword/keyword-node (read-keyword reader)))
44-
(u/throw-reader reader "unexpected EOF while reading keyword.")))
17+
(nkeyword/keyword-node (reader/read-keyword reader)))
18+
(reader/throw-reader reader "unexpected EOF while reading keyword.")))

src/rewrite_clj/parser/namespaced_map.cljc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
(ns ^:no-doc rewrite-clj.parser.namespaced-map
22
(:require [rewrite-clj.node.namespaced-map :as nsmap]
33
[rewrite-clj.node.protocols :as node]
4-
[rewrite-clj.parser.utils :as u]
54
[rewrite-clj.reader :as reader] ))
65

76
#?(:clj (set! *warn-on-reflection* true))
@@ -21,18 +20,17 @@
2120
(recur (conj nodes n))
2221
[nodes n]))))
2322

24-
2523
(defn parse-namespaced-map
2624
"The caller has parsed up to `#:` and delegates the details to us."
2725
[reader read-next]
2826
(reader/ignore reader)
2927
(let [qualifier-node (parse-qualifier reader)]
3028
(when (and (not (:auto-resolved? qualifier-node))
3129
(nil? (:prefix qualifier-node)))
32-
(u/throw-reader reader "namespaced map expects a namespace"))
30+
(reader/throw-reader reader "namespaced map expects a namespace"))
3331
(let [[whitespace-nodes map-node] (parse-to-next-elem reader read-next)]
3432
(when (or (not map-node)
3533
(not= :map (node/tag map-node)))
36-
(u/throw-reader reader "namespaced map expects a map"))
34+
(reader/throw-reader reader "namespaced map expects a map"))
3735
(nsmap/namespaced-map-node
3836
(concat [qualifier-node] whitespace-nodes [map-node])))))

src/rewrite_clj/parser/string.cljc

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
(ns ^:no-doc rewrite-clj.parser.string
22
(:require [clojure.string :as string]
3-
[clojure.tools.reader.reader-types :as r]
43
[rewrite-clj.node.stringz :as nstring]
5-
[rewrite-clj.parser.utils :as u])
4+
[rewrite-clj.reader :as reader])
65
#?(:cljs (:import [goog.string StringBuffer])))
76

87
#?(:clj (set! *warn-on-reflection* true))
@@ -16,11 +15,11 @@
1615

1716
(defn- read-string-data
1817
[#?(:cljs ^not-native reader :default reader)]
19-
(u/ignore reader)
18+
(reader/ignore reader)
2019
(let [buf (StringBuffer.)]
2120
(loop [escape? false
2221
lines []]
23-
(if-let [c (r/read-char reader)]
22+
(if-let [c (reader/next reader)]
2423
(cond (and (not escape?) (identical? c \"))
2524
(flush-into lines buf)
2625

@@ -31,7 +30,7 @@
3130
(do
3231
(.append buf c)
3332
(recur (and (not escape?) (identical? c \\)) lines)))
34-
(u/throw-reader reader "Unexpected EOF while reading string.")))))
33+
(reader/throw-reader reader "Unexpected EOF while reading string.")))))
3534

3635
(defn parse-string
3736
[#?(:cljs ^not-native reader :default reader)]

src/rewrite_clj/parser/utils.cljc

Lines changed: 0 additions & 42 deletions
This file was deleted.

src/rewrite_clj/reader.cljc

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
(:refer-clojure :exclude [peek next])
33
(:require #?@(:clj [[clojure.java.io :as io]])
44
[clojure.tools.reader.edn :as edn]
5+
[clojure.tools.reader.impl.commons :as reader-impl-commons]
6+
[clojure.tools.reader.impl.errors :as reader-impl-errors]
7+
[clojure.tools.reader.impl.utils :as reader-impl-utils]
58
[clojure.tools.reader.reader-types :as r]
69
[rewrite-clj.interop :as interop])
710
#?(:cljs (:import [goog.string StringBuffer])
@@ -124,7 +127,12 @@
124127
(defn peek
125128
"Peek next char."
126129
[#?(:cljs ^not-native reader :default reader)]
127-
(r/peek-char reader))
130+
(let [ch (r/peek-char reader)]
131+
;; compensate for cljs newline normalization in tools reader v1.3.5
132+
;; see https://clojure.atlassian.net/browse/TRDR-65
133+
(if (identical? \return ch)
134+
\newline
135+
ch)))
128136

129137
(defn position
130138
"Create map of `row-k` and `col-k` representing the current reader position."
@@ -169,13 +177,36 @@
169177
(if (= n 1) "" "s")))
170178
vs)))
171179

180+
;;
181+
;; ## Customizations
182+
;;
183+
(defn read-keyword
184+
"This customized version of clojure.tools.reader.edn's read-keyword allows for
185+
an embedded `::` in a keyword to to support [garden-style keywords](https://github.com/noprompt/garden)
186+
like `:&::before`. This function was transcribed from clj-kondo."
187+
[reader]
188+
(let [ch (r/read-char reader)]
189+
(if-not (reader-impl-utils/whitespace? ch)
190+
(let [#?(:clj ^String token :default token) (#'edn/read-token reader :keyword ch)
191+
s (reader-impl-commons/parse-symbol token)]
192+
(if (and s
193+
;; (== -1 (.indexOf token "::")) becomes:
194+
(not (zero? (.indexOf token "::"))))
195+
(let [#?(:clj ^String ns :default ns) (s 0)
196+
#?(:clj ^String name :default name) (s 1)]
197+
(if (identical? \: (nth token 0))
198+
(reader-impl-errors/throw-invalid reader :keyword token) ; No ::kw in edn.
199+
(keyword ns name)))
200+
(reader-impl-errors/throw-invalid reader :keyword token)))
201+
(reader-impl-errors/throw-single-colon reader))))
202+
172203
;; ## Reader Types
173204

174205
;;
175206
;; clojure.tools.reader (at the time of this writing v1.3.5) does not seem to normalize Windows \r\n newlines
176207
;; properly to \n for Clojure
177208
;;
178-
;; ClojureScript seems to work fine - but note that for peek it can return \r instead of \n.
209+
;; ClojureScript seems to work fine - but note that for peek it will return \r for \r\n and \r\f instead of \n.
179210
;;
180211
;; see https://clojure.atlassian.net/browse/TRDR-65
181212
;;
@@ -185,39 +216,34 @@
185216
#?(:clj
186217
(deftype NewlineNormalizingReader
187218
[rdr
188-
^:unsynchronized-mutable next-char
189-
^:unsynchronized-mutable peeked-char]
219+
^:unsynchronized-mutable read-ahead-char
220+
^:unsynchronized-mutable user-peeked-char]
190221
r/Reader
191222
(read-char [_reader]
192-
(if peeked-char
193-
(let [ch peeked-char]
194-
(set! peeked-char nil)
195-
ch)
196-
(let [ch (or next-char (r/read-char rdr))]
197-
(when next-char (set! next-char nil))
198-
(cond (identical? \return ch)
199-
(let [next-ch (r/read-char rdr)]
200-
(when (not (identical? \newline next-ch))
201-
(set! next-char next-ch))
202-
\newline)
203-
204-
(identical? \formfeed ch)
205-
\newline
206-
207-
:else
208-
ch))))
223+
(if-let [ch user-peeked-char]
224+
(do (set! user-peeked-char nil) ch)
225+
(let [ch (or read-ahead-char (r/read-char rdr))]
226+
(when read-ahead-char (set! read-ahead-char nil))
227+
(if (not (identical? \return ch))
228+
ch
229+
(let [read-ahead-ch (r/read-char rdr)]
230+
(when (not (or (identical? \newline read-ahead-ch)
231+
(identical? \formfeed read-ahead-ch)))
232+
(set! read-ahead-char read-ahead-ch))
233+
\newline)))))
209234

210235
(peek-char [reader]
211-
(let [ch (or peeked-char (.read-char reader))]
212-
(when-not peeked-char (set! peeked-char ch))
213-
ch))))
236+
(or user-peeked-char
237+
(let [ch (.read-char reader)]
238+
(set! user-peeked-char ch)
239+
ch)))))
214240

215241
#?(:clj
216242
(defn ^Closeable newline-normalizing-reader
217243
"Normalizes the following line endings to LF (line feed - 0x0A):
218244
- LF (remains LF)
219245
- CRLF (carriage return 0x0D line feed 0x0A)
220-
- FF (form feed 0x0C)"
246+
- CRFF (carriage return 0x0D form feed 0x0C)"
221247
[rdr]
222248
(NewlineNormalizingReader. (r/to-rdr rdr) nil nil)))
223249

test/rewrite_clj/parser_test.cljc

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -543,22 +543,20 @@
543543
";; comment\r\n(+ 1 2 3)\r\n"
544544
";; comment\n(+ 1 2 3)\n"
545545

546-
"1\r2\r\n3\f4"
546+
"1\r2\r\n3\r\f4"
547547
"1\n2\n3\n4"
548548

549-
"\n\n\n\n\n"
550-
"\n\n\n\n\n"
549+
"\n\n\n\n"
550+
"\n\n\n\n"
551551

552552
"\r\r\r\r\r"
553553
"\n\n\n\n\n"
554554

555-
"\r\n\r\n\r\n\r\n\r\n"
556-
"\n\n\n\n\n"
557-
558-
"\f\f\f\f\f"
559-
"\n\n\n\n\n"
555+
"\r\n\r\n\r\n\r\n\r\n\r\n"
556+
"\n\n\n\n\n\n"
560557

561-
"\r\n\r\r\f\r\n"
562-
"\n\n\n\n\n"))
558+
;1 2 3 4 5 6 7
559+
"\r\n\r\r\f\r\n\r\r\n\r"
560+
"\n\n\n\n\n\n\n"))
563561

564562

0 commit comments

Comments
 (0)