Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved var-usages analysis for multimethods #2264

Open
1 task done
beoliver opened this issue Jan 30, 2024 · 1 comment
Open
1 task done

Improved var-usages analysis for multimethods #2264

beoliver opened this issue Jan 30, 2024 · 1 comment

Comments

@beoliver
Copy link

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

  • I have read the Clojure etiquette and will respect it when communicating on this platform.

Is your feature request related to a problem? Please describe.

I would like to use the :var-usages data from the :analysis to perform static analysis.

Currently, it appears that no information about referenced vars is preserved for defmethod expressions and while referenced vars are preserved for defmulti expressions, the fact that the source var is a multimethod is not.

As a concrete example, assuming that I have the following namespace

(ns my-ns)
(defn foo [x] x)
(defn bar [x] (foo x))
(defmulti baz identity)
(defmethod baz :call-foo [x] (foo x))

If I run the following analysis

(kondo/run! {:lint [<path>] :skip-lint true :config {:analysis true}})

I get the following under [:analysis :var-usages] (I have removed some keys to reduce noise)

  1. {:name defn, :from my-ns, :macro true, :to clojure.core}
  2. {:name foo, :from my-ns, :from-var bar, :to my-ns}
  3. {:name defn, :from my-ns, :macro true, :to clojure.core}
  4. {:name identity, :from my-ns, :from-var baz, :to clojure.core}
  5. {:name defmulti, :from my-ns, :macro true, :to clojure.core}
  6. {:name baz, :defmethod true, :dispatch-val-str ":call-foo", :from my-ns, :to my-ns}
  7. {:name foo, :from my-ns, :to my-ns}
  8. {:name defmethod, :from my-ns, :macro true, :to clojure.core}

From this we can see that foo is called from var bar (entry 2) and that identity is called from the var bar (entry 4).

Note that in entry 4 baz is not marked as a multimethod. The fact that the identity function is (part of) the dispatch-fn for the multimethod is not captured. Moreover, it is not shown that foo is also called from the multimethod baz with dispatch-val :call-foo.

Entry 6 suggests that baz is called from the top level (there is no :from-var).

Ideally, multimethods could be handled in a more nuanced way for the var-usage analysis.

Describe the solution you'd like
A clear and concise description of what you want to happen.

I would like entry 4 in the :var-usages to include the key/val pair :defmulti true To indicate that identity must have been referenced from the dispatch-fn. (I don't think there would be another reason to reference a var here?)

{:name identity, :from my-ns, :from-var bar, :to clojure.core :defmulti true}

Entry 6 is slightly more problematic. Currently it is

{:name baz :defmethod true, :dispatch-val-str ":call-foo", :from my-ns, :to my-ns}

The lack of a :from-var indicates that baz is referenced from the top level. Perhaps this is fine and should stay - however, I would like to see the following (or similar) included.

{:name foo, :from-var baz, :defmethod true, :dispatch-val-str ":call-foo", :from my-ns, :to my-ns}

I am not sure if this would make sense coexisting with entry 6?

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Writing my own tool for performing such analysis.

Additional context
Add any other context or screenshots about the feature request here.

Open to looking into this, but not sure about the scope/how difficult it would be?

@borkdude
Copy link
Member

Open to looking into this, but not sure about the scope/how difficult it would be?

I haven't had a look yet, but I'd be happy to answer any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants