Is there a reason why var-usages might contain :name-col nil?
I'm thinking maybe something here is not working as intended https://clojurians.slack.com/archives/CHY97NXE2/p1664897879130349 sorry I never got around to testing this back when it came up initially.
not sure why, but I bet you have a repro with some code? ;)
I'm looking at the Dewey dataset and seeing quite a few nils in :row, :col, :name-col and :name-row.
Just trying to confirm we're not aware of a logical reason why it might be this way before going further with the detective work.
I'm afraid it's hard to say in general without a repro and it'll be detective work for someone
Okay, fair enough. If you don't have a hunch I'll see if I can pull together a working example. Thank you @borkdude.
@borkdude https://github.com/sheluchin/clj-kondo-nil-usage-locs lmk if there's anything else I can add that would help
thanks. in addition to this, a smaller repro would definitely help, e.g. if you can find a specific piece of code from fast-edn that returns nil for some fields, then we can look more into detail into that specific code. with those pieces of data, please file an issue
my suspicion is that . and is etc generate name-col name when they are actually the result of an expansion of some other undesugared form or macro, e.g. from are
so in this case there isn't a location for the name of the call
e.g. are is expanded here:
https://github.com/clj-kondo/clj-kondo/blob/19a89107b94d133f6bf3e4dc31f7cbd6bd1fe8aa/src/clj_kondo/impl/analyzer/test.clj#L43-L52
e.g. here is a node that is generated without a location: https://github.com/clj-kondo/clj-kondo/blob/19a89107b94d133f6bf3e4dc31f7cbd6bd1fe8aa/src/clj_kondo/impl/analyzer/test.clj#L45 so I think that's the answer probably
Would you consider that a bug or just something that needs to be documented?
not sure about the other normal functions like assoc but perhaps some more detective work would help here.
I don't consider the above a bug, it doesn't make sense to make up some location for stuff that isn't really there in code
Does it make sense for it to be counted as a usage there? I guess it's something like an "indirect usage" in this case.
it's an indirect usage indeed
I can investigate assoc, conj and say count... unless there are some specific names you want me to check out.
does it also say something like "generated location" in the usage data? I forgot the exact term, but I did add this to communicate that it's a generated node
random example:
Name |Value |
-----------------+----------------------------+
fixed-arities | |
end-row |35 |
name-end-col | |
name-end-row | |
private | |
name-row | |
name |. |
defmethod | |
dispatch-val-str | |
lang | |
filename |clojure/fira_code/glyphs.clj|
alias | |
from |fira-code.glyphs |
macro | |
col |29 |
deprecated | |
name-col | |
from-var |parse-escaped-string! |
end-col |44 |
arity |3 |
varargs-min-arity| |
refer | |
row |35 |
to |clojure.core |
basis-id |0 |I don't see anything to the tune of "generated loc".
no wait, it doesn't make sense to say "generated location" if there is no location. It does sometimes say this when it has inherited a location from a parent node in the case of macro hooks
ok, that's interesting that it does have a col but nothing else
let me check out the code
end-col too
row 35 says this: https://github.com/tonsky/FiraCode/blob/7e5bff99c479b731eb18d528320f8bca98a6ac19/clojure/fira_code/glyphs.clj#L35 which version of fira-code is this?
7e5bff99c479b731eb18d528320f8bca98a6ac19
yep same one
weird, I don't even see a call on that line
and I didn't see any calls to do in https://github.com/tonsky/fast-edn/blob/main/test/fast_edn/test.clj but one of the usages shows:
{:filename
"/home/alex/repos/clojure-stuff/fast-edn/test/fast_edn/test.clj",
:row nil,
:col nil,
:from fast-edn.test,
:to clojure.core,
:name do,
:arity 15,
:from-var basics-test}ah that's an expansion
| :fixed-arities | :end-row | :name-end-col | :name-end-row | :name-row | :name | :filename | :from | :col | :name-col | :from-var | :end-col | :arity | :row | :to |
|----------------+----------+---------------+---------------+-----------+--------------+-----------+------------------+------+-----------+-----------------------+----------+--------+------+------------------|
| #{0} | 27 | 12 | 27 | 27 | skip-ws! | <stdin> | fira-code.glyphs | 3 | 4 | parse-escaped-string! | 13 | 0 | 27 | fira-code.glyphs |
| #{0} | 28 | 28 | 28 | 28 | current-char | <stdin> | fira-code.glyphs | 15 | 16 | parse-escaped-string! | 29 | 0 | 28 | fira-code.glyphs |
| #{1 2} | 28 | 11 | 28 | 28 | = | <stdin> | fira-code.glyphs | 9 | 10 | parse-escaped-string! | 30 | 2 | 28 | clojure.core |
| #{0} | 32 | 20 | 32 | 32 | advance! | <stdin> | fira-code.glyphs | 11 | 12 | parse-escaped-string! | 21 | 0 | 32 | fira-code.glyphs |
| #{0} | 33 | 33 | 33 | 33 | current-char | <stdin> | fira-code.glyphs | 20 | 21 | parse-escaped-string! | 34 | 0 | 33 | fira-code.glyphs |
| #{1 2} | 35 | 17 | 35 | 35 | = | <stdin> | fira-code.glyphs | 15 | 16 | parse-escaped-string! | 24 | 2 | 35 | clojure.core |
| | 35 | | | | . | <stdin> | fira-code.glyphs | 29 | | parse-escaped-string! | 44 | 3 | 35 | clojure.core |
| #{0} | 35 | 54 | 35 | 35 | advance! | <stdin> | fira-code.glyphs | 45 | 46 | parse-escaped-string! | 55 | 0 | 35 | fira-code.glyphs |
| #{0} | 35 | 81 | 35 | 35 | current-char | <stdin> | fira-code.glyphs | 68 | 69 | parse-escaped-string! | 82 | 0 | 35 | fira-code.glyphs |
| | 35 | | | | . | <stdin> | fira-code.glyphs | 56 | | parse-escaped-string! | 83 | 3 | 35 | clojure.core |
| | 35 | 90 | 35 | 35 | recur | <stdin> | fira-code.glyphs | 84 | 85 | parse-escaped-string! | 91 | 0 | 35 | clojure.core |
| | 35 | 28 | 35 | 35 | do | <stdin> | fira-code.glyphs | 25 | 26 | parse-escaped-string! | 92 | 4 | 35 | clojure.core |
| #{1 2} | 36 | 17 | 36 | 36 | = | <stdin> | fira-code.glyphs | 15 | 16 | parse-escaped-string! | 24 | 2 | 36 | clojure.core |
| #{0} | 36 | 38 | 36 | 36 | advance! | <stdin> | fira-code.glyphs | 29 | 30 | parse-escaped-string! | 39 | 0 | 36 | fira-code.glyphs |
| #{0 1} | 36 | 44 | 36 | 36 | str | <stdin> | fira-code.glyphs | 40 | 41 | parse-escaped-string! | 48 | 1 | 36 | clojure.core |
| | 36 | 28 | 36 | 36 | do | <stdin> | fira-code.glyphs | 25 | 26 | parse-escaped-string! | 49 | 2 | 36 | clojure.core |
| | 37 | | | | . | <stdin> | fira-code.glyphs | 29 | | parse-escaped-string! | 44 | 3 | 37 | clojure.core |
| | 37 | 51 | 37 | 37 | recur | <stdin> | fira-code.glyphs | 45 | 46 | parse-escaped-string! | 52 | 0 | 37 | clojure.core |
| | 37 | 28 | 37 | 37 | do | <stdin> | fira-code.glyphs | 25 | 26 | parse-escaped-string! | 53 | 2 | 37 | clojure.core |
| | 37 | 18 | 34 | 34 | cond | <stdin> | fira-code.glyphs | 13 | 14 | parse-escaped-string! | 54 | 6 | 34 | clojure.core |
| | 37 | 15 | 33 | 33 | let | <stdin> | fira-code.glyphs | 11 | 12 | parse-escaped-string! | 55 | 2 | 33 | clojure.core |
| | 37 | 14 | 31 | 31 | loop | <stdin> | fira-code.glyphs | 9 | 10 | parse-escaped-string! | 56 | 3 | 31 | clojure.core |
| #{3} | 38 | 21 | 38 | 38 | replace | <stdin> | fira-code.glyphs | 9 | 10 | parse-escaped-string! | 35 | 3 | 38 | clojure.string |
| #{3} | 39 | 21 | 39 | 39 | replace | <stdin> | fira-code.glyphs | 9 | 10 | parse-escaped-string! | 34 | 3 | 39 | clojure.string |
| #{3} | 40 | 21 | 40 | 40 | replace | <stdin> | fira-code.glyphs | 9 | 10 | parse-escaped-string! | 34 | 3 | 40 | clojure.string |
| | 40 | 10 | 30 | 30 | -> | <stdin> | fira-code.glyphs | 7 | 8 | parse-escaped-string! | 35 | 4 | 30 | clojure.core |
| | 40 | 9 | 29 | 29 | let | <stdin> | fira-code.glyphs | 5 | 6 | parse-escaped-string! | 36 | 2 | 29 | clojure.core |
| | 40 | 8 | 28 | 28 | when | <stdin> | fira-code.glyphs | 3 | 4 | parse-escaped-string! | 37 | 2 | 28 | clojure.core |This is from:
(->> (with-in-str (slurp "") (k/run! {:lint ["-"] :config {:analysis true}}) ) :analysis :var-usages (filter #(= (:from-var %) 'parse-escaped-string!)) (clojure.pprint/print-table)) ah yes:
| | 37 | | | | . | <stdin> | fira-code.glyphs | 29 | | parse-escaped-string! | 44 | 3 | 37 | clojure.core |
so the name is .
which is an expansion of (StringBuilder.)
makes sense
so that one seems correct, it's an expansion
Here's an assoc example from babashka 4c6fe98236aa63e0044a7caff772510e441c2af0:
Name |Value |
-----------------+---------------------+
fixed-arities |#{3} |
end-row |828 |
name-end-col | |
name-end-row | |
private | |
name-row | |
name |assoc |
defmethod | |
dispatch-val-str | |
lang | |
filename |src/babashka/main.clj|
alias | |
from |babashka.main |
macro | |
col |59 |
deprecated | |
name-col | |
from-var |readers-fn |
end-col |64 |
arity |3 |
varargs-min-arity|3 |
refer | |
row |828 |
to |clojure.core |
basis-id |375 |I think that's from this line:
_ (swap! seen-urls assoc urls parsed-resources)Yeah, that should have complete loc info, no?
probably you can assume here that the name-col is the same as the call location since it's called as an argument
yep, that's where it lands
I think clojure-lsp makes such assumptions too
Okay, so that's two edge cases: expansions and non-invocation usages
perhaps we can just document this
yes, but the second edge case does have full location information other than the name where the expansion had none right?
looks to be the case
Sounds like a bit of docs should suffice. I think in my case I will just filter out records that don't have this information and call it a day.
Appreciate you diving into this. I'll add a PR with a mention of these cases for the docs if you don't prefer to do it yourself.
thanks a lot, a PR would be appreciated!
ty for merge. link for ref: https://github.com/clj-kondo/clj-kondo/pull/2505