Option to enable the fixable errors only?

[davegaeddert]

Is there an option to easily enable all the error codes that are fixable (and disable the non-fixable ones)?

Something like:

ruff --select-fixable

Basically I'm looking at pairing this with black in a format command, so black --check and ruff --select-fixable could run together as format --check, and then black and ruff --fix --select-fixable could run together as format without failing on all the other ruff errors that have to be manually fixed (those will be show to the user at a different time)... Just wondering if there's an easier way to select them all than ruff --select A --select B etc.

Thanks!

[charliermarsh]

Another way to look at this could be to add a ruff fix command that fixes anything that's fixable, but doesn't report on remaining errors.

(One thing that could be useful here in the interim is the --exit-zero flag? That way, Ruff at least won't fail if it detects errors after fixing.)

[davegaeddert]

I like the ruff fix idea — when thinking about this, I did run ruff --fix wondering if it would show/exit the other unfixable errors or not (honestly wasn't sure which way it would go). If ruff fix were a command, personally I'd want to see ruff fix --check then to also get the "error if there are potential fixes" behavior. Having ruff fix --check would also align pretty well with how black, isort, etc. are used. Whatever you prefer is good by me! Kinda cool that it only has one command currently :)

At least personally, I'm ok without the --exit-zero flag I think. I can ignore the error code and just assume it wasn't a "real" error for now... either way it will still have same stdout which is almost the bigger issue for my particular use case.

[charliermarsh]

Maybe it's a flag like ruff --fix-only instead of ruff --fix?

[davegaeddert]

Thanks for merging something! The --fix-only flag helps, but is there any combination of options that lets you report the fixable errors without fixing them? I think the other ideas allowed for that, but not sure with --fix-only. Examples on the others:

ruff fix --check
ruff fix

ruff --select-fixable
ruff --select-fixable --fix

??
ruff --fix-only

Random other thing — I tested it real quick and looks like --fix-only has the potential to mask a major goof where the target files/dir doesn't exist:

Appreciate your work!

[charliermarsh]

Oh maybe I misunderstood. Can you provide a bit more context on when it'd be useful to report fixable errors without fixing them?

[davegaeddert]

Yep, sorry. I've got a forge format command that runs a combination of formatters (now just black and ruff): https://github.com/forgepackages/forge/blob/90ff92c85180cf91969d07d76f9a42555a492b0d/forge/cli.py#L131-L137

It has a --check option which was passed through to black and isort (which ruff replaces). With ruff I just couldn't find an easy way to "check" the potential autofixes. The --fix-only situation is certainly related, but for me it actually stemmed from the "check" part first which isn't solved yet. Does that help?

If you want to stop here though, I think that's totally fine — --fix-only is helpful and I'm realizing now that since I use all of the ruff checks in forge pre-commit anyway (https://github.com/forgepackages/forge/blob/90ff92c85180cf91969d07d76f9a42555a492b0d/forge/cli.py#L108-L110), I can maybe just remove the forge format --check option (which is what forge pre-commit used to leverage, and I don't really need available to users).

Anyway, pulling you into my problems now but I think you can consider this closed if you want! I do think the missing file/dir bug is legit though, and I can open a separate issue for that if you want.

[charliermarsh]

Ahh, I see, ok. I guess, separately from --fix-only, we could have ruff --fix --check that exits 1 if there's anything to change (but doesn't report anything)?

[charliermarsh]

I'm imagining:

• ruff --check (don't print anything, just exit 1 if there are any errors)
• ruff --fix --check (don't print anything, just exit 1 if there's anything fixable)
• ruff --fix-only (fix errors and exit)
• ruff --fix-only --check (same as ruff --fix --check)

[charliermarsh]

I guess I'm not sure how ruff --fix --check should behave (ruff --check and ruff --fix-only --check have clearer semantics).

Should it exit with a non-zero status code if anything is fixable, or if there are any errors?

[charliermarsh]

@andersk - Without reading this entire thread, do you have any gut reaction to what you'd expect ruff --check, ruff --fix --check, and ruff --fix-only --check to do?

[davegaeddert]

I'm realizing some of this may have to do with how ruff works, but personally I think it'd be more helpful to give some context on --check (and on --fix/--fix-only). As a user, when I use black I always appreciate it listing out the files it would/did change (easier to compare to git status, etc.):

But honestly, my problem is solved! Happy to give more feedback but don't feel like you have to force it on my behalf. In some ways I almost lean back towards one of the original suggestions of --select-fixable, or based on this issue about ALL, maybe --select FIXABLE? I feel like that would give you all combinations of uses?

[davegaeddert]

Whoops, just saw your latest comments. FWIW, I agree that --check is sort of unclear... feels like it already has "check" behavior by default due to (mostly) being a linter.

[charliermarsh]

It could be useful, because right now, --fix will actually write the fixes to disk. --check could be useful to detect that there are fixes to commit, without actually writing to disk. That's how I view it. But I'm def worried that I'm introducing too many CLI arguments here and making things more confusing.

[davegaeddert]

Yeah, I'm just saying that ruff --check is potentially confusing to me because it wouldn't write fixes anyway (no --fix), so it seems like a strange combination of options (would make more sense to me as ruff fix --check and not an option on ruff, but don't know if a new command is desirable...). Why would you want use ruff --check over just ruff (same thing, less output)?

Does a special select make any sense, like you suggested in that other issue? Then you'd only be adding one option value (if you removed --fix-only) and have:

• ruff --select FIXABLE (check for fixes)
• ruff --select FIXABLE --fix (run fixes, don't report anything else)
• ruff --fix (current behavior, fix and report other errors)

So you could have select ALL, FIXABLE, UNFIXABLE?

[charliermarsh]

Yeah, the only issue with FIXABLE is that some codes are marked as fixable, but can't be fixed in all cases. For example, the check that converts TypedDict to use the class-based syntax (class Foo(TypedDict): ...) is fixable, but we can't fix cases in which you use property names that aren't valid class property identifiers. So there is a list of fixable codes, but it's not always true that errors tied to those codes can actually be fixed due to these edge cases.

[davegaeddert]

Ah, good to know! If you ran ruff --select FIXABLE --fix and it couldn't fix something, I guess I wouldn't be too surprised then if it reported one of those edge case errors ("fixed X errors, couldn't fix 1")?

[andersk]

@andersk - Without reading this entire thread, do you have any gut reaction to what you'd expect ruff --check, ruff --fix --check, and ruff --fix-only --check to do?

My first reaction was that ruff --check sounds like what ruff already does, but since that’s clearly wrong, I was reminded of black --check, which sounds like what ruff --silent already does, but is different in the --fix case.

By analogy, ruff --fix --check would exit with failure if ruff --fix would change anything. There’s still a question as to whether it would exit with failure if ruff --fix would report errors. If so, it would be equivalent to ruff --check and ruff --silent, so that’s probably wrong; if not, it would be equivalent to ruff --fix-only --check.

From reading the thread now, it looks like I mostly got it right?

[andersk]

If we’re analogizing ALL, --select FIXABLE sounds like it would select fixable rules from Ruff’s entire list of rules, regardless of whether they’re enabled in the current configuration. That doesn’t seem like desirable behavior (maybe it’s useful in the form of --ignore UNFIXABLE?). But if that isn’t the behavior and it instead magically depends on the current configuration, then some funny circular reasoning is needed to figure out what --select FIXABLE,F401,F402 would mean.

[davegaeddert]

I think I saw somewhere that you were messing with a --diff flag? Would that be an alternative to --check that is a little less ambiguous (i.e. you can only diff fixes)?

• ruff --fix --diff - run checks and show diffs for fixes w/o applying them
• ruff --fix-only --diff - show diffs for fixes w/o applying them (exit failure if there are available fixes)
• ruff --diff - doesn't change behavior at all

[charliermarsh]

Oh yeah, that does exist already. I have to look and see how the exit codes work but I believe it’s as-described above.