clj-kondo / clj-kondo

Static analyzer and linter for Clojure code that sparks joy
Eclipse Public License 1.0
1.69k stars 292 forks source link

Include whether something is a protocol or a multimethod in analysis #405

Closed jimmyhmiller closed 2 years ago

jimmyhmiller commented 5 years ago

Currently analysis includes if something is a macro. This is useful information and I think should be extended to at least include protocols and multimethods.

This could be done by adding similar keys for protocol and multimethod (:protocol and :multimethod) or some generalized notion like "type" (This is how codox does it).

I think having this information would be useful for a number of different tools. One I'm considering using the analysis part of clj-kondo to produce information similar to that consumed by cljdoc (which uses codox). Getting fast feedback on what my docs would look like would be super valuable and the current process is a little clunky.

If you are okay with including that information, I'd be happy to do the work to add it. Thanks for creating this. Super excited to use this analysis data.

borkdude commented 5 years ago

@jimmyhmiller Seems like a good feature to add. I find "type" a little bit undescriptive, so I think I prefer the proposed booleans.

borkdude commented 5 years ago

@jimmyhmiller Just to be clear: you have my blessing to work on this.

vemv commented 3 years ago

Perhaps an obvious observation, but implementing this would allow one to implement "find protocol implementations" which is a commonly desired IDE feature

borkdude commented 3 years ago

Maybe @ericdallo would like to use this in clojure-lsp?

ericdallo commented 3 years ago

Yes! Actually, there is a whole LSP method called textDocument/implementation that could use that as @vemv mentioned :) ATM clojure-lsp doesn't support that method, with that feature, this would be possible!

borkdude commented 3 years ago

I think we should distinguish between multimethod and protocol definitions and their implementations somehow. Is there any prior work we can look at to adopt a standard way of naming? The linked codox code in the OP only shows definitions I believe?

stale[bot] commented 2 years ago

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 contributions.

lread commented 2 years ago

TLDR; depending on the use case, we might be able to infer what we need to with existing clj-kondo analysis.

But... also maybe not? I've jotted my understanding down here and would love to know where I've got things wrong. Also, very happy to help if there is work to be done here.

Context: this is currently interesting to me as I am taking a look at using clj-kondo as an alternate API analyzer for cljdoc.

I've been doing a little poking around.

Codox (and its offstring, cljdoc-analyzer) has a :type which can be:

macros In clj-kondo, macros are covered (as stated in OP).

I think I can rely on :macro true - but could also possibly have relied on :defined-by clojure.core/defmacro?

Sample var definition:

{:end-row 709,
 :meta nil,
 :name-end-col 17,
 :name-end-row 703,
 :name-row 703,
 :ns rewrite-clj.zip,
 :name edit->,
 :defined-by clojure.core/defmacro,
 :lang :clj,
 :filename "rewrite_clj/zip.cljc",
 :macro true,
 :col 1,
 :name-col 11,
 :end-col 64,
 :arglist-strs ["[zloc & body]"],
 :varargs-min-arity 1,
 :doc
 "Like `->`, threads `zloc` through forms.\n   The resulting zipper will be located at the same path (i.e. the same\n   number of downwards and right movements from the root) as incoming `zloc`.\n   \n   See also [[subedit->]] for an isolated edit.",
 :row 703}

multimethods In clj-kondo :define-by clojure.core/defmulti helps us to identify multimethods?

{:end-row 42,
 :meta {:private true},
 :name-end-col 32,
 :name-end-row 41,
 :private true,
 :name-row 41,
 :ns rewrite-clj.parser.core,
 :name parse-next*,
 :defined-by clojure.core/defmulti,
 :lang :clj,
 :filename "rewrite_clj/parser/core.cljc",
 :col 1, 
 :name-col 21, 
 :end-col 33, 
 :row 41}

An example of a cljdoc-analyzer parsed multi-method:

{:name start
 :type :multimethod
 :file "metagetta_test/test_ns1/multimethod.cljc"
 :line 6}

I'm not sure about associated defmethods, but these don't seem to be handled by codox/cljoc-analyzer either. Probably because they haven't been interesting to API docs?

protocols Protocols in codox/cljdoc-analyzer are interesting in that they are returned with :members.
For example, this

