Skip to content

Commit de478bd

Browse files
borkdudeborkdude
authored
Fix #931: support multiple catch in combination with ^:sci/error (#1019)
1 parent acc970c commit de478bd

File tree

4 files changed

+105
-68
lines changed

4 files changed

+105
-68
lines changed

‎src/sci/impl/analyzer.cljc‎

Lines changed: 35 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
{:no-doc true
33
:clj-kondo/config '{:linters {:unresolved-symbol {:exclude [ctx this bindings]}}}}
44
(:require
5+
#?(:clj [sci.impl.reflector :as reflector])
56
#?(:clj [sci.impl.types :as t :refer [#?(:cljs ->Node) ->constant]])
67
#?(:cljs [cljs.tagged-literals :refer [JSValue]])
78
#?(:cljs [goog.object :as gobj])
@@ -20,8 +21,7 @@
2021
[ana-macros constant? macro? rethrow-with-location-of-node
2122
set-namespace! recur special-syms]]
2223
[sci.impl.vars :as vars]
23-
[sci.lang]
24-
#?(:clj [sci.impl.reflector :as reflector]))
24+
[sci.lang])
2525
#?(:cljs
2626
(:require-macros
2727
[sci.impl.analyzer :refer [gen-return-recur
@@ -895,19 +895,16 @@
895895
(assoc binding ex-iden))))
896896
(assoc-in [:iden->invoke-idx ex-iden] ex-idx))
897897
analyzed-body (analyze ctx
898-
(cons 'do body))]
898+
(cons 'do body))
899+
sci-error (some-> ex meta :sci/error)]
899900
{:class clazz
900901
:ex-idx ex-idx
901902
:body analyzed-body
902-
:ex ex})
903+
:ex ex
904+
:sci-error sci-error})
903905
(throw-error-with-location (str "Unable to resolve classname: " ex) ex))))
904906
catches)
905-
sci-error (let [fst (when (= 1 (count catches))
906-
(nth catches 0))
907-
ex (:ex fst)]
908-
(and (= #?(:clj 'Exception
909-
:cljs 'js/Error) ex)
910-
(some-> ex meta :sci/error)))
907+
sci-error (some :sci-error catches)
911908
finally (when finally
912909
(analyze ctx (cons 'do (rest finally))))]
913910
(sci.impl.types/->Node
@@ -1444,36 +1441,36 @@
14441441
:file @utils/current-file)]
14451442
(cond (str/starts-with? meth ".")
14461443
(let [meth (subs meth 1)
1447-
arg-types (when-let [param-tags (some-> (meta expr) :param-tags)]
1448-
(let [param-count (count param-tags)
1449-
^"[Ljava.lang.Class;" arg-types (when (pos? param-count)
1450-
(make-array Class param-count))]
1451-
(areduce arg-types idx _ret nil
1452-
(when-let [t (nth param-tags idx)]
1453-
(when-not (= '_ t)
1454-
(when-let [t (interop/resolve-type-hint ctx t)]
1455-
(aset arg-types idx t)))))
1456-
arg-types))
1457-
f (fn [obj & args]
1458-
(let [args (object-array args)
1459-
arg-count (alength args)
1460-
^java.util.List methods (interop/meth-cache ctx clazz meth arg-count #(reflector/get-methods clazz arg-count meth false) :instance-methods)]
1461-
(reflector/invoke-matching-method meth methods clazz obj args arg-types)))]
1462-
(sci.impl.types/->Node
1463-
f
1464-
stack))
1444+
arg-types (when-let [param-tags (some-> (meta expr) :param-tags)]
1445+
(let [param-count (count param-tags)
1446+
^"[Ljava.lang.Class;" arg-types (when (pos? param-count)
1447+
(make-array Class param-count))]
1448+
(areduce arg-types idx _ret nil
1449+
(when-let [t (nth param-tags idx)]
1450+
(when-not (= '_ t)
1451+
(when-let [t (interop/resolve-type-hint ctx t)]
1452+
(aset arg-types idx t)))))
1453+
arg-types))
1454+
f (fn [obj & args]
1455+
(let [args (object-array args)
1456+
arg-count (alength args)
1457+
^java.util.List methods (interop/meth-cache ctx clazz meth arg-count #(reflector/get-methods clazz arg-count meth false) :instance-methods)]
1458+
(reflector/invoke-matching-method meth methods clazz obj args arg-types)))]
1459+
(sci.impl.types/->Node
1460+
f
1461+
stack))
14651462
(try (reflector/get-static-field ^Class clazz ^String meth)
14661463
(catch IllegalArgumentException _
14671464
nil))
14681465
(sci.impl.types/->Node
1469-
(interop/get-static-field clazz meth)
1470-
stack)
1466+
(interop/get-static-field clazz meth)
1467+
stack)
14711468
:else (sci.impl.types/->Node
1472-
(fn [& args]
1473-
(reflector/invoke-static-method
1474-
clazz meth
1475-
^objects (into-array Object args)))
1476-
stack)))))
1469+
(fn [& args]
1470+
(reflector/invoke-static-method
1471+
clazz meth
1472+
^objects (into-array Object args)))
1473+
stack)))))
14771474

