Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clojure-LSP doesn't seem to find broch.core/meters #7

Closed
teodorlu opened this issue Oct 19, 2023 · 3 comments
Closed

Clojure-LSP doesn't seem to find broch.core/meters #7

teodorlu opened this issue Oct 19, 2023 · 3 comments

Comments

@teodorlu
Copy link

teodorlu commented Oct 19, 2023

Hi!

My clojure-lsp setup doesn't seem to be able to find the definition for broch.core/meters (and more) variables.

broch.core/meters works perfectly in the REPL. And broch.core/meters looks like a normal def, so I'd expect this to just work.

This is what I'm seeing, lint error for broch.core/meters and no completion for meters (but completion for other symbols!).

image image

Broch version:

        no.anteo/broch {:mvn/version "0.1.83"}

Screenshots were produced with this LSP+clj-kondo version:

 :server-version "2023.08.06-00.33.37-nightly",
 :clj-kondo-version "2023.07.14-SNAPSHOT",

(up-to-date versions provided by Doom Emacs (clojure +lsp) as of 2023-10-19).


For all that I can see, this is more likely to indicate an issue in Clojure-LSP than broch. But I thought I'd ask if you guys have any ideas first -- because you know the broch codebase.

I think it's weird that some vars (like b/* og b/max) were found without issues, whereas other vars (like b/meters) were not found. Perhaps a first step is to try reproduce the issue.

Teodor

@teodorlu
Copy link
Author

teodorlu commented Oct 19, 2023

If you don't have any immediate ideas, asking in #lsp on Slack might be a good plan B.

@2food
Copy link
Contributor

2food commented Oct 19, 2023

Hi @teodorlu

I think this is because the quantitity functions are defined with a special macro broch.core/defunit. This does mean that static analysis like lsp doesn't know that it's def-ed (unless you tell it that defunit can def things like def. (I know you can do this with kondo, but I don't know about lsp)).

But luckily your timing couldn't be better, because I've actually just changed this to use the regular defs, because the macro caused issues with cljs compatibility. So your issue should go away because of it.

You can use the latest git sha if you like, or just wait until the next release. (which I plan to do today!)

@2food 2food closed this as completed Oct 19, 2023
@teodorlu
Copy link
Author

teodorlu commented Oct 19, 2023

Oh, amazing!

I realize that a source of my confusion was that I was:

  1. Using the latest maven release in my code
  2. But I was reading your latest code on master, where meters is just a def.

Thanks for making this, and thanks for supporting static analysis! Looking forward to the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants