->/->> not handled

[vemv]

The problem

The two following pieces of code should get the same completions, but they don't:

(defn x [^String arg]
  (__prefix__ arg)) ;; for a prefix value of ".", String completions will be offered

(defn x [^String arg]
  (-> arg __prefix__)) ;; for a prefix value of ".", Object completions will be offered

Possible solution

Perform certain macroexpansions as an early step in compliment.context/parse-context.

The following POC worked fine for me:

(ns compliment.sources.poc
  (:require
   [compliment.sources.class-members :as src]
   [compliment.context :as ctx]
   [compliment.t-helpers :refer :all]
   [clojure.walk :as walk]))

(defn macroexpanded-context [form]
  (->> form
       (walk/postwalk (fn [x]
                        (if (and (list? x)
                                 (-> x first #{'-> '->>}))
                          (macroexpand-1 x)
                          x)))
       ctx/parse-context))

(strip-tags (src/members-candidates "." *ns* (macroexpanded-context '(defn x [^String arg]
                                                                       (-> arg __prefix__)))))

WDYT?

Cheers - Victor

[alexander-yakushev]

That's a neat idea, Victor! I also don't like the fact that typetag-based completion doesn't work when being threaded.

Unfortunately, we cannot do macroexpansion in context parsing – because that potentially executes arbitrary code, and we don't want to do that on behalf of the user when just doing the completions. Imagine this very implausible but hypothetically possible scenario:

(defmacro bomb-away! [& body]
  (launch-bombs-for-real!)
  (clojure.java.shell/sh "rm" "-rf" "/")
  (System/exit 0)
  ~@body)

(bomb-away! (let [s "this is the code I never intended to execute"]
                  kw :pri___ ;; and this is where completion is automatically triggered

However, I'm open to hard-coding the handling of ->/->> specifically.

[alexander-yakushev]

Oops, sorry, I didn't look at your suggestion carefully enough to see that you are only macroexpending the threading form. That's much better. However, if this is already close to the hard-coding approach, I'd say it's better to be on the safe side and not use macroexpand at all.

[vemv]

Thanks for the reply!

I see :)

Do you find it likely that this feature will make it reasonably soon?

[alexander-yakushev]

If you submitted a PR, it would be almost instant:) Otherwise, I'll have to find the time to look into it.

[vemv]

👍 !

I might try things, but before I do, it's worth asking: when you see a -> token, how do you know it corresponds to clojure.core/-> and not something else?

• A var of this ns overriding ->
• A refered var overriding ->
• A local let binding overriding ->

The two first cases seem easy to check, by inspecting *ns*.

How to you typically handle this?

[alexander-yakushev]

Yes, you can try to resolve -> symbol in the current namespace to see if it's not rebound to something else. And yes, you can't really know if it's not shadowed by a local var, however it's not really likely that someone would rebind threading macros to some functions locally.

That being said, I don't think you need to check for that at all. The cases where -> doesn't stand for clojure.core/-> are vanishingly infrequent, and the "cost of error" for us is very low – the worst thing that could happen is that we hide some completion candidates which we shouldn't have, no big deal.

[didibus]

If you checked that -> was indeed bound to clojure.core/-> I see no reason why the macro-expansion solution would not work. The -> and ->> do not execute arbitrary code, that's known, so I don't see the risk here.

[vemv]

Hi again!

I wonder if you could give a shot to this feature. Currently I'm unable to contribute to OSS due to IP concerns (I might get that fixed at some point though... hard to tell when)

That being said, I don't think you need to check for that at all.

I tend to agree now. Even if a custom -> as in scope, it's pretty safe to infer that it essentially has -> semantics, so macroexpanding it as clojure.core/-> would be equivalent (that is, for what compliment cares about)