refactor-nrepl

refactor-nrepl copied to clipboard

Expected behavior

Symbol renamed

Actual behavior

java.lang.IllegalStateException: refactor-nrepl is unable to build an AST for myapp.core. tools.analyzer encountered the following problem: Can't set!: *warn-on-reflection* from non-binding thread

Steps to reproduce the problem

I've created a simple clojure file with (set! *warn-on-reflection* true) on top and tried renaming the symbol.

Environment & Version information

clj-refactor.el version information

clj-refactor 2.5.1, refactor-nrepl 2.5.1

CIDER version information

;; CIDER 1.1.0snapshot, nREPL 0.8.3 ;; Clojure 1.10.3, Java 11.0.10

Leiningen or Boot version

Leiningen 2.9.3 on Java 11.0.10 OpenJDK 64-Bit Server VM

Emacs version

GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.13, cairo version 1.16.0) of 2021-03-17

Operating system

Fedora 33

Also when simply launching CIDER repl with jack in I see this in the *Messages* buffer:

error in process filter: Some namespaces are in a bad state: error "Can’t set!: *warn-on-reflection* from non-binding thread" in myapp.core

for every namespace where I set this var. When working with GraalVM this setting is very handy and I would like to keep it.

Although popular, (set! *warn-on-reflection*) is not guaranteed to work. No top-level (set! ... ) is - personally I keep seeing projects tripping up on this nuance.

Furthermore (set! *warn-on-reflection*), while is thread-local, is not local to your ns, or to your project. So setting it can change its value for anyone consuming your ns.

tldr it's not an ideal mechanism.

What can you use instead:

• lein check and grep for reflection warnings
• Same, but with https://github.com/athos/clj-check
• Use https://github.com/jonase/eastwood#reflection as a linter

We're using Eastwood as a reflection linter in most https://github.com/clojure-emacs projects as a hard check in CI and it's working nicely for us. It has advantages over a simple grep laid out in its documentation.

(I'm biased though because I authored it)

I've (unsuccessfully) tried to reproduce the issue here https://github.com/clojure-emacs/refactor-nrepl/pull/325

Feel free to hack it, you might be able to break the build with a sophisticated-enough example :)

I can confirm this happens here as well. It was is a legacy thing that need to be removed. I resolved by removing all the instances of *warn-on-reflection*. The problem was definitely there though and happening on "rename symbol"

I'll see if I can add a temporary binding for *warn-on-reflection*.

Maybe it's worth adding all Clojure's special variables that can be set with set!? There aren't many AFAIK

Cheers yes there's similar tech in eastwood https://github.com/jonase/eastwood/blob/8a6655f592004a7d6e028380144cb1210bb7a9f5/src/eastwood/analyze_ns.clj#L422-L435

Although it's not as easy as a binding: given that tools.analyzer is used, any analyzed form itself could be performing a set!. There's a workaround in Eastwood which I could replicate here.

Just ran into this as well. My workaround was to find and replace all (set! *warn-on-reflection* true) with #_(set! *warn-on-reflection* true). When I'm done with the refactorings, I'll undo that commit.

~~I'm a bit surprised this doesn't happen more, since clj-kondo by default tells you to add (set! *warn-on-reflection* true) when using java interop.~~ (edit: I'm silly, by defaults it's off)

@filipesilva Sorry, I missed your message.

I find that clj-kondo recommendation a bit odd. You don't really need to litter clojure namespaces with that flag. You can simply lint for reflection with Eastwood, Leiningen, certain Clojure CLI tools.

Anyway I could give this a try again.