Figure out how to encode nil and an empty list differently

[bbatsov]

One big pain when working with nREPL today is that in Emacs a nil and an empty list are the same thing. This means that nils and empty lists get encoded the same way (as empty lists) in nREPL requests and we can't differentiate them in the nREPL middlewares.

We need to figure out a some special way to encode nil to avoid problems like #2357 down the road.

@vspinu Any thoughts on this? I was thinking we might just treat some symbol specially - e.g. nrepl/nil or something like this. It's extremely unlikely something like this would come up in user code.

[vspinu]

I think this part is best handled in the bencode/nrepl side. If you pass a symbol you will have to check for that symbol in every middleware, or have a special middleware which would replace all those recursively in each received message.

Let me check the bencode decoder on the nrepl side. Maybe there is a loophole which could be exploited.

[vspinu]

Cannot be done without overwriting one or two nrepl.bencode functions. Especially that nil is used as a termination tocken for the entire message.

The write part uses multimethods, meaning that we can write a custom nil on the clojure side. Unfortunately it's not what is needed here.

I think nrepl/nil what you have in mind is the best option without changes to tools.nrepl. A shorter alternative might be a control character like ^N.

[vspinu]

But I am not sure if this nrep/nil workaround would address the core issue. Middleware should not be relying on nil values because nrepl cannot pass those in either direction.

[bbatsov]

I guess when you send nil from the server to the client that'd be OK even for Emacs. What I meant was actually to patch the bencoding on Emacs's side just to add a difference between nil and empty list there. While I agree that passing nils to APIs is the same as not passing anything it seems to me we should be able to do it, without breaking anything and without polluting the middleware with Emacs specific checks (e.g. (seq something) to coerce empty lists to nil).

[vspinu]

I guess when you send nil from the server to the client that'd be OK even for Emacs.

You cannot send nil from server. It is send as an empty list.

actually to patch the bencoding on Emacs's side just to add a difference between nil and empty list there

This won't help, because nrepl itself doesn't differentiate between nil or an empty list. One needs to patch the bencode on the server side or workaround it by encoding nil as nrepl/nil string, then substitute nil in some coerce middleware on the server side. The latter doesn't make much sense for orchard because other clients beside cider won't be doing the same hack. Thus, relying on explicit nil in the orchard middleware is just not a good idea in the first place.

One way to patch the emacs bencode is to drop all nil components and thus enforce nil-punning. But then if one wants to send an empty list you have to send nrepl/empty-list token and substitute with empty list in the emacs bencode encoder. Doesn't sound worthwhile.

I would still insist that the easiest solution for the middleware for which this matters is just not send anything. But again, if nil or [] matters for a middleware, that middleware is probably doing a wrong thing in the first place.

[bbatsov]

I think nrepl/nil what you have in mind is the best option without changes to tools.nrepl. A shorter alternative might be a control character like ^N.

Changes to nREPL are easy to do as well now, given the fact that I was recently handed over the project. :-) I never read the bencode code, and I'm pretty surprised they decided to treat nil in this manner originally.

I was also thinking that we can also just filter out all nils on the Emacs side, but unfortunately this has to be done manually, unless we introduce some notion of a nil there, so we can differentiate between a nil passed in the middleware request and an empty list. The nrepl-send-request can just remove problematic keys automatically.

But I am not sure if this nrep/nil workaround would address the core issue. Middleware should not be relying on nil values because nrepl cannot pass those in either direction.

Yeah, I think now that probably we should just stop passing nils completely from Emacs. The only problem with this is that it's going to be error-prone - some nils will definitely leak from time to time to the middleware and it'd better for us to know those are nils, not empty lists.

[vspinu]

act that I was recently handed over the project. :-)

Cool! Then it might be really worth extending bencode/nrepl protocol a bit.

The recent change that made empty RETs in the REPL not propagating to the server probably has something to do with this issue. If eval middleware could accept both strings and nils, then one could differentiate between an empty command and an empty string.

it'd better for us to know those are nils, not empty lists.

Indeed.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!