Rename symbol breaks in the presence of :: keywords (regression)

[DerGuteMoritz]

Steps to reproduce the problem

When :: style keywords (e.g. ::foo or ::foo/bar) are present anywhere in the project, the rename symbol refactoring fails with this exception:

Caused by: clojure.lang.ExceptionInfo: Invalid token: ::foo/bar

Seems to be a regression of #106

Environment & Version information

clj-refactor.el and refactor-nrepl version information

clj-refactor 2.2.0, refactor-nrepl 2.2.0

CIDER version information

;; CIDER 0.12.0 (Seattle), nREPL 0.2.12
;; Clojure 1.8.0, Java 1.8.0_102

Leiningen or Boot version

Leiningen 2.6.1

Emacs version

GNU Emacs 24.5.2

Operating system

NixOS 16.09pre85533.2a16e37 (Flounder)

[DerGuteMoritz]

I just experimented some more and noticed that it only happens some of the time. So far I can only reproduce it with a partially compiled project. Calling cider-refresh makes the problem go away for some reason. Maybe the stacktrace helps:

Caused by: clojure.lang.ExceptionInfo: Invalid token: ::foo/bar
 at clojure.core$ex_info.invokeStatic (core.clj:4617)
    clojure.core$ex_info.invoke (core.clj:4617)
    refactor_nrepl.util$ex_info_assoc.invokeStatic (util.clj:26)
    refactor_nrepl.util$ex_info_assoc.doInvoke (util.clj:23)
    clojure.lang.RestFn.applyTo (RestFn.java:139)
    clojure.core$apply.invokeStatic (core.clj:648)
    clojure.core$apply.invoke (core.clj:641)
    refactor_nrepl.find.find_macros$find_macro_definitions_in_file.invokeStatic (find_macros.clj:51)
    refactor_nrepl.find.find_macros$find_macro_definitions_in_file.invoke (find_macros.clj:49)
    refactor_nrepl.find.find_macros$put_cached_macro_definitions.invokeStatic (find_macros.clj:71)
    refactor_nrepl.find.find_macros$put_cached_macro_definitions.invoke (find_macros.clj:70)
    refactor_nrepl.find.find_macros$get_macro_definitions_in_file_with_caching.invokeStatic (find_macros.clj:79)
    refactor_nrepl.find.find_macros$get_macro_definitions_in_file_with_caching.invoke (find_macros.clj:76)
    clojure.core$map$fn__4785.invoke (core.clj:2646)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:49)
    clojure.lang.Cons.next (Cons.java:39)
    clojure.lang.RT.boundedLength (RT.java:1749)
    clojure.lang.RestFn.applyTo (RestFn.java:130)
    clojure.core$apply.invokeStatic (core.clj:646)
    clojure.core$mapcat.invokeStatic (core.clj:2674)
    clojure.core$mapcat.doInvoke (core.clj:2674)
    clojure.lang.RestFn.invoke (RestFn.java:423)
    refactor_nrepl.find.find_macros$find_macro_definitions_in_project.invokeStatic (find_macros.clj:85)
    refactor_nrepl.find.find_macros$find_macro_definitions_in_project.invoke (find_macros.clj:81)
    refactor_nrepl.find.find_macros$find_macro.invokeStatic (find_macros.clj:211)
    refactor_nrepl.find.find_macros$find_macro.invoke (find_macros.clj:206)
    refactor_nrepl.find.find_symbol$find_symbol$fn__19072.invoke (find_symbol.clj:237)
    clojure.core$binding_conveyor_fn$fn__4676.invoke (core.clj:1938)
    clojure.lang.AFn.call (AFn.java:18)
    java.util.concurrent.FutureTask.run (FutureTask.java:266)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1142)
    java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:617)
    java.lang.Thread.run (Thread.java:745)

[expez]

Ya, the project has to be loaded into the repl for some of this stuff to work. Otherwise the reader won't know what namespace foo should resolve to when reading ::foo/bar.

Most people seem to use the reloaded workflow, with cider-refresh, so this probably isn't a huge problem in practice. I'm hesitant to add extra cider-refresh before our refactorings because that's often redundant and then we'd have to add another setting to toggle this behavior on and off :/

[DerGuteMoritz]

Oh, I see! I figured that the evaluation that mode warns about for certain refactorings would already do something like this so I was a bit confused as to why cider-refresh would have an effect at all. Couldn't this evaluation just be changed to do a cider-refresh at least once per nrepl session the first time necessary? That shouldn't be too invasive, I guess. Alternatively, I would suggest to issue a hint to the user what might be going on in this case perhaps..?

[DerGuteMoritz]

Oops, accidentally hit "close and comment" ;-)

[expez]

I figured that the evaluation that mode warns about for certain refactorings would already do something like this

That's a reasonable assumption, but that's the evaluation related to building an AST. In order to work with macros (which have been expanded away by the AST gets built) we have to do some manual reading of our own. This is what fails in the stacktrace above.

Couldn't this evaluation just be changed to do a cider-refresh at least once per nrepl session the first time necessary?

Yeah, but as soon as a new occurrence of a keyword of type ::foo/bar shows up we'd have to do it again. So we either do it / prompt every time or leave it to the user to have the relevant code loaded into the repl

I agree that the situation as it is now is pretty bad, and I think it could be improved by just providing a better error message. The only reason I haven't done that is because we'd have to rethrow based on the text in the exception-info :/

[expez]

Can't reproduce this on the latest snapshot. Closing this for now.

We're doing a push for another stable release, sometime later this month, if you don't want to run a snapshot.