clojure-lsp / clojure-lsp

Clojure & ClojureScript Language Server (LSP) implementation
https://clojure-lsp.io
MIT License
1.16k stars 152 forks source link

clojure-lsp misreports unused public var in bb.edn project #1246

Open ikappaki opened 2 years ago

ikappaki commented 2 years ago

Describe the bug clojure-lsp diagnostics misreports as unused public var fns that are referenced by bb.edn tasks.

To Reproduce

create a test directory with the following files (say in ~/clsp-diags-miss-bb) :

bb.edn:

{:paths ["scripts"]
 :tasks
 {hi make/hi}}

scripts/make.clj

(ns make)
(defn hi []
  (println :hi))
  1. cd to the project dir and run diagnostics command:
    ~/clsp-diags-miss-bb $ clojure-lsp diagnostics --verbose
    [  5%] Finding kondo config        [INFO] Folder /home/ikappaki/clsp-diags-miss-bb/.clj-kondo not found, creating for necessary clj-kondo analysis...
    [ 10%] Finding cache               [ERROR] [DB] No cache DB file found
    [INFO] [DB] Reading transit analysis cache from /home/ikappaki/clsp-diags-miss-bb/.lsp/.cache/db.transit.json db took 0ms
    [ 15%] Discovering classpath       [INFO] Finding classpath via `/home/ikappaki/.local/bin/bb print-deps --format classpath`
    [DEBUG] Classpath found, paths:  ["scripts" "/home/ikappaki/.gitlibs/libs/babashka/babashka.core/52a6037bd4b632bffffb04394fb4efd0cdab6b1e/src" "/home/ikappaki/.m2/repository/babashka/babashka.curl/0.1.1/babashka.curl-0.1.1.jar" "/home/ikappaki/.m2/repository/babashka/fs/0.1.2/fs-0.1.2.jar" "/home/ikappaki/.m2/repository/cheshire/cheshire/5.11.0/cheshire-5.11.0.jar" "/home/ikappaki/.m2/repository/clj-commons/clj-yaml/0.7.108/clj-yaml-0.7.108.jar" "/home/ikappaki/.m2/repository/com/cognitect/transit-clj/1.0.329/transit-clj-1.0.329.jar" "/home/ikappaki/.m2/repository/com/taoensso/timbre/5.2.1/timbre-5.2.1.jar" "/home/ikappaki/.m2/repository/hiccup/hiccup/2.0.0-alpha2/hiccup-2.0.0-alpha2.jar" "/home/ikappaki/.m2/repository/http-kit/http-kit/2.6.0-RC1/http-kit-2.6.0-RC1.jar" "/home/ikappaki/.m2/repository/insn/insn/0.5.2/insn-0.5.2.jar" "/home/ikappaki/.m2/repository/nrepl/bencode/1.1.0/bencode-1.1.0.jar" "/home/ikappaki/.m2/repository/org/babashka/babashka.impl.reify/0.1.1/babashka.impl.reify-0.1.1.jar" "/home/ikappaki/.m2/repository/org/babashka/cli/0.3.34/cli-0.3.34.jar" "/home/ikappaki/.m2/repository/org/babashka/sci.impl.types/0.0.2/sci.impl.types-0.0.2.jar" "/home/ikappaki/.m2/repository/org/clojure/clojure/1.11.1/clojure-1.11.1.jar" "/home/ikappaki/.m2/repository/org/clojure/core.async/1.5.648/core.async-1.5.648.jar" "/home/ikappaki/.m2/repository/org/clojure/core.match/1.0.0/core.match-1.0.0.jar" "/home/ikappaki/.m2/repository/org/clojure/core.rrb-vector/0.1.2/core.rrb-vector-0.1.2.jar" "/home/ikappaki/.m2/repository/org/clojure/data.csv/1.0.0/data.csv-1.0.0.jar" "/home/ikappaki/.m2/repository/org/clojure/data.priority-map/1.1.0/data.priority-map-1.1.0.jar" "/home/ikappaki/.m2/repository/org/clojure/data.xml/0.2.0-alpha6/data.xml-0.2.0-alpha6.jar" "/home/ikappaki/.m2/repository/org/clojure/test.check/1.1.1/test.check-1.1.1.jar" "/home/ikappaki/.m2/repository/org/clojure/tools.cli/1.0.206/tools.cli-1.0.206.jar" "/home/ikappaki/.m2/repository/org/clojure/tools.logging/1.1.0/tools.logging-1.1.0.jar" "/home/ikappaki/.m2/repository/rewrite-clj/rewrite-clj/1.0.699-alpha/rewrite-clj-1.0.699-alpha.jar" "/home/ikappaki/.m2/repository/selmer/selmer/1.12.50/selmer-1.12.50.jar" "/home/ikappaki/.m2/repository/com/fasterxml/jackson/core/jackson-core/2.13.3/jackson-core-2.13.3.jar" "/home/ikappaki/.m2/repository/com/fasterxml/jackson/dataformat/jackson-dataformat-cbor/2.13.3/jackson-dataformat-cbor-2.13.3.jar" "/home/ikappaki/.m2/repository/com/fasterxml/jackson/dataformat/jackson-dataformat-smile/2.13.3/jackson-dataformat-smile-2.13.3.jar" "/home/ikappaki/.m2/repository/tigris/tigris/0.1.2/tigris-0.1.2.jar" "/home/ikappaki/.m2/repository/org/flatland/ordered/1.5.9/ordered-1.5.9.jar" "/home/ikappaki/.m2/repository/org/yaml/snakeyaml/1.26/snakeyaml-1.26.jar" "/home/ikappaki/.m2/repository/com/cognitect/transit-java/1.0.362/transit-java-1.0.362.jar" "/home/ikappaki/.m2/repository/com/taoensso/encore/3.21.0/encore-3.21.0.jar" "/home/ikappaki/.m2/repository/io/aviso/pretty/1.1.1/pretty-1.1.1.jar" "/home/ikappaki/.m2/repository/org/clojure/core.specs.alpha/0.2.62/core.specs.alpha-0.2.62.jar" "/home/ikappaki/.m2/repository/org/clojure/spec.alpha/0.3.218/spec.alpha-0.3.218.jar" "/home/ikappaki/.m2/repository/org/clojure/tools.analyzer.jvm/1.2.2/tools.analyzer.jvm-1.2.2.jar" "/home/ikappaki/.m2/repository/org/clojure/data.codec/0.1.0/data.codec-0.1.0.jar" "/home/ikappaki/.m2/repository/org/clojure/tools.reader/1.3.6/tools.reader-1.3.6.jar" "/home/ikappaki/.m2/repository/javax/xml/bind/jaxb-api/2.3.0/jaxb-api-2.3.0.jar" "/home/ikappaki/.m2/repository/org/msgpack/msgpack/0.6.12/msgpack-0.6.12.jar" "/home/ikappaki/.m2/repository/com/taoensso/truss/1.6.0/truss-1.6.0.jar" "/home/ikappaki/.m2/repository/org/clojure/core.memoize/1.0.253/core.memoize-1.0.253.jar" "/home/ikappaki/.m2/repository/org/clojure/tools.analyzer/1.1.0/tools.analyzer-1.1.0.jar" "/home/ikappaki/.m2/repository/org/ow2/asm/asm/9.2/asm-9.2.jar" "/home/ikappaki/.m2/repository/com/googlecode/json-simple/json-simple/1.1.1/json-simple-1.1.1.jar" "/home/ikappaki/.m2/repository/org/javassist/javassist/3.18.1-GA/javassist-3.18.1-GA.jar" "/home/ikappaki/.m2/repository/org/clojure/core.cache/1.0.225/core.cache-1.0.225.jar"]
    [INFO] [Startup] Using source-paths from classpath: ["/home/ikappaki/clsp-diags-miss-bb/scripts"]
    [ 20%] Copying kondo configs       [INFO] Copying kondo configs from classpath to project if any...
    Configs copied:
    - .clj-kondo/babashka/fs
    - .clj-kondo/rewrite-clj/rewrite-clj
    [INFO] Copied kondo configs, took 60ms secs.
    [ 25%] Analyzing external classpath[INFO] Analyzing classpath for project root /home/ikappaki/clsp-diags-miss-bb
    [INFO] Analyzing 49 paths with clj-kondo
    [ 44%] Analyzing external classpathConfigs copied:
    - .clj-kondo/babashka/fs
    - .clj-kondo/rewrite-clj/rewrite-clj
    [INFO] External classpath paths analyzed, took 2160ms
    [INFO] :maintain-dep-graph 35ms
    [INFO] Manual GC after classpath scan took 16ms
    [INFO] Caching db for next startup...
    [INFO] [DB] Upserting transit analysis to /home/ikappaki/clsp-diags-miss-bb/.lsp/.cache/db.transit.json cache took 320ms
    [ 50%] Analyzing project files     [INFO] [Startup] Analyzing source paths for project root /home/ikappaki/clsp-diags-miss-bb
    [INFO] [Startup] Project only paths analyzed by clj-depend, took 0ms
    [ 99%] Analyzing project files     No configs copied.
    [INFO] :maintain-dep-graph 0ms
    [INFO] Linting whole project for unused-public-var took 1ms
    [INFO] [Startup] Project only paths analyzed by clj-kondo, took 8ms
    [INFO] :maintain-dep-graph 0ms
    [100%] Project analyzed            
    Finding diagnostics...
    scripts/make.clj:1:6: info: [clojure-lsp/unused-public-var] Unused public var 'make/hi'

