BetterThanTomorrow / calva

Clojure & ClojureScript Interactive Programming for VS Code
https://marketplace.visualstudio.com/items?itemName=betterthantomorrow.calva
Other
1.62k stars 212 forks source link

Go to definition sometimes works for Java sources #1436

Open Cyrik opened 2 years ago

Cyrik commented 2 years ago
calva v2.0.229 and calva 2.0.230
nrepl: 0.8.3
cider-nrepl: 0.26.0
cider/piggieback: 0.5.2
clojure-lsp version used: 2021.12.01-12.28.16
clj-kondo version used: 2021.10.20-SNAPSHOT
(def testing (.toUpperCase "test")) ;;always broken
(doto (new java.util.HashMap) (.put "a" 1) (.put "b" 2)) ;;HashMap sometimes works, Map also sometimes works, but less often?
(System/getProperties) ;; also worked for a bit

It’s the strangest thing, it just worked for two things, then I wanted to check my calva version, went back to the file, and its broke for everything again, without a reload. Lsp logs don't show any problems. After reloading with calva 2.0.230 System works again while the rest is broken.

bpringe commented 2 years ago

This feature doesn't work for me either. To be clear, I think it only should work with a repl connection, and not via clojure-lsp. Is that right @ericdallo?

I did some brief investigating and found I have a src.zip file inside my $JAVA_HOME/lib directory. This doesn't seem to help, though.

I've never really needed this feature, so I've never looked much into making it work. Maybe someone else can provide us more info here. I'll post in the Slack channel.

bpringe commented 2 years ago

Related information, just to centralize the info here: https://github.com/clojure-emacs/orchard/issues/103#issuecomment-764936527.

pratik97 commented 2 years ago

I am on v2.0.231 and these things are working for me with a REPL connected:

(:import java.time.LocalDateTime)
(doto (new java.util.HashMap) (.put "a" 1) (.put "b" 2)) ;;HashMap sometimes works, Map also sometimes works, but less often?
(System/getProperties) ;; also worked for a bit

(The hashmap bit will work, when we go to definition for java.util.HashMap and not for put) this is not working

(def testing (.toUpperCase "test"))
PEZ commented 2 years ago

It mostly just works for me. (I'm using 12.0.2-zulu installed by SDK-MAN). Same as for @pratik97, that things like (.put "a" 1) does not work.

In this comment: https://github.com/clojure-emacs/cider/issues/2819#issuecomment-907732646 @vemv points out a way to stablize at least the working things. (I am not sure if that .put is supposed to work). Copying the steps here:

  • make sure the JDK sources are in your classpath
    • sometimes they are by default, otherwise add them by hand
  • Disable dynapath
  • Use https://github.com/clojure-emacs/enrich-classpath/ for adding arbitrary Java sources for your whole dependency tree
    • ~(it doesn't add sources for the JDK itself atm, but it's easy enough to do that one by hand)~

I am not sure yet how to use enrich-classpath from a deps.edn or shadow-cljs project, but we can probably figure that out.

We should be able to add an option to Calva that makes it do these things.

vemv commented 2 years ago

Good timing! Last weekend I released enrich-classpath 1.5.0. It now includes the JDK sources so this does not apply anymore:

~(it doesn't add sources for the JDK itself atm, but it's easy enough to do that one by hand)~

We have it pending to cut new Orchard + cider-nrepl releases. If Calva uses them, it should upgrade those.

I am not sure yet how to use enrich-classpath from a deps.edn or shadow-cljs project, but we can probably figure that out.

That is drafted at https://github.com/clojure-emacs/enrich-classpath/pull/8 . There might be also alternative ways to accomplish this. In the end enrich-classpath is a vanilla library (not Lein plugin) which main defn takes a map in, and returns a map out: https://github.com/clojure-emacs/enrich-classpath/blob/c0dcc2c1b32184d152e05672f9b5fade9ef41837/src/cider/enrich_classpath.clj#L266

I also made a comment about that PR today here: https://app.slack.com/client/T03RZGPFR/C6QH853H8

PEZ commented 2 years ago

Thanks!

Calva uses cider-nrepl, is there a dev version we can test?

vemv commented 2 years ago

cider-nrepl 0.27.4 is out, which transitively also upgrades Orchard.

(enrich-classpath is not part of these dependencies: one needs to "enrich" the classpath caller-side, it's not something that can be self-contained)

PEZ commented 2 years ago

This has stopped working for me entirely since a while. No idea what is going on.

ericdallo commented 2 years ago

It's a calva bug I'm not sure how to solve as well, looks related with custom calva code that handle jar openening

ericdallo commented 2 years ago

Let me re-phrase: go to definition for java sources never worked on clojure-lsp, only via REPL, clojure-lsp don't have this go to java source yet. What seems to be broken is the go to definition of any external clojure jar

ericdallo commented 2 years ago

I'll open a new issue since this one seems related with java sources https://github.com/BetterThanTomorrow/calva/issues/1494