Skip to content

Fix #3307: Incorrect special-form eldocs #3323

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
- Allow using `npx nbb` as `cider-nbb-command`.
- [#3281](https://github.com/clojure-emacs/cider/pull/3281): Replace newline chars with actual newlines in `*cider-test-report*` buffer, for prettier error messages.
- Bump the injected `cider-nrepl` to 0.30.
- [#3307](https://github.com/clojure-emacs/cider/issues/3307): Make eldoc highlighting on emacs special forms better match the location of the point when latest `cider-nrepl` is used.

## 1.6.0 (2022-12-21)

Expand Down
25 changes: 22 additions & 3 deletions cider-eldoc.el
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,22 @@ arglists. ELDOC-INFO is a p-list containing the eldoc information."
(cider-eldoc-format-thing ns symbol thing 'fn)
(cider-eldoc-format-arglist arglists pos))))

(defun cider-eldoc-format-special-form (thing pos eldoc-info)
"Return the formatted eldoc string for a special-form.
THING is the special form's name. POS is the argument index of the
special-form's arglists. ELDOC-INFO is a p-list containing the eldoc
information."
(let* ((ns (lax-plist-get eldoc-info "ns"))
(symbol (lax-plist-get eldoc-info "symbol"))
(arglists (mapcar (lambda (arglist)
(if (equal (car arglist) symbol)
(cdr arglist)
arglist))
(lax-plist-get eldoc-info "arglists"))))
(format "%s: %s"
(cider-eldoc-format-thing ns symbol thing 'fn)
(cider-eldoc-format-arglist arglists pos))))

(defun cider-highlight-args (arglist pos)
"Format the the function ARGLIST for eldoc.
POS is the index of the currently highlighted argument."
Expand Down Expand Up @@ -470,9 +486,12 @@ Only useful for interop forms. Clojure forms would be returned unchanged."
(pos (lax-plist-get sexp-eldoc-info "pos"))
(thing (lax-plist-get sexp-eldoc-info "thing")))
(when eldoc-info
(if (eq (cider-eldoc-thing-type eldoc-info) 'var)
(cider-eldoc-format-variable thing eldoc-info)
(cider-eldoc-format-function thing pos eldoc-info))))))
(cond
((eq (cider-eldoc-thing-type eldoc-info) 'var)
(cider-eldoc-format-variable thing eldoc-info))
((eq (cider-eldoc-thing-type eldoc-info) 'special-form)
(cider-eldoc-format-special-form thing pos eldoc-info))
(t (cider-eldoc-format-function thing pos eldoc-info)))))))

(defun cider-eldoc-setup ()
"Setup eldoc in the current buffer.
Expand Down
56 changes: 56 additions & 0 deletions test/cider-eldoc-tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,37 @@
(expect (cider--eldoc-format-class-names class-names)
:to-equal "(String StringBuffer CharSequence & 1 more)")))))

(describe "cider-eldoc-thing-type"
(it "Identifies special forms correctly"
(let ((eldoc-info '("type" "special-form")))
(expect (cider-eldoc-thing-type eldoc-info) :to-equal 'special-form)))
(it "Identifies functions correctly"
(let ((eldoc-info '("type" "function")))
(expect (cider-eldoc-thing-type eldoc-info) :to-equal 'fn))))

(describe "cider-eldoc-format-special-form"
(before-each
(spy-on 'cider-eldoc-format-thing :and-return-value "formatted thing!")
(spy-on 'cider-eldoc-format-arglist :and-return-value "formatted arglist!"))
(it "Should remove duplicates from special-form arglists that have duplicates"
(let ((eldoc-info '("ns" nil
"symbol" "if"
"arglists" (("if" "test" "then" "else?"))
"type" "special-form")))
(cider-eldoc-format-special-form 'if 0 eldoc-info)
(expect 'cider-eldoc-format-arglist :to-have-been-called-with '(("test" "then" "else?")) 0)))
(it "Should not remove duplicates from special-form arglists that do not have duplicates"
(let ((eldoc-info '("ns" nil
"symbol" "."
"arglists" ((".instanceMember" "instance" "args*") (".instanceMember" "Classname" "args*") ("Classname/staticMethod" "args*") ("Classname/staticField"))
"type" "special-form")))
(cider-eldoc-format-special-form 'if 0 eldoc-info)
(expect 'cider-eldoc-format-arglist :to-have-been-called-with '((".instanceMember" "instance" "args*")
(".instanceMember" "Classname" "args*")
("Classname/staticMethod" "args*")
("Classname/staticField"))
0))))

(describe "cider-eldoc-format-thing"
:var (class-names)
(before-each
Expand Down Expand Up @@ -238,6 +269,31 @@
(expect (cider-eldoc-info-in-current-sexp) :to-equal
'("eldoc-info" (("java.lang.String") ".length" (("this"))) "thing" "java.lang.String/.length" "pos" 0)))))))

(describe "cider-eldoc"
(before-each
(spy-on 'cider-connected-p :and-return-value t)
(spy-on 'cider-eldoc--edn-file-p :and-return-value nil))
(it "Should call cider-eldoc-format-variable for vars"
(spy-on 'cider-eldoc-info-in-current-sexp :and-return-value '("thing" "foo" "pos" 0 "eldoc-info" ("ns" "clojure.core" "symbol" "foo" "type" "variable" "docstring" "test docstring")))
(spy-on 'cider-eldoc-format-variable)
(cider-eldoc)
(expect 'cider-eldoc-format-variable :to-have-been-called-with "foo" '("ns" "clojure.core" "symbol" "foo" "type" "variable" "docstring" "test docstring")))
(it "Should call cider-eldoc-format-special-form for special forms"
(spy-on 'cider-eldoc-info-in-current-sexp :and-return-value '("thing" "if" "pos" 0 "eldoc-info" ("ns" "clojure.core" "symbol" "if" "type" "special-form" "arglists" ("special form arglist"))))
(spy-on 'cider-eldoc-format-special-form)
(cider-eldoc)
(expect 'cider-eldoc-format-special-form :to-have-been-called-with "if" 0 '("ns" "clojure.core" "symbol" "if" "type" "special-form" "arglists" ("special form arglist"))))
(it "Should call cider-eldoc-format-function for functions"
(spy-on 'cider-eldoc-info-in-current-sexp :and-return-value '("thing" "a-fn" "pos" 0 "eldoc-info" ("ns" "foo.bar" "symbol" "a-fn" "type" "function" "arglists" ("function arglist"))))
(spy-on 'cider-eldoc-format-function)
(cider-eldoc)
(expect 'cider-eldoc-format-function :to-have-been-called-with "a-fn" 0 '("ns" "foo.bar" "symbol" "a-fn" "type" "function" "arglists" ("function arglist"))))
(it "Should call cider-eldoc-format-function for macros"
(spy-on 'cider-eldoc-info-in-current-sexp :and-return-value '("thing" "a-macro" "pos" 0 "eldoc-info" ("ns" "clojure.core" "symbol" "a-macro" "type" "macro" "arglists" ("macro arglist"))))
(spy-on 'cider-eldoc-format-function)
(cider-eldoc)
(expect 'cider-eldoc-format-function :to-have-been-called-with "a-macro" 0 '("ns" "clojure.core" "symbol" "a-macro" "type" "macro" "arglists" ("macro arglist")))))

(describe "cider-eldoc-format-sym-doc"
:var (eldoc-echo-area-use-multiline-p)
(before-each
Expand Down