[analysis] A way to differ defmethod analysis from defmethod usages

[ericdallo]

To upvote this issue, give it a thumbs up. See this list for the most upvoted issues.

Is your feature request related to a problem? Please describe.
Related clojure-lsp/clojure-lsp#656.
I'd like to implement find implementations on multimethods, for that work I need a way to tell that a analysis element is a defmulti definition, a defmethod definition or a usage of a defmethod.
Code

(defmulti greeting :type)
(defmethod greeting :hello [])
(greeting {:type :hello})

Analysis output

;; defmulti
{:ns clojure-lsp.queries,
 :name greeting,
 :defined-by clojure.core/defmulti,
 :bucket :var-definitions
 ...position and filename...}
 
 ;;defmethod
 {:name greeting,
  :from clojure-lsp.queries,
  :context {},
  :bucket :var-usages,
  :to clojure-lsp.queries
  ...position and filename...}
 
 ;; usage
 {:name greeting,
  :from clojure-lsp.queries,
  :context {},
  :arity 1,
  :bucket :var-usages,
  :to clojure-lsp.queries
  ...position and filename...}

you can see both usage and defmethod are the same.

Describe the solution you'd like
I'd like a way to tell that one is defmethod and other is usage, the most simple solution would be to have a defined-by field with a value clojure.core/defmethod when from defmethod otherwise nil

Describe alternatives you've considered
@borkdude suggested having a separated multimethod-impls, that would work, but not sure multimethods are as complex as protocols/impls to have a separated bucket for them and since we already have the :defined-by on var-definitions, that sounds like a good approach to me.

Additional context
I can work in a PR when I find the time

[ericdallo]

@borkdude Do you think my suggestion is valid? Can I open a PR following that idea?

[borkdude]

@ericdallo Sorry, it slipped off my radar. I think I'm going to have some minor things to change, but overall I think it's good to make a PR, if you don't mind changing some things after the fact. Else, I'm going to look at this more closely in the weekend, hopefully.

[ericdallo]

No problem, take your time!

[borkdude]

@ericdallo Finally looked at this. I think defined-by clojure.core/defmethod would maybe be a bit weird since this isn't defining a new var, which is what defined-by is for.

Should we move the second case to a new bucket, since it's not really a var usage, but more like protocol-impls: it adds another dispatch value to the multimethod table. So maybe multimethod-impls?

[ericdallo]

I see, it's not a defined-by indeed, I think the way is multimethod-impls the closest to what it means.

[borkdude]

@ericdallo Or we could just add a key to the var usage's :context that it is a defmethod implementation., perhaps:

:context {:clojure.core/defmethod {:some-stuff ...}}

[ericdallo]

Not sure, context sounds like something extra outside core clojure, but it could be my impression though

[borkdude]

Or maybe just another key :defmethod-impl true. We could later on add more stuff like :defmethod-impl {:dispatch-value ...}} if we need to.

[borkdude]

Maybe the last one is the best option. A better name maybe: :defmethod true or :defmethod {....}

[ericdallo]

:defmethod true sounds enough and easy to enhance in the future

[ericdallo]

@borkdude There is a new clojure-lsp issue that depends on this issue, so I can take this one and implement it, so, are we good to go with the :defmethod true on the var-usage itself?

[borkdude]

Yes.

[ericdallo]

#1608

[borkdude]

Thanks!