?

I've got data like

{:name-end-col 17,
  :name-end-row 24,
  :name-row 24,
  :name a,
  :filename
  "...",
  :alias d,
  :from amperity.web.workflows.workflow.resolutions.plugin,
  :col nil,
  :name-col 14,
  :from-var credentials-page-link-component,
  :arity 3,
  :row nil,
  :to helix.dom}

in the analysis data

it's not every usage, but a lot

This might come from a hook which doesn't propagate location metadata

ah. we do have a few kondo hooks for helix

is there an example of how to propagate location metadata?

when generating the a call, you have to set metadata, e.g. from the outer node

e.g. (with-meta (list-node ...) (meta node))

That might be a useful warning for hook devs.

I came across this when running analysis on a bunch of libraries. It is a fairly common thing in the wild.

What clj-kondo could maybe do is remember the col / row of the parent and report that as a fallback

(if there is a parent)

Actually, I thought I did something to fix that in https://github.com/clj-kondo/clj-kondo/issues/1812

But this isn't released yet. Perhaps @U4YGF4NGM can try with the master version. I hope to finish this release tomorrow

just tried what's in master f0935686f0c4e5690b0965189a7bb11f85b5510d and i still get nil for :row and :col for the same number of vars

Ah, I'm afk right now so can't browse the rest of the kondo source but it looks like the tests I added were only for :var-definitions. I guess usages has a similar problem that was not fixed there.

I still suspect this to be a hook-specific problem

Yeah, it's just many hooks out there currently have this problem. I guess it depends on the use case. If there can be some fallback added that provides better results without fixing every hook that omits meta, it would be nice.

Issue welcome

I can take a shot at a fix in a week or two if no one gets around to it by then.

this change seems to fix most of the issues for helix code https://github.com/lilactown/helix/commit/09a72365004ee788b8d3f74d7cb2fd4144a17f00

I'm not sure how to add the line numbers back to the new-opts value

Issue still welcome

in case we forget

I'm working on it already

@UPWHQK562 By introducing the fix, I could also revert your change in PR 1813. Can you check if that makes sense? https://github.com/clj-kondo/clj-kondo/compare/fallback-location-for-hooks?w=1 @U4YGF4NGM Could you test with this branch / SHA of clj-kondo?

That is: SHA 34b751fda59dc1f709f55f04554bbe329d4d4ac1

@U04V15CAJ traveling right now so can't do the most thorough review but it looks good. I wonder if you can also simplify the :name-row etc. fields so the logic a few lines below in reg-var! can be consistent.

ok, I'll continue tomorrow

The gist of it is you're moving the fallback logic to a more generic location and giving it the explicit name :fallback-loc?

Yes

I looked at your other (or (:name-row ...) ...) stuff @UPWHQK562 but this was still necessary

Alright, it would have been nice to make it consistent but this is still a nice improvement. Thanks for working on this, @U04V15CAJ 🙏

ok @U4YGF4NGM and @UPWHQK562 I merged the final solution to master now. I already had a similar solution for :macroexpand hooks but I now applied the same solution to :analyze-hook for nodes that don't have a location https://github.com/clj-kondo/clj-kondo/commit/17423d1b90498c4667663bd256056fd38bffc46c Please test

e556f9c5f53620cc9acb2030257c1461556dab84 would be the best commit to test with

Let's say, just latest master commit

@U04V15CAJ Nice, that looks like a much more elegant overall solution. Looking forward to testing this out shortly.