14781475
(defn analyze-call [ctx expr m top-level?]
14791476
(with-top-level-loc top-level? m
@@ -1569,12 +1566,11 @@
15691566
:sci.impl/f-meta f-meta)
15701567
^"[Ljava.lang.Class;" arg-types (when (pos? arg-count)
15711568
(make-array Class arg-count))
1572-
has-types? (volatile! nil)
1573-
]
1569+
has-types? (volatile! nil)]
15741570
(when arg-types
15751571
(or (when-let [param-tags (-> f* (some-> meta :param-tags))]
15761572
(vreset! has-types? true)
1577-
(areduce arg-types idx _ret nil
1573+
(areduce arg-types idx _ret nil
15781574
(when-let [t (nth param-tags idx)]
15791575
(when-not (= '_ t)
15801576
(when-let [t (interop/resolve-type-hint ctx t)]
@@ -1690,7 +1686,7 @@
16901686
arg1 (nth children 1)]
16911687
(sci.impl.types/->Node
16921688
(f* (t/eval arg0 ctx bindings)
1693-
(t/eval arg1 ctx bindings))
1689+
(t/eval arg1 ctx bindings))
16941690
nil))
16951691
(throw-error-with-location (str "Wrong number of args (" ccount ") passed to: " f*) expr)))
16961692
:else

‎src/sci/impl/evaluator.cljc‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
(ns sci.impl.evaluator
22
{:no-doc true}
3-
(:refer-clojure :exclude [eval])
43
(:require
54
[clojure.string :as str]
65
[sci.impl.deftype]
@@ -9,8 +8,7 @@
98
[sci.impl.records]
109
[sci.impl.resolve :as resolve]
1110
[sci.impl.types :as types]
12-
[sci.impl.utils :as utils :refer [#?(:cljs kw-identical?)
13-
rethrow-with-location-of-node
11+
[sci.impl.utils :as utils :refer [rethrow-with-location-of-node
1412
throw-error-with-location]]
1513
[sci.impl.vars :as vars]
1614
[sci.lang])
@@ -84,21 +82,25 @@
8482
(types/eval body ctx bindings))
8583
(catch #?(:clj Throwable :cljs :default) e
8684
(if-let
87-
[[_ r]
88-
(reduce (fn [_ c]
89-
(let [clazz (:class c)]
90-
(when #?(:cljs
91-
(or (kw-identical? :default clazz)
92-
(if (instance? sci.impl.types/NodeR clazz)
93-
(instance? (types/eval clazz ctx bindings) e)
94-
(instance? clazz e)))
95-
:clj (instance? clazz e))
96-
(reduced
97-
[::try-result
98-
(do (aset ^objects bindings (:ex-idx c) e)
99-
(types/eval (:body c) ctx bindings))]))))
100-
nil
101-
catches)]
85+
[[_ r]
86+
(reduce (fn [_ c]
87+
(let [clazz (:class c)
88+
e (if (and sci-error
89+
(not (:sci-error c)))
90+
(ex-cause e)
91+
e)]
92+
(when #?(:cljs
93+
(or (utils/kw-identical? :default clazz)
94+
(if (instance? sci.impl.types/NodeR clazz)
95+
(instance? (types/eval clazz ctx bindings) e)
96+
(instance? clazz e)))
97+
:clj (instance? clazz e))
98+
(reduced
99+
[::try-result
100+
(do (aset ^objects bindings (:ex-idx c) e)
101+
(types/eval (:body c) ctx bindings))]))))
102+
nil
103+
catches)]
102104
r
103105
(rethrow-with-location-of-node ctx bindings e body)))
104106
(finally

‎test/sci/core_test.cljc‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
(ns sci.core-test
22
(:require
3+
#?(:clj [sci.ctx-store :as store])
34
[clojure.edn :as edn]
45
[clojure.string :as str]
56
[clojure.test :as test :refer [deftest is testing]]
6-
#?(:clj [sci.ctx-store :as store])
77
[sci.copy-ns-test-ns]
8-
[sci.core :as sci :refer [eval-string]]
8+
[sci.core :as sci]
99
[sci.impl.unrestrict :as unrestrict]
1010
[sci.test-utils :as tu]))
1111

@@ -755,7 +755,7 @@
755755
(def state (atom []))
756756
(doseq [i [1 2 3]] (swap! state conj i))
757757
@state"
758-
{}))))
758+
{}))))
759759

