Skip to content

Commit

Permalink
Fix clj-kondo#2391: support :discouraged-var on global js values (clj…
Browse files Browse the repository at this point in the history 
…-kondo#2331)
  • Loading branch information
@borkdude
borkdude authored May 22, 2024
1 parent a41a91e commit f8a5909
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 53 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ For a list of breaking changes, check [here](#breaking-changes).
- [#2227](https://github.com/clj-kondo/clj-kondo/issues/2227): allow `:flds` to be used in keys destructuring for ClojureDart
- [#2316](https://github.com/clj-kondo/clj-kondo/issues/2316): handle ignore hint on protocol method
- [#2322](https://github.com/clj-kondo/clj-kondo/issues/2322): add location to warning about invalid unicode character
- [#2391](https://github.com/clj-kondo/clj-kondo/issues/2391): support `:discouraged-var` on global JS values, like `js/fetch`

## 2024.03.13

Expand Down
78 changes: 30 additions & 48 deletions src/clj_kondo/impl/linters.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,23 @@
(set (rest fst-sexpr)))]
(when init
(when-let
[case-expr
(let [c (first
(reduce
(fn [acc sexpr]
(if (=? sexpr)
(let [new-acc
(set/intersection acc
(set (rest sexpr)))]
(if (= 1 (count new-acc))
new-acc
(reduced nil)))
(if (= :else sexpr)
acc
(reduced nil))))
init
rest-sexprs))]
c)]
[case-expr
(let [c (first
(reduce
(fn [acc sexpr]
(if (=? sexpr)
(let [new-acc
(set/intersection acc
(set (rest sexpr)))]
(if (= 1 (count new-acc))
new-acc
(reduced nil)))
(if (= :else sexpr)
acc
(reduced nil))))
init
rest-sexprs))]
c)]
(findings/reg-finding!
(node->line filename expr :warning :cond-as-case
(format "cond can be written as (case %s ...)"
Expand Down Expand Up @@ -132,10 +132,10 @@
([clojure.core cond] [cljs.core cond])
(lint-cond ctx (:expr call))
([clojure.core if-let] [clojure.core if-not] [clojure.core if-some]
[cljs.core if-let] [cljs.core if-not] [cljs.core if-some])
[cljs.core if-let] [cljs.core if-not] [cljs.core if-some])
(lint-missing-else-branch ctx (:expr call))
([clojure.core get-in] [clojure.core assoc-in] [clojure.core update-in]
[cljs.core get-in] [cljs.core assoc-in] [cljs.core update-in])
[cljs.core get-in] [cljs.core assoc-in] [cljs.core update-in])
(lint-single-key-in ctx called-name (:expr call))
#_([clojure.test is] [cljs.test is])
#_(lint-test-is ctx (:expr call))
Expand Down Expand Up @@ -365,25 +365,7 @@
fn-sym (symbol (str resolved-ns)
(str fn-name))
_
(let [discouraged-var-config
(get-in (:config call) [:linters :discouraged-var])]
(when-not (or (identical? :off (:level discouraged-var-config))
(empty? (dissoc discouraged-var-config :level)))
(let [candidates (cons (symbol (str resolved-ns) (str fn-name))
(map #(symbol (str %) (str fn-name))
(config/ns-groups call-config resolved-ns filename)))]
(doseq [fn-lookup-sym candidates]
(when-let [cfg (get discouraged-var-config fn-lookup-sym)]
(when-not (identical? :off (:level cfg))
(findings/reg-finding! ctx {:filename filename
:level (:level cfg)
:row row
:end-row end-row
:col col
:end-col end-col
:type :discouraged-var
:message (or (:message cfg)
(str "Discouraged var: " fn-sym))})))))))]
(namespace/lint-discouraged-var! ctx call-config resolved-ns fn-name filename row end-row col end-col fn-sym)]
:when valid-call?
:let [fn-name (:name called-fn)
_ (when (and ;; unresolved?
Expand Down Expand Up @@ -455,14 +437,14 @@
(str fn-sym))}))
(when-let [deprecated (:deprecated called-fn)]
(when-not
(or
(or
;; recursive call
recursive?
(utils/linter-disabled? call :deprecated-var)
(config/deprecated-var-excluded
(:config call)
fn-sym
caller-ns-sym in-def))
recursive?
(utils/linter-disabled? call :deprecated-var)
(config/deprecated-var-excluded
(:config call)
fn-sym
caller-ns-sym in-def))
(findings/reg-finding! ctx
{:filename filename
:row row
Expand Down Expand Up @@ -591,9 +573,9 @@
config (:config v)
ctx (assoc ctx :config config)]
(when-not
(or (contains? used-referred-vars k)
(config/unused-referred-var-excluded config var-ns k)
(contains? refer-all-nss var-ns))
(or (contains? used-referred-vars k)
(config/unused-referred-var-excluded config var-ns k)
(contains? refer-all-nss var-ns))
(let [filename (:filename v)
referred-ns (export-ns-sym var-ns)]
(findings/reg-finding!
Expand Down
50 changes: 45 additions & 5 deletions src/clj_kondo/impl/namespace.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,27 @@
(format "Namespace only aliased but wasn't loaded: %s"
ns-sym))))))))))

