Fix read with nil/false eof-value

When calling (read stream false nil), the eof-value nil was treated as
falsy by if-let, causing an EOF exception instead of returning nil.
Same issue for false as eof-value.

Fix: use contains? to check for :eof key in opts, and use the
eof-error? parameter to decide whether to include :eof in opts.

Author: borkdude

CHANGELOG.md

@@ -12,6 +12,7 @@ SCI is used in [babashka](https://github.com/babashka/babashka),
 
 ## Unreleased
 
+- Fix `read` with `nil` or `false` as eof-value throwing instead of returning the eof-value
 - `deftype` now macroexpands to `deftype*`, matching JVM Clojure, enabling code walkers like riddley
 - `case` now macroexpands to JVM-compatible `case*` format, enabling tools like riddley and cloverage to work with SCI
 - Fix `.method` on class objects (e.g. `(.getDeclaredField String "value")`) routing to static instead of instance method path

src/sci/impl/read.cljc

@@ -9,12 +9,13 @@
 
 (defn- eof-or-throw [opts v]
   (if (utils/kw-identical? parser/eof v)
-    (if-let [eof (:eof opts)]
-      (if (not (utils/kw-identical? :eofthrow eof))
-        eof
-        (throw (ex-info "EOF while reading"
-                        {:type :sci.error/parse
-                         :opts opts})))
+    (if (contains? opts :eof)
+      (let [eof (:eof opts)]
+        (if (not (utils/kw-identical? :eofthrow eof))
+          eof
+          (throw (ex-info "EOF while reading"
+                          {:type :sci.error/parse
+                           :opts opts}))))
       (throw (ex-info "EOF while reading"
                       {:type :sci.error/parse
                        :opts opts})))
@@ -44,12 +45,14 @@
    (read stream true nil))
   ([stream eof-error? eof-value]
    (read stream eof-error? eof-value false))
-  ([stream _eof-error? eof-value _recursive?]
-   (let [v (parser/parse-next (store/get-ctx) stream
-                              (-> {:eof eof-value}
+  ([stream eof-error? eof-value _recursive?]
+   (let [opts (cond-> {}
+                (not eof-error?) (assoc :eof eof-value))
+         v (parser/parse-next (store/get-ctx) stream
+                              (-> opts
                                   (with-resolver)
                                   (with-suppressed)))]
-     (eof-or-throw {:eof eof-value} v)))
+     (eof-or-throw opts v)))
   ([opts stream]
    (let [opts (-> opts with-resolver with-suppressed)
          opts (if (:read-cond opts)

test/sci/read_test.cljc

@@ -102,6 +102,34 @@ form
      (testing "symbol normally gets line number attached, but we can't do this with pushbackreader"
        (is (= 'a (sci/eval-string (pr-str '(read (java.io.PushbackReader. (java.io.StringReader. "a")))) {:classes {'java.io.PushbackReader java.io.PushbackReader}}))))))
 
+#?(:clj
+   (deftest read-eof-value-test
+     (let [opts {:classes {'java.io.PushbackReader java.io.PushbackReader
+                           'java.io.StringReader java.io.StringReader}}]
+       (testing "read with nil eof-value returns nil at EOF"
+         (is (= [42 nil]
+                (sci/eval-string
+                 "(with-open [r (java.io.PushbackReader. (java.io.StringReader. \"42\"))]
+                    [(read r false nil) (read r false nil)])"
+                 opts))))
+       (testing "read with false eof-value returns false at EOF"
+         (is (= [42 false]
+                (sci/eval-string
+                 "(with-open [r (java.io.PushbackReader. (java.io.StringReader. \"42\"))]
+                    [(read r false false) (read r false false)])"
+                 opts))))
+       (testing "read with custom eof-value returns it at EOF"
+         (is (= [42 :my-eof]
+                (sci/eval-string
+                 "(with-open [r (java.io.PushbackReader. (java.io.StringReader. \"42\"))]
+                    [(read r false :my-eof) (read r false :my-eof)])"
+                 opts))))
+       (testing "read with eof-error? true throws at EOF"
+         (is (thrown-with-msg? Exception #"EOF while reading"
+               (sci/eval-string
+                "(read (java.io.PushbackReader. (java.io.StringReader. \"\")))"
+                opts)))))))
+
 #?(:clj (deftest suppress-read-test
           (sci/eval-string
            "(tagged-literal? (binding [*suppress-read* true] (read-string \"#foo 1\")))")))