Skip to content

Commit

Permalink
[#518] Fix error reporting in case of arity error
Browse files Browse the repository at this point in the history
  • Loading branch information
borkdude committed Jan 27, 2021
1 parent 71e61e1 commit 002cc94
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 22 deletions.
21 changes: 12 additions & 9 deletions src/sci/impl/fns.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
(:require [sci.impl.faster :refer [nth-2 assoc-3 get-2]]
[sci.impl.macros :as macros :refer [?]]
[sci.impl.types :as t]
[sci.impl.utils :as utils])
[sci.impl.utils :as utils]
[sci.impl.vars :as vars])
#?(:cljs (:require-macros [sci.impl.fns :refer [gen-fn
gen-fn-varargs]])))

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

(defn throw-arity [ctx fn-name macro? args]
(defn throw-arity [ctx nsm fn-name macro? args]
(when-not (:disable-arity-checks ctx)
(throw (new #?(:clj Exception
:cljs js/Error)
(let [actual-count (if macro? (- (count args) 2)
(count args))]
(str "Wrong number of args (" actual-count ") passed to: " fn-name))))))
(str "Wrong number of args (" actual-count ") passed to: " (str nsm "/" fn-name)))))))

(deftype Recur #?(:clj [val]
:cljs [val])
Expand Down Expand Up @@ -62,7 +63,7 @@
~@(? :cljs
(when-not disable-arity-checks
`[(when-not (= ~n (.-length (~'js-arguments)))
(throw-arity ~'ctx ~'fn-name ~'macro? (vals (~'js->clj (~'js-arguments)))))]))
(throw-arity ~'ctx ~'nsm ~'fn-name ~'macro? (vals (~'js->clj (~'js-arguments)))))]))
(let [;; tried making bindings a transient, but saw no perf improvement (see #246)
~'bindings (get-2 ~'ctx :bindings)
~@assocs
Expand Down Expand Up @@ -93,12 +94,12 @@
(assoc ret (second params) args*)
(do
(when-not args*
(throw-arity ctx fn-name macro? args))
(throw-arity ctx nsm fn-name macro? args))
(recur (next args*) (next params)
(assoc-3 ret fp (first args*))))))
(do
(when args*
(throw-arity ctx fn-name macro? args))
(throw-arity ctx nsm fn-name macro? args))
ret)))
ctx (assoc-3 ctx :bindings bindings)
ret (interpret ctx body)
Expand All @@ -121,11 +122,13 @@
#_:clj-kondo/ignore fn-name
#_:clj-kondo/ignore macro?
with-meta?]
(let [disable-arity-checks? (get-2 ctx :disable-arity-checks)
(let [nsm (vars/current-ns-name)
disable-arity-checks? (get-2 ctx :disable-arity-checks)
min-var-args-arity (when var-arg-name fixed-arity)
;; body-count (count body)
f (if-not (or var-arg-name
#?(:clj disable-arity-checks?))
f (if-not #?(:clj (or var-arg-name
disable-arity-checks?)
:cljs var-arg-name)
(case (int fixed-arity)
0 (fn arity-0 []
(let [ret (interpret ctx body)
Expand Down
17 changes: 8 additions & 9 deletions src/sci/impl/utils.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@
#?(:clj (or (-ex-cause e) e)
:cljs e)
e)
ex-msg #?(:clj (.getMessage e)
:cljs (.-message e))
ex-msg (or (:message d)
#?(:clj (.getMessage e)
:cljs (.-message e)))
{:keys [:line :column :file]
:or {line (:line ctx)
column (:column ctx)}}
Expand All @@ -111,16 +112,14 @@
(meta node))
ex-msg (if (and ex-msg (:name fm))
(str/replace ex-msg #"(sci\.impl\.)?fns/fun/[a-zA-Z0-9-]+--\d+"
(str (:name fm)))
(str (:ns fm) "/" (:name fm)))
ex-msg)]
(if (and line column)
(let [m ex-msg
new-exception
(let [
base {:type :sci/error
(let [new-exception
(let [base {:type :sci/error
:line line
:column column
:message m
:message ex-msg
:sci.impl/callstack
(delay (when-let
[v (get-in @(:env ctx) [:sci.impl/callstack (:id ctx)])]
Expand All @@ -132,7 +131,7 @@
base (if phase
(assoc base :phase phase)
base)]
(ex-info m (merge base d) e))]
(ex-info ex-msg (merge base d) e))]
(throw new-exception))
(throw e))))))

Expand Down
8 changes: 4 additions & 4 deletions test/sci/core_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -368,26 +368,26 @@
{:line 1 :column 19}
(eval* "(+ 1 2 3 4 5) (do x)")))
(tu/assert-submap {:type :sci/error, :line 1, :column 15,
:message #"Wrong number of args \(1\) passed to: foo"}
:message #"Wrong number of args \(1\) passed to: user/foo"}
(try (eval* "(defn foo []) (foo 1)")
(catch #?(:clj clojure.lang.ExceptionInfo :cljs ExceptionInfo) ex
(let [d (ex-data ex)]
d))))
(tu/assert-submap {:type :sci/error, :line 1, :column 21,
:message #"Wrong number of args \(0\) passed to: foo"}
:message #"Wrong number of args \(0\) passed to: user/foo"}
(try (eval* "(defn foo [x & xs]) (foo)")
(catch #?(:clj clojure.lang.ExceptionInfo :cljs ExceptionInfo) ex
(let [d (ex-data ex)]
d))))
(tu/assert-submap {:type :sci/error, :line 1, :column 93,
:message #"Wrong number of args \(2\) passed to: bindings"}
:message #"Wrong number of args \(2\) passed to: user/bindings"}
(try (eval* (str "(defmacro bindings [a] (zipmap (mapv #(list 'quote %) (keys &env)) (keys &env))) "
"(let [x 1] (bindings))"))
(catch #?(:clj clojure.lang.ExceptionInfo :cljs ExceptionInfo) ex
(let [d (ex-data ex)]
d))))
(tu/assert-submap {:type :sci/error, :line 1, :column 25,
:message #"Wrong number of args \(0\) passed to: foo"}
:message #"Wrong number of args \(0\) passed to: user/foo"}
(try (eval* (str "(defmacro foo [x & xs]) "
"(foo)"))
(catch #?(:clj clojure.lang.ExceptionInfo :cljs ExceptionInfo) ex
Expand Down
18 changes: 18 additions & 0 deletions test/sci/error_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,21 @@
(:locals (ex-data e))))
ks (keys locals)]
(is (= '[x] ks)))))

(deftest arity-error-test
(testing "The name of the correct function is reported"
(is (thrown-with-msg?
#?(:clj clojure.lang.ExceptionInfo :cljs cljs.core/ExceptionInfo)
#"Wrong number of args \(0\) passed to: foo/echo-msg"
(eval-string "
(ns foo)
(defn echo-msg [msg]
msg)
(ns bar (:require foo))
(defn main []
(foo/echo-msg)) ;; called with the wrong arity
(main)")))))

0 comments on commit 002cc94

Please sign in to comment.