(defn lint-discouraged-var! [ctx call-config resolved-ns fn-name filename row end-row col end-col fn-sym]
(let [discouraged-var-config
(get-in call-config [:linters :discouraged-var])]
(when-not (or (identical? :off (:level discouraged-var-config))
(empty? (dissoc discouraged-var-config :level)))
(let [candidates (cons (symbol (str resolved-ns) (str fn-name))
(map #(symbol (str %) (str fn-name))
(config/ns-groups call-config resolved-ns filename)))]
(doseq [fn-lookup-sym candidates]
(when-let [cfg (get discouraged-var-config fn-lookup-sym)]
(when-not (identical? :off (:level cfg))
(findings/reg-finding! ctx {:filename filename
:level (:level cfg)
:row row
:end-row end-row
:col col
:end-col end-col
:type :discouraged-var
:message (or (:message cfg)
(str "Discouraged var: " fn-sym))}))))))))

(defn resolve-name
[ctx call? ns-name name-sym expr]
(let [lang (:lang ctx)
Expand Down Expand Up @@ -639,9 +660,9 @@
(when alias?
(reg-used-alias! ctx ns-name ns-sym))
(cond->
{:ns ns*
:name var-name
:interop? (and cljs? (boolean interop))}
{:ns ns*
:name var-name
:interop? (and cljs? (boolean interop))}
alias?
(assoc :alias ns-sym)
core?
Expand Down Expand Up @@ -675,11 +696,20 @@
:unresolved-ns ns-sym})
(if cljs?
;; see https://github.com/clojure/clojurescript/blob/6ed949278ba61dceeafb709583415578b6f7649b/src/main/clojure/cljs/analyzer.cljc#L781
(when-not (one-of ns* ["js" "goog"
(if-not (one-of ns* ["js" "goog"
"Math" "String"])
{:name (symbol (name name-sym))
:unresolved? true
:unresolved-ns ns-sym})
:unresolved-ns ns-sym}
(let [var-name (-> (str/split (name name-sym) #"\." 2) first symbol)
{:keys [row end-row col end-col]} (meta expr)]
;; interop calls don't make it into linters/lint-var-usage,
;; this is why we call discouraged var here
(lint-discouraged-var! ctx (:config ctx) ns-sym var-name
(:filename ctx)
row end-row col end-col
(symbol (str ns-sym) ns*))
nil))
{:name (symbol (name name-sym))
:unresolved? true
:unresolved-ns ns-sym}))))
Expand Down Expand Up @@ -767,6 +797,16 @@
:allow-forward-reference? (:in-comment ctx)
:clojure-excluded? clojure-excluded?})))))))))))))

#_
(do
(def resolve-name* resolve-name)

(defn resolve-name [ctx call? ns-name name-sym expr]
(prn :resolve)
(let [x (resolve-name* ctx call? ns-name name-sym expr)]
(prn x)
x)))

;;;; Scratch

(comment)
11 changes: 11 additions & 0 deletions test/clj_kondo/discouraged_var_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,14 @@
:message "Discouraged var: foo/bar"}]
(lint! "(ns foo) (defn bar []) (bar)"
'{:linters {:discouraged-var {foo/bar {:level :error}}}})))

(deftest js-test
(assert-submaps2
'({:file "<stdin>", :row 1, :col 1, :level :warning, :message "Use web.http/js-fetch instead"}
{:file "<stdin>", :row 2, :col 1, :level :warning, :message "Use web.http/js-fetch instead"}
{:file "<stdin>", :row 3, :col 1, :level :warning, :message "Use web.http/js-fetch instead"})
(lint! "js/fetch
(js/fetch)
(js/fetch.foo)"
'{:linters {:discouraged-var {js/fetch {:message "Use web.http/js-fetch instead"}}}}
"--lang" "cljs")))

0 comments on commit f8a5909

Please # to comment.