Incomplete specification of the info and eldoc ops

[yuhan0]

https://docs.cider.mx/cider-nrepl/nrepl-api/ops.html#info
The specification for the info op only mentions the following keys in the return message:

:doc-block-tags-fragments - may be absent
:doc-first-sentence-fragments - may be absent
:doc-fragments - may be absent
:status - `done`

In reality, the frontend (Emacs / cider-doc) relies on various other compulsory (non-nil) keys in the map, such as :name and :ns.

When the info op is requested for ill-formed input (eg. a non-existent var), the cider-nrepl implementation signals this by returning :no-info in its status:

cider-nrepl/src/cider/nrepl/middleware/info.clj

Line 129 in 11ae4ee

{:status :no-info}))

This is also undocumented, but relied upon by the frontend to guard against parsing of an empty (malformed) var-info response.

https://github.com/clojure-emacs/cider/blob/810337cee931d9f14aa16550a74516f8337a47f3/cider-client.el#L691
(Relevant call stack: cider-doc-lookup -> cider-create-doc-buffer -> cider-var-info -> cider-sync-request:info)

Similarly, the eldoc op implicitly relies on a :no-eldoc status to signal an empty return value.

The problem occurs with the babashka.nrepl implementation of the protocol, which also claims to provide the info and eldoc ops, but sometimes returns an map missing many 'required' keys such as :name, and a done status. This results in varying degrees of breakage when CIDER is connected to a babashka-type REPL.

Clearly this would be a simple fix on the impl to return the appropriate :no-info / :no-eldoc statuses, but I thought it would be worth properly pinning down the return contracts before raising an issue there.

Otherwise given the current underspecified docs, it could technically be seen as a well-formed response that requires additional logic on the frontend(s) to guard against nil values.

Steps to reproduce (Emacs / Cider 1.15)

• Create a standalone .bb file with the contents:

;; repro.bb
(ns repro)

(defn foo
  "See also: `bar`"
  [])

• cider-jack-in with the babashka project type (C-u 3 C-c M-j)

• cider-load-buffer, then cider-doc on the foo symbol.

• Click the xref link for bar in the doc buffer (a non-existent var).

• The elisp error is thrown: Wrong type argument: stringp, nil
  (Note: cider-doc on a non-existent symbol is wrapped in a condition-case, and falls back to a minibuffer prompt on encountering this error)

• Also observe the broken eldoc displayed in calls to non-existent vars:

(oops  ) ; <- with cursor in form

Environment & Version information

NA

[yuhan0]

Another related error with cider-eldoc + regular JVM Clojure:

;; repro.clj
(ns repro)

(def undocumented ;; <- cursor here
  123)

• Ensure the default config of (setq cider-eldoc-display-for-symbol-at-point t)
• cider-jack-in-clj, cider-load-buffer
• Place cursor on the symbol undocumented, and observe in the echo area: eldoc error: (wrong-type-argument arrayp nil)

Here, the issue is that CIDER expects the eldoc op response to contain a string under the key :docstring, which is unspecified: https://docs.cider.mx/cider-nrepl/nrepl-api/ops.html#eldoc
The error is thrown from cider-docstring--format -> replace-regexp-in-string.

Again it's unclear where the responsibility for the fix should lie - on the Elisp side of things, or having the middleware eldoc op guarantee to always return a string-valued :docstring response?

[vemv]

or having the middleware eldoc op guarantee to always return a string-valued :docstring response?

I don't think we can possibly guarantee this. What should the returned value be for a (def undocumented 123)?

Other than that, PR welcome with documentation changes!

In case you weren't familiar, the workflow is editing src/cider/nrepl.clj and then running lein docs.

[yuhan0]

Actually, the contents of the response originate from Orchard, which simply assocs :docstring = nil to the map when the var has no :doc metadata:
https://github.com/clojure-emacs/orchard/blob/6e845a7fe364839e99bb4b4f59ecde5f698d0583/src/orchard/eldoc.clj#L63

Ideally, the user-facing Eldoc message should display something like "(not documented)", but would that be an upstream change in Orchard? Or simply propagate the nil and defer the presentation to the frontend? (Do we differentiate between missing vs. empty-string :doc meta?)