760760
(deftest for-test
761761
(is (= '([1 4] [1 6])
@@ -902,7 +902,7 @@
902902
(is (thrown? js/Error
903903
(sci/eval-string "(try (throw \"blah\") (catch js/Error e (str e e)))")))
904904
(is (= "blahblah" (sci/eval-string "(try (throw \"blah\") (catch :default e (str e e)))")))))
905-
(is (= [1](sci/eval-string "(defn catch [] 1) (try [(catch)])"))))
905+
(is (= [1] (sci/eval-string "(defn catch [] 1) (try [(catch)])"))))
906906

907907
(deftest syntax-quote-test
908908
(is (= '(clojure.core/list 10 10)
@@ -967,7 +967,7 @@
967967
(when-not tu/native?
968968
(is (= ":dude\n:dude\n"
969969
(let [out (sci/with-out-str
970-
(eval-string "(defmacro foo [x] (list 'do x x)) (foo (prn :dude))"))]
970+
(sci/eval-string "(defmacro foo [x] (list 'do x x)) (foo (prn :dude))"))]
971971
out))))))
972972

973973
(deftest declare-test
@@ -1107,8 +1107,7 @@
11071107
(is (= '(clojure.core/defrecord Foo []) (eval* "(macroexpand '(defrecord Foo []))")))
11081108
(is (= '(. nil log) (eval* "(macroexpand-1 '(.log))")))
11091109
(is (= '(. js/console log) (eval* "(macroexpand-1 '(.log js/console))")))
1110-
(is (= '(. js/console log 1 2 3) (eval* "(macroexpand-1 '(.log js/console 1 2 3))")))
1111-
)
1110+
(is (= '(. js/console log 1 2 3) (eval* "(macroexpand-1 '(.log js/console 1 2 3))"))))
11121111

11131112
(deftest macroexpand-call-test
11141113
(is (= [1 1] (eval* "(defmacro foo [x] `(bar ~x)) (defmacro bar [x] [x x]) (macroexpand '(foo 1))")))
@@ -1157,7 +1156,7 @@
11571156
{:load-fn (fn [{:keys [:namespace]}]
11581157
(case namespace
11591158
foo {:file "foo.clj"
1160-
:source "(ns foo) (println \"hello\")"}))})))))))
1159+
:source "(ns foo) (println \"hello\")"}))})))))))
11611160

