Skip to content

Commit

Permalink
CLJS-519: namespace aliased keywords throw Clojure reader exception
Browse files Browse the repository at this point in the history
`forms-seq` was not as lazy at it should have been, we might
accidentally look ahead past the ns form and consume a namespace
aliased keyword before we get a chance to apply the side effects of
the `parse 'ns` multimethod in the analyzer.

We were incorrectly setting `*ns*` in `forms-seq` to
`*ana/*reader-ns*` instead of `*ana/cljs-ns*` which is what the analyzer
actually side effects.
  • Loading branch information
swannodette committed Jun 13, 2013
1 parent 8220ebd commit 4a04114
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 4 deletions.
5 changes: 5 additions & 0 deletions src/clj/cljs/analyzer.clj
Expand Up @@ -25,6 +25,7 @@
;; to resolve keywords like ::foo - the namespace
;; must be determined during analysis - the reader
;; did not know
;; TODO: probably remove, see bottom of file - David
(def ^:dynamic *reader-ns-name* (gensym))
(def ^:dynamic *reader-ns* (create-ns *reader-ns-name*))

Expand Down Expand Up @@ -1007,6 +1008,10 @@
(= form ()) (analyze-list env form name)
:else {:op :constant :env env :form form})))))

;; TODO: CLJS-519, refactor, could use forms-seq from compiler
;; eliminate *reader-ns* and *reader-ns-name*, seems redundant now that
;; we know to use *cljs-ns* when reading - David

(defn analyze-file
[^String f]
(let [res (if (re-find #"^file://" f) (java.net.URL. f) (io/resource f))]
Expand Down
7 changes: 4 additions & 3 deletions src/clj/cljs/compiler.clj
Expand Up @@ -735,9 +735,10 @@
([f]
(forms-seq f (clojure.lang.LineNumberingPushbackReader. (io/reader f))))
([f ^java.io.PushbackReader rdr]
(if-let [form (binding [*ns* ana/*reader-ns*] (read rdr nil nil))]
(lazy-seq (cons form (forms-seq f rdr)))
(.close rdr))))
(lazy-seq
(if-let [form (binding [*ns* (create-ns ana/*cljs-ns*)] (read rdr nil nil))]
(cons form (forms-seq f rdr))
(.close rdr)))))

(defn rename-to-js
"Change the file extension from .cljs to .js. Takes a File or a
Expand Down
4 changes: 4 additions & 0 deletions test/cljs/cljs/keyword_other.cljs
@@ -0,0 +1,4 @@
(ns cljs.keyword-other)

(defn foo [a b]
(+ a b))
5 changes: 5 additions & 0 deletions test/cljs/cljs/keyword_test.cljs
@@ -0,0 +1,5 @@
(ns cljs.keyword-test
(:require [cljs.keyword-other :as other]))

(defn test-keyword []
(assert (= ::other/foo :cljs.keyword-other/foo)))
4 changes: 3 additions & 1 deletion test/cljs/test_runner.cljs
Expand Up @@ -9,7 +9,8 @@
[cljs.letfn-test :as letfn-test]
[foo.ns-shadow-test :as ns-shadow-test]
[cljs.top-level :as top-level]
[cljs.reducers-test :as reducers-test]))
[cljs.reducers-test :as reducers-test]
[cljs.keyword-test :as keyword-test]))

(set-print-fn! js/print)

Expand All @@ -25,6 +26,7 @@
(ns-shadow-test/test-shadow)
(top-level/test)
(reducers-test/test-all)
(keyword-test/test-keyword)

(println "Tests completed without exception")

Expand Down

0 comments on commit 4a04114

Please sign in to comment.