diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn index e496e89bd0b19f2e24b97afaded7791b291f008b..4ac114994a342a5459415d696c686aae818a37e4 100644 --- a/.clj-kondo/config.edn +++ b/.clj-kondo/config.edn @@ -1,5 +1,5 @@ ;; -*- comment-column: 100; -*- -{:config-paths ["macros"] +{:config-paths ["macros" "src"] :linters {:aliased-namespace-symbol {:level :warning} :invalid-arity {:skip-args [metabase.lib.util.match/match]} ; TODO (cam): can we fix these? diff --git a/.clj-kondo/hooks/clojure/core.clj b/.clj-kondo/src/hooks/clojure/core.clj similarity index 100% rename from .clj-kondo/hooks/clojure/core.clj rename to .clj-kondo/src/hooks/clojure/core.clj diff --git a/.clj-kondo/hooks/clojure/test.clj b/.clj-kondo/src/hooks/clojure/test.clj similarity index 100% rename from .clj-kondo/hooks/clojure/test.clj rename to .clj-kondo/src/hooks/clojure/test.clj diff --git a/.clj-kondo/hooks/common.clj b/.clj-kondo/src/hooks/common.clj similarity index 96% rename from .clj-kondo/hooks/common.clj rename to .clj-kondo/src/hooks/common.clj index 6b054c931c1f9690e5adeec45ce6340505de396d..625f47440c6f12904d1a384941a3ad06a25cdc89 100644 --- a/.clj-kondo/hooks/common.clj +++ b/.clj-kondo/src/hooks/common.clj @@ -376,8 +376,15 @@ (when (hooks/token-node? node) (let [sexpr (hooks/sexpr node)] (when (symbol? sexpr) - (when-let [resolved (hooks/resolve {:name sexpr})] - (symbol (name (:ns resolved)) (name (:name resolved))))))) + (let [resolved (hooks/resolve {:name sexpr})] + (cond + (and resolved (:ns resolved)) + (symbol (name (:ns resolved)) (name (:name resolved))) + + ;; if it wasn't resolved but is still qualified it's probably using the full namespace name rather than an + ;; alias. + (qualified-symbol? sexpr) + sexpr))))) ;; some symbols like `*count/Integer` aren't resolvable. (catch Exception _ nil))) diff --git a/.clj-kondo/hooks/metabase/api/automagic_dashboards_test.clj b/.clj-kondo/src/hooks/metabase/api/automagic_dashboards_test.clj similarity index 100% rename from .clj-kondo/hooks/metabase/api/automagic_dashboards_test.clj rename to .clj-kondo/src/hooks/metabase/api/automagic_dashboards_test.clj diff --git a/.clj-kondo/hooks/metabase/api/common.clj b/.clj-kondo/src/hooks/metabase/api/common.clj similarity index 100% rename from .clj-kondo/hooks/metabase/api/common.clj rename to .clj-kondo/src/hooks/metabase/api/common.clj diff --git a/.clj-kondo/hooks/metabase/api/search_test.clj b/.clj-kondo/src/hooks/metabase/api/search_test.clj similarity index 100% rename from .clj-kondo/hooks/metabase/api/search_test.clj rename to .clj-kondo/src/hooks/metabase/api/search_test.clj diff --git a/.clj-kondo/hooks/metabase/db/schema_migrations_test/impl.clj b/.clj-kondo/src/hooks/metabase/db/schema_migrations_test/impl.clj similarity index 100% rename from .clj-kondo/hooks/metabase/db/schema_migrations_test/impl.clj rename to .clj-kondo/src/hooks/metabase/db/schema_migrations_test/impl.clj diff --git a/.clj-kondo/hooks/metabase/driver.clj b/.clj-kondo/src/hooks/metabase/driver.clj similarity index 100% rename from .clj-kondo/hooks/metabase/driver.clj rename to .clj-kondo/src/hooks/metabase/driver.clj diff --git a/.clj-kondo/hooks/metabase/legacy_mbql/schemas/macros.clj b/.clj-kondo/src/hooks/metabase/legacy_mbql/schemas/macros.clj similarity index 100% rename from .clj-kondo/hooks/metabase/legacy_mbql/schemas/macros.clj rename to .clj-kondo/src/hooks/metabase/legacy_mbql/schemas/macros.clj diff --git a/.clj-kondo/hooks/metabase/lib/schema/mbql_clause.clj b/.clj-kondo/src/hooks/metabase/lib/schema/mbql_clause.clj similarity index 100% rename from .clj-kondo/hooks/metabase/lib/schema/mbql_clause.clj rename to .clj-kondo/src/hooks/metabase/lib/schema/mbql_clause.clj diff --git a/.clj-kondo/hooks/metabase/lib/test_util/macros.clj b/.clj-kondo/src/hooks/metabase/lib/test_util/macros.clj similarity index 100% rename from .clj-kondo/hooks/metabase/lib/test_util/macros.clj rename to .clj-kondo/src/hooks/metabase/lib/test_util/macros.clj diff --git a/.clj-kondo/hooks/metabase/models/interface.clj b/.clj-kondo/src/hooks/metabase/models/interface.clj similarity index 100% rename from .clj-kondo/hooks/metabase/models/interface.clj rename to .clj-kondo/src/hooks/metabase/models/interface.clj diff --git a/.clj-kondo/hooks/metabase/models/setting.clj b/.clj-kondo/src/hooks/metabase/models/setting.clj similarity index 100% rename from .clj-kondo/hooks/metabase/models/setting.clj rename to .clj-kondo/src/hooks/metabase/models/setting.clj diff --git a/.clj-kondo/hooks/metabase/public_settings/premium_features.clj b/.clj-kondo/src/hooks/metabase/public_settings/premium_features.clj similarity index 100% rename from .clj-kondo/hooks/metabase/public_settings/premium_features.clj rename to .clj-kondo/src/hooks/metabase/public_settings/premium_features.clj diff --git a/.clj-kondo/hooks/metabase/query_processor_test/expressions_test.clj b/.clj-kondo/src/hooks/metabase/query_processor_test/expressions_test.clj similarity index 100% rename from .clj-kondo/hooks/metabase/query_processor_test/expressions_test.clj rename to .clj-kondo/src/hooks/metabase/query_processor_test/expressions_test.clj diff --git a/.clj-kondo/hooks/metabase/test/data.clj b/.clj-kondo/src/hooks/metabase/test/data.clj similarity index 100% rename from .clj-kondo/hooks/metabase/test/data.clj rename to .clj-kondo/src/hooks/metabase/test/data.clj diff --git a/.clj-kondo/hooks/metabase/test/util.clj b/.clj-kondo/src/hooks/metabase/test/util.clj similarity index 100% rename from .clj-kondo/hooks/metabase/test/util.clj rename to .clj-kondo/src/hooks/metabase/test/util.clj diff --git a/.clj-kondo/hooks/metabase/util.clj b/.clj-kondo/src/hooks/metabase/util.clj similarity index 100% rename from .clj-kondo/hooks/metabase/util.clj rename to .clj-kondo/src/hooks/metabase/util.clj diff --git a/.clj-kondo/hooks/metabase/util/log.clj b/.clj-kondo/src/hooks/metabase/util/log.clj similarity index 81% rename from .clj-kondo/hooks/metabase/util/log.clj rename to .clj-kondo/src/hooks/metabase/util/log.clj index 2617e89a5133cc125d9ab86367043654b9596202..61ba11785ab6d706f1687530446e04924fadbc52 100644 --- a/.clj-kondo/hooks/metabase/util/log.clj +++ b/.clj-kondo/src/hooks/metabase/util/log.clj @@ -124,40 +124,3 @@ (check-for-i18n-format-specifiers node format-string) (check-format-string-arg-count node format-string args))) x) - -(comment - ;; should fail, missing a format arg - (infof - {:node (api/parse-string - (str '(log/warnf "Migration lock was acquired after %d retries.")))}) - - (infof - {:node (api/parse-string - (str '(log/warnf e "Migration lock was acquired after %d retries.")))}) - - ;; should fail, too many format args - (infof - {:node (api/parse-string - (str '(log/warnf "Migration lock was acquired after %d retries." 1 2)))}) - - (infof - {:node (api/parse-string - (str '(log/warnf e "Migration lock was acquired after %d retries." 1 2)))}) - - (infof - {:node (api/parse-string - (str '(log/warnf "Migration lock was acquired after {0} retries." 1)))}) - - ;; should fail, has format args but is warn rather than warnf - (info - {:node (api/parse-string - (str '(log/warn e "Migration lock was acquired after %d retries." 1 2)))}) - - ;; should fail -- should not use i18n/tru or i18n/trs - (infof - {:node (api/parse-string - (str '(log/warnf (metabase.util.i18n/tru "Migration lock was acquired after {0} retries." 1))))}) - - (info - {:node (api/parse-string - (str '(log/warn (metabase.util.i18n/trs "Migration lock was acquired after {0} retries." 1))))})) diff --git a/.clj-kondo/hooks/metabase/util/malli/registry.clj b/.clj-kondo/src/hooks/metabase/util/malli/registry.clj similarity index 100% rename from .clj-kondo/hooks/metabase/util/malli/registry.clj rename to .clj-kondo/src/hooks/metabase/util/malli/registry.clj diff --git a/.clj-kondo/hooks/toucan/util/test.clj b/.clj-kondo/src/hooks/toucan/util/test.clj similarity index 100% rename from .clj-kondo/hooks/toucan/util/test.clj rename to .clj-kondo/src/hooks/toucan/util/test.clj diff --git a/.clj-kondo/test/hooks/common_test.clj b/.clj-kondo/test/hooks/common_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..7b6d697975b990bc2fe0413fa7457984bf1ab84d --- /dev/null +++ b/.clj-kondo/test/hooks/common_test.clj @@ -0,0 +1,19 @@ +(ns ^:mb/once hooks.common-test + (:require + [clj-kondo.hooks-api :as api] + [clj-kondo.impl.utils] + [clojure.test :refer :all] + [hooks.common])) + +(deftest ^:parallel node->qualified-symbol-test + (binding [clj-kondo.impl.utils/*ctx* {:namespaces (atom nil)}] + (is (= 'metabase.util.i18n/tru + (hooks.common/node->qualified-symbol (api/parse-string "metabase.util.i18n/tru")))))) + +(deftest ^:parallel node->qualified-symbol-test-2 + (binding [clj-kondo.impl.utils/*ctx* {:namespaces (atom {:clj {:clj '{hooks.common-test {:qualify-ns {i18n metabase.util.i18n}}}}}) + :lang :clj + :base-lang :clj + :ns {:name 'hooks.common-test}}] + (is (= 'metabase.util.i18n/tru + (hooks.common/node->qualified-symbol (api/parse-string "i18n/tru")))))) diff --git a/.clj-kondo/test/hooks/metabase/util/log_test.clj b/.clj-kondo/test/hooks/metabase/util/log_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..5259c320a09c10546695847607ff015e7b5d1d05 --- /dev/null +++ b/.clj-kondo/test/hooks/metabase/util/log_test.clj @@ -0,0 +1,77 @@ +(ns ^:mb/once hooks.metabase.util.log-test + (:require + [clj-kondo.hooks-api :as api] + [clj-kondo.impl.utils] + [clojure.string :as str] + [clojure.test :refer :all] + [hooks.metabase.util.log])) + +(defn- warnings + [form] + (let [f (if (str/ends-with? (name (first form)) "f") + hooks.metabase.util.log/infof + hooks.metabase.util.log/info)] + (binding [clj-kondo.impl.utils/*ctx* {:config {:linters {:metabase/validate-logging {:level :warning}}} + :ignores (atom nil) + :findings (atom []) + :namespaces (atom {})}] + (f {:node (api/parse-string (pr-str form))}) + (mapv :message @(:findings clj-kondo.impl.utils/*ctx*))))) + +(deftest ^:parallel warn-on-missing-format-args-test + (testing "should fail, missing a format arg" + (are [form] (= ["log format string expects 1 arguments instead of 0."] + (warnings (quote form))) + (metabase.util.log/warnf "Migration lock was acquired after %d retries.") + (metabase.util.log/warnf e "Migration lock was acquired after %d retries.")))) + +(deftest ^:parallel warn-on-too-many-format-args-test-1 + (testing "should fail, too many format args" + (are [form] (= ["log format string expects 1 arguments instead of 2."] + (warnings (quote form))) + (metabase.util.log/warnf "Migration lock was acquired after %d retries." 1 2) + (metabase.util.log/warnf e "Migration lock was acquired after %d retries." 1 2)))) + +(deftest ^:parallel warn-on-too-many-format-args-test-2 + (testing "should fail, too many format args" + (are [form] (= ["this looks like an i18n format string. Don't use identifiers like {0} in logging." + "Don't use metabase.util.log/warnf with no format string arguments, use metabase.util.log/warn instead." + "log format string expects 0 arguments instead of 1."] + (warnings (quote form))) + (metabase.util.log/warnf "Migration lock was acquired after {0} retries." 1) + (metabase.util.log/warnf e "Migration lock was acquired after {0} retries." 1)))) + +(deftest ^:parallel warn-on-format-args-without-logf-test + (testing "should fail, has format args but is warn rather than warnf" + (are [form] (= ["metabase.util.log/warn used with a format string, use metabase.util.log/warnf instead."] + (warnings (quote form))) + (metabase.util.log/warn "Migration lock was acquired after %d retries." 1) + (metabase.util.log/warn e "Migration lock was acquired after %d retries." 1)))) + +(deftest ^:parallel warn-on-format-without-logf-test + (testing "should fail, has format args but is warn rather than warnf" + (are [form] (= ["Use metabase.util.log/warnf instead of metabase.util.log/warn + format"] + (warnings (quote form))) + (metabase.util.log/warn (format "Migration lock was acquired after %d retries." 1)) + (metabase.util.log/warn e (format "Migration lock was acquired after %d retries." 1))))) + +(deftest ^:parallel warn-on-logf-with-no-format-args-test + (testing "should fail, has format args but is warn rather than warnf" + (are [form] (= ["Don't use metabase.util.log/warnf with no format string arguments, use metabase.util.log/warn instead."] + (warnings (quote form))) + (metabase.util.log/warnf "Migration lock cleared.") + (metabase.util.log/warnf e "Migration lock cleared.")))) + +(deftest ^:parallel warn-on-i18n-test-1 + (testing "should fail -- should not use i18n/tru or i18n/trs" + (are [form] (= ["do not i18n the logs!"] + (warnings (quote form))) + (metabase.util.log/warnf (metabase.util.i18n/tru "Migration lock was acquired after {0} retries." 1)) + (metabase.util.log/warnf e (metabase.util.i18n/tru "Migration lock was acquired after {0} retries." 1))))) + +(deftest ^:parallel warn-on-i18n-test-2 + (testing "should fail -- should not use i18n/tru or i18n/trs" + (are [form] (= ["do not i18n the logs!"] + (warnings (quote form))) + (metabase.util.log/warn (metabase.util.i18n/trs "Migration lock was acquired after {0} retries." 1)) + (metabase.util.log/warn e (metabase.util.i18n/trs "Migration lock was acquired after {0} retries." 1))))) diff --git a/.gitignore b/.gitignore index 726a6f243a76090e6e4045918aec0b291e70bb33..558e60c2072acdbc329fe4f9db7c3d9d1c5df476 100644 --- a/.gitignore +++ b/.gitignore @@ -96,7 +96,8 @@ modules/drivers/*/.lsp/* .clj-kondo/* !.clj-kondo/README.md !.clj-kondo/config.edn -!.clj-kondo/hooks/ +!.clj-kondo/src/ +!.clj-kondo/test/ !.clj-kondo/macros/ # Editor- or environment-specific local files diff --git a/deps.edn b/deps.edn index 519015685fc8306aaff4762d1b7a31da5daa55f9..8dfa5fc995d2b4c91bdcec2fbc69525387c1c747 100644 --- a/deps.edn +++ b/deps.edn @@ -254,7 +254,7 @@ ring/ring-mock {:mvn/version "0.4.0"} ; creating Ring request maps for testing purposes talltale/talltale {:mvn/version "0.5.14"}} ; generates fake data, useful for prototyping or load testing - :extra-paths ["dev/src" "local/src" "test" "test_resources"] + :extra-paths ["dev/src" "local/src" "test" "test_resources" ".clj-kondo/src" ".clj-kondo/test"] :jvm-opts ["-Dmb.run.mode=dev" "-Dmb.field.filter.operators.enabled=true" "-Dmb.test.env.setting=ABCDEFG" @@ -619,9 +619,9 @@ :extra-deps {org.clojure/data.json {:mvn/version "2.5.0"}}} - ;;; Run tests for the build scripts: - ;;; - ;;; clj -X:dev:drivers:build:build-dev:build-test + ;; Run tests for the build scripts: + ;; + ;; clj -X:dev:drivers:build:build-dev:build-test :build-test {:exec-fn mb.hawk.core/find-and-run-tests-cli :exec-args {:only ["bin/build/test"]}} @@ -703,6 +703,12 @@ {:exec-args {:only [metabase.api.search-test "test/metabase/search"]}} + ;; test custom clj-kondo linters and hooks. + ;; + ;; clj -X:dev:test:test/kondo + :test/kondo + {:exec-args {:only [".clj-kondo/test"]}} + ;; watch and reload clojure namespaces :watch {:extra-deps {com.nextjournal/beholder {:mvn/version "1.0.2"} ;; watcher diff --git a/src/metabase/util/jvm.clj b/src/metabase/util/jvm.clj index 1527ba10a12bff08f150a1cf4a8c49a0c4425551..458007725e950a2fab5d0f7f97ba20d7f99b3941 100644 --- a/src/metabase/util/jvm.clj +++ b/src/metabase/util/jvm.clj @@ -315,5 +315,5 @@ (if (>= elapsed-time timeout-ms) nil ; timeout reached (do - (Thread/sleep interval-ms) + (Thread/sleep (long interval-ms)) (recur))))))))) diff --git a/test/metabase/test_runner.clj b/test/metabase/test_runner.clj index 2c3d280750588fbd4dbfb7dbbcdc88bc7bf34829..ee4b19d4152e91fcb5388287536eb54eba11c21a 100644 --- a/test/metabase/test_runner.clj +++ b/test/metabase/test_runner.clj @@ -82,7 +82,7 @@ "test_resources"]) (defn- default-options [] - {:namespace-pattern #"^metabase.*" + {:namespace-pattern #"^(?:(?:metabase.*)|(?:hooks\..*))" ; anything starting with `metabase*` (including `metabase-enterprise`) or `hooks.*` :exclude-directories excluded-directories :test-warn-time 3000})