clojure-lsp / clojure-lsp

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

Can't seem to configure :clojure-lsp/unused-public-var at the namespace level #1453

Open ikappaki opened 1 year ago

ikappaki commented 1 year ago

Describe the bug Can't turn off :clojure-lsp/unused-public-var level in namespace metadata under the :clj-kondo/config key

To Reproduce

  1. Create a src/exp.clj file with an unused var while turning the unused-public-var linter off.
    
    (ns exp
    {:clj-kondo/config '{:linters {:clojure-lsp/unused-public-var {:level :off}}}})

(def a nil)

2. Run `clojure-lsp diagnostics`, you should get an message about unused public var `exp/a`

$ clojure-lsp diagnostics [100%] Project analyzed
Finding diagnostics... src\exp.clj:3:5: info: [clojure-lsp/unused-public-var] Unused public var 'exp/a'

**Expected behavior**
There should be no informational produced since the linter is turned off in the clj-kondo namespace configuration.

**User details (please complete the following information):**
 - OS: Windows 10
 - Version: clojure-lsp 2022.12.09-15.51.10

**Additional context**

The namespace config works for other linters, e.g.

unresolved symbol `u` is reported below alongside `:clojure-lsp/unused-public-var`

```clojure
(ns exp
  {:clj-kondo/config '{:linters {:clojure-lsp/unused-public-var {:level :off}
                                 }}})

(def a nil)

(+ u 1)
clojure-lsp diagnostics
[100%] Project analyzed            
Finding diagnostics...
src\exp.clj:5:5: info: [clojure-lsp/unused-public-var] Unused public var 'exp/a'
src\exp.clj:7:3: error: [unresolved-symbol] Unresolved symbol: u

but is not reported as expected when the unresolved linter is turned off, i.e. the clj-kondo/config works for unresolved-symbol but not for unused-public-var

(ns exp
  {:clj-kondo/config '{:linters {:clojure-lsp/unused-public-var {:level :off}
                                 :unresolved-symbol {:level :off}}}})

(def a nil)

(+ u 1)
$ clojure-lsp diagnostics
[100%] Project analyzed            
Finding diagnostics...
src\exp.clj:4:5: info: [clojure-lsp/unused-public-var] Unused public var 'exp/a'
ericdallo commented 1 year ago

This happens because this linter is a clj-kondo custom linter used in :custom-lint-fn, I think clj-kondo should support that but not sure how hard would be, WDYT @borkdude ? (I can't recall or find a issue related to that if already exists)

borkdude commented 1 year ago

I think it might be better to do this as :config-in-ns and then the lsp linter itself can look into the config?

ericdallo commented 1 year ago

I was trying to avoid having more custom lint logic on lsp side, since other clj-kondo users can use custom-lint-fn, if they apply a reg-finding! they would face the same issue not being able to ns meta config or need to manually read/handle :config-in-ns, right?

borkdude commented 1 year ago

Makes sense, but the reg-finding is called without any reference to the namespace, so how would clj-kondo know how to suppress that lint warning?

ericdallo commented 1 year ago

Good point, maybe we could add a extra arg to that reg-finding?

borkdude commented 1 year ago

I don't see the complete picture of this yet, but I'm open to an experimental PR to explore that idea

ikappaki commented 1 year ago

Hi @ericdallo,

is there anything I can help you out with this to get it moving?

thanks

ericdallo commented 1 year ago

@ikappaki my idea is: currently clj-kondo exposes a :custom-lint-fn arg that clojure-lsp passes during every lint, as arg of that function, clj-kondo passes reg-findin!, which currently clojure-lsp calls it here, I was thinking if clj-kondo could receive on that map arg the :ns and internally check the ns config as it does for other linters

mainej commented 1 year ago

@borkdude is it correct that clj-kondo collects ns metadata config while a file is being read, uses it while processing that file, and then discards it?

If so, your plan sounds hard @ericdallo. Consider the following two files:

;; using.clj
(ns using
  (:require [defining]))

defining/a-var

;; defining.clj
(ns defining
  {:clj-kondo/config '{:linters {:clojure-lsp/unused-public-var {:level :off}}}})

(def a-var 1)

If the user deletes the var-usage from using.clj, then the var-definition in defining.clj is unused.

However, in this situation we ask clj-kondo to analyze the contents of using.clj, but not defining.clj. During the analysis run clj-kondo won't ever see the ns metadata config from defining.clj, the config which it needs to decide whether to include lint on a var defined in the defining ns.

It should have seen that config in a previous run, whenever defining.clj was last analyzed, but that means you need to give clj-kondo a durable cache of namespace -> config (or maybe [filename namespace] -> config) which it can re-use in later runs. It's not impossible, just sounds like quite a lot of work.

I wish I had a better suggestion about how to implement this, but unfortunately I don't. Just pointing out problems 😛

borkdude commented 1 year ago

For this situation it's preferred to use :config-in-ns