clojure-emacs / cider-nrepl

A collection of nREPL middleware to enhance Clojure editors with common functionality like definition lookup, code completion, etc.
https://docs.cider.mx/cider-nrepl
670 stars 175 forks source link

The cache initialized can perform side-effectful `Class/forName`s #842

Closed vemv closed 5 months ago

vemv commented 5 months ago

https://clojurians.slack.com/archives/C0617A8PQ/p1705167808132879

Although https://github.com/clojure-emacs/cider-nrepl/blob/c80f47c231acce5d6508c56e7d5868e4c726704c/src/cider/nrepl.clj#L45-L47 aims to avoid static initializers, perhaps there are similar calls nearby.

vemv commented 5 months ago

False alarm - the user's java code was based on final static fields, not static initializers.

We cannot work around those. It's a pretty extreme edge case and there's an easy refactoring path (static fields -> static initializers).

hugoduncan commented 5 months ago

Is there anything to be done to avoid someone in this situation going down the path of trying to debug what is going on?

hugoduncan commented 5 months ago

Unfortunately the call to orchard.info/info in warmup-orchard-caches! is initialising the java class.

info calls orchard.meta/resolve-var which calls ns-resolve.

vemv commented 5 months ago

Is there anything to be done to avoid someone in this situation going down the path of trying to debug what is going on?

Considering that as of today we offer support practically 7 days a week (my pleasure 🙂) I'm not immediately concerned by this meta-problem.

Perhaps we can add an integration test ensuring that no requires are performed, for the supported use cases:

info calls orchard.meta/resolve-var which calls ns-resolve.

Good catch! I see, unfortunately the Java class/record type symbol clause is not the first one in the or: https://github.com/clojure-emacs/orchard/blob/951fdbe26cf5146ef070b905a99508e6ef9997e1/src/orchard/info.clj#L72-L81

However there's an easy fix - change info for info-java

vemv commented 5 months ago

https://github.com/clojure-emacs/cider/releases/tag/v1.13.0 https://github.com/clojure-emacs/cider-nrepl/blob/master/CHANGELOG.md#0450-2024-01-14