search

clojure-emacs / refactor-nrepl

nREPL middleware to support refactorings in an editor agnostic way
Eclipse Public License 1.0
256 stars 69 forks source link

suggest-libspec is suggesting clojure.math as clj only in cljc files #408

Open dgtized opened 1 month ago

dgtized commented 1 month ago

Expected behavior

Typing math/ in a CLJC file should insert the libspec [clojure.math :as math] into the ns form.

Actual behavior

The libspec inserted is #?(:clj [clojure.math :as math]).

Steps to reproduce the problem

Using clj-refactor in a CLJC file type math/ in a namespace with no existing alias named math.

Commentary

This is using a default cljr-magic-require-namespaces which does not specify clojure.math as :only ("clj") but does include it as a magic namespace. This works correctly for clojure.set :as set and clojure.string :as str and even clojure.walk and clojure.zip, which are also listed in the magic namespaces, so it's confusing that clojure.math behaves differently. The project I'm testing with has a mixture of cljs and cljc files but has no instances where the math libspec is wrapped in a reader conditional. It does correctly return clojure.math :as math in cljs files in that project, but add the unnecessary :clj language conditional if math/ invokes cljr-slash in a CLJC file.

Note that clojure.math is one of the CLJS namespaces that is magically mapped to the equivalent namespace in Clojure as described in https://clojurescript.org/guides/ns-forms#_clojure_namespace_aliasing. I don't know if that's related but it seems like it might be.

Environment & Version information

clj-refactor.el and refactor-nrepl version information

clj-refactor 3.11.3, refactor-nrepl 3.10.0

CIDER version information

;; CIDER 1.14.0-snapshot (package: 20240510.1436), nREPL 1.1.1
;; Clojure 1.11.1, Java 17.0.10

Emacs version

GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.38, cairo version 1.18.0) of 2024-05-08

Operating system

Ubuntu 23.10

vemv commented 1 month ago

Hi @dgtized , good to see you here.

Can you attach the nrepl logs as well?

Thanks - V

dgtized commented 1 month ago 
(-->
  id                           "416"
  op                           "cljr-suggest-libspecs"
  session                      "a8396f47-6c61-46f7-81dc-320bc33add03"
  time-stamp                   "2024-05-13 14:20:49.421752067"
  buffer-language-context      "cljc"
  debug                        "false"
  input-language-context       "cljc"
  insert-newline-after-require "true"
  language-context             "cljc"
  lib-prefix                   "math"
  preferred-aliases            "((\"edn\" . \"clojure.edn\") (\"io\" \"clojure.java.io\" :only (\"clj\")) (\"math\" . \"clojure.math\") (\"set\" . \"clojure.set\") (\"str\" . \"clojure.string\") (\"walk\" . \"clojure.walk\") (\"zip\" . \"clojure.zip\"))"
  prefix-rewriting             "false"
)
(<--
  id          "416"
  session     "a8396f47-6c61-46f7-81dc-320bc33add03"
  time-stamp  "2024-05-13 14:20:49.507559109"
  status      ("done")
  suggestions "[\"#?(:clj [clojure.math :as math])\"]"
)

Not sure if this is the right log, I toggled nrepl-toggle-message-logging and grabbed the output that seemed related. Is that what you meant by the nrepl logs?

vemv commented 1 month ago

Yes, that's very useful as it includes the entire preferred-aliases value, besides from language-context, etc.

I'll try to give it a shot, if there are no news, feel free to nudge.

Or if you'd be willing, you can try reproducing the bug by adding that data to https://github.com/clojure-emacs/refactor-nrepl/blob/4a873ca41da1dcb1728109c8829d6c2465110c21/test/refactor_nrepl/ns/suggest_libspecs_test.clj#L51-L60 - should be an easy way to get started.