Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto gen config not working? #16

Closed
svdo opened this issue May 7, 2024 · 12 comments
Closed

Auto gen config not working? #16

svdo opened this issue May 7, 2024 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@svdo
Copy link

svdo commented May 7, 2024

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
Copy link
Owner

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

@NoahTheDuke NoahTheDuke added the bug Something isn't working label May 7, 2024
@NoahTheDuke NoahTheDuke self-assigned this May 7, 2024
@svdo
Copy link
Author

svdo commented May 8, 2024

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

In

rules-by-type (prepare-rules config (or rules (:rules @global-rules)))
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
(let [all-enabled (update-vals @conf/default-config #(assoc % :enabled true))]
.

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
Copy link
Author

svdo commented May 8, 2024

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
Copy link
Author

svdo commented May 8, 2024

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
Copy link
Author

svdo commented May 8, 2024

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

@svdo
Copy link
Author

svdo commented May 8, 2024

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

@NoahTheDuke
Copy link
Owner

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
Copy link
Author

svdo commented May 8, 2024

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
Copy link
Owner

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
Copy link
Owner

NoahTheDuke commented May 9, 2024

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

@NoahTheDuke
Copy link
Owner

Released in 1.15.2! Thanks so much.

@svdo
Copy link
Author

svdo commented May 10, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants