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

False positive: unused-public-var with #(fn) and macroexpand hook #1766

Open
mrkam2 opened this issue Feb 3, 2024 · 4 comments
Open

False positive: unused-public-var with #(fn) and macroexpand hook #1766

mrkam2 opened this issue Feb 3, 2024 · 4 comments
Labels
bug Something isn't working waiting upstream Waiting for lib changes/fix on any dependency lib
Projects

Comments

@mrkam2
Copy link

mrkam2 commented Feb 3, 2024

Describe the bug
clojure-lsp/unused-public-var reports a function as unused when it is used in an anonymous function literal inside a macro configured with a hook.

a.clj:

(ns a)

(defn my-fn [x] x)

(defmacro my-do [& body]
  `(do ~@body))

(defn -main []
  ; Leaving only this line will trigger the unused public var message.
  (my-do (map #(my-fn %) (range 10))))

To Reproduce

  1. Unzip issue.zip
  2. Run clojure-lsp diagnostics
  3. See error
    src/a.clj:3:7: info: [clojure-lsp/unused-public-var] Unused public var 'a/my-fn'

Expected behavior
No such error should be reported.

When either :analyze-call hook is used, or #(my-fn %) is replaced with (fn [x] (my-fn [x])), unused variable is not reported.

@mrkam2 mrkam2 added API Related to API / CLI bug Something isn't working labels Feb 3, 2024
@ericdallo
Copy link
Member

Thanks for the detailed repro @mrkam2, that happens because clj-kondo doesn't provide the var-definition when using the :macroexpand like you mentioned in the .clj-kondo/config.edn file, I can't see anything to do on clojure-lsp, we would need to make clj-kondo export that.

@borkdude could you try that zip project and confirm it's a desired behavior?

@ericdallo ericdallo added waiting upstream Waiting for lib changes/fix on any dependency lib and removed API Related to API / CLI labels Feb 4, 2024
@ericdallo ericdallo added this to Low priority in clojure-lsp via automation Feb 4, 2024
@borkdude
Copy link
Contributor

borkdude commented Feb 6, 2024

The var is available in the analysis but has a :derived-location because #(...) cannot be represented as an s-expression, so when going through macro-expansion the position is derived. Probably lsp ignores this usage, but that's not clj-kondo's issue.

@ericdallo
Copy link
Member

Yeah, we ignore :derived-location otherwise we will cause a breaking change on custom hooks, this was implemented here, so not sure how to fix that in clojure-lsp without breaking custom hooks that was why we ignored derived-location.

@borkdude
Copy link
Contributor

You don't need a location to know that a var is used, even if the var usage has a derived location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working waiting upstream Waiting for lib changes/fix on any dependency lib
Projects
clojure-lsp
Low priority
Development

No branches or pull requests

3 participants