Fix formatting when requires have metadata

[camsaul]

Fixes #351

While working on this I tried to think of all the possible weird edge cases I could and found that it also broke in a few even weirder situations e.g. if you did something like put metadata on or comments before the x in [x :as y] itself. I added a pretty comprehensive test makes sure things work as expected no matter what shenanigans you're up to in your ns form.

[camsaul]

I tried to write out the reasoning behind the changes here as opposed to in the PR, hope that's ok. Maybe it will save the next person who comes along some time so they don't have to work out what's going on the hard way

[weavejester]

I think in this case it's a lot of information that isn't relevant to most readers. Consider instead something like:

current-namespace (some-> as-zloc z/leftmost find-first-symbol z/sexpr)

This informs the reader of the intent, without needing to go into detail. If the reader is curious of why, they can look up the information in the associated commit.

It's important to explain the source code, but also to be concise. Any information that isn't universally relevant is noise to those that don't need it.

[camsaul]

if we have something like

^{:clj-kondo/ignore [:discouraged-namespace} [my.namespace :as n]

then grandparent-node in this case is a :meta node so by adding the vector or string check we prevent that from accidentally returning an alias map like

{"n" "{:clj-kondo/ignore [:discouraged-namespace]}.my.namespace"}

[camsaul]

If you have something like

[#_my.old.namespace my.namespace :as n]

then z/leftmost will be an :uneval node (see clj-commons/rewrite-clj#70) so we need to skip it until we find the first symbol node, hence the use of z/find. I didn't use token? here since keywords are tokens too and we don't want it to accidentally return :as if it runs into some other issue.

[weavejester]

What about factoring that out:

(defn- symbol-node? [zloc]
  (some-> zloc z/node n/symbol-node?))

(defn- leftmost-symbol [zloc]
  (some-> zloc z/leftmost (z/find (comp symbol-node? skip-meta))))

That might make it a little more readable:

current-namespace (some-> as-zloc leftmost-symbol z/sexpr)

[camsaul]

Hope using doseq to test different combinations of stuff here is ok. Writing out 40 separate test cases seemed a bit noisy

[weavejester]

That's fine, but why did you use doseq instead of are?

[camsaul]

The main reason was that I wanted to try all the different permutations of ignore-str and ns-vec-str which would have required a nested ares which besides being hard to read would then macroexpand into nested is assertions as well. doseq is clearer and doesn't do dumb things

[weavejester]

I think in this case it's a lot of information that isn't relevant to most readers. Consider instead something like:

current-namespace (some-> as-zloc z/leftmost find-first-symbol z/sexpr)

This informs the reader of the intent, without needing to go into detail. If the reader is curious of why, they can look up the information in the associated commit.

It's important to explain the source code, but also to be concise. Any information that isn't universally relevant is noise to those that don't need it.

[weavejester]

What about factoring that out:

(defn- symbol-node? [zloc]
  (some-> zloc z/node n/symbol-node?))

(defn- leftmost-symbol [zloc]
  (some-> zloc z/leftmost (z/find (comp symbol-node? skip-meta))))

That might make it a little more readable:

current-namespace (some-> as-zloc leftmost-symbol z/sexpr)

[weavejester]

That's fine, but why did you use doseq instead of are?

[camsaul]

Updated with suggestions