Fix #842: error metadata missing on some interop call exceptions (#843)

babashka · Nov 28, 2022 · 05c738b · 05c738b
1 parent de6a915
commit 05c738b
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ SCI is used in [babashka](https://github.com/babashka/babashka),
 ## Unreleased
 
 - Fix for `SCI_ELIDE_VARS`
+- [#842](https://github.com/babashka/sci/issues/842) Error metadata missing on some interop calls ([@bobisageek](https://github.com/bobisageek))
 
 ## 0.5.36 (2022-11-14)
 

diff --git a/src/sci/impl/analyzer.cljc b/src/sci/impl/analyzer.cljc
@@ -1011,13 +1011,16 @@
         (let [field-access (str/starts-with? method-name "-")
               meth-name (if field-access
                           (subs method-name 1)
-                          method-name)]
+                          method-name)
+              stack (assoc (meta expr)
+                      :ns @utils/current-ns
+                      :file @utils/current-file)]
           #?(:clj (if (class? instance-expr)
                     (if (nil? args)
                       (if field-access
                         (sci.impl.types/->Node
                          (interop/get-static-field [instance-expr (subs method-name 1)])
-                         nil)
+                         stack)
                         ;; https://clojure.org/reference/java_interop
                         ;; If the second operand is a symbol and no args are
                         ;; supplied it is taken to be a field access - the
@@ -1031,35 +1034,23 @@
                                       (catch IllegalArgumentException _ nil))]
                           (sci.impl.types/->Node
                            (interop/get-static-field [instance-expr method-name])
-                           nil)
-                          (let [stack (assoc (meta expr)
-                                             :ns @utils/current-ns
-                                             :file @utils/current-file)]
-                            (sci.impl.types/->Node
-                             (eval/eval-static-method-invocation
+                           stack)
+                          (sci.impl.types/->Node
+                            (eval/eval-static-method-invocation
                               ctx bindings
                               (cons [instance-expr method-name] args))
-                             stack))))
-                      (let [stack (assoc (meta expr)
-                                         :ns @utils/current-ns
-                                         :file @utils/current-file)]
-                        (sci.impl.types/->Node
-                         (eval/eval-static-method-invocation
+                            stack)))
+                      (sci.impl.types/->Node
+                        (eval/eval-static-method-invocation
                           ctx bindings (cons [instance-expr method-name] args))
-                         stack)))
-                    (let [stack (assoc (meta expr)
-                                       :ns @utils/current-ns
-                                       :file @utils/current-file)]
-                      (with-meta (sci.impl.types/->Node
-                                  (eval/eval-instance-method-invocation
+                        stack))
+                    (with-meta (sci.impl.types/->Node
+                                 (eval/eval-instance-method-invocation
                                    ctx bindings instance-expr meth-name field-access args)
-                                  stack)
-                        {::instance-expr instance-expr
-                         ::method-name method-name})))
-             :cljs (let [stack (assoc (meta expr)
-                                      :ns @utils/current-ns
-                                      :file @utils/current-file)
-                         allowed? (identical? method-expr utils/allowed-append)]
+                                 stack)
+                      {::instance-expr instance-expr
+                       ::method-name   method-name}))
+             :cljs (let [allowed? (identical? method-expr utils/allowed-append)]
                      (with-meta (sci.impl.types/->Node
                                  (eval/eval-instance-method-invocation
                                   ctx bindings instance-expr meth-name field-access args allowed?)
@@ -1082,7 +1073,7 @@
   (when (< (count expr) 2)
     (throw (new #?(:clj IllegalArgumentException :cljs js/Error)
                 "Malformed member expression, expecting (.member target ...)")))
-  (analyze-dot ctx (list '. obj (cons (symbol (subs (name method-name) 1)) args))))
+  (analyze-dot ctx (with-meta (list '. obj (cons (symbol (subs (name method-name) 1)) args)) (meta expr))))
 
 (defn analyze-new [ctx [_new class-sym & args :as expr]]
   (let [ctx (without-recur-target ctx)]

diff --git a/test/sci/interop_test.cljc b/test/sci/interop_test.cljc
@@ -1,9 +1,9 @@
 (ns sci.interop-test
   (:require
-   [clojure.test :as test :refer [deftest is testing #?(:cljs async)]]
+   [clojure.string :as str]
+   [clojure.test :as test :refer [are deftest is testing #?(:cljs async)]]
    [sci.core :as sci]
    [sci.test-utils :as tu]
-   #?(:cljs [clojure.string :as str])
    #?(:cljs [goog.object :as gobj]))
   #?(:clj (:import PublicFields)))
 
@@ -115,6 +115,44 @@
        (is (thrown-with-msg? Exception #"not"
                              (eval* "(clojure.lang.Var/find 'clojure.core/int)"))))))
 
+
+(when-not tu/native?
+  (deftest exception-data
+    (testing "top-level interop forms have line and column data"
+      (letfn [(form-ex-data [form]
+                (try
+                  (tu/eval* (str form) {:classes {:allow :all
+                                                  #?@(:clj ['Long Long])}})
+                  (is (= nil "shouldn't reach here"))
+                  (catch #?(:clj Exception :cljs :default) e
+                    (ex-data e))))]
+        (testing "instance members"
+          (are [form]
+            (let [actual (form-ex-data form)]
+              (and (tu/submap? {:type   :sci/error
+                           :line   1
+                           :column 1}
+                actual)
+                (str/includes? (:message actual) "missingMem")))
+            '(. 3 missingMem) '(. 3 missingMem 1 2)
+            '(.missingMem 3)  '(.missingMem 3 1 2)
+            ; these return nil in cljs
+            #?@(:clj ['(.-missingMem 3) '(. 3 -missingMem)])))
+        #?(:clj
+           (testing "static members"
+             (are [form]
+               (let [actual (form-ex-data form)]
+                 (and (tu/submap? {:type   :sci/error
+                                   :line   1
+                                   :column 1}
+                        actual)
+                   (str/includes? (:message actual) "missingMem")))
+               '(. Long missingMem)  '(. Long missingMem 1 2)
+               '(.missingMem Long)   '(.missingMem Long 1 2)
+               '(Long/missingMem)    '(Long/missingMem 1 2)
+               '(. Long -missingMem) '(.-missingMem Long)
+               'Long/missingMem      '(Long/-missingMem))))))))
+
 (deftest syntax-test
   (when-not tu/native?
     (doseq [expr ["(.)" "(. {})" "(.foo)"]]