Fork me on GitHub
#clj-kondo
<
2020-12-09
>
Aleksander Rendtslev09:12:57

Sweet! Just while I was looking for it. I was about to try to write it out, but I'm still pretty new to clojure. So i was scratching my head a little on this one (the rum hook does look like a good place to start though)

borkdude09:12:40

Let me know if you need any help or have any questions

seancorfield20:12:51

@borkdude Just spotted this in the analysis of deps.edn:

#:worldsingles{worldsingles-test
                  #:local{:root "../worldsingles-test"}}
and clj-kondo flags worldsingles-test as being an unqualified lib name (which it isn't). I guess clj-kondo isn't reading namespaced-maps correctly?

borkdude20:12:17

crap. I just pushed the button for release ;)

seancorfield20:12:39

And it actually recommends changing the above to this, which is definitely wrong:

{worldsingles-test {:local/root "../worldsingles-test"}}/{worldsingles-test {:local/root "../worldsingles-test"}}

borkdude20:12:03

@lee ^ interesting issue with namespaced maps in rewrite-clj wrt analysis..

borkdude20:12:29

yes, that's because it doesn't look at a namespaced map like it should.

borkdude20:12:33

issue welcome

borkdude20:12:45

I'll fix it in the next release, I just hit the release button

lread20:12:59

Interesting, thanks for the ping! I should have a look at how clj-kondo is using rewrite-clj here and make sure we are properly covered for rewrite-cljc.

borkdude20:12:34

It could in fact be a bug in my own fork of rewrite-clj now I think about it. I'll hold off on releasing clj-kondo for today and take a look.

lread20:12:54

In any case, I’m interested to learn more. I’ll be lurking! simple_smile

borkdude20:12:13

I'm calling sexpr on those nodes... and probably my version of sexpr for this thing doesn't work correctly... https://github.com/borkdude/clj-kondo/blob/master/src/clj_kondo/impl/linters/deps_edn.clj

borkdude20:12:33

hmm, it does appear to work:

user=> (n/sexpr (p/parse-string "#:foo{:a 1}"))
#:foo{:a 1}

borkdude20:12:42

Then maybe the bug is in the deps_edn.clj code itself.

lread20:12:10

yeah, namespaced element support is weak in rewrite-clj but my notes say that particular case is covered.

borkdude20:12:00

Hmm, this looks wrong:

user=> (n/sexpr (p/parse-string "#:worldsingles{worldsingles-test #:local{:root \"../worldsingles-test\"}}"))
{worldsingles-test #:local{:root "../worldsingles-test"}}

borkdude20:12:46

It appears to go wrong when the map has symbols:

user=> (n/sexpr (p/parse-string "#:foo{x 1}"))
{x 1}

borkdude20:12:31

I have never seen this case before. TIL.

lread20:12:38

But symbols shouldn’t be namespaced, only keywords, right?

borkdude20:12:20

user=> (keys (edn/read-string "#:foo{x 1}"))
(foo/x)

borkdude20:12:47

otoh:

user=> (keys (edn/read-string "#:foo{\"x\" 1}"))
("x")

borkdude20:12:44

@lee does rewrite-cljc do this correctly?

lread20:12:20

So I’m looking at my handy reference https://clojure.atlassian.net/browse/CLJ-1910

borkdude20:12:13

> A keyword or symbol key without a namespace is read with the default namespace as its namespace.

lread20:12:13

hmm… I think I missed that “symbol key” detail…

lread20:12:21

so no, rewrite-cljc namespaced element support WIP does not handle this correctly yet!

borkdude20:12:52

In clj-kondo this is a small fix:

user=> (keys (n/sexpr (p/parse-string "#:foo{x 1}")))
(foo/x)

borkdude20:12:32

modified   parser/clj_kondo/impl/rewrite_clj/node/seq.clj
@@ -56,9 +56,11 @@
           m (first (node/sexprs children))
           nspace (name nspace-k)]
       (->> (for [[k v] m
-                 :let [k' (cond (not (keyword? k)) k
+                 :let [k' (cond (not (ident? k)) k
                                 (namespace k)      k
-                                :else (keyword nspace (name k)))]]
+                                :else (if (keyword? k)
+                                        (keyword nspace (name k))
+                                        (symbol nspace (name k))))]]

lread20:12:33

oh foo! simple_smile …but am happy to realize this fact!

lread20:12:46

CLJ-1910 does deliver the gritty details, it just needs to be absorbed.

borkdude22:12:51

@seancorfield Pushed a fix for https://github.com/borkdude/clj-kondo/issues/1093 to master. Re-scheduled the release for coming weekend.

seancorfield22:12:24

Nice! Thank you, sir!