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

Error with suggest_libspecs #556

Closed
chpill opened this issue Dec 2, 2023 · 6 comments
Closed

Error with suggest_libspecs #556

chpill opened this issue Dec 2, 2023 · 6 comments

Comments

@chpill
Copy link

chpill commented Dec 2, 2023

Hi, thanks for this amazing package!

I've refreshed my spacemacs packages and I see an error message each time I type a namespace alias that is not already in the namespace declaration (for example, I will see the error if I type str/ and the [clojure.string :as str] is not in the ns form).

Expected behavior

Adds the correct missing libspec.

Actual behavior

Error with the following stacktrace:

cljr--get-error-value: Error in nrepl-refactor: java.lang.ClassCastException: class clojure.lang.PersistentList cannot be cast to class clojure.lang.Named (clojure.lang.PersistentList and clojure.lang.Named are in unnamed module of loader ’app’)
 at clojure.core$name.invokeStatic (core.clj:1610)
    clojure.core$name.invoke (core.clj:1604)
    clojure.core$comp$fn__5876.invoke (core.clj:2586)
    clojure.core$mapv$fn__8535.invoke (core.clj:6979)
    clojure.lang.PersistentVector.reduce (PersistentVector.java:343)
    clojure.core$reduce.invokeStatic (core.clj:6885)
    clojure.core$mapv.invokeStatic (core.clj:6970)
    clojure.core$mapv.invoke (core.clj:6970)
    clojure.core$partial$fn__5908.invoke (core.clj:2641)
    clojure.core$mapv$fn__8535.invoke (core.clj:6979)
    clojure.lang.PersistentVector.reduce (PersistentVector.java:343)
    clojure.core$reduce.invokeStatic (core.clj:6885)
    clojure.core$mapv.invokeStatic (core.clj:6970)
    clojure.core$mapv.invoke (core.clj:6970)
    refactor_nrepl.ns.suggest_libspecs$parse_preferred_aliases_STAR___102053.invokeStatic (suggest_libspecs.clj:26)
    refactor_nrepl.ns.suggest_libspecs/parse_preferred_aliases_STAR_ (suggest_libspecs.clj:18)
    clojure.lang.AFn.applyToHelper (AFn.java:154)
    clojure.lang.AFn.applyTo (AFn.java:144)
    clojure.core$apply.invokeStatic (core.clj:667)
    clojure.core$memoize$fn__6946.doInvoke (core.clj:6388)
    clojure.lang.RestFn.invoke (RestFn.java:408)
    refactor_nrepl.ns.suggest_libspecs$suggest_libspecs_response.invokeStatic (suggest_libspecs.clj:270)
    refactor_nrepl.ns.suggest_libspecs$suggest_libspecs_response.invoke (suggest_libspecs.clj:218)
    clojure.lang.Var.invoke (Var.java:384)
    refactor_nrepl.middleware$suggest_libspecs_reply$fn__78564.invoke (middleware.clj:213)
    refactor_nrepl.ns.libspec_allowlist$with_memoized_libspec_allowlist_STAR_.invokeStatic (libspec_allowlist.clj:41)
    refactor_nrepl.ns.libspec_allowlist$with_memoized_libspec_allowlist_STAR_.invoke (libspec_allowlist.clj:39)
    refactor_nrepl.middleware$suggest_libspecs_reply.invokeStatic (middleware.clj:211)
    refactor_nrepl.middleware$suggest_libspecs_reply.invoke (middleware.clj:210)
    refactor_nrepl.middleware$wrap_refactor$fn__78575.invoke (middleware.clj:244)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_resource$fn__81710.invoke (nrepl.clj:613)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_log$fn__81643.invoke (nrepl.clj:362)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_refresh$fn__81702.invoke (nrepl.clj:587)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_test$fn__81734.invoke (nrepl.clj:700)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_apropos$fn__81498.invoke (nrepl.clj:165)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_complete$fn__81514.invoke (nrepl.clj:179)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_classpath$fn__81506.invoke (nrepl.clj:173)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.completion$wrap_completion$fn__76945.invoke (completion.clj:58)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_xref$fn__81776.invoke (nrepl.clj:774)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.interruptible_eval$interruptible_eval$fn__76435.invoke (interruptible_eval.clj:154)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.sideloader$wrap_sideloader$fn__77058.invoke (sideloader.clj:108)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.load_file$wrap_load_file$fn__76977.invoke (load_file.clj:81)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_content_type$fn__81470.invoke (nrepl.clj:143)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.session$add_stdin$fn__76553.invoke (session.clj:379)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_slurp$fn__81482.invoke (nrepl.clj:157)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_ns$fn__81678.invoke (nrepl.clj:505)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_debug$fn__81526.invoke (nrepl.clj:199)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_enlighten$fn__81548.invoke (nrepl.clj:226)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.piggieback$wrap_cljs_repl$fn__77493.invoke (piggieback_impl.clj:370)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_out$fn__81686.invoke (nrepl.clj:541)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_tracker$fn__81750.invoke (nrepl.clj:742)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_inspect$fn__81613.invoke (nrepl.clj:283)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.caught$wrap_caught$fn__76368.invoke (caught.clj:97)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    cider.nrepl$wrap_profile$fn__81694.invoke (nrepl.clj:550)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    shadow.cljs.devtools.server.nrepl_impl$handle.invokeStatic (nrepl_impl.clj:329)
    shadow.cljs.devtools.server.nrepl_impl$handle.invoke (nrepl_impl.clj:228)
    shadow.cljs.devtools.server.nrepl$middleware$fn__77615.invoke (nrepl.clj:46)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.print$wrap_print$fn__76335.invoke (print.clj:234)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.middleware.session$session$fn__76538.invoke (session.clj:325)
    nrepl.middleware$wrap_conj_descriptor$fn__76099.invoke (middleware.clj:16)
    nrepl.server$default_handler$fn__77106.invoke (server.clj:141)
    nrepl.server$handle_STAR_.invokeStatic (server.clj:24)
    nrepl.server$handle_STAR_.invoke (server.clj:21)
    nrepl.server$handle$fn__77074.invoke (server.clj:41)
    clojure.core$binding_conveyor_fn$fn__5823.invoke (core.clj:2047)
    clojure.lang.AFn.call (AFn.java:18)
    java.util.concurrent.FutureTask.run (FutureTask.java:264)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1136)
    java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:635)
    java.lang.Thread.run (Thread.java:833)

