clojure-emacs / refactor-nrepl

nREPL middleware to support refactorings in an editor agnostic way
Eclipse Public License 1.0
256 stars 69 forks source link

Visiting `data-readers` files can cause issues #373

Closed vemv closed 2 years ago

vemv commented 2 years ago

https://clojurians.slack.com/archives/C0617A8PQ/p1647128245198459?thread_ts=1647064392.173409&cid=C0617A8PQ

dgtized commented 2 years ago

Here is the minimal way to reproduce the hanging problem:

  1. Create an empty project like so:
    mkdir test
    cd test
    echo {} > deps.edn
    mkdir -p src/test
    echo {} > src/data_readers.cljc
    echo "(ns test.core)" > src/test/core.clj
  2. visit src/test/core.clj and cider-jack-in
  3. move to the last line and type (str/ to trigger cljr-slash and refactor-nrepl will hang

After ctrl-g the following error will show up in the repl buffer:

ERROR: Unhandled REPL handler exception processing message {:op namespace-aliases, :prefix-rewriting false, :insert-newline-after-require true, :debug false, :suggest true, :session 8d29373d-0def-47b6-8f84-a6490e93b83a, :id 10}
java.lang.IllegalStateException: No ns form at test/src/data_readers.cljc
    at refactor_nrepl.core$read_ns_form_with_meta.invokeStatic(core.clj:383)
    at refactor_nrepl.core$read_ns_form_with_meta.invoke(core.clj:372)
    at clojure.lang.AFn.applyToHelper(AFn.java:156)
    at clojure.lang.AFn.applyTo(AFn.java:144)
    at clojure.core$apply.invokeStatic(core.clj:667)
    at clojure.core$memoize$fn__6894.doInvoke(core.clj:6342)
    at clojure.lang.RestFn.invoke(RestFn.java:421)
    at refactor_nrepl.ns.ns_parser$get_libspecs_from_file.invokeStatic(ns_parser.clj:141)
    at refactor_nrepl.ns.ns_parser$get_libspecs_from_file.invoke(ns_parser.clj:126)
    at refactor_nrepl.ns.libspecs$put_cached_ns_info_BANG_.invokeStatic(libspecs.clj:43)
    at refactor_nrepl.ns.libspecs$put_cached_ns_info_BANG_.invoke(libspecs.clj:40)
    at refactor_nrepl.ns.libspecs$get_ns_info_from_file_with_caching.invokeStatic(libspecs.clj:59)
    at refactor_nrepl.ns.libspecs$get_ns_info_from_file_with_caching.invoke(libspecs.clj:56)
    at refactor_nrepl.ns.libspecs$add_tentative_aliases$fn__6397.invoke(libspecs.clj:79)
    at clojure.core$map$fn__5884.invoke(core.clj:2759)
    at clojure.lang.LazySeq.sval(LazySeq.java:42)
    at clojure.lang.LazySeq.seq(LazySeq.java:51)
    at clojure.lang.RT.seq(RT.java:535)
    at clojure.core$seq__5419.invokeStatic(core.clj:139)
    at clojure.core$filter$fn__5911.invoke(core.clj:2813)
    at clojure.lang.LazySeq.sval(LazySeq.java:42)
    at clojure.lang.LazySeq.seq(LazySeq.java:51)
    at clojure.lang.RT.seq(RT.java:535)
    at clojure.core$seq__5419.invokeStatic(core.clj:139)
    at clojure.core$keep$fn__8582.invoke(core.clj:7332)
    at clojure.lang.LazySeq.sval(LazySeq.java:42)
    at clojure.lang.LazySeq.seq(LazySeq.java:51)
    at clojure.lang.Cons.next(Cons.java:39)
    at clojure.lang.RT.boundedLength(RT.java:1793)
    at clojure.lang.RestFn.applyTo(RestFn.java:130)
    at clojure.core$apply.invokeStatic(core.clj:669)
    at clojure.core$apply.invoke(core.clj:662)
    at refactor_nrepl.ns.libspecs$add_tentative_aliases.invokeStatic(libspecs.clj:92)
    at refactor_nrepl.ns.libspecs$add_tentative_aliases.invoke(libspecs.clj:64)
    at clojure.core$update.invokeStatic(core.clj:6191)
    at clojure.core$update.invoke(core.clj:6177)
    at refactor_nrepl.ns.libspecs$namespace_aliases.invokeStatic(libspecs.clj:142)
    at refactor_nrepl.ns.libspecs$namespace_aliases.invoke(libspecs.clj:125)
    at refactor_nrepl.ns.libspecs$namespace_aliases_response.invokeStatic(libspecs.clj:146)
    at refactor_nrepl.ns.libspecs$namespace_aliases_response.invoke(libspecs.clj:145)
    at clojure.lang.Var.invoke(Var.java:384)
    at refactor_nrepl.middleware$namespace_aliases_reply.invokeStatic(middleware.clj:189)
    at refactor_nrepl.middleware$namespace_aliases_reply.invoke(middleware.clj:188)
    at refactor_nrepl.middleware$wrap_refactor$fn__3039.invoke(middleware.clj:221)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_resource$fn__3303.invoke(nrepl.clj:406)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_apropos$fn__3195.invoke(nrepl.clj:129)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_macroexpand$fn__3263.invoke(nrepl.clj:287)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_ns$fn__3271.invoke(nrepl.clj:300)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_profile$fn__3287.invoke(nrepl.clj:343)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at nrepl.middleware.sideloader$wrap_sideloader$fn__1901.invoke(sideloader.clj:108)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_undef$fn__3351.invoke(nrepl.clj:493)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_classpath$fn__3203.invoke(nrepl.clj:137)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_clojuredocs$fn__3377.invoke(nrepl.clj:529)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at nrepl.middleware.load_file$wrap_load_file$fn__1820.invoke(load_file.clj:81)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at nrepl.middleware.caught$wrap_caught$fn__1211.invoke(caught.clj:97)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_content_type$fn__3179.invoke(nrepl.clj:107)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at nrepl.middleware.print$wrap_print$fn__1178.invoke(print.clj:234)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at cider.nrepl$wrap_tracker$fn__3343.invoke(nrepl.clj:482)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at nrepl.middleware.completion$wrap_completion$fn__1788.invoke(completion.clj:58)
    at nrepl.middleware$wrap_conj_descriptor$fn__942.invoke(middleware.clj:16)
    at nrepl.middleware.session$session$fn__1381.invoke(session.clj:325)
    at nrepl.middlew

As discussed in the Slack, the fix for this for data_readers in particular is that it can be moved into resources so that it's not parsed and doesn't throw an error.

However, the hang will happen if any clj/cljs/cljc file in src lacks a ns header, and I suspect if any other exception occurs in the process of suggesting candidates. As a quick fix for the data_readers/ns header problem it seems plausible to skip over any files that fail to parse and only suggest ns candidate aliases from files that parse.

More broadly though, it seems like this should report the error to the user in emacs without requiring a ctrl-g to interrupt the hanging loop. Not sure what all that entails or if that is relevant for other nrepl or nrepl refactor operations besides cljr-slash.