orchard.namespace/read-namespace fails if the ns form includes reader conditionals

[alasdairm]

Expected behavior

orchard.namespace/read-namespace should pull out the name from the first (ns ..,) form in any cljc file.

Actual behavior

orchard.namespace/read-namespace returns nil if the ns form includes a reader conditional.
(the EDN read throws an exception that is caught by the catch block).

Steps to reproduce the problem

Can be reproduced in two ways - from CIDER or by adding a test to Orchard that (IMO) should pass (but fails).

From CIDER

create a project with a mix of files with and without reader conditionals in their ns forms
E.g.

(ns my-namespace
  (:require #?(:clj [clojure.test :refer :all]
                     :cljs [cljs.test :as t :include-macros true])))

Select load all project files
Browse all namespaces
observe that the files with reader conditionals have not been loaded.

By adding a test to Orchard

Add a test that passes a file with reader conditionals to read-namespace

--- a/test/orchard/namespace_test.clj
+++ b/test/orchard/namespace_test.clj
@@ -65,8 +65,13 @@
       (testing "of top-level forms only"
         (spit url "(comment (ns ns1)) (ns ns2) (ns ns3)")
         (is (= (n/read-namespace uri) 'ns2)))
+      (testing "of ns forms with reader conditionals"
+        (spit url "(ns ns1 (:require #?(:clj [clojure.test] :cljs [cljs.test])))")
+        (is (= 'ns1
+               (n/read-namespace uri))))
       (io/delete-file url))))

+
 (deftest namespace-resolution
   (testing "Resolving"
     (let [nses '[clojure.java.io

Run the test
Error reported - read-namespace returns nil rather than ns1

Environment & Version information

CIDER 1.2.0 (Nice), nREPL 0.9.0
refactor-nrepl/refactor-nrepl "3.1.0"
cider/cider-nrepl "0.27.4"
cider/orchard "0.8.0"

Clojure version

Clojure 1.10.1

Java version

Java 14.0.2

Operating system

Windows 10

Suggested fix [edited]

The problem is that clojure.edn/read throws an exception if it encounters a reader conditional. The exception is caught and nil returned.

Instead of clojure.edn/read use clojure.core/read with read-eval bound to false and passing in an option of :read-cond :allow.

Another solution would be to first try the clojure.edn/read and, if that fails, fall back to a regex match if the clojure.edn/read fails. There is a trade-off here - a regex will find non-top-level forms.

Sample code can be supplied in a pull request if this approach is acceptable.

[vemv]

Thanks much for the report!

Not sure if agree with the suggested fix. Isn't it possible to read .cljc files using the same mechanism that Clojure does?

• regexes seem a workaround
• reminder, we can't use tools.reader (Orchard aims to be dependency-free)

[alasdairm]

@vemv My apologies, I had looked at clojure.core/read, miss-read the docs and discounted - because I wasn't using correctly.
This uses the Clojure reader and passes the local test cases, including the one I suggest above.

(defn read-namespace
  "Returns the namespace name from the first top-level `ns` form in the file."
  [url]
  (try
    (binding [*read-eval* false]
      (let [opts {:read-cond :allow}]
        (with-open [r (PushbackReader. (io/reader url))]
          (->> (repeatedly #(clojure.core/read opts r))
               (filter #(and (list? %) (= (first %) 'ns))) ; ns form
               (map second)                                ; ns name
               (first)))))
    (catch Exception _))))

The changes:

• use clojure.core/read rather than clojure.edn/read
• bind *read-eval* to false
• include options map with :read-cond :allow

[vemv]

Sounds good like that! Makes sense.

Let us know if you're up for a PR, otherwise we can take it from here 👍

[alasdairm]

Yup, will do a PR

[vemv]

@alasdairm I went ahead with #145, still thanks much for the snippet! 🍻

[alasdairm]

@vemv You're welcome.
I've been very busy last few days so fair enough. Glad I could be of help.
And thank you for your work.