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

Latest update breaks namespace > filename check #1607

Closed
LouDnl opened this issue Mar 8, 2022 · 18 comments
Closed

Latest update breaks namespace > filename check #1607

LouDnl opened this issue Mar 8, 2022 · 18 comments
Labels
help wanted Extra attention is needed windows
Projects

Comments

@LouDnl
Copy link

LouDnl commented Mar 8, 2022

[ 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
Copy link
Member

borkdude commented Mar 8, 2022

/cc @svdo

@borkdude borkdude added this to Needs triage in clj-kondo via automation Mar 8, 2022
@borkdude borkdude moved this from Needs triage to High priority (next release) in clj-kondo Mar 8, 2022
@borkdude borkdude added help wanted Extra attention is needed windows labels Mar 8, 2022
@borkdude
Copy link
Member

borkdude commented Mar 8, 2022

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

@borkdude
Copy link
Member

borkdude commented Mar 8, 2022

@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
Copy link
Member

borkdude commented Mar 10, 2022

@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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor

svdo commented Mar 10, 2022

@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
Copy link
Member

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

@borkdude
Copy link
Member

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

@borkdude
Copy link
Member

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

@borkdude
Copy link
Member

Added more logging:

extension2.zip

@borkdude
Copy link
Member

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
Copy link
Member

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

@borkdude
Copy link
Member

Removed conditional, always replace backslashes:

extension3.zip

@borkdude
Copy link
Member

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

@borkdude
Copy link
Member

borkdude commented Mar 11, 2022

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
Copy link
Member

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 \

clj-kondo automation moved this from High priority (next release) to Done Mar 11, 2022
@svdo
Copy link
Contributor

svdo commented Mar 11, 2022

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!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed windows
Projects
clj-kondo
  
Done
Development

No branches or pull requests

3 participants