`cider-find-ns` can't open files in jars

rpkarlsson commented 5 years ago

cider-find-ns can't open files in jars and throws an error instead.

Expected behavior

That a buffer containing the ns is opened.

Actual behavior

The previouly opened buffer is focused and the following error is shown

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  set-buffer(nil)
  (save-current-buffer (set-buffer buffer) (widen) (goto-char (point-min)) (cider-mode 1) (cond ((consp pos) (forward-line (1- (or (car pos) 1))) (if (cdr pos) (move-to-column (cdr pos)) (back-to-indentation))) ((numberp pos) (goto-char pos)) (pos (if (or (save-excursion (search-forward-regexp (format "(def.*\\s-\\(%s\\)" ...) nil (quote noerror))) (let ((name ...)) (or (save-excursion ...) (save-excursion ...)))) (goto-char (match-beginning 0)) (message "Can't find %s in %s" pos (buffer-file-name)))) (t nil)))
  cider-jump-to(nil nil nil)
  (if path (cider-jump-to (cider-find-file path) nil other-window) (user-error "Can't find namespace `%s'" ns))
  (let* ((path (and t (cider-sync-request:ns-path ns)))) (if path (cider-jump-to (cider-find-file path) nil other-window) (user-error "Can't find namespace `%s'" ns)))
  cider--find-ns("aleph.flow" nil)
  cider-find-ns(nil)
  funcall-interactively(cider-find-ns nil)
  #<subr call-interactively>(cider-find-ns record nil)
  apply(#<subr call-interactively> cider-find-ns (record nil))
  call-interactively@ido-cr+-record-current-command(#<subr call-interactively> cider-find-ns record nil)
  apply(call-interactively@ido-cr+-record-current-command #<subr call-interactively> (cider-find-ns record nil))
  call-interactively(cider-find-ns record nil)
  command-execute(cider-find-ns record)
  execute-extended-command(nil "cider-find-ns")
  smex-read-and-run(("toggle-debug-on-error" "emacs-version" "cider-find-ns" "eshell" "cider" "buttercup-run-at-point" "edebug-defun" "recompile" "rk-insert-js-log" "mu4e" "cider-inspect-last-result" "rk-geoplanner-app-state-console-log" "kpsystem-geoserver-cider-inspect-app-state" "cider-debug-defun-at-point" "eval-buffer" "rk-punch-new-day" "elfeed" "cider-pprint-eval-last-sexp-to-comment" "magit-status" "rk-insert-js-log-let" "package-list-packages" "cider-refresh" "cider-inspect" "rk-init.el" "org-capture" "org-static-blog-publish" "checkdoc" "cider-mode" "load-theme" "magit" "paredit-mode" "disable-theme" "org-fill-paragraph" "find-library" "check-parens" "package-install" "godoc" "org-agenda" "magit-blame" "delete-matching-lines" "cider-inspect-last-sexp" "clojure-mode" "cider-scratch" "browse-url-emacs" "diary" "cider-jack-in" "diary-view-entries" "package-autoremove" "apropos" "cider-version" ...))
  smex()
  funcall-interactively(smex)
  #<subr call-interactively>(smex nil nil)
  apply(#<subr call-interactively> smex (nil nil))
  call-interactively@ido-cr+-record-current-command(#<subr call-interactively> smex nil nil)
  apply(call-interactively@ido-cr+-record-current-command #<subr call-interactively> (smex nil nil))
  call-interactively(smex nil nil)
  command-execute(smex)

Steps to reproduce the problem

Jack in
M-x cider-find-ns
Select a ns that supplied in a jar.

Environment & Version information

CIDER version information

;; CIDER 0.22.0snapshot (package: 20190720.1656), nREPL 0.6.0
;; Clojure 1.10.0, Java 1.8.0_212

Lein/Boot version

clojure-cli

Emacs version

26.2

Operating system

Fedora 30

rpkarlsson commented 5 years ago

Did a workaround available at: https://github.com/rpkarlsson/cider/commit/68c2ff09b567932823c82eed74c7c22221c93d3a, but I feel that the fix perhaps should be done directly in nREPL instead?

nREPL's cider-sync-request:ns-path returns file:/path/to/jar.jar!/path/file.clj" instead of the expected jar:file:/path/to/jar.jar!/path/file.clj"

bbatsov commented 5 years ago

Seems like some bug in Orchard or cider-nrepl. There were a lot of changes there recently. //cc @arichiardi @jeffvalk

rpkarlsson commented 5 years ago

Doing some digging the bug seems to be introduced with the changes in: https://github.com/clojure-emacs/orchard/commit/ddd6aeaa58f7f3fbc259a7fcc317ab6bf9643615#diff-015a5738d798be18495eaebdc7362ee8L106

Changing nREPL to use the new orchard.ns/canonical-source was done in https://github.com/clojure-emacs/cider-nrepl/commit/49462f13c5a87ff071551150cc6ceb61ef967c8c#diff-5f80328b3910bd3a21646a0c244ba0b3R88.

orchard.ns/canonical-source returns clojure.java.io data so I'm reluctant to change anything on that side. I'm thinking nREPL could handle things like this https://github.com/rpkarlsson/cider-nrepl/commit/2d955e06a56646cb332175100a56bc0ffaa5be7e

Would that be an OK solution or do you have any thoughts/suggestions? At least feels better than the initial workaround where cider checked the string directly.

jeffvalk commented 5 years ago

orchard.namespace has tests (here) to validate that canonical-source finds namespaces in jar files. You can check this by invoking that function directly, e.g.:

(cider.nrepl.inlined-deps.orchard.v0v5v0-beta9.orchard.namespace/canonical-source
 'clojure.data) ; Provided you have the source on your classpath

If that fails, some assumption of orchard.namespace has been violated. But I suspect the problem is downstream of this.

rpkarlsson commented 5 years ago

orchard.namespace finds the source. It's that .getPath seems to be missing the "jar:" prefix.

cider.nrepl.middleware.ns/ns-path does something like:

(.getPath (orchard.namespace/canonical-source 'clojure.string))
; => "file:/home/rpkarlsson/.m2/repository/org/clojure/clojure/1.10.0/clojure-1.10.0.jar!/clojure/string.clj"

https://github.com/clojure-emacs/cider-nrepl/blob/49462f13c5a87ff071551150cc6ceb61ef967c8c/src/cider/nrepl/middleware/ns.clj#L88

Cider expects that path to be "jar:file:[...]".

jeffvalk commented 5 years ago

Seems Cider should be calling (str ...) rather than (.getPath ...) on the result of canonical-source.

clojure-emacs / cider