(defprotocol InnerNode
  "Protocol for non-leaf EDN/Clojure/ClojureScript nodes."
  (inner? [node]
    "Returns true if `node` can have children.")
  (children [node]
    "Returns child nodes for `node`.")
  (replace-children [node children]
    "Returns `node` replacing current children with `children`.")
  (leader-length [node]
    "Returns number of characters before children for `node`."))

is parsed by codox/cljdoc-analyzer as:

{:name InnerNode,
 :file "rewrite_clj/node/protocols.cljc",
 :type :protocol,
 :line 77,
 :members
 ({:name children,
   :type :var,
   :arglists ([node]),
   :doc "Returns child nodes for `node`.\n"}
  {:name inner?,
   :type :var,
   :arglists ([node]),
   :doc "Returns true if `node` can have children.\n"}
  {:name leader-length,
   :type :var,
   :arglists ([node]),
   :doc "Returns number of characters before children for `node`.\n"}
  {:name replace-children,
   :type :var,
   :arglists ([node children]),
   :doc
   "Returns `node` replacing current children with `children`.\n"}),
 :doc "Protocol for non-leaf EDN/Clojure/ClojureScript nodes.\n"}

When I look at clj-kondo analysis, I do see a :defined-by which might be a helpful identifier. If I look at the :var-definitions for the above in clj-kondo, I see the protocol itself:

{:end-row 86,
 :meta nil,
 :name-end-col 23,
 :name-end-row 77,
 :name-row 77,
 :ns rewrite-clj.node.protocols,
 :name InnerNode,
 :defined-by clojure.core/defprotocol,
 :lang :clj,
 :filename "rewrite_clj/node/protocols.cljc",
 :col 1,
 :name-col 14,
 :end-col 65,
 :doc "Protocol for non-leaf EDN/Clojure/ClojureScript nodes.", 
 :row 77}

But don't see a strong distinction/link-back for its members, for example, here's children

{:fixed-arities #{1},
 :end-row 86,
 :meta nil,
 :name-end-col 12,
 :name-end-row 81,
 :name-row 81,
 :ns rewrite-clj.node.protocols,
 :name children,
 :defined-by clojure.core/defprotocol,
 :lang :clj,
 :filename "rewrite_clj/node/protocols.cljc",
 :col 1,
 :name-col 4,
 :end-col 65,
 :arglist-strs ["[node]"],
 :doc "Returns child nodes for `node`.", 
 :row 77}

I think maybe I could infer that InnerNode is the protocol by seeing it has no arglists. And then I could infer that children is part of InnerNode checking :row for overlap.

I do see the new :protocol-impls work, and that looks cool, but I don't think it helps with protocol definitions. For handy reference here's an example :protocol-impl entry:

{:impl-ns rewrite-clj.node.keyword,
 :end-row 43,
 :name-end-col 10,
 :protocol-ns rewrite-clj.node.protocols,
 :name-end-row 41,
 :method-name string,
 :name-row 41,
 :defined-by cljs.core/defrecord,
 :protocol-name Node,
 :filename "rewrite_clj/node/keyword.cljc",
 :col 3, 
 :name-col 4, 
 :end-col 22, 
 :row 41}

One idea would be to add the :protocol-name to the :var-definitions protocol members.

notes clj-kondo analysis for most example output above was generated with:

clj-kondo --config '{:config-paths ^:replace [] :output {:format :edn :analysis {:arglists true :namespace-definitions {:meta true} :var-definitions {:meta true} :protocol-impls true}}}' --lint  ~/.m2/repository/rewrite-clj/rewrite-clj/1.0.699-alpha/rewrite-clj-1.0.699-alpha.jar | jet --pretty > dump.edn
ericdallo commented 2 years ago

I just forgot about this issue and opened #1570, which is related

borkdude commented 2 years ago

but implementing this would allow one to implement "find protocol implementations"

This is already available now. See https://github.com/clj-kondo/clj-kondo/tree/master/analysis#extra-analysis.

I think this issue is becoming too bloated and partially solved. #1570 is the remaining issue about multimethods.

borkdude commented 2 years ago

@lread Please create a new issue about your protocol stuff.