Expected behavior no diagnostics message should be produced about Unused public var, since make/hi is referenced by bb.edn.

User details (please complete the following information):

ericdallo commented 2 years ago

We would need to make clj-kondo return analysis for bb.edn, then clojure-lsp would find that those functions are really being used as bb tasks, what you think @borkdude?

borkdude commented 2 years ago

Yeah, there is a clj-kondo issue to provide linting for bb.edn as well. If we implement that, then this would work as well. But for now you could maybe just treat the task .clj code as a library (so it will ignore unused public vars)?

ericdallo commented 2 years ago

@borkdude what do you mean to treat the task .clj as a library? How can we know that a specific user ns is supposed to be a library?

borkdude commented 2 years ago

@ericdallo "as a library" I mean: a namespace in which all vars should not be linted as unused public vars. This is a general problem with that linter right?

ericdallo commented 2 years ago

Yes, I think that would be a new feature for clojure-lsp where user specifies those ns, right? not sure how common is that since most cases I saw are related to functions that are being used on edn files

borkdude commented 2 years ago

If you write a library, isn't it common that many functions in the public API are not used anywhere else? Except in tests.

ericdallo commented 2 years ago

Yes, but we already have the config to allow that, right? .clj-kondo/config.edn

{:linters {:clojure-lsp/unused-public-var {:exclude #{my-ns}}}}
borkdude commented 2 years ago

Indeed, that's what I intended to say :)

mainej commented 2 years ago

So, @ericdallo, it sounds like it would have been better if I had done {:linters {:clojure-lsp/unused-public-var {:exclude #{make}}}} rather than ignoring each of the make tasks individually. Would you like a PR that corrects that?

ericdallo commented 2 years ago

I'm fine with both

ikappaki commented 2 years ago

Yeah, there is a clj-kondo issue to provide linting for bb.edn as well. If we implement that, then this would work as well. But for now you could maybe just treat the task .clj code as a library (so it will ignore unused public vars)?

HI @borkdude, is this the one you are referring to please? https://github.com/clj-kondo/clj-kondo/issues/1576

borkdude commented 2 years ago

Yup!