From caa47dcc4ad351f0b660aab7c5ecd9643ddba942 Mon Sep 17 00:00:00 2001 From: Case Nelson <case@metabase.com> Date: Fri, 9 Dec 2022 09:50:04 -0700 Subject: [PATCH] Add meta to kondo hook tokens (#27099) Kondo made a recent change to try deriving positional information for introduced tokens, unfortunately it is rarely accurate and can make working with our macros difficult in lsp. --- .clj-kondo/hooks/common.clj | 6 +++++ .clj-kondo/hooks/metabase/api/common.clj | 23 +++++++++++--------- .clj-kondo/hooks/metabase/models/setting.clj | 5 +++-- .clj-kondo/hooks/metabase/test/util.clj | 18 ++++++++------- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/.clj-kondo/hooks/common.clj b/.clj-kondo/hooks/common.clj index fa3bef8380e..3b5636f8051 100644 --- a/.clj-kondo/hooks/common.clj +++ b/.clj-kondo/hooks/common.clj @@ -2,6 +2,12 @@ (:require [clj-kondo.hooks-api :as hooks] [clojure.pprint])) +(defn with-macro-meta + "When introducing internal nodes (let, defn, etc) it is important to provide a meta of an existing token + as the current version of kondo will use the whole form by default." + [new-node hook-node] + (with-meta new-node (meta (first (:children hook-node))))) + ;;; This stuff is to help debug hooks. Trace a function and it will pretty-print the before & after sexprs. ;;; ;;; (hooks.common/trace #'calculate-bird-scarcity) diff --git a/.clj-kondo/hooks/metabase/api/common.clj b/.clj-kondo/hooks/metabase/api/common.clj index a8ac33d7b31..a7272ae3463 100644 --- a/.clj-kondo/hooks/metabase/api/common.clj +++ b/.clj-kondo/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])) + [clj-kondo.hooks-api :as hooks] + [clojure.string :as str] + [hooks.common :as common])) (defn route-fn-name [method route] @@ -13,11 +14,13 @@ (defn defendpoint [{:keys [node]}] (let [[method route & body] (rest (:children node))] {:node - (api/list-node [(api/token-node 'do) - ;; register usage of compojure core var - (api/token-node (symbol (str "compojure.core") - (str method))) - ;; define function with route-fn-name - (api/list-node (list* (api/token-node 'clojure.core/defn) - (route-fn-name (api/sexpr method) (api/sexpr route)) - body))])})) + (hooks/vector-node [;; register usage of compojure core var + (common/with-macro-meta (hooks/token-node (symbol (str "compojure.core") + (str method))) + node) + ;; define function with route-fn-name + (hooks/list-node + (list* (common/with-macro-meta (hooks/token-node 'clojure.core/defn) + node) + (route-fn-name (hooks/sexpr method) (hooks/sexpr route)) + body))])})) diff --git a/.clj-kondo/hooks/metabase/models/setting.clj b/.clj-kondo/hooks/metabase/models/setting.clj index fba086662bb..bc56be64080 100644 --- a/.clj-kondo/hooks/metabase/models/setting.clj +++ b/.clj-kondo/hooks/metabase/models/setting.clj @@ -1,5 +1,6 @@ (ns hooks.metabase.models.setting - (:require [clj-kondo.hooks-api :as hooks])) + (:require [clj-kondo.hooks-api :as hooks] + [hooks.common :as common])) (defn defsetting "Rewrite a [[metabase.models.defsetting]] form like @@ -38,7 +39,7 @@ {:node (-> (list (hooks/token-node 'let) ;; include description and the options map so they can get validated as well. - (hooks/vector-node (vec (interleave (repeat (hooks/token-node '_)) + (hooks/vector-node (vec (interleave (repeat (common/with-macro-meta (hooks/token-node '_) node)) args))) getter-node setter-node) diff --git a/.clj-kondo/hooks/metabase/test/util.clj b/.clj-kondo/hooks/metabase/test/util.clj index c44e0f2a98d..01938afc88e 100644 --- a/.clj-kondo/hooks/metabase/test/util.clj +++ b/.clj-kondo/hooks/metabase/test/util.clj @@ -1,5 +1,6 @@ (ns hooks.metabase.test.util - (:require [clj-kondo.hooks-api :as hooks])) + (:require [clj-kondo.hooks-api :as hooks] + [hooks.common :as common])) (defn- namespaced-symbol-node? [node] (when (hooks/token-node? node) @@ -20,16 +21,17 @@ for analysis purposes. We only need to 'capture' namespaced Setting bindings with a `_` so Kondo considers their namespace to be 'used' and to catch undefined var usage." - [{{[_ bindings & body] :children} :node}] - (let [bindings (if (hooks/vector-node? bindings) + [{:keys [node]}] + (let [{[_ bindings & body] :children} node + bindings (if (hooks/vector-node? bindings) (hooks/vector-node (into [] (mapcat (fn [[setting-name v]] (concat - [(hooks/token-node '_) v] - ;; if the setting name is namespace-qualified add a `_` - ;; entry for it too. - (when (namespaced-symbol-node? setting-name) - [(hooks/token-node '_) setting-name])))) + [(with-meta (hooks/token-node '_) (meta setting-name)) v] + ;; if the setting name is namespace-qualified add a `_` + ;; entry for it too. + (when (namespaced-symbol-node? setting-name) + [(with-meta (hooks/token-node '_) (meta setting-name)) setting-name])))) (partition 2 (:children bindings)))) bindings)] {:node (-> (list* -- GitLab