Error when completing (System/getProperty ..)

[borkdude]

#error {
 :cause No namespace: System found
 :via
 [{:type clojure.lang.ExceptionInfo
   :message No namespace: System found [at line 1, column 7]
   :data {:type :sci/error, :line 1, :column 7, :message No namespace: System found [at line 1, column 7]}

[kwrooijen]

When connecting with lein I get the following error. Probably related?

lein repl :connect localhost:23456
Connecting to nREPL at localhost:23456
REPL-y 0.5.1, nREPL 
Clojure 1.10.3
: Could not resolve symbol: java.lang.System/getProperty user 
nil
Error loading namespace; falling back to user
nil

[benjamin-asdf]

  (require '[sci.addons :as addons])

  (def opts (-> {:namespaces {'foo.bar {'x 1}}} addons/future))
  (def sci-ctx (sci/init opts))

  (require '[sci.impl.interop :as interop])

  (interop/resolve-class sci-ctx 'System)
  nil
  (interop/resolve-class sci-ctx 'Object)
  java.lang.Object

I came this far. I when I chek the sci context I see some raw classes. Maybe the fix involves fixing System there
@borkdude

[borkdude]

@benjamin-asdf The issue is not about System, the issue is that completions can fail when trying to complete an interop form.

[benjamin-asdf]

@borkdude my current thinking is that impl/server.clj should check if the prefix is a static class (e.g. "System/") and then return the static members of the class. I guess I can check how cider-nrepl is completing interop.

[borkdude]

Yes, something like that. I think the nbb completions might be doing this better, maybe it's also worth checking there.

[benjamin-asdf]

master...benjamin-asdf:master this is my first version that works. I feel like I should understand the nature of match and ns->alias etc. to know if we need this or not @borkdude
Also can I use clojure.reflect like that?

edit: it only accidentally works rn. I'm going to fix it

[benjamin-asdf]

Getting inspired by the nbb code indeed helped.

showcase:
https://user-images.githubusercontent.com/38900087/168864361-977356e2-e2e6-4ef6-92ed-de3cd25481c5.mp4

for System to work I did need to add it to the sci default imports though.

  (require '[sci.impl.opts])
  (alter-var-root #'sci.impl.opts/default-classes assoc  'java.lang.System System)
  (alter-var-root #'sci.impl.opts/default-imports assoc  'System 'java.lang.System)

else calling resolve inside sci/eval-string does not resolve System.
Next improvement would be to get all imported members I suppose. I mean when you type method names without namespace prefix.
code: master...benjamin-asdf:master (will write tests)

[borkdude]

@benjamin-asdf babashka.nrepl is more general than babashka only, it is an nREPL suited for babashka-like projects. Maybe the name should be changed to sci.nrepl, but anyway. This issue is not about System only, it is about crashing when you try to complete System: the nREPL server should not give any errors if System is not there. We should not add it to the context in this project (perhaps only for tests), as there are projects using this code which should not automatically have access to System.

[benjamin-asdf]

@borkdude yes you are right. In a running bb instance System is imported correctly.
I fixed the error by doing this:

ns-found? (sci/eval-string* ctx (format "(find-ns '%s)" query-ns))

Added 2 functions

ns-imports->completions similar to nbb version but uses clojure.reflect
import-symbols->completions this is so you get the completion for example for "Sys" -> "System". It just seems like the correct behaviour to me.
https://github.com/babashka/babashka.nrepl/pull/54/files

[benjamin-asdf]

I'm thinking about the code and I wonder if import-symbols->completions should not exist and

                from-current-ns (fully-qualified-syms ctx (sci/eval-string* ctx "(ns-name *ns*)"))
                from-current-ns (map (fn [sym]
                                       [(namespace sym) (name sym) :unqualified])
                                     from-current-ns)

this should somehow end up returning those class names.