NoahTheDuke / splint

A Clojure linter focused on style and code shape.
https://cljdoc.org/d/io.github.noahtheduke/splint/
Mozilla Public License 2.0
109 stars 2 forks source link

Auto gen config not working? #16

Closed svdo closed 4 months ago

svdo commented 4 months ago

Hi!

I really like the concept of splint (thanks!!!🙏), but we have so many warnings that we can't just get started. Today I noticed the "--auto-gen-config" option, which sounds like what we need, but when I run it I get an empty config file:

;; Splint configuration auto-generated on 2024-05-07.
;; All failing rules have been disabled and can be enabled as time allows.

{

}

I tried it both with using the provided deps.edn alias, and with via Babushka (the bbin installation method), they give the same result.

Is there anything I'm doing wrong or that I can try? Thank you!

NoahTheDuke commented 4 months ago

Uh oh! It certainly used to work, so let me take a look.

svdo commented 4 months ago

I'm trying to debug this. So far I noticed this:

In https://github.com/NoahTheDuke/splint/blob/adc9eda1b6c9ad0f8eb722d7575e67d47069e90c/src/noahtheduke/splint/runner.clj#L347 it says: (prepare-rules config (or rules (:rules @global-rules))) but the format of (:rules @global-rules) is very different from the rules in the case of auto-gen-config. In that case rules is the all-enabled on line https://github.com/NoahTheDuke/splint/blob/adc9eda1b6c9ad0f8eb722d7575e67d47069e90c/src/noahtheduke/splint/runner.clj#L376.

If I change all-enabled to nil on that line, I do get a config file, but that's probably not the right solution.

svdo commented 4 months ago

When all-enabled is as it currently is defined in the source code, rules looks like this:

Screenshot 2024-05-08 at 12 35 25

When I set it to nil, it looks like this:

Screenshot 2024-05-08 at 12 34 14
svdo commented 4 months ago

In the existing situation, many keys are missing, notably init-type which causes the map of prepared rules (rules-by-type) to contain only one single key: nil.

svdo commented 4 months ago

(note: that config file I get with setting all-enabled to nil suppresses only a relatively small part of the findings)

svdo commented 4 months ago

Update: it turned out that the remaining warnings were all of one single type: lint/thread-macro-one-arg.

NoahTheDuke commented 4 months ago

Oh dang, you dove in right when I did a bunch of work internally to change how I build and store the rules on the ctx during runs lol. Congrats on being maybe the only other person to look at the code!

If you've actually fixed it, I'd love to take a look.

svdo commented 4 months ago

Not fixed unfortunately, I was hoping you would say something like "ah right then I know what's wrong!" :) I'd be happy to take a shot at it (with a bit of help maybe) but probably I should wait for you to finish what you're working on now...?

NoahTheDuke commented 4 months ago

I suspect the solution here is pretty simple: instead of passing all-enabled in as the third parameter, write something like (spit-config (run-impl paths {:test-config (merge all-enabled options)})). To rubber duck: we don't want to load any local config, we want to treat the all-enabled config as our primary config, so use :test-config to bypass the load-config call. Due to my changes yesterday, prepare-rules starts from scratch instead of using the given rules object as the base, which is why this is failing (I think).

I'm at work right now and might not get to work on this tonight, so if you feel up to it, give this a try.

NoahTheDuke commented 4 months ago

I ended up having some time to fix this. Hopefully it should work for you once I release it in the morning.

NoahTheDuke commented 4 months ago

Released in 1.15.2! Thanks so much.

svdo commented 4 months ago

And thank you, it was great that you were able to fix (and release!) it so quickly! Indeed it is working now :)