Fix #937: throw when copying non-existent namespace (#938)

babashka · Oct 8, 2024 · 54a2ca9 · 54a2ca9
1 parent bd4d6ba
commit 54a2ca9
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,7 @@ SCI is used in [babashka](https://github.com/babashka/babashka),
 - Support `aset` on primitive array using reflection
 - Fix [#928](https://github.com/babashka/sci/issues/928): record constructor supports optional meta + ext map
 - Fix [#934](https://github.com/babashka/sci/issues/934): `:allow` may contain namespaced symbols
+- Fix [#937](https://github.com/babashka/sci/issues/937): throw when copying non-existent namespace
 
 ## 0.8.43 (2024-08-06)
 

diff --git a/deps.edn b/deps.edn
@@ -7,10 +7,10 @@
   :dev {:extra-paths ["reflector/src-java11"]}
   :test {:extra-paths ["test" "test-resources"]
          :extra-deps {org.clojure/clojure {:mvn/version "1.9.0"}
-                      org.clojure/clojurescript {:mvn/version "1.10.866"}
+                      org.clojure/clojurescript {:mvn/version "1.11.132"}
                       clj-commons/conch {:mvn/version "0.9.2"}
                       funcool/promesa {:mvn/version "8.0.450"}}}
-  :shadow {:extra-deps {thheller/shadow-cljs {:mvn/version "2.28.11"}}}
+  :shadow {:extra-deps {thheller/shadow-cljs {:mvn/version "2.28.16"}}}
   :cljs {:extra-deps {org.clojure/clojurescript {:mvn/version "1.11.54"}}}
   :clj-test-runner
   {:extra-deps {com.cognitect/test-runner

diff --git a/src/sci/core.cljc b/src/sci/core.cljc
@@ -451,7 +451,11 @@
                         ;; this branch is hit by macroexpanding within the CLJS
                         ;; compiler on the JVM. At ths point, cljs-ns-publics
                         ;; refers to the right var.
-                        (let [publics-map
+                        (let [ns? #_:clj-kondo/ignore
+                              (sci.impl.cljs/cljs-find-ns ns-sym)
+                              _ (when-not ns?
+                                  (throw (ex-info (str "Copying non-existent namespace: " ns-sym) {:ns ns-sym})))
+                              publics-map
                               #_:clj-kondo/ignore
                               (sci.impl.cljs/cljs-ns-publics ns-sym)
                               publics-map (process-publics publics-map opts)
@@ -473,7 +477,12 @@
                           `(-copy-ns ~publics-map ~sci-ns))
                         :cljs
                         ;; this branch is hit by self-hosted
-                        (let [publics-map
+                        (let [ns?
+                              #_:clj-kondo/ignore
+                              (cljs.analyzer.api/find-ns ns-sym)
+                              _ (when-not ns?
+                                  (throw (ex-info (str "Copying non-existent namespace: " ns-sym) {:ns ns-sym})))
+                              publics-map
                               #_:clj-kondo/ignore
                               (cljs.analyzer.api/ns-publics ns-sym)
                               publics-map (process-publics publics-map opts)

diff --git a/src/sci/impl/cljs.cljc b/src/sci/impl/cljs.cljc
@@ -4,7 +4,8 @@
   #?(:cljs (:require-macros [sci.impl.cljs :refer [require-cljs-analyzer-api when-not-var-exists]])))
 
 (macros/deftime
-  #?(:clj (def cljs-ns-publics (resolve 'cljs.analyzer.api/ns-publics)))
+  #?(:clj (do (def cljs-ns-publics (resolve 'cljs.analyzer.api/ns-publics))
+              (def cljs-find-ns (resolve 'cljs.analyzer.api/find-ns))))
   #_:clj-kondo/ignore
   (defmacro ^:private require-cljs-analyzer-api []
     (macros/? :clj
@@ -13,7 +14,8 @@
               :cljs #?(;; macro executed from JVM Clojure, within CLJS compiler
                        :clj
                        (do (require '[cljs.analyzer.api])
-                           (def cljs-ns-publics (resolve 'cljs.analyzer.api/ns-publics)))
+                           (def cljs-ns-publics (resolve 'cljs.analyzer.api/ns-publics))
+                           (def cljs-find-ns (resolve 'cljs.analyzer.api/find-ns)))
                        ;; self-hosted CLJS, no require supported but also not necessary
                        :cljs nil)))
 

diff --git a/test/sci/core_test.cljc b/test/sci/core_test.cljc
@@ -1525,7 +1525,9 @@
     (is (= "YOLO" (:doc (meta (get sci-ns 'foo)))))
     (is (:copy-this (meta (get sci-ns 'foo))))
     (is (:awesome-meta (meta (get sci-ns 'baz))))
-    (is (= [1 1] (sci/eval-string "(vec-macro 1)" {:namespaces {'user sci-ns}})))))
+    (is (= [1 1] (sci/eval-string "(vec-macro 1)" {:namespaces {'user sci-ns}}))))
+  ;; throws at compile time, so it's difficult to test this:
+  #_(is (thrown? Exception (sci/copy-ns #_:clj-kondo/ignore non-existent.namespace nil))))
 
 (deftest copy-ns-default-meta-test
   (testing "copy-ns default meta includes name"