Steps to reproduce the problem

I see an error message each time I type a namespace alias that is not already in the namespace declaration (for example, I will see the error if I type str/ and the [clojure.string :as str] is not in the ns form).

Environment & Version information

clj-refactor.el version information

3.11.1

CIDER version information

Include here the version string displayed when
CIDER's REPL is launched. Here's an example:

CIDER 1.13.0-snapshot (package: 20231127.825), nREPL 1.0.0
Clojure 1.11.1, Java 17.0.7

This was injected when jacking in

nrepl/nrepl {:mvn/version "1.0.0"}
cider/cider-nrepl {:mvn/version "0.44.0"}
refactor-nrepl/refactor-nrepl {:mvn/version "3.9.0"}

Leiningen or Boot version

Clojure CLI version 1.11.1.1413

Emacs version

GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.18.0, Xaw3d scroll bars)

Operating system

Linux NixOS 23.11

@chpill
Copy link
Author

chpill commented Dec 2, 2023

I've just confirmed that the feature works as expected when I manually override the files in .emacs.d/elpa/28.2/develop/clj-refactor-20231202.445 with the files from the 3.11.0 version of this repo, and then rm *.elc files.

@vemv
Copy link
Member

vemv commented Dec 2, 2023

Thanks for the report!

What's your value of cljr-magic-require-namespaces? and cljr-slash-uses-suggest-libspec?

Does it get fixed if you remove ("io" "clojure.java.io" :only ("clj")) from the former?

@chpill
Copy link
Author

chpill commented Dec 2, 2023

cljr-magic-require-namespaces:

(("edn" . "clojure.edn")
 ("io" "clojure.java.io" :only
  ("clj"))
 ("math" . "clojure.math")
 ("set" . "clojure.set")
 ("str" . "clojure.string")
 ("walk" . "clojure.walk")
 ("zip" . "clojure.zip"))

cljr-slash-uses-suggest-libspec: t

Indeed, when I remove ("io" "clojure.java.io" :only ("clj")) from cljr-magic-require-namespaces, the issue goes away.

@vemv
Copy link
Member

vemv commented Dec 2, 2023

Indeed, when I remove ("io" "clojure.java.io" :only ("clj")) from cljr-magic-require-namespaces, the issue goes away.

Thanks!

Feel free to give it a go @dgtized

@dgtized
Copy link
Contributor

dgtized commented Dec 3, 2023

Yea I'm seeing that now. I believe that's on the parsing side in the middleware given it's a java stacktrace though. The clj-refactor client is sending across "preferred-aliases" with prin1-to-string, which has the default value:

"((\"edn\" . \"clojure.edn\") 
  (\"io\" \"clojure.java.io\" :only (\"clj\")) 
  (\"math\" . \"clojure.math\") 
  (\"set\" . \"clojure.set\") 
  (\"str\" . \"clojure.string\") 
  (\"walk\" . \"clojure.walk\") 
  (\"zip\" . \"clojure.zip\"))"

See https://github.com/clojure-emacs/clj-refactor.el/blob/master/clj-refactor.el#L2032.

So I suspect the failure here is that the middleware is having difficulty in parse-preferred-aliases. It might be that read-string is having trouble reading back ("clj") as it's not actually a list because it's not '("clj") or (list "clj"). Alternatively, it looks like the first pass of the parser may be assuming a syntax like: ["io" "clojure.java.io" :only :clj]" and not accounting for a list of language contexts for the value of :only. However, that is the syntax we decided on in #530 (comment). Specifically, by supporting a set of contexts instead of singular context, we can future proof our syntax for something like ("io" "clojure.java.io" :only ("bb" "clj")).

Looking through the suggest_libspec tests it looks like they may all assume a single value for :only instead of a set of accepted language contexts. I'm not sure if that's just a simplification in the tests or if that is also a constraint in the implementation. If it's the latter it seems reasonable to update the middleware to accept a list, but not accept any lists with multiple values yet.

So short term, it seems reasonable to revert to the old syntax here until the middleware can parse :only statements with a list. We can't add tests to verify this here as they all mock the interface with the middleware. I think just reverting 8806352 would be sufficient for now?

@vemv
Copy link
Member

vemv commented Dec 3, 2023

Thanks much for the accurate analysis @dgtized !

I went ahead and fixed the middleware.

I've cut a new clj-refactor.el release, to be melpa-visible within the next few hours.

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

3 participants