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

cljr-rename-symbol crashes for the first time on an unevaluated namespace (cljr-warn-on-eval 't) #401

Open
developer2019 opened this issue Dec 14, 2017 · 3 comments
Labels

Comments

@developer2019
Copy link

Hi!

Cljr fails to rename symbol in code which is successfully compiled and run bylein test.

Expected behavior

  1. cd /Users/YourName/Desktop.
  2. git clone https://github.com/developer2019/example
  3. Open /Users/YourName/Desktop/example/src/anagram.clj in Emacs.
  4. M-x cider-mode
  5. <menu-bar> <CIDER> <Start a REPL>
  6. Position cursor at anagrams-for (line 4, symbol 10) and press M-x cljr-rename-symbol RET.
    It tells that project needs to be evaluated, type y RET.
  7. Cljr should ask for a new symbol name and then rename this symbol in all project files.

Actual behavior

  1. cd /Users/YourName/Desktop.
  2. git clone https://github.com/developer2019/example
  3. Open /Users/YourName/Desktop/example/src/anagram.clj in Emacs.
  4. M-x cider-mode
  5. <menu-bar> <CIDER> <Start a REPL>
  6. Position cursor at anagrams-for (line 4, symbol 10) and press M-x cljr-rename-symbol RET.
    It tells that project needs to be evaluated, type y RET.
  7. It fails with Wrong type argument: char-or-string-p, nil.

Steps to reproduce the problem

  1. cd /Users/YourName/Desktop.
  2. git clone https://github.com/developer2019/example
  3. Open /Users/YourName/Desktop/example/src/anagram.clj in Emacs.
  4. M-x cider-mode
  5. <menu-bar> <CIDER> <Start a REPL>
  6. Position cursor at anagrams-for (line 4, symbol 10) and press M-x cljr-rename-symbol RET.
    It tells that project needs to be evaluated, type y RET.
  7. It fails with Wrong type argument: char-or-string-p, nil.

Environment & Version information

lein test compiles this code and runs successfully.

clj-refactor.el version information

clj-refactor 2.4.0-SNAPSHOT (package: 20171117.317), refactor-nrepl 2.4.0-SNAPSHOT from MELPA (i. e. latest clj-refactor).

CIDER version information

;; CIDER 0.15.1 (London), nREPL 0.2.12
;; Clojure 1.8.0, Java 1.8.0_111

Leiningen or Boot version

$ lein --version
Leiningen 2.8.1 on Java 1.8.0_111 Java HotSpot(TM) 64-Bit Server VM

lein test compiles this project and runs successfully.

Emacs version

GNU Emacs 25.3.1

Operating system

OS X 10.11 "El Capitan"
@developer2019
Copy link
Author

Had the same issue on clj-refactor.el version: 2.3.1.
Uninstalled it and installed clj-refactor 2.4.0-SNAPSHOT (package: 20171117.317), refactor-nrepl 2.4.0-SNAPSHOT from MELPA (i. e. latest clj-refactor). No changes and issue still occurs.

@benedekfazekas
Copy link
Member

managed to reproduce this. The problem essentially is that cljr can not get the ns for the symbol to be renamed. we use these calls to resolve the ns of the symbol

           (symbol (cider-symbol-at-point))
           (var-info (cljr--var-info symbol))
           (ns (nrepl-dict-get var-info "ns"))

this calls a cider function which only returns something if the namespace is loaded. this results in nil because the code is not loaded at this point.

a proper fix would be to call warm-ast-cache before trying to execute find-symbol-sync (given that the user answered y)

there are several workarounds:

  • load the namespace into your REPL session (either C-c C-k or refresh from your cider REPL, latter loads the whole project actually). after that rename-symbol should work
  • it actually works for the second attempt. not really sure why that is but most likely when cljr tries to resolve the symbol to be renamed it builds an AST for the namespace and that also results in evaluating the it
  • set cljr-warn-on-eval to nil ((setq cljr-warn-on-eval nil)) this will result in evaluating the whole project at startup time. if you are worried to set it globally set it in a dir local for the project you are confident about that this won't cause any harm

@benedekfazekas benedekfazekas changed the title cljr-rename-symbol crashes on simple program cljr-rename-symbol crashes for the first time on an unevaluated namespace (cljr-warn-on-eval 't) Dec 14, 2017
@developer2019
Copy link
Author

Thank you, @benedekfazekas, for exploration of this bug and finding workarounds for it.

I wonder why code is not loaded if cljr have asked about evaluation before asking for a new name for the symbol and I confirmed evaluation:

To perform this op the project needs to be evaluated.
  Analyzing a large project might take a while: hit C-g to abort.
  (Set cljr-warn-on-eval to nil to analyze the project without warning)
  Do you want to proceed? (y or n) y

Any why after (setq cljr-warn-on-eval nil) when there are no message (see above) project evaluation happens. It seems that showing this message prevents project evaluation even when y is entered.

load the namespace into your REPL session (either C-c C-k or refresh from your cider REPL, latter loads the whole project actually). after that rename-symbol should work

I actually need to load the whole project (not that example project) to be able to refactor symbol uses through all project files so tried to refresh from cider REPL:

user> refresh
CompilerException java.lang.RuntimeException: Unable to resolve symbol: refresh in this context, compiling:(*cider-repl example*:1:7527) 
user> (clojure.repl/refresh)
CompilerException java.lang.RuntimeException: No such var: clojure.repl/refresh, compiling:(*cider-repl example*:49:7) 

Doesn't work.

it actually works for the second attempt. not really sure why that is but most likely when cljr tries to resolve the symbol to be renamed it builds an AST for the namespace and that also results in evaluating the it

Could work but haven't tried it yet.

set cljr-warn-on-eval to nil ((setq cljr-warn-on-eval nil)) this will result in evaluating the whole project at startup time. if you are worried to set it globally set it in a dir local for the project you are confident about that this won't cause any harm

This works.

@clojure-emacs clojure-emacs deleted a comment from MalloZup Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants