Fix #942: improve error location of invalid destructuring (#946)

CHANGELOG.md

@@ -10,6 +10,10 @@ SCI is used in [babashka](https://github.com/babashka/babashka),
 [joyride](https://github.com/BetterThanTomorrow/joyride/) and many
 [other](https://github.com/babashka/sci#projects-using-sci) projects.
 
+## Unreleased
+
+- Fix [#942](https://github.com/babashka/sci/issues/942): improve error location of invalid destructuring
+
 ## 0.9.44 (2024-10-24)
 
 - Fix [#917](https://github.com/babashka/sci/issues/917): support new Clojure 1.12 Java interop: `String/new`, `String/.length` and `Integer/parseInt` as fns

src/sci/impl/analyzer.cljc

@@ -695,12 +695,6 @@
                        (t/eval body ctx bindings))))))
              stack))))))
 
-(defn analyze-let
-  "The let macro from clojure.core"
-  [ctx [_let let-bindings & exprs :as expr]]
-  (let [let-bindings (destructure let-bindings)]
-    (analyze-let* ctx expr let-bindings exprs)))
-
 (defn init-var! [ctx name expr]
   (let [cnn (utils/current-ns-name)
         env (:env ctx)

src/sci/impl/destructure.cljc

@@ -3,7 +3,7 @@
   {:no-doc true}
   (:refer-clojure :exclude [destructure]))
 
-(defn destructure* [bindings]
+(defn destructure* [bindings loc]
   (let [bents (partition 2 bindings)
         pb (fn pb [bvec b v]
              (let [pvec
@@ -38,7 +38,8 @@
                                                   firstb
                                                   (if has-rest
                                                     gfirst
-                                                    (list nth gvec n nil)))
+                                                    (cond-> (list nth gvec n nil)
+                                                      loc (with-meta loc))))
                                               (inc n)
                                               (next bs)
                                               seen-rest?))))
@@ -109,5 +110,7 @@
             :cljs (new js/Error (str "Unsupported binding key: " (ffirst kwbs)))))
         (reduce process-entry [] bents)))))
 
-(defn destructure [b]
-  (destructure* b))
+(defn destructure
+  ([b] (destructure b nil))
+  ([b loc]
+   (destructure* b loc)))

src/sci/impl/namespaces.cljc

@@ -879,7 +879,7 @@
     (utils/throw-error-with-location "let requires a vector for its binding" expr))
   (when-not (even? (count bindings))
     (utils/throw-error-with-location "let requires an even number of forms in binding vector" expr))
-  `(let* ~(destructure/destructure bindings)
+  `(let* ~(destructure/destructure bindings (meta expr))
      ~@body))
 
 (defn loop**

test/sci/error_test.cljc

@@ -179,3 +179,12 @@
        (let [f (sci.core/eval-string "(fn [] (try (/ 1 0) (catch ^:sci/error Exception e (throw e))))")]
          (is (let [res @(future (invoke-ex-fn f))]
                (is (= {:line 1 :column 13} (select-keys res [:line :column])))))))))
+
+(deftest let-test
+  (let [{:keys [line column]}
+        (try
+          (sci.core/eval-string "(str (let [[a] 1] a))")
+          (catch Exception e
+            (ex-data e)))]
+    (is (= 1 line))
+    (is (= 6 column))))