Fork me on GitHub
#lsp
<
2021-04-02
>
NoahTheDuke16:04:56

maybe a bug? in a clojure file, i have (:require [clojure.string :as string]) in my ns macro, and then hover over (string/join ", " ...) and press my keys for "go to definition". it loads the clojurescript string file in my ~/.m2 directory

NoahTheDuke16:04:57

if i "go to defintion" on clojure core library functions, it goes to the right clojure file

NoahTheDuke16:04:21

even if I [clojure.string :refer [join]], when i "go to definition" on join, it loads ~/.m2/.../clojurescript-1.10.238.jar::clojure/string.cljs

ericdallo16:04:44

hum, could you try removing your .lsp/sqlite.db file and restart the lsp?

ericdallo16:04:40

odd, is your project a lein one?

ericdallo16:04:34

could you run lein classpath on your project root?

NoahTheDuke16:04:53

it's very long lol

ericdallo16:04:14

I just want to know if it's pointing to the correct clojure only classpath

ericdallo16:04:35

is your project clojure only? or clojurec or clojurescript?

ericdallo16:04:54

hum, maybe clojure-lsp is not getting the right one when there are the clojure and cljs available

ericdallo16:04:09

could you confirm if that happens in a minimal repro project?

NoahTheDuke16:04:42

sure, let me see what i can whip up for you

thanks 1
NoahTheDuke16:04:03

if i run lein new minimal and then open the "core.clj" file, it works correctly

ericdallo16:04:42

yeah, I think it'll only happen for projects where it has clojure core and cljs core on classpath

NoahTheDuke16:04:47

if i change the :dependencies to those from my primary project, it shows the error again

ericdallo16:04:37

nice, I think I can try to repro with that

ericdallo16:04:41

I'll let you know, thanks

NoahTheDuke16:04:33

looks like it happens with

:dependencies [[org.clojure/clojure "1.9.0"]
                 [org.clojure/clojurescript "1.10.238"]]

NoahTheDuke16:04:39

thanks so much, and good luck!

NoahTheDuke18:04:42

ayyyyyy! Thank you so much!

๐Ÿ‘ 1
NoahTheDuke16:04:17

I forgot to mention that I'm using vim and coc.nvim, so maybe there's some funny business happening with that configuration

borkdude20:04:10

Can people who use lsp linting (not clj-kondo standalone) try this snippet:

(defmulti foo :bar)
(defmethod foo :test
  [{:keys [a b] :or {a 1 b 2}}]
  (let [q (* 10 (dec a))]
    (+ b q)))
Do you get any warnings?

ericdallo20:04:49

I get both unused-binding on a and b :

borkdude20:04:11

that's bad then, because that's a false positive

borkdude20:04:22

and it isn't happening with vanilla clj-kondo

ericdallo20:04:47

Hum, it seems clj-kondo standalone doesn't print that unless you pass :locals true

clj -Sdeps '{:deps {clj-kondo {:mvn/version "2021.03.31"}}}' -m clj-kondo.main  --lint src/clojure_sample/lsp/definition/a.clj --config '{:output {:analysis {:arglists true :locals true :keywords true} :canonical-paths true}}

/home/greg/dev/clojure-sample/src/clojure_sample/lsp/definition/a.clj:6:22: warning: unused default for binding a
/home/greg/dev/clojure-sample/src/clojure_sample/lsp/definition/a.clj:6:26: warning: unused default for binding b
linting took 156ms, errors: 0, warnings: 2 
clj -Sdeps '{:deps {clj-kondo {:mvn/version "2021.03.31"}}}' -m clj-kondo.main  --lint src/clojure_sample/lsp/definition/a.clj --config '{:output {:analysis {:arglists true :keywords true} :canonical-paths true}}'

linting took 160ms, errors: 0, warnings: 0

borkdude20:04:48

that's interesting

borkdude20:04:55

but analysis data should not have an effect on unused defaults

ericdallo21:04:05

yeah, you are right, must be something else

borkdude21:04:44

Smaller repro:

(fn
  [{:keys [a] :or {a 1}}]
  a)

๐Ÿ‘ 1
borkdude21:04:40

I think it might be in here:

v (cond-> (assoc m
                                          :name s
                                          :filename (:filename ctx)
                                          :tag t)
                             (:analyze-locals? ctx)
                             (-> (assoc :id (swap! (:id-gen ctx) inc)
                                        :str (str expr))
                                 (merge (scope-end scoped-expr))))
So there is a condition if locals are being analyzed or not, and if so, something is added, which may cause some comparison to go wrong

๐Ÿ‘€ 1
borkdude21:04:57

so I think we'll have to pull the reg-binding apart for analysis

borkdude21:04:29

or dissoc some stuff before it's registered into the other bucket

borkdude21:04:08

so I can repro with this:

$ clj-kondo --config '{:output {:analysis {:locals true}}}' --lint /tmp/sean.clj
/tmp/sean.clj:3:20: warning: unused default for binding a

๐Ÿ‘ 1
ericdallo21:04:46

yes, I just used the same args from clojure-lsp, but yeah, it's the locals that cause that

borkdude21:04:45

ok, I'll work on a fix

๐Ÿ™Œ 1
seancorfield21:04:59

Wow! Nice detective work!

ericdallo21:04:08

Thanks! I should bump and release a new clojure-lsp with that fix tomorrow, then @U9A1RLFNV can bump on Calva soon ๐Ÿ™‚

๐Ÿ‘ 2
borkdude21:04:36

@UKFSJSM38 cool. I think people can bump their lsp version manually now in #calva

yes 1
borkdude21:04:51

so as soon as there is a new version, Sean should be able to use it already

pez22:04:29

:male-detective:

borkdude20:04:25

If not, then the problem is #calva specific and not something in the (latest) lsp