Closed mitchell-horning closed 4 years ago
Just hit this same issue yesterday. After debugging it a bit, the problem seems to lie in this line: https://github.com/clojure-emacs/clj-refactor.el/blob/master/clj-refactor.el#L2500
When the symbol to be renamed appears in several places, the list of locations we get from nREPL is inserted as independent maps in the temporary buffer. E.g., this is the temporary buffer that I get in the project where I hit the problem (reformatted for clarity, it's a single line in the original buffer):
{:line-beg 5,
:line-end 7,
:col-beg 1,
:col-end 20,
:file "/home/magnet/sandbox/applications/hydrogen-modules/rbac.git/src/magnet/rbac.clj",
:name "magnet.rbac/kw->str",
:match "(defn- kw->str\n [k]\n (str (symbol k)))"}
{:line-beg 67,
:line-end 67,
:col-beg 66,
:col-end 73,
:file "/home/magnet/sandbox/applications/hydrogen-modules/rbac.git/src/magnet/rbac.clj",
:name "magnet.rbac/kw->str",
:match "(format \"%s = ?\" (kw->str column))"}
{:line-beg 122,
:line-end 122,
:col-beg 29,
:col-end 36,
:file "/home/magnet/sandbox/applications/hydrogen-modules/rbac.git/src/magnet/rbac.clj",
:name "magnet.rbac/kw->str",
:match "(update :context-type kw->str)))"}
{:line-beg 159,
:line-end 159,
:col-beg 23,
:col-end 30,
:file "/home/magnet/sandbox/applications/hydrogen-modules/rbac.git/src/magnet/rbac.clj",
:name "magnet.rbac/kw->str",
:match "(kw->str context-type))]"}
{:line-beg 192,
:line-end 192,
:col-beg 29,
:col-end 36,
:file "/home/magnet/sandbox/applications/hydrogen-modules/rbac.git/src/magnet/rbac.clj",
:name "magnet.rbac/kw->str",
:match "(update :context-type kw->str)))"}
{:line-beg 227,
:line-end 227,
:col-beg 23,
:col-end 30,
:file "/home/magnet/sandbox/applications/hydrogen-modules/rbac.git/src/magnet/rbac.clj",
:name "magnet.rbac/kw->str",
:match "(kw->str context-type)"}
parseedn-read
reads the whole temporary buffer in a single go (I stepped through the function in edebug
, and verified that the (while (not (cljr--end-of-buffer-p)) ...
) loop is executed only once). And it returns a list of hash-maps, with all the EDN maps in it.
edn-read
on the other hand seems to read a single EDN map at a time and loops as many times as needed to read the whole temporary buffer (haven't verified this in edebug yet, but I will).
That means that while edn-read
returns a single hash-map at a time, parseedn-read
returns a list of hash-maps. The surrounding code assumes a single hash-map is returned, and pushes the returned value to the occurrences
list. So occurrences
ends up being a list of a list of hash-maps, instead of a list of hash-maps (which is what the rest of the code expects).
One quick (and very dirty) way of confirming this is changing the code from:
(with-temp-buffer
(insert (cljr--call-middleware-sync request "occurrence"))
(unless (cljr--empty-buffer-p)
(goto-char (point-min))
(while (not (cljr--end-of-buffer-p))
(push (parseedn-read) occurrences))))
to
(with-temp-buffer
(insert (cljr--call-middleware-sync request "occurrence"))
(unless (cljr--empty-buffer-p)
(goto-char (point-min))
(while (not (cljr--end-of-buffer-p))
(dolist (edn (parseedn-read))
(push edn occurrences)))))
This fixes the issue and makes symbol renaming work as expected.
OpenJDK 8
clj-refactor 2.5.0 refactor-nrepl 2.5.0
CIDER 0.25 nREPL 0.6.0 Clojure 1.10.0 OpenJDK Runtime Environment (build 1.8.0_222-b10) OpenJDK 64-Bit Server VM (build 25.222-b10, mixed mode)
Leiningen 2.9.1 on Java 1.8.0_222 OpenJDK 64-Bit Server VM
GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars) of 2019-09-23, modified by Debian
Debian GNU/Linux 10.1 (buster)
Just a quick note to confirm that I have verified that edn-read
reads a single EDN map at a time and loops as many times as needed to read the whole temporary buffer.
So there is clearly a change in behavior from edn-read
to parseedn-read
, that is causing this issue.
Thanks for that @iarenaza . I did notice though that only every rename attempt is successful. Otherwise it fails with:
cljr-rename-symbol: Wrong type argument: char-or-string-p, nil
In any case, much much better than it not working at all
@anonimitoraf It is a bit difficult to pinpoint the code that might be producing such error (and I don't get it myself). If you could enable "debug on error", with toggle-debug-on-error
and attach the resulting stack trace, that could help a bit.
I ran into the same problem and the solution above by @iarenaza fixed it. Any change this would be accepted as a PR?
The suggested fix seemed OK to me, so I applied it. Sorry for the breakage! I didn't test everything after replacing the EDN parser.
Thank you for the great product. I'll try to resolve on my own, but am an Emacs debugging novice.
Expected behavior
The behavior specified in the wiki.
Actual behavior
In a new lein project, calling
cljr-rename-symbol
onx
in the below function:evokes the below error:
C-c C-k
or re-runningcljr-rename-symbol
a second time does not fix the issue.Steps to reproduce the problem
init.el
:Then call
lein new cljr_test
and callcljr-rename-symbol
.Environment & Version information
OpenJDK 8, 11, or 13 all seem to produce the same error.
clj-refactor.el version information
clj-refactor 2.5.0-SNAPSHOT (package:20200221.1329), refactor-nrepl 2.5.0-SNAPSHOT
CIDER version information
CIDER 0.25.0snapshot (package: 20200220.1307), nREPL 0.6.0 Clojure 1.10.0, Java 1.8.0_242
Leiningen or Boot version
Leiningen 2.9.1 on Java 1.8.0_242 OpenJDK 64-Bit Server VM
Emacs version
26.3
Operating system
Arch linux, kernel 5.5.4-arch1-1