11621161
(deftest reload-all-test
11631162
(when-not tu/native?
@@ -1454,7 +1453,7 @@
14541453
(is (= 2 (sci/eval-form @C 'n/foo)))))
14551454

14561455
(deftest merge-opts-preserves-features-test
1457-
(let [ctx(sci/init {:features #{:cljs}})]
1456+
(let [ctx (sci/init {:features #{:cljs}})]
14581457
(is (= 2 (sci/eval-string* ctx "#?(:clj 1 :cljs 2)")))
14591458
(is (= 2 (sci/eval-string* (sci/merge-opts ctx {}) "#?(:clj 1 :cljs 2)")))))
14601459

@@ -1654,6 +1653,47 @@
16541653
'format-stacktrace sci/format-stacktrace}}})]
16551654
(is (str/includes? (str st) "1:31"))))
16561655

1656+
(deftest sci-error-multiple-catches-test
1657+
(testing "^:sci/error works with multiple catch clauses - specific catch first"
1658+
(let [result (sci/eval-string
1659+
(-> "(defn foo [] (/ 1 0))
1660+
(defn bar []
1661+
(try (foo)
1662+
(catch ArithmeticException e
1663+
{:caught :arithmetic :msg (ex-message e)})
1664+
(catch ^:sci/error Exception e
1665+
{:caught :exception})))
1666+
(bar)"
1667+
#?(:cljs (str/replace "ArithmeticException" "js/RangeError"))
1668+
#?(:cljs (str/replace "Exception" "js/Error"))
1669+
#?(:cljs (str/replace "(/ 1 0)" "(js/Array. -1)")))
1670+
{:classes #?(:clj {'ArithmeticException ArithmeticException}
1671+
:cljs {:allow :all 'js js/global})})]
1672+
(is (= :arithmetic (:caught result)))
1673+
(is (str/includes? (:msg result) #?(:clj "Divide by zero"
1674+
:cljs "length")))))
1675+
(testing "^:sci/error Exception catch still gets location info with multiple catches"
1676+
(let [result (sci/eval-string
1677+
(-> "(require '[sci.core :as sci])
1678+
(defn foo [] (assoc :foo :bar))
1679+
(defn bar []
1680+
(try (foo)
1681+
(catch ArithmeticException e
1682+
{:caught :not-found})
1683+
(catch ^:sci/error Exception e
1684+
{:caught :exception :st (sci/format-stacktrace (sci/stacktrace e))})))
1685+
(bar)"
1686+
#?(:cljs (str/replace "ArithmeticException" "js/RangeError"))
1687+
#?(:cljs (str/replace "Exception" "js/Error")))
1688+
{:classes #?(:clj {'ArithmeticException ArithmeticException}
1689+
:cljs {:allow :all 'js js/global})
1690+
:namespaces {'sci.core {'stacktrace sci/stacktrace
1691+
'format-stacktrace sci/format-stacktrace}}})]
1692+
(is (= :exception (:caught result)))
1693+
;; Check that we have location info (user/foo on line 2)
1694+
(is (str/includes? (str (:st result)) "user/foo"))
1695+
(is (str/includes? (str (:st result)) ":2:")))))
1696+
16571697
(deftest var->sym-test
16581698
(is (= 'clojure.core/inc (sci/var->symbol (sci/eval-string "#'inc")))))
16591699

@@ -1726,7 +1766,7 @@
17261766
(is (true? (sci/eval-string "(exists? js/console.log)" {:classes {'js js/globalThis
17271767
:allow :all}})))
17281768
(is (false? (sci/eval-string "(exists? js/foo.bar)" {:classes {'js js/globalThis
1729-
:allow :all}})))
1769+
:allow :all}})))
17301770
(is (false? (sci/eval-string "(exists? js/console.log.foobar)" {:classes {'js js/globalThis
17311771
:allow :all}})))
17321772
(is (false? (sci/eval-string "(exists? console.log)" {:classes {'js js/globalThis

‎test/sci/test_utils.cljc‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,3 @@
4040
(comment
4141
(eval* "*in*" {'*in* 1}) ;;=> 1
4242
)
43-

0 commit comments

Comments
 (0)