houseabsolute / precious

One code quality tool to rule them all
Apache License 2.0
82 stars 4 forks source link

Consider adding a fix subcommand in addition to tidy & lint #44

Closed autarch closed 1 year ago

autarch commented 1 year ago

Some tools, notably golangci-lint and clippy, have a mode where they will attempt to semantically modify code to fix linting issues. Running these as part of tidying is almost certainly a mistake, since they could actually make changes you don't want.

But having a third mode, precious fix, to support this, might be nice.

mmcclimon commented 1 year ago

I have taken a preliminary pass at this in #45.

oschwald commented 1 year ago

How is this different than tidy? For our golangci-lint config, we have lint for running without --fix and tidy for running with it. I worry adding yet another mode will make it even more confusing. I think if #38 or something similar was done, I would almost never run a particular mode.

autarch commented 1 year ago

Hmm, that's a good question. My initial thinking was that tidy was for things you always want to apply and fix is for things you might want to apply.

Personally I don't have golangci-lint or clippy set up to run their fix stuff as part of tidying, although to be honest I haven't even tried it! I guess I was worried that this would produce too many false positives in terms of changes I'd rather not apply.

In your experience with golangci-lint --fix, do you ever want to undo its changes? And does it respect //nolint comments so you don't have to keep undoing it?

oschwald commented 1 year ago

We have a fairly extensive golangci-lint config that uses many different linters. I can't really think of a time where there was a suggestion that we didn't want to apply as most of the things that are automatically fixed are pretty non-controversial. It does respect nolint for the fixers. My experience with eslint has been similar.

Although I don't use clippy with precious, I can think of a few instances where its suggestions were not what I wanted. It seems to be much more aggressive in what it fixes. However, I can also think of instances where code formatters made the code harder to read and the result wasn't necessarily what I wanted to apply, causing me to go back and disable the formatter for that block of code.

Given the distinction you outline, I wonder if a more general solution would be the ability to apply labels to particular commands and run commands based on those labels. This would allow someone to have a set of tidiers that they absolutely want to apply no matter what. It would also allow them to have a set of linters where they expect no errors, while having another set where some errors may be expected. It would also allow people to create other groups that might make sense for their workflow, e.g., commands for a particular language.

autarch commented 1 year ago

I wonder if a more general solution would be the ability to apply labels to particular commands and run commands based on those labels.

Yes, this makes a lot of sense, and is more or less captured in #8. I will update that one to specifically talk about labels.

autarch commented 1 year ago

I'm going to close this in favor of #8.