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

Latest update breaks namespace > filename check #1607

Closed LouDnl closed 2 years ago

LouDnl commented 2 years ago

[ To keep development of this project going, consider sponsoring. If you are already a sponsor, thank you! ]

version

v2022.3.8

platform

Windows 10 Microsoft Windows [Version 10.0.19043.1526]

editor

VSCode: image

problem

Latest update that fixes the namespace bug introduces new namespace bug: image image

repro

Create project, create clojure file with namespace. Error occurs immediatly.

config

{:hooks   {:analyze-call {taoensso.timbre/refer-timbre hooks.taoensso.timbre/refer-timbre
                          sibiro.params/defnp hooks.sibiro.params/defnp
                          sibiro.params/defnp- hooks.sibiro.params/defnp
                          slingshot.slingshot/try+ hooks.slingshot.try-plus/try+}}
 :lint-as {mount.lite/defstate clojure.core/def
           taoensso.encore/defonce clojure.core/def}
 :linters {:unused-referred-var {:exclude {mount.lite [defstate]}}
           :unused-namespace    {:exclude ["^aorta.*"]}
           :clojure-lsp/unused-public-var {:level :off}}
 :skip-comments true}

expected behavior

Do not give namespace / filename error

borkdude commented 2 years ago

/cc @svdo

borkdude commented 2 years ago

Worth mentioning that with a single segment namespace it did work correctly.

borkdude commented 2 years ago

@svdo If you want to look into it, the error happens when calling clj-kondo via this LSP server:

https://github.com/clj-kondo/clj-kondo.lsp/blob/c7f741917ed2d40274ae8696ce5efce23df4a3d4/server/src/clj_kondo/lsp_server/impl/server.clj#L125-L143

borkdude commented 2 years ago

@svdo @LouDnl I have a clj-kondo vsix for debugging here:

extension.zip

I added (spit (io/file "clj-kondo.log") [filename filename* ns-name munged-ns] :append true) to the code in case of a name mismatch, so we can see what clj-kondo encountered. Please post the contents of this file so we can debug further. You still have to enable the linter.

borkdude commented 2 years ago

Using that extension, I was able to reproduce it again on Windows and got the following clj-kondo.log:

type .\clj-kondo.log
["/c:/Users/borkdude/dev/clj-kondo.lsp/server/src/clj_kondo/lsp_server/impl/server.clj" "c:\\Users\\borkdude\\dev\\clj-kondo.lsp\\server\\src\\clj_kondo\\lsp_server\\impl\\server" clj-kondo.lsp-server.impl.server "clj_kondo.lsp_server.impl.server"]["/c:/Users/borkdude/dev/clj-kondo.lsp/server/src/clj_kondo/lsp_server/impl/server.clj" "c:\\Users\\borkdude\\dev\\clj-kondo.lsp\\server\\src\\clj_kondo\\lsp_server\\impl\\server" clj-kondo.lsp-server.impl.server "clj_kondo.lsp_server.impl.server"]["/c:/Users/borkdude/dev/clj-kondo.lsp/server/src/clj_kondo/lsp_server/impl/server.clj" "c:\\Users\\borkdude\\dev\\clj-kondo.lsp\\server\\src\\clj_kondo\\lsp_server\\impl\\server" clj-kondo.lsp-server.impl.server "clj_kondo.lsp_server.impl.server"]
borkdude commented 2 years ago

It looks correct in the REPL:

user=> (def filename "/c:/Users/borkdude/dev/clj-kondo.lsp/server/src/clj_kondo/lsp_server/impl/server.clj")
#'user/filename
user=> (some-> filename
                                    ^String (fs/strip-ext)
                                    ^String (.replace "/" ".")
                                    (cond-> (not= fs/file-separator "/")
                                      (.replace ^CharSequence fs/file-separator ".")))
"c:.Users.borkdude.dev.clj-kondo.lsp.server.src.clj_kondo.lsp_server.impl.server"
svdo commented 2 years ago

@borkdude Is the problem maybe that the return value of babashka.fs/strip-ext appears to be a java.nio.File.Path instance, but in the code that I added there is a ^String type hint; maybe that gives problems with GraalVM (and not in the REPL)?

borkdude commented 2 years ago
user=> (require '[babashka.fs :as fs])
nil
user=> (fs/strip-ext "foo.bar")
"foo"
borkdude commented 2 years ago

The code isn't running in GraalVM in this extension, but in a normal JVM

borkdude commented 2 years ago

I'll add some more logging tomorrow and then I'll publish a new plugin here

borkdude commented 2 years ago

Added more logging:

extension2.zip

borkdude commented 2 years ago

Logs:

c:\Users\borkdude\dev\clj-kondo\src\clj_kondo\core :<-:stripped
c:\Users\borkdude\dev\clj-kondo\src\clj_kondo\core :<-replaced
[:filename "/c:/Users/borkdude/dev/clj-kondo/src/clj_kondo/core.clj" "c:\\Users\\borkdude\\dev\\clj-kondo\\src\\clj_kondo\\core" "c:\\Users\\borkdude\\dev\\clj-kondo\\src\\clj_kondo\\core" :ns-name clj-kondo.core :munged-ns "clj_kondo.core"] :append true

It seems the (not= fs/file-separator "/") condition isn't hit in the extension... since I would have also expected :replace2 to be logged:

            (let [log (fn [& strs]
                        (spit (io/file "clj-kondo.log") (str (str/join " " strs) "\n") :append true))
                  filename* (some-> filename
                                    ^String (fs/strip-ext)
                                    (doto (log :<-:stripped))
                                    ^String (.replace "/" ".")
                                    (doto (log :<-replaced))
                                    (cond-> (not= fs/file-separator "/")
                                      (-> (doto (.replace ^CharSequence fs/file-separator ".")
                                            (log :<-replace2)))))
                  munged-ns (str (munge ns-name))]
              (when (and filename*
                         (not (str/ends-with? filename* munged-ns)))
                (log [:filename filename filename* filename* :ns-name ns-name :munged-ns munged-ns] :append true)
borkdude commented 2 years ago

Could it be that an extension is running inside linux in Visual Studio Code on Windows...?

borkdude commented 2 years ago

Removed conditional, always replace backslashes:

extension3.zip

borkdude commented 2 years ago

That seems to work. Well, let's just always replace backslashes, no matter the OS, it seems a sane thing to do.

borkdude commented 2 years ago

One last time, I'm logging the file separator and OS name at startup within the extension:

extension4.zip

     (info "Clj-kondo language server loaded. Please report any issues to https://github.com/borkdude/clj-kondo.")
     (info "OS" (System/getProperty "os.name"))
     (info "FS separator" java.io.File/separator)
borkdude commented 2 years ago

It beats me...

[Info  - 2:46:39 PM] Clj-kondo language server loaded. Please report any issues to https://github.com/borkdude/clj-kondo.
[Info  - 2:46:39 PM] OS Windows 10
[Info  - 2:46:39 PM] FS separator \
svdo commented 2 years ago

Yeah I guess that's the best you can do, good enough for me to be sure. Thanks for all the effort you have put into this thing that seemed so simple!!