Fix #923: check for duplicate keys in dynamic set of map literal (#924)

babashka · Jul 23, 2024 · 213ce7a · 213ce7a
1 parent dfadc12
commit 213ce7a
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ SCI is used in [babashka](https://github.com/babashka/babashka),
 - Fix [#626](https://github.com/babashka/sci/issues/626): add `cljs.core/exists?`
 - Fix [#919](https://github.com/babashka/sci/issues/919): :js-libs + refer + rename clashes with core var
 - Fix [#906](https://github.com/babashka/sci/issues/906): `merge-opts` loses `:features` or previous context
+- Fix [#923](https://github.com/babashka/sci/issues/923): check for duplicate keys in dynamic set of map literals
 
 ## 0.8.41 (2023-11-24)
 

diff --git a/src/sci/impl/analyzer.cljc b/src/sci/impl/analyzer.cljc
@@ -1698,7 +1698,14 @@
 
 (defn map-fn [children-count]
   (if (<= children-count 16)
-    array-map hash-map))
+    #?(:clj #(let [^objects arr (into-array Object %&)]
+               (clojure.lang.PersistentArrayMap/createWithCheck arr))
+       :cljs #(PersistentArrayMap.createWithCheck (into-array %&))
+       :default array-map)
+    #?(:clj #(let [^objects arr (into-array Object %&)]
+               (clojure.lang.PersistentHashMap/createWithCheck arr))
+       :cljs #(PersistentHashMap.createWithCheck (into-array %&))
+       :default hash-map)))
 
 (defn return-map [ctx the-map analyzed-children]
   (let [mf (map-fn (count analyzed-children))]
@@ -1832,7 +1839,11 @@
                                           ;; return a vector
                                           identity
                                           vector expr m)
-       (set? expr) (analyze-vec-or-set ctx set hash-set expr m)
+       (set? expr) (analyze-vec-or-set ctx set
+                                       #?(:clj #(clojure.lang.PersistentHashSet/createWithCheck %&)
+                                          :cljs #(PersistentHashSet.createWithCheck (into-array %&))
+                                          :default vector)
+                                       expr m)
        (seq? expr) (if (seq expr)
                      (analyze-call ctx expr m top-level?)
                      ;; the empty list

diff --git a/test/sci/core_test.cljc b/test/sci/core_test.cljc
@@ -7,6 +7,8 @@
    [sci.core :as sci :refer [eval-string]]
    [sci.test-utils :as tu]))
 
+#?(:cljs (def Exception js/Error))
+
 #?(:cljs
    (defn testing-vars-str
      "Returns a string representation of the current test.  Renders names
@@ -107,6 +109,9 @@
                       :c [*in* *in*]
                       :d #{*in* (inc *in*)}
                       :e {:a *in*}}))))
+  (testing "duplicate keys"
+    (is (thrown-with-msg? Exception #"Duplicate key" (sci/eval-string "(let [a 1 b 1] #{a b})")))
+    (is (thrown-with-msg? Exception #"Duplicate key" (sci/eval-string "(let [a 1 b 1] {a 1 b 2})"))))
   (testing "quoting"
     (is (= {:a '*in*} (eval* 1 (str "'{:a *in*}"))))
     (is (= '#{1 2 3 *in*} (eval* 4 "'#{1 2 3 *in*}")))
@@ -638,8 +643,6 @@
                                                  'js js/global}})))
       (str "FAIL: " expr)))
 
-#?(:cljs (def Exception js/Error))
-
 (deftest recur-test
   (is (= 10000 (tu/eval* "(defn hello [x] (if (< x 10000) (recur (inc x)) x)) (hello 0)"
                          {})))