From d86597b19c9efef97ca5f9771c6882cf4b77e624 Mon Sep 17 00:00:00 2001
From: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Date: Fri, 31 Mar 2023 11:21:53 +0200
Subject: [PATCH] Revert "shared.ns/import-fn(s) emit optimized defn forms
(#29700)" (#29715)
This reverts commit a817ed8aff5ef4c0048c747caaecaeed5c143a31.
---
.../src/metabase/shared/util/namespaces.clj | 61 +++----------------
.../metabase/shared/util/namespaces_test.cljc | 39 ------------
2 files changed, 7 insertions(+), 93 deletions(-)
delete mode 100644 shared/test/metabase/shared/util/namespaces_test.cljc
diff --git a/shared/src/metabase/shared/util/namespaces.clj b/shared/src/metabase/shared/util/namespaces.clj
index 52e2f4f2d54..5e2e0869db3 100644
--- a/shared/src/metabase/shared/util/namespaces.clj
+++ b/shared/src/metabase/shared/util/namespaces.clj
@@ -1,59 +1,12 @@
(ns metabase.shared.util.namespaces
"Potemkin is Java-only, so here's a basic function-importing macro that works for both CLJS and CLJ."
(:require
- [net.cgrand.macrovich :as macros]
- [potemkin :as p]))
+ [net.cgrand.macrovich :as macros]
+ [potemkin :as p]))
-(defn- arity-form
- "Generate the form for a given arity with `arglist` for a `defn` form created by [[-redef]]."
- [imported-symbol arglist]
- (let [variadic? (some (partial = '&) arglist)
- simplified-arglist (mapv (fn [arg]
- (if (symbol? arg)
- arg
- (gensym "arg-")))
- arglist)]
- (list simplified-arglist
- (cond->> (list* imported-symbol (remove (partial = '&) simplified-arglist))
- variadic? (cons 'apply)))))
-
-(defn- defn-form
- "Generate the `defn` form for [[-redef]]."
- [imported-symbol defn-name docstring arglists]
- `(defn ~defn-name
- ~docstring
- ;; ClojureScript seems to ignore this metadata anyway, oh well. Make sure we re-quote arglists so the reader
- ;; doesn't try to evaluate them.
- {:arglists '~arglists}
- ~@(for [arglist arglists]
- (arity-form imported-symbol arglist))))
-
-(defmacro -redef
- "Impl for [[import-fn]] and [[import-fns]]."
- [imported-symbol created-symbol]
- {:pre [(qualified-symbol? imported-symbol)
- ((some-fn nil? simple-symbol?) created-symbol)]}
- ;; In ClojureScript compilation, `imported-symbol` will come in like `u/format-seconds` instead of
- ;; `metabase.util/format-seconds`, thus we'll need to use [[cljs.analyzer/resolve-symbol]] to resolve it to the real
- ;; unaliased version.
- (let [resolve-symbol (macros/case
- :cljs (requiring-resolve 'cljs.analyzer/resolve-symbol)
- :clj identity)
- imported-var (requiring-resolve (resolve-symbol imported-symbol))
- imported-metadata (meta imported-var)
- metadata (merge
- {:doc "docstring"
- :arglists '([& args])}
- ;; preserve important metadata from the imported var.
- imported-metadata
- ;; preserve metadata attached to the symbol(s) passed in to import-fn(s).
- (meta created-symbol)
- (meta imported-symbol))
- defn-name (-> (or created-symbol (symbol (name imported-symbol)))
- ;; attach everything except for `:doc` and `:arglists` to the symbol we're deffing; we'll
- ;; deal with `:doc` and `:arglists` separately.
- (with-meta (dissoc metadata :doc :arglists)))]
- (defn-form imported-symbol defn-name (:doc metadata) (:arglists metadata))))
+(defn- redef [target sym]
+ (let [defn-name (or sym (symbol (name target)))]
+ `(def ~defn-name "docstring" (fn [& args#] (apply ~target args#)))))
(defmacro import-fn
"Imports a single defn from another namespace.
@@ -64,7 +17,7 @@
([target]
`(import-fn ~target nil))
([target sym]
- `(-redef ~target ~sym)))
+ (redef target sym)))
(defmacro import-fns
"Imports defns from other namespaces.
@@ -79,5 +32,5 @@
:let [target-sym (if (vector? f) (first f) f)
new-sym (if (vector? f) (second f) f)
target (symbol (name target-ns) (name target-sym))]]
- `(-redef ~target ~new-sym)))
+ (redef target new-sym)))
:clj `(p/import-vars ~@spaces)))
diff --git a/shared/test/metabase/shared/util/namespaces_test.cljc b/shared/test/metabase/shared/util/namespaces_test.cljc
deleted file mode 100644
index 54983b53f82..00000000000
--- a/shared/test/metabase/shared/util/namespaces_test.cljc
+++ /dev/null
@@ -1,39 +0,0 @@
-(ns metabase.shared.util.namespaces-test
- (:require
- [clojure.test :refer [deftest is testing are]]
- [metabase.shared.formatting.date :as shared.formatting.date]
- [metabase.shared.util.namespaces :as shared.ns]))
-
-#?(:clj
- (deftest ^:synchronized arity-form-test
- (with-redefs [gensym (fn [prefix]
- (symbol (str prefix "1234")))]
- (are [arglist expected-form] (= expected-form
- (#'shared.ns/arity-form 'imported/fn arglist))
- '[x]
- '([x]
- (imported/fn x))
-
- '[x y]
- '([x y]
- (imported/fn x y))
-
- '[{:destructured :map} x]
- '([arg-1234 x]
- (imported/fn arg-1234 x))
-
- '[x & more]
- '([x & more]
- (apply imported/fn x more))))))
-
-;;; this is just an arbitrarily selected function that has `:export` metadata; we can change it to something else if
-;;; it changes upstream.
-(shared.ns/import-fn ^::extra-key shared.formatting.date/format-for-parameter)
-
-(deftest ^:parallel import-fn-test
- (testing "Should copy important metadata from the source var"
- (is (:export (meta #'format-for-parameter)))
- (is (= (select-keys (meta #'shared.formatting.date/format-for-parameter) [:export :arglists :doc])
- (select-keys (meta #'format-for-parameter) [:export :arglists :doc]))))
- (testing "Should preserve metadata on the symbol(s) passed to import-fn(s)"
- (is (::extra-key (meta #'format-for-parameter)))))
--
GitLab