diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index cb5d4b7d3fb59e78b79f5af1f7dac7954ac563c9..462bad5591f7f137998a15ca673af19feb62eba5 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -1,4 +1,45 @@ -{:linters {:unresolved-symbol {:exclude [schema= re=]}} +{:config-paths ["macros"] + :linters {:unresolved-symbol {:exclude [schema= re= + (metabase.util/prog1 [<>]) + (metabase.mbql.util/match-one) + (metabase.mbql.util/match) + (metabase.mbql.util.match/match) + (metabase.mbql.util.match/match-one) + (metabase.mbql.util.match/replace) + (metabase.mbql.util/replace) + (metabase.mbql.util/replace-in) + (metabase.query-processor.middleware.cache-backend.interface/with-cached-results) + (metabase.util.regex/rx [opt]) + (metabase.async.streaming-response/streaming-response) + (clojure.core.logic/fresh) + (clojure.core.logic/matcha) + (clojure.core.logic/run)]} + ;; TODO: clj-kondo should have a way to disable this in certain macro calls like + ;; metabase.mbql.util.match/replace + :unexpected-recur {:level :off} + :unused-referred-var {:exclude {compojure.core [GET DELETE POST PUT]}}} + :lint-as {metabase.api.common/let-404 clojure.core/let + metabase.db.data-migrations/defmigration clojure.core/def + metabase.query-processor.error-type/deferror clojure.core/def + metabase.models.setting/defsetting clj-kondo.lint-as/def-catch-all + metabase.mbql.schema.macros/defclause clj-kondo.lint-as/def-catch-all + metabase.public-settings.premium-features/define-premium-feature clojure.core/def + metabase.sync.util/sum-for clojure.core/for + metabase.sync.util/with-emoji-progress-bar clojure.core/let + metabase.driver.sql-jdbc.execute.diagnostic/capturing-diagnostic-info clojure.core/fn + metabase.util.files/with-open-path-to-resource clojure.core/let + metabase.db.liquibase/with-liquibase clojure.core/let + metabase.models.setting.multi-setting/define-multi-setting clojure.core/def + metabase.integrations.ldap/with-ldap-connection clojure.core/fn + toucan.db/with-call-counting clojure.core/fn + + potemkin.types/defprotocol+ clojure.core/defprotocol + potemkin/defprotocol+ clojure.core/defprotocol + potemkin.types/defrecord+ clojure.core/defrecord + potemkin/defrecord+ clojure.core/defrecord + potemkin.types/deftype+ clojure.core/deftype + potemkin/deftype+ clojure.core/deftype + clojurewerkz.quartzite.jobs/defjob clojure.core/defn} :hooks {:analyze-call {metabase.test.data/dataset hooks.metabase.test.data/dataset metabase.test/dataset hooks.metabase.test.data/dataset metabase.test.data/$ids hooks.metabase.test.data/$ids @@ -6,4 +47,13 @@ metabase.test.data/mbql-query hooks.metabase.test.data/$ids metabase.test/mbql-query hooks.metabase.test.data/$ids metabase.test.data/run-mbql-query hooks.metabase.test.data/$ids - metabase.test/run-mbql-query hooks.metabase.test.data/$ids}}} + metabase.test/run-mbql-query hooks.metabase.test.data/$ids} + :macroexpand {metabase.api.common/defendpoint* metabase.api.common/defendpoint* + metabase.api.common/defendpoint metabase.api.common/defendpoint + metabase.api.common/defendpoint-async metabase.api.common/defendpoint + metabase.query-processor.streaming/streaming-response + metabase.query-processor.streaming/streaming-response + + toucan.models/defmodel toucan.models/defmodel + }} + :config-in-comment {:linters {:unresolved-symbol {:level :off}}}} diff --git a/.clj-kondo/macros/metabase/api/common.clj b/.clj-kondo/macros/metabase/api/common.clj new file mode 100644 index 0000000000000000000000000000000000000000..bd2bcb345941e7e83cc6fa4772e3adc53f6efc1b --- /dev/null +++ b/.clj-kondo/macros/metabase/api/common.clj @@ -0,0 +1,102 @@ +(ns metabase.api.common + (:require [clojure.string :as str])) + +(defn optional + "Helper function for defining functions that accept optional arguments. If `pred?` is true of the first item in `args`, + a pair like `[first-arg other-args]` is returned; otherwise, a pair like `[default other-args]` is returned. + + If `default` is not specified, `nil` will be returned when `pred?` is false. + + (defn + ^{:arglists ([key? numbers])} + wrap-nums [& args] + (let [[k nums] (optional keyword? args :nums)] + {k nums})) + (wrap-nums 1 2 3) -> {:nums [1 2 3]} + (wrap-nums :numbers 1 2 3) -> {:numbers [1 2 3]}" + {:arglists '([pred? args] + [pred? args default])} + [pred? args & [default]] + (if (pred? (first args)) [(first args) (next args)] + [default args])) + +(defn add-route-param-regexes + "Expand a `route` string like \"/:id\" into a Compojure route form that uses regexes to match parameters whose name + matches a regex from `auto-parse-arg-name-patterns`. + + (add-route-param-regexes \"/:id/card\") -> [\"/:id/card\" :id #\"[0-9]+\"]" + [route] + (if (vector? route) + route + (let [arg-types nil #_(typify-args (route-arg-keywords route))] + (if (empty? arg-types) + route + (apply vector route arg-types))))) + +(defn route-fn-name + "Generate a symbol suitable for use as the name of an API endpoint fn. Name is just `method` + `route` with slashes + replaced by underscores. + + (route-fn-name GET \"/:id\") ;-> GET_:id" + [method route] + ;; if we were passed a vector like [":id" :id #"[0-9+]"] only use first part + (let [route (if (vector? route) (first route) route)] + (-> (str (name method) route) + (str/replace #"/" "_") + symbol))) + +(defmacro defendpoint* + "Impl macro for `defendpoint`; don't use this directly." + [{:keys [method route fn-name docstr args body]}] + `(def ~(vary-meta fn-name + assoc + + :doc docstr + :is-endpoint? true) + (~(symbol "compojure.core" (name method)) ~route ~args + ~@body))) + +(defn- parse-defendpoint-args [[method route & more]] + (let [fn-name (route-fn-name method route) + route (add-route-param-regexes route) + [docstr [args & more]] (optional string? more) + [arg->schema body] (optional (every-pred map? #(every? symbol? (keys %))) more)] + (when-not docstr + ;; Don't i18n this, it's dev-facing only + :TODO/emit-clj-kondo-warning-for-missing-docstring + #_(log/warn (u/format-color 'red "Warning: endpoint %s/%s does not have a docstring. Go add one." + (ns-name *ns*) fn-name))) + {:method method + :route route + :fn-name fn-name + ;; eval the vals in arg->schema to make sure the actual schemas are resolved so we can document + ;; their API error messages + :docstr "foo" + :args args + :arg->schema arg->schema + :body body})) + +(defmacro defendpoint + "Define an API function. + This automatically does several things: + + - calls `auto-parse` to automatically parse certain args. e.g. `id` is converted from `String` to `Integer` via + `Integer/parseInt` + + - converts `route` from a simple form like `\"/:id\"` to a typed one like `[\"/:id\" :id #\"[0-9]+\"]` + + - sequentially applies specified annotation functions on args to validate them. + + - automatically calls `wrap-response-if-needed` on the result of `body` + + - tags function's metadata in a way that subsequent calls to `define-routes` (see below) will automatically include + the function in the generated `defroutes` form. + + - Generates a super-sophisticated Markdown-formatted docstring" + ;; {:arglists '([method route docstr? args schemas-map? & body])} + [& defendpoint-args] + (let [{:keys [args body arg->schema], :as defendpoint-args} (parse-defendpoint-args defendpoint-args)] + `(defendpoint* ~(assoc defendpoint-args + :body `(do ~args + ~arg->schema + (do ~@body)))))) diff --git a/.clj-kondo/macros/metabase/query_processor/streaming.clj b/.clj-kondo/macros/metabase/query_processor/streaming.clj new file mode 100644 index 0000000000000000000000000000000000000000..af76e51f84adc63160d1d262ef6af80b4ef37c69 --- /dev/null +++ b/.clj-kondo/macros/metabase/query_processor/streaming.clj @@ -0,0 +1,6 @@ +(ns metabase.query-processor.streaming) + +(defmacro streaming-response [[x y z] & body] + `(clojure.core/let [~x ~y] + ~z + ~@body)) diff --git a/.clj-kondo/macros/toucan/models.clj b/.clj-kondo/macros/toucan/models.clj new file mode 100644 index 0000000000000000000000000000000000000000..6bac0387b8942d9754e778d609ad81e35e4d6fd6 --- /dev/null +++ b/.clj-kondo/macros/toucan/models.clj @@ -0,0 +1,7 @@ +(ns toucan.models) + +(defmacro defmodel [model-name & ks] + `(do (clojure.core/defrecord ~model-name ~(vec (map (comp symbol name) ks))) + (clojure.core/defrecord + ~(symbol (str model-name "Instance")) + ~(vec (map (comp symbol name) ks))))) diff --git a/.clj-kondo/test.clj b/.clj-kondo/test.clj new file mode 100644 index 0000000000000000000000000000000000000000..5630f7de300ecf7dc5e59eb92362eb9f0527d219 --- /dev/null +++ b/.clj-kondo/test.clj @@ -0,0 +1,10 @@ +(ns test + (:require [metabase.query-processor.streaming :as streaming] + [toucan.models :as models])) + +(models/defmodel Card :foo) + +(map->Card {:foo 1}) +(map->CardInstance {:foo 1}) + +(streaming/streaming-response [x 1 (inc x)]) diff --git a/shared/src/metabase/mbql/normalize.cljc b/shared/src/metabase/mbql/normalize.cljc index e307df9d51f3e312ff6a4b24e8047bb5282029ce..5e630ffd69d06b28fc07819aff8b81b75780d018 100644 --- a/shared/src/metabase/mbql/normalize.cljc +++ b/shared/src/metabase/mbql/normalize.cljc @@ -392,7 +392,7 @@ (canonicalize-mbql-clause (wrap-implicit-field-id clause))) (defmethod canonicalize-mbql-clause :field - [[_ id-or-name opts :as clause]] + [[_ id-or-name opts]] (if (is-clause? :field id-or-name) (let [[_ nested-id-or-name nested-opts] id-or-name] (canonicalize-mbql-clause [:field nested-id-or-name (not-empty (merge nested-opts opts))])) diff --git a/shared/src/metabase/mbql/schema.cljc b/shared/src/metabase/mbql/schema.cljc index 2e274d7f3eaa052c4a377c5d32ad9c635a7c8514..e7fa07429932afc423a21cc5719f7dd833ffc87f 100644 --- a/shared/src/metabase/mbql/schema.cljc +++ b/shared/src/metabase/mbql/schema.cljc @@ -774,6 +774,7 @@ (defclause ^{:requires-features #{:standard-deviation-aggregations}} stddev field-or-expression FieldOrExpressionDef) +(declare ag:var) ;; for clj-kondo (defclause ^{:requires-features #{:standard-deviation-aggregations}} [ag:var var] field-or-expression FieldOrExpressionDef) diff --git a/src/metabase/api/collection.clj b/src/metabase/api/collection.clj index 97ac1f3533d9da655b8f7b2ab592fa5eb4aec659..2ec43dcfe4dda511c7b175d72e12d34f6c66a52b 100644 --- a/src/metabase/api/collection.clj +++ b/src/metabase/api/collection.clj @@ -15,6 +15,7 @@ [metabase.models.card :refer [Card]] [metabase.models.collection :as collection :refer [Collection]] [metabase.models.collection.graph :as collection.graph] + #_:clj-kondo/ignore ;; bug: when alias defined for namespaced keywords is run through kondo macro, ns should be regarded as used [metabase.models.collection.root :as collection.root] [metabase.models.dashboard :refer [Dashboard]] [metabase.models.interface :as mi] @@ -82,7 +83,7 @@ [exclude-archived namespace] {exclude-archived (s/maybe su/BooleanString) namespace (s/maybe su/NonBlankString)} - (let [coll-type-ids (reduce (fn [acc {:keys [collection_id dataset] :as x}] + (let [coll-type-ids (reduce (fn [acc {:keys [collection_id dataset] :as _x}] (update acc (if dataset :dataset :card) conj collection_id)) {:dataset #{} :card #{}} @@ -190,7 +191,7 @@ (dissoc row :description :display :authority_level :moderated_status))) (defmethod collection-children-query :snippet - [_ collection {:keys [archived? pinned-state]}] + [_ collection {:keys [archived?]}] {:select [:id :name [(hx/literal "snippet") :model]] :from [[NativeQuerySnippet :nqs]] :where [:and @@ -442,7 +443,7 @@ [:model :desc] [[:model_ranking :desc] [:%lower.name :asc]])) (defn- collection-children* - [collection models {:keys [collection-namespace sort-info] :as options}] + [collection models {:keys [sort-info] :as options}] (let [sql-order (children-sort-clause sort-info @mdb.env/db-type) models (sort (map keyword models)) queries (for [model models] @@ -477,7 +478,7 @@ (s/defn ^:private collection-children "Fetch a sequence of 'child' objects belonging to a Collection, filtered using `options`." [{collection-namespace :namespace, :as collection} :- collection/CollectionWithLocationAndIDOrRoot - {:keys [models collections-only? pinned-state], :as options} :- CollectionChildrenOptions] + {:keys [models], :as options} :- CollectionChildrenOptions] (let [valid-models (for [model-kw [:collection :dataset :card :dashboard :pulse :snippet] ;; only fetch models that are specified by the `model` param; or everything if it's empty :when (or (empty? models) (contains? models model-kw)) diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj index ff10d7c87581dcb40a68a9356a377daf7c467294..778ca073ee10c90bb5fb6dc63ad562846a1328e0 100644 --- a/src/metabase/api/common.clj +++ b/src/metabase/api/common.clj @@ -246,7 +246,7 @@ (defmacro defendpoint* "Impl macro for `defendpoint`; don't use this directly." - [{:keys [method route fn-name docstr args arg->schema original-body body]}] + [{:keys [method route fn-name docstr args body]}] {:pre [(or (string? route) (vector? route))]} `(def ~(vary-meta fn-name assoc @@ -423,9 +423,8 @@ (defn check-valid-page-params "Check on paginated stuff that, if the limit exists, the offset exists, and vice versa." [limit offset] - (do - (check (not (and limit (not offset))) [400 (tru "When including a limit, an offset must also be included.")]) - (check (not (and offset (not limit))) [400 (tru "When including an offset, a limit must also be included.")]))) + (check (not (and limit (not offset))) [400 (tru "When including a limit, an offset must also be included.")]) + (check (not (and offset (not limit))) [400 (tru "When including an offset, a limit must also be included.")])) (s/defn column-will-change? :- s/Bool "Helper for PATCH-style operations to see if a column is set to change when `object-updates` (i.e., the input to the @@ -491,7 +490,7 @@ updating an existing instance, but creating a new one)." ([new-model-data :- ModelWithPosition] (maybe-reconcile-collection-position! nil new-model-data)) - ([{old-collection-id :collection_id, old-position :collection_position, :as before-update} :- (s/maybe ModelWithPosition) + ([{old-collection-id :collection_id, old-position :collection_position, :as _before-update} :- (s/maybe ModelWithPosition) {new-collection-id :collection_id, new-position :collection_position, :as model-updates} :- ModelWithOptionalPosition] (let [updated-collection? (and (contains? model-updates :collection_id) (not= old-collection-id new-collection-id)) diff --git a/src/metabase/api/common/internal.clj b/src/metabase/api/common/internal.clj index 4653b4d548ba2416dcf681d6711dae6d9464ecb5..0ecfac7f61b5a4995ee37ec159c9b3588359dee2 100644 --- a/src/metabase/api/common/internal.clj +++ b/src/metabase/api/common/internal.clj @@ -3,7 +3,7 @@ These are primarily used as the internal implementation of `defendpoint`." (:require [clojure.string :as str] [clojure.tools.logging :as log] - metabase.async.streaming-response + [metabase.async.streaming-response :as streaming-response] [metabase.config :as config] [metabase.util :as u] [metabase.util.i18n :as ui18n :refer [tru]] @@ -13,7 +13,7 @@ (:import clojure.core.async.impl.channels.ManyToManyChannel metabase.async.streaming_response.StreamingResponse)) -(comment metabase.async.streaming-response/keep-me) +(comment streaming-response/keep-me) ;;; +----------------------------------------------------------------------------------------------------------------+ ;;; | DOCSTRING GENERATION | diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj index 73d93809d4239a0a1756a6807088fd6781214ee0..007df7ee82858cad54c83d0597212f988b173a34 100644 --- a/src/metabase/api/dashboard.clj +++ b/src/metabase/api/dashboard.clj @@ -120,7 +120,7 @@ (defn- hide-unreadable-cards "Replace the `:card` and `:series` entries from dashcards that they user isn't allowed to read with empty objects." - [{public-uuid :public_uuid, :as dashboard}] + [dashboard] (update dashboard :ordered_cards (fn [dashcards] (vec (for [dashcard dashcards] (-> dashcard @@ -699,7 +699,7 @@ (letfn [(parse-ids [s] (set (cond (string? s) [(Integer/parseUnsignedInt s)] - (sequential? s) (map #(Integer/parseUnsignedInt %) s))))] + (sequential? s) (map (fn [i] (Integer/parseUnsignedInt i)) s))))] (let [filtered-field-ids (parse-ids filtered) filtering-field-ids (parse-ids filtering)] (doseq [field-id (set/union filtered-field-ids filtering-field-ids)] diff --git a/src/metabase/api/public.clj b/src/metabase/api/public.clj index cf5aeb9bb13491c2afd2d1b7079d48e7d3a62352..604eaa55c0849e746201878d3a5494fcea14e520 100644 --- a/src/metabase/api/public.clj +++ b/src/metabase/api/public.clj @@ -81,7 +81,7 @@ :status])) (defmethod transform-results :failed - [{:keys [error], error-type :error_type, :as results}] + [{error-type :error_type, :as results}] ;; if the query failed instead, unless the error type is specified and is EXPLICITLY allowed to be shown for embeds, ;; instead of returning anything about the query just return a generic error message (merge diff --git a/src/metabase/api/query_description.clj b/src/metabase/api/query_description.clj index 7a8e1543a2d712da8c157e79e348714ce33b5e3e..af83bdc4b1e5908415f1f7edbf8da170a0cdb984 100644 --- a/src/metabase/api/query_description.clj +++ b/src/metabase/api/query_description.clj @@ -32,6 +32,7 @@ {:type :aggregation :arg (:display-name options)} [:aggregation-options ag _] + #_:clj-kondo/ignore (recur ag) [(operator :guard #{:+ :- :/ :*}) & args] diff --git a/src/metabase/driver/sql/query_processor.clj b/src/metabase/driver/sql/query_processor.clj index 83a6da179c1459f1100d92c8f6eebe0c5ee3db62..c7bc95d79027bf9fe7a5f0b275790fbc809fb5db 100644 --- a/src/metabase/driver/sql/query_processor.clj +++ b/src/metabase/driver/sql/query_processor.clj @@ -511,6 +511,7 @@ (->honeysql driver (hx/identifier :field-alias (:name options))) [:aggregation-options ag _] + #_:clj-kondo/ignore (recur ag) ;; For some arcane reason we name the results of a distinct aggregation "count", everything else is named the diff --git a/src/metabase/models/params/chain_filter/dedupe_joins.clj b/src/metabase/models/params/chain_filter/dedupe_joins.clj index 89b834fe35b68dc23189c2ca32fb0fce4efd2865..5212106ab5536b4310a3fa851723dfdd2afbf6db 100644 --- a/src/metabase/models/params/chain_filter/dedupe_joins.clj +++ b/src/metabase/models/params/chain_filter/dedupe_joins.clj @@ -39,6 +39,7 @@ (defn- list-beforeo "A relation such that `sublist` is all items in `lst` up to (but not including) `item`." [lst sublist item] + #_:clj-kondo/ignore (l/matcha [lst sublist] ([[] []]) ([[item . _] []]) diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj index 9c7718264e3f325a5d665df3640a0d162fc6f4de..a41fb7801bcf0bb07b5150f9bc57560f2ee6ddb2 100644 --- a/src/metabase/public_settings.clj +++ b/src/metabase/public_settings.clj @@ -24,8 +24,8 @@ (boolean (setting/get :google-auth-client-id))) (defn- ldap-configured? [] - (do (classloader/require 'metabase.integrations.ldap) - ((resolve 'metabase.integrations.ldap/ldap-configured?)))) + (classloader/require 'metabase.integrations.ldap) + ((resolve 'metabase.integrations.ldap/ldap-configured?))) (defn- ee-sso-configured? [] (u/ignore-exceptions diff --git a/src/metabase/query_processor/middleware/annotate.clj b/src/metabase/query_processor/middleware/annotate.clj index 0d4635bd97715b8096de8f61feb736ffdebc440e..2cea2c54861b2f93dd13b69da58795b27b214d10 100644 --- a/src/metabase/query_processor/middleware/annotate.clj +++ b/src/metabase/query_processor/middleware/annotate.clj @@ -295,6 +295,7 @@ (:name options) [:aggregation-options ag _] + #_:clj-kondo/ignore (recur ag) ;; For unnamed expressions, just compute a name like "sum + count" @@ -336,6 +337,7 @@ (:display-name options) [:aggregation-options ag _] + #_:clj-kondo/ignore (recur ag) [(operator :guard #{:+ :- :/ :*}) & args] diff --git a/src/metabase/query_processor/middleware/permissions.clj b/src/metabase/query_processor/middleware/permissions.clj index ebdcb046c64af23d1eee2834b5c4d781260f4017..a8234bf0c437ca54628ea7c18991cf42e9321a86 100644 --- a/src/metabase/query_processor/middleware/permissions.clj +++ b/src/metabase/query_processor/middleware/permissions.clj @@ -83,7 +83,8 @@ (throw (perms-exception required-perms)))) ;; check perms for any Cards referenced by this query (if it is a native query) (doseq [{query :dataset_query} (qp.resolve-referenced/tags-referenced-cards outer-query)] - (check-query-permissions* query))) + ;; TODO: review needed: + (check-query-permissions* query context))) (s/defn ^:private check-query-permissions* "Check that User with `user-id` has permissions to run `query`, or throw an exception."