(Also, this was extra confusing because I was under the impression
that Bencode had no way to encode nils and instead uses empty strings or surrogates like "-1" to pun for a missing value. But it turns out that Clojure nils are simply sent over the wire as empty lists :/
This probably deserves to be a separate issue but I couldn't figure out which repo it belonged to 👀)

[vemv]

"(not documented)"

Not sure if this would be pretty or desirable, from a user perspective.

For clarity, can't we just deal with the empty list Elisp side?

[yuhan0]

can't we just deal with the empty list Elisp side?

That's the bit I was unsure about, relying on the fact that Clojure nil -> Bencode list -> Emacs nil.

Going by the comment here by @bbatsov clojure-emacs/cider#3711 (comment)

Indeed. Bencode has no representation for nil.

Are there other places in the code that implicitly rely on this sort of empty-list = nil punning?
I see a lot of empty-string punning around Cider, ie. checks like (string= var "").

For example the ns-path op returns empty strings when given 'bad' inputs:

(with-current-buffer "repro.clj"
  (cider-nrepl-send-sync-request '("op" "ns-path" "ns" "clorjue.core")))
;; =>
(dict "status" ("done")
      "id" "22"
      "session" "1a73fedf-5506-4623-9ec4-3b93b199f668" 
      "path" ""
      "url" "")

But I don't have a broad enough overview of the shapes of data flowing back and forth through all these various ops, and it doesn't help that the docs are incomplete and vague about input / return types.

[vemv]

That's the bit I was unsure about

Personally I'd advocate for omitting the inclusion of keys that cannot have a meaningul value and return err keys instead.

Regardless of bencode intricacies, it feels generally cleaner.

In many codebases, return types look like this:

{age: Int} | {error: String}

...but one doesn't intentionally include an age of -1 to signal an error, or accidentally include it while :error is also present.

Elisp side, this seems easy to handle - if an expected key is absent or if an err key is present, one handles the error. That's two ways to signal errors, non exclusive. So plenty of chances for consumers to realise what's wrong!

Not sure of @bbatsov agrees with me.

[ikappaki]

(@yuhan0, sorry for jumping into this issue for my problem, but I thought this thread is most relevant)

Hi @vemv,

I plan to raise a PR to update the info op doc entry in the manual to document the Returns value for the basic case where the nREPL client request info about a symbol defined in a local file. I'll also note that the info doc is still incomplete unless someone knowledgeable or confident enough can update it with the full spec? Unfortunately, after reviewing both cider-nrepl and orchard source code and tests, the full spec is still unclear to me.

I've encountered an issue with the Basilisp nREPL server and VS Code Calva, where info's return :file key value, in retrospect, should be a URI but instead is set to an absolute file path (e.g. d:\dev\clj\issue-cider-info\src\a.clj instead of file:/D:/dev/clj/issue-cider-info/src/a.clj). This causes Calva to fail when trying to switch to a symbol definition in another file.

I plan to fix this in the Basilisp nREPL server but need to reference the spec for proper housekeeping, which is why I'm proposing this partial doc update with your blessing.

Below is a CIDER log extract showing the return value for an info request on a/abc from the Clojure nREPL server, demonstrating that the :file return value should be a URI (not an absolute file path as in the Basilisp nREPL case), and should be documented as such

(-->
  id         "18"
  op         "info"
  session    "cdbe8f64-e972-482c-9677-9ae456aaae96"
  time-stamp "2024-09-17 18:59:41.733894000"
  ns         "b" 
  sym        "a/abc"
)
(<--
  id           "18"
  session      "cdbe8f64-e972-482c-9677-9ae456aaae96"
  time-stamp   "2024-09-17 18:59:41.736375000"
  arglists-str "[]"
  column       1
  file         "file:/D:/dev/clj/issue-cider-info/src/a.clj"
  line         3
  name         "abc"
  ns           "a"
  resource     "a.clj"
  status       ("done")
)

Thanks

[vemv]

Sounds good, thanks!