Fork me on GitHub
#lsp
<
2022-05-14
>
lassemaatta12:05:02

if I run lsp-find-implementation (or -definition) on a macro call in a clojurescript file (e.g. something like (reagent.core/with-let [...) I get LSP :: Not found for: <name of macro>. Is this normal and expected, a bug, or a user/configuration error?

lassemaatta12:05:11

docstrings don't seem to work either

lassemaatta12:05:32

on the other hand stuff from cljs.core.cljc seem to resolve ok, stuff like or.

ericdallo13:05:23

It should work I think, any simple repro so I can try?

lassemaatta13:05:51

Sure, I’ll try if I can create a minimal repro.

lassemaatta10:05:31

basically just lein new reagent <name of app>

lassemaatta10:05:07

note the {:lint-as {reagent.core/with-let clojure.core/let}}

lassemaatta10:05:40

lsp-mode , Emacs 29.0.50 + clojure-lsp 2022.05.03-12.35.40

lassemaatta10:05:54

also, note that navigating to reagent.core takes me to reagent.core.cljs, in which that CLJS namespace requires the macros with (:require-macros [reagent.core]) from reagent.core.clj . I can't even figure out how can I navigate to that reagent.core.clj file.

lassemaatta10:05:40

I think this kind of "have a clj and cljs version of the same namespace for macros" approach is described in https://code.thheller.com/blog/shadow-cljs/2019/10/12/clojurescript-macros.html, if it's somehow related to the problem

ericdallo15:05:43

I can repro that issue, it seems to me a clj-kondo issue where clj-kondo doesn't find the var-definitions exposed by require-macros If I understood it correctly. • the cljs ns macro-repro.core requires [reagent.core :as reagent] • the cljs ns reagent.core doesn't have a with-let because it's from clj ns reagent.core and it's imported with (:require-macros [reagent.core]) Does that makes sense for you @U04V15CAJ? should we create a issue on kondo with that minimal repro? I think we should have var-definitions of the clj required-macro ns, on the cljs ns, right?

ericdallo15:05:05

I think we can summarize as: clj-kondo doesn't define var-definitions of :require-macros , it should have a reagent.core/with-let of that cljs ns

borkdude15:05:40

The macro occurs in a .clj file so clj-kondo registers that in var-definitions

ericdallo15:05:06

shouldn't we have a var-definition on the ragent.corecljs file as well?

borkdude15:05:17

No, since it's not a var in a .cljs file

ericdallo15:05:06

hum, but when we call reagent.core/with-let , isn't referencing a var-definition on that cljs ns?

borkdude15:05:34

No, it is referencing the macro in JVM Clojure and it then emits clojurescript code

ericdallo15:05:36

Otherwise clojure-lsp would need to check all require-macros when a find-definition is not found, sounds weird to me

borkdude15:05:01

That's not weird, that's how it should work

ericdallo15:05:04

so a custom cljs code is created with sometihing lke a var that can be called right?

borkdude15:05:53

The macro runs in the JVM

borkdude15:05:17

it does not generate a reagent.core/with-let var in ClojureScript (unless you define it in a .cljc file, there are some edge cases)

borkdude15:05:47

so in general you should keep on looking for a var with the same name in the .clj side of things

ericdallo15:05:36

right, but we could have something from kondo that could help identifying that, otherwise seems to me clojure-lsp would be a little bit blind and need to check multiple places for each find definition of a cljs usage

ericdallo15:05:42

maybe something on context, or something that helps saying: that macro comes from this other ns

borkdude15:05:10

perhaps yes

borkdude15:05:48

Not sure it's that simple. I think it usually works correctly if you use the .clj var as a fallback

ericdallo15:05:20

does the name would be always the same?

ericdallo15:05:59

I mean, would be always exist a var with the same name?

borkdude15:05:18

yes, fall back on the clj var with the same ns + name + macro true

lassemaatta15:05:32

(Just curious: what do you mean with ”the clj var” wrt a macro?)

borkdude15:05:58

@U0178V2SLAY The macro var in the JVM

lassemaatta15:05:03

Ah, defmacro creates a var. Never really realized that :)

lassemaatta15:05:56

Thanks a lot to both of you for looking into this 👍

👍 1
ericdallo15:05:54

borkdude’s suggestion of falling back to the var on clj works, just not sure if will work for all use cases of :require-macros . For example, I don't think it would work for (ns foo (:require-macros [bar])) because the ns would be different, right?

borkdude15:05:49

(:require-macros [bar]) The calls would then look like:

(bar/macro ...)

borkdude15:05:03

so they would already refer to the namespace bar

borkdude15:05:14

and then you can fall back on the .clj namespace bar

ericdallo15:05:01

oh, didn't know that works. So one would not be able to (foo/my-macro) ? because the definition is really on bar ?

borkdude15:05:28

:require-macros is basically the same as :require but for macros

ericdallo15:05:43

Got it, alright, will try to fix it on clojure-lsp side so, thanks for the help and info @U04V15CAJ @U0178V2SLAY

borkdude15:05:43

it is not like :refer-all

👍 1
ericdallo16:05:16

@U0178V2SLAY it should be fixed and available in some minutes on #clojure-lsp-builds, LMK if any issues

lassemaatta17:05:54

Awesome, thanks a lot! I’ll test it tomorrow and report back

👍 1
lassemaatta05:05:34

Seems to be working great! 🎉

🎉 1
witek19:05:59

Hello. How can I run the new move-form feature from Emacs with lsp-mode? It does not show up in the list when I run lsp-execute-code-action.

ericdallo19:05:06

It was renamed to drag, you can find as "drag ..." In the code actions

witek19:05:22

No, I mean moving a function to a different namespace. This one: https://github.com/clojure-lsp/clojure-lsp/pull/871

ericdallo19:05:00

Oh, right, this one is available only as commands for now, I I tend to add support to code actions soon

ericdallo19:05:22

In lsp-mode it's called lsp-clojure-move-form I think, let me double check

witek19:05:25

Nothing there for me 😞

ericdallo19:05:49

You probably need to update ls-mode, even so, I just found that I accidentally committed a hardcoded path 😂 https://github.com/emacs-lsp/lsp-mode/blob/master/clients/lsp-clojure.el#L224

witek19:05:36

Thank you! 🙏

ericdallo19:05:03

@U2ERGD6UD There was a bug on clojure-lsp side as well, I fixed right now both on lsp-mode and clojure-lsp so to use it you will need to update lsp-mode + use a clojure-lsp from #clojure-lsp-builds if you want.