From 7fb88340d4045bc0b95f97d1bc33f2db28f7c4db Mon Sep 17 00:00:00 2001 From: Cam Saul <1455846+camsaul@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:24:33 -0700 Subject: [PATCH] Update Kondo to `2024.08.01` and add `deps.edn` aliases to run from the JVM (#47370) * Add `clojure -M:kondo` and `clojure -M:kondo:kondo/all` and bump version * Fix Kondo errors * Fix Kondo+LSP issues with `defendpoint`, `defenterprise`, etc. * Use replace-deps instead of deps for speed * Ok apparently maybe we do need to copy configs when we run Kondo on CI * Oops `./bin/kondo.sh` should not try to use `clj-kondo` * Remove references to GA driver folders --- .clj-kondo/src/hooks/common.clj | 28 ++++++-- .clj-kondo/src/hooks/metabase/api/common.clj | 5 +- .../src/hooks/metabase/models/interface.clj | 7 +- .../src/hooks/metabase/models/setting.clj | 23 ++++--- .../public_settings/premium_features.clj | 6 +- .github/workflows/backend.yml | 14 ---- bin/kondo-updated.sh | 5 +- bin/kondo.sh | 22 +++--- deps.edn | 67 ++++++++++++++++++- src/metabase/api/table.clj | 7 +- src/metabase/driver/sql_jdbc/execute.clj | 2 +- src/metabase/query_processor/context.clj | 2 +- src/metabase/util/honey_sql_2.clj | 2 +- 13 files changed, 133 insertions(+), 57 deletions(-) diff --git a/.clj-kondo/src/hooks/common.clj b/.clj-kondo/src/hooks/common.clj index 625f47440c6..172f268e6f2 100644 --- a/.clj-kondo/src/hooks/common.clj +++ b/.clj-kondo/src/hooks/common.clj @@ -288,8 +288,8 @@ (-> (hooks/vector-node [(hooks/vector-node (map :model binding-infos)) (-> (hooks/list-node (list* (hooks/token-node `let) - (hooks/vector-node (mapcat (juxt :binding :value) binding-infos)) - body)) + (hooks/vector-node (mapcat (juxt :binding :value) binding-infos)) + body)) (with-meta (meta body)))]) (with-meta (meta body))))) @@ -364,11 +364,11 @@ where the first arg should be linted and appear to be used." [{{[_ arg & body] :children} :node}] (let [node* (hooks/list-node - (list* - (hooks/token-node 'let) - (hooks/vector-node [(hooks/token-node (gensym "_")) - arg]) - body))] + (list* + (hooks/token-node 'let) + (hooks/vector-node [(hooks/token-node (gensym "_")) + arg]) + body))] {:node node*})) (defn node->qualified-symbol [node] @@ -407,3 +407,17 @@ ;; should be 1 (format-string-specifier-count "%-02d")) + +(defn add-lsp-ignore-unused-public-var-metadata + "Add + + ^{:clj-kondo/ignore [:clojure-lsp/unused-public-var]} + + metadata to a node to suppress LSP warnings." + [node] + (letfn [(update-ignores [existing-ignores] + (hooks/vector-node + (cons + (hooks/keyword-node :clojure-lsp/unused-public-var) + (:children existing-ignores))))] + (vary-meta node update :clj-kondo/ignore update-ignores))) diff --git a/.clj-kondo/src/hooks/metabase/api/common.clj b/.clj-kondo/src/hooks/metabase/api/common.clj index 622ff3c8c2d..ad3e7556709 100644 --- a/.clj-kondo/src/hooks/metabase/api/common.clj +++ b/.clj-kondo/src/hooks/metabase/api/common.clj @@ -1,7 +1,8 @@ (ns hooks.metabase.api.common (:require [clj-kondo.hooks-api :as api] - [clojure.string :as str])) + [clojure.string :as str] + [hooks.common])) (defn route-fn-name "route fn hook" @@ -25,7 +26,7 @@ (api/token-node (route-fn-name (api/sexpr method) (api/sexpr route))) body)) (with-meta (meta node))))) - (with-meta {:clj-kondo/ignore [:clojure-lsp/unused-public-var]}))))] + hooks.common/add-lsp-ignore-unused-public-var-metadata)))] (update arg :node update-defendpoint))) (comment diff --git a/.clj-kondo/src/hooks/metabase/models/interface.clj b/.clj-kondo/src/hooks/metabase/models/interface.clj index eff17e414b6..48e7c09c344 100644 --- a/.clj-kondo/src/hooks/metabase/models/interface.clj +++ b/.clj-kondo/src/hooks/metabase/models/interface.clj @@ -1,5 +1,7 @@ (ns hooks.metabase.models.interface - (:require [clj-kondo.hooks-api :as hooks])) + (:require + [clj-kondo.hooks-api :as hooks] + [hooks.common])) (defn define-hydration-method "This is used for both [[metabase.models.interface/define-simple-hydration-method]] and @@ -14,5 +16,6 @@ (hooks/token-node 'defn) fn-name fn-tail)))) - (with-meta (update (meta node) :clj-kondo/ignore #(hooks/vector-node (cons :clojure-lsp/unused-public-var (:children %))))))] + (with-meta (meta node)) + hooks.common/add-lsp-ignore-unused-public-var-metadata)] {:node node})) diff --git a/.clj-kondo/src/hooks/metabase/models/setting.clj b/.clj-kondo/src/hooks/metabase/models/setting.clj index 9e65a8093ae..befb21b19ac 100644 --- a/.clj-kondo/src/hooks/metabase/models/setting.clj +++ b/.clj-kondo/src/hooks/metabase/models/setting.clj @@ -181,12 +181,13 @@ setter-node (-> (list (hooks/token-node 'defn) (with-meta - (hooks/token-node (symbol (str (hooks/sexpr setting-name) \!))) - (meta setting-name)) + (hooks/token-node (symbol (str (hooks/sexpr setting-name) \!))) + (meta setting-name)) (hooks/string-node "Docstring.") (hooks/vector-node [(hooks/token-node '_value-or-nil)])) hooks/list-node - (with-meta (update (meta node) :clj-kondo/ignore #(hooks/vector-node (cons :clojure-lsp/unused-public-var (:children %))))))] + (with-meta (meta node)) + common/add-lsp-ignore-unused-public-var-metadata)] (when (nil? (second (drop-while (comp not #{[:k :export?]} first) options-list))) (when-not (contains? ignored-implicit-export? (:value setting-name)) @@ -242,14 +243,14 @@ (comment (defn- defsetting* [form] (hooks/sexpr - (:node - (defsetting - {:node - (hooks/parse-string - (with-out-str - #_{:clj-kondo/ignore [:unresolved-namespace]} - (clojure.pprint/pprint - form)))})))) + (:node + (defsetting + {:node + (hooks/parse-string + (with-out-str + #_{:clj-kondo/ignore [:unresolved-namespace]} + (clojure.pprint/pprint + form)))})))) (defn x [] (defsetting* diff --git a/.clj-kondo/src/hooks/metabase/public_settings/premium_features.clj b/.clj-kondo/src/hooks/metabase/public_settings/premium_features.clj index 703515c067c..06aa4b4a0a3 100644 --- a/.clj-kondo/src/hooks/metabase/public_settings/premium_features.clj +++ b/.clj-kondo/src/hooks/metabase/public_settings/premium_features.clj @@ -1,6 +1,7 @@ (ns hooks.metabase.public-settings.premium-features (:require - [clj-kondo.hooks-api :as hooks])) + [clj-kondo.hooks-api :as hooks] + [hooks.common])) (defn defenterprise [{node :node}] (let [[_defenterprise fn-name & args] (:children node) @@ -26,7 +27,8 @@ fn-name docstring)) fn-tail)) - (with-meta (update (meta node) :clj-kondo/ignore #(hooks/vector-node (cons :clojure-lsp/unused-public-var (:children %))))))))})) + (with-meta (meta node)) + hooks.common/add-lsp-ignore-unused-public-var-metadata)))})) (comment (defn- defenterprise* [form] diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index cfb85ca74d7..2164800aa9e 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -12,9 +12,6 @@ concurrency: group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }} cancel-in-progress: true -env: - CLJ_KONDO_VERSION: "2023.09.07" - jobs: files-changed: name: Check which files changed @@ -70,23 +67,12 @@ jobs: (needs.files-changed.outputs.backend_all == 'true' || needs.static-viz-files-changed.outputs.static_viz == 'true') runs-on: ubuntu-22.04 timeout-minutes: 10 - env: - DOWNLOAD_URL: https://github.com/clj-kondo/clj-kondo/releases/download steps: - uses: actions/checkout@v4 - name: Prepare back-end environment uses: ./.github/actions/prepare-backend with: m2-cache-key: 'kondo' - - name: Install clj-kondo - run: | - curl -OL ${DOWNLOAD_URL}/v${CLJ_KONDO_VERSION}/clj-kondo-${CLJ_KONDO_VERSION}-linux-static-amd64.zip - curl -OL ${DOWNLOAD_URL}/v${CLJ_KONDO_VERSION}/clj-kondo-${CLJ_KONDO_VERSION}-linux-static-amd64.zip.sha256 - cat clj-kondo-${CLJ_KONDO_VERSION}-linux-static-amd64.zip.sha256 >> SHA256sum.txt - echo " clj-kondo-${CLJ_KONDO_VERSION}-linux-static-amd64.zip" >> SHA256sum.txt - sha256sum -c SHA256sum.txt - unzip -d /usr/local/bin clj-kondo-${CLJ_KONDO_VERSION}-linux-static-amd64.zip - - run: clj-kondo --version - name: Run clj-kondo run: ./bin/kondo.sh diff --git a/bin/kondo-updated.sh b/bin/kondo-updated.sh index 7157b9500e1..e243f1d0083 100755 --- a/bin/kondo-updated.sh +++ b/bin/kondo-updated.sh @@ -21,14 +21,15 @@ fi echo "Linting Clojure source files that have changes compared to $diff_target..." -UPDATED_FILES=$(git diff --name-only "$diff_target" -- '*.clj' '*.cljc' '*.cljs') +# ignore files in the Kondo config directory and dev directory +UPDATED_FILES=$(git diff --name-only "$diff_target" -- '*.clj' '*.cljc' '*.cljs' ':!/.clj-kondo' ':!/dev') if [ -z "$UPDATED_FILES" ]; then echo 'No updated Clojure source files.' exit 0 fi -command="clj-kondo --parallel --lint ${UPDATED_FILES[*]}" +command="clojure -M:kondo --lint ${UPDATED_FILES[*]}" set -x diff --git a/bin/kondo.sh b/bin/kondo.sh index a241744fa68..23113d4550d 100755 --- a/bin/kondo.sh +++ b/bin/kondo.sh @@ -1,24 +1,26 @@ #! /usr/bin/env bash +# Convenience for running clj-kondo against all the appropriate directories. + set -euo pipefail -# Convenience for running clj-kondo against all the appropriate directories. +# make sure we're in the root dir of the metabase repo i.e. the parent dir of the dir this script lives in +script_dir=`dirname "${BASH_SOURCE[0]}"` +cd "$script_dir/.." # Copy over Kondo configs from libraries we use. -clj-kondo --copy-configs --dependencies --lint "$(clojure -A:dev -Spath)" --skip-lint --parallel +clojure -M:kondo --copy-configs --dependencies --lint "$(clojure -A:dev -Spath)" --skip-lint --parallel + +# Clear cache and delete the broken type signatures automatically generated by Malli. rm -rf .clj-kondo/metosin/malli-types-clj/ rm -rf .clj-kondo/.cache # Initialize cache required for `hooks.metabase.test.data/dataset` hook. -clj-kondo --dependencies --lint "test/metabase/test/data/dataset_definitions.clj" +clojure -M:kondo --dependencies --lint "test/metabase/test/data/dataset_definitions.clj" # Run Kondo against all of our Clojure files in the various directories they might live. -find modules/drivers enterprise/backend \ - -maxdepth 2 \ - -type d \ - -name src -or -name test \ - | xargs clj-kondo \ - --parallel \ - --lint bin src test +set -x + +clojure -M:kondo:kondo/all diff --git a/deps.edn b/deps.edn index 13ea61cb2e6..f6400d55a3e 100644 --- a/deps.edn +++ b/deps.edn @@ -235,7 +235,7 @@ {:mvn/version "0.3.0"} ; Enables easy memory measurement clj-http-fake/clj-http-fake {:mvn/version "1.0.4" :exclusions [slingshot/slingshot]} - clj-kondo/clj-kondo {:mvn/version "2024.03.13"} ; this is not for RUNNING kondo, but so we can hack on custom hooks code from the REPL. + clj-kondo/clj-kondo {:mvn/version "2024.08.01"} ; included here mainly to facilitate working on the stuff in `.clj-kondo/src` cloverage/cloverage {:mvn/version "1.2.4"} com.gfredericks/test.chuck {:mvn/version "0.2.14"} ; generating strings from regexes (useful with malli) djblue/portal {:mvn/version "0.56.0"} ; ui for inspecting values @@ -401,6 +401,71 @@ ;;; Linters + ;; clojure -M:kondo --lint src test + ;; + ;; clojure -M:kondo --version + ;; + ;; clojure -M:kondo --copy-configs --dependencies --lint "$(clojure -A:dev -Spath)" --skip-lint --parallel + ;; + ;; Run Kondo from the JVM using the pinned version. Preferable to running the installed command since we can pin the + ;; version here which may be different from the version installed on your computer. + ;; + ;; Use this to only run Kondo against specific files. + :kondo + {:replace-deps + {clj-kondo/clj-kondo {:mvn/version "2024.08.01"}} + + :main-opts + ["-m" "clj-kondo.main"]} + + ;; clojure -M:kondo:kondo/all + ;; + ;; Like the command above but includes arguments so it lints everything. + :kondo/all + {:main-opts + ["-m" "clj-kondo.main" + ;; disabled for now since this seems to cause random failures -- see https://github.com/clj-kondo/clj-kondo/issues/2218 + #_"--parallel" + ;; enable this if Kondo isn't working as expected or if LSP jump-to-definition is being fussy + #_"--debug" + "--lint" + "src" + "test" + "enterprise/backend/src" + "enterprise/backend/test" + "bin/build/src" + "bin/build/test" + "bin/lint-migrations-file/src" + "bin/lint-migrations-file/test" + "bin/release-list/src" + "bin/release-list/test" + "modules/drivers/athena/src" + "modules/drivers/athena/test" + "modules/drivers/bigquery-cloud-sdk/src" + "modules/drivers/bigquery-cloud-sdk/test" + "modules/drivers/druid-jdbc/src" + "modules/drivers/druid-jdbc/test" + "modules/drivers/druid/src" + "modules/drivers/druid/test" + "modules/drivers/mongo/src" + "modules/drivers/mongo/test" + "modules/drivers/oracle/src" + "modules/drivers/oracle/test" + "modules/drivers/presto-jdbc/src" + "modules/drivers/presto-jdbc/test" + "modules/drivers/redshift/src" + "modules/drivers/redshift/test" + "modules/drivers/snowflake/src" + "modules/drivers/snowflake/test" + "modules/drivers/sparksql/src" + "modules/drivers/sparksql/test" + "modules/drivers/sqlite/src" + "modules/drivers/sqlite/test" + "modules/drivers/sqlserver/src" + "modules/drivers/sqlserver/test" + "modules/drivers/vertica/src" + "modules/drivers/vertica/test"]} + ;; clojure -M:ee:drivers:check ;; ;; checks that all the namespaces we actually ship can be compiled, without any dependencies that we don't diff --git a/src/metabase/api/table.clj b/src/metabase/api/table.clj index c3618dac8bd..cd57f53486d 100644 --- a/src/metabase/api/table.clj +++ b/src/metabase/api/table.clj @@ -8,6 +8,7 @@ [metabase.driver.h2 :as h2] [metabase.driver.util :as driver.u] [metabase.events :as events] + [metabase.lib.schema.id :as lib.schema.id] [metabase.models.card :refer [Card]] [metabase.models.database :refer [Database]] [metabase.models.field :refer [Field]] @@ -64,11 +65,11 @@ (t2/hydrate :db :pk_field) fix-schema))) -(defn- update-table!* +(mu/defn ^:private update-table!* "Takes an existing table and the changes, updates in the database and optionally calls `table/update-field-positions!` if field positions have changed." - [{:keys [id] :as existing-table} body] - {id ms/PositiveInt} + [{:keys [id] :as existing-table} :- [:map [:id ::lib.schema.id/table]] + body] (when-let [changes (not-empty (u/select-keys-when body :non-nil [:display_name :show_in_getting_started :entity_type :field_order] :present [:description :caveats :points_of_interest :visibility_type]))] diff --git a/src/metabase/driver/sql_jdbc/execute.clj b/src/metabase/driver/sql_jdbc/execute.clj index c6fc0675d6e..3dfd8cf0ce4 100644 --- a/src/metabase/driver/sql_jdbc/execute.clj +++ b/src/metabase/driver/sql_jdbc/execute.clj @@ -313,8 +313,8 @@ (defn recursive-connection? "Whether or not we are in a recursive call to [[do-with-connection-with-options]]. If we are, you shouldn't set Connection options AGAIN, as that may override previous options that we don't want to override." - [] {:added "0.47.0"} + [] (pos? *connection-recursion-depth*)) (mu/defn do-with-resolved-connection diff --git a/src/metabase/query_processor/context.clj b/src/metabase/query_processor/context.clj index 0d9fb423d76..a98f4e59256 100644 --- a/src/metabase/query_processor/context.clj +++ b/src/metabase/query_processor/context.clj @@ -9,6 +9,6 @@ (defn timeout "DEPRECATED: use [[metabase.query-processor.pipeline/*query-timeout-ms*]] directly instead." - [_context] {:deprecated "0.50.0"} + [_context] qp.pipeline/*query-timeout-ms*) diff --git a/src/metabase/util/honey_sql_2.clj b/src/metabase/util/honey_sql_2.clj index d2b4ccb7dbb..7200305e3d4 100644 --- a/src/metabase/util/honey_sql_2.clj +++ b/src/metabase/util/honey_sql_2.clj @@ -284,8 +284,8 @@ (defn type-info->db-type "For a given type-info, returns the `database-type`." - [type-info] {:added "0.39.0"} + [type-info] (:database-type type-info)) (defn database-type -- GitLab