Requesting a review on https://github.com/pantsbuild/pants/pull/20098 if anyone has time - this would add support for the Ruff formatter to Pants 🙂

Are you still looking for a solution to being able to disable certain

ruff

modes?

Yeah. I see that there is a

skip

flag: https://github.com/pantsbuild/pants/blob/9adee6d33afccc657175c3d2d7abd4dc33c92745/src/python/pants/backend/python/lint/ruff/subsystem.py#L56 My main idea was to break this flag into two separate flags to skip formatting and linting, and default the formatting one to

true

instead of false. This would mean that people don’t suddenly overnight get opted into Ruff formatting if they don’t want it. Does that make sense, or is there a better way?

I think it might be worth having a new flag, e.g.

--ruff-modes=

which would let you specify one or more of

lint

,

fmt

and

fix

. If unspecified, it would run all three.

And how would that interact with the

skip

flag (if at all)?

Let's see

Ok will give that a shot! Thanks for the guidance

So

skip

isn't a flag, it's a field that you can declare on a single

python_source

target in order to have

ruff

not run on that source

Isn’t that something slightly different? It seems like what you’re referring to is defined here: https://github.com/pantsbuild/pants/blob/9adee6d33afccc657175c3d2d7abd4dc33c92745/src/python/pants/backend/python/lint/ruff/skip_field.py I can see that the field is called

skip_ruff

and can be applied to Python sources

never mind, I see that there's also a

SkipOption

Right, yes. That’s what I’m referring to here

Right, you should be able to amend

skip

in

subsystem.py

Sorry for being obtuse - I’m not sure this fixes my problem though. Basically, the issue is: 1. We add Ruff formatting (by merging the PR) 2. Currently, there is no Ruff formatting and people only use Ruff for linting 3. If people upgrade to a newer version of Pants that supports Ruff formatting, they will get opted into Ruff formatting by default (?) 4. The

skip

flag does not let you selectively skip tasks, and if we were to modify it similar to

cue

then we’d end up skipping all Ruff tasks rather than just formatting

Yup, sorry, it's early in the morning and I'm confused. 🙂

So then, going back to the beginning my initial idea was to split the

skip

option into separate options for each goal (

fmt

,

lint

,

fix

)

Yeah, I understand what you want, I'm just surprised that

SkipOption

doesn't work that way already.

Anyhow - I was trying to figure out if there is a better solution than splitting the options. But it doesn’t seem like there is anything obvious

I think having a list option,

--ruff-modes

would work well.

--ruff-skip

will need to stick around because of our deprecation policy, which is a bit annoying

To me I think having the skip option as-is the correct (the option to selectively use ruff or not for particular targets). Then in the main configuration (e.g.

pants.toml

) we can choose the ruff modes we want to use in our project. I think this is what @ancient-vegetable-10556 suggested as well in the end. The ruff modes could default to only

lint

when introduced to not change the behaviour for those already using the ruff backend, while having the option for all to opt in to the new modes of ruff as they please.

I don’t think so - people would probably enable formatting/linting globally or not at all

so keeping the skip option as is (along the example from the cue backend for instance) would make sense then, and just adding to the ruff subsystem the modes option 🙂

So here’s what I don’t get - how do I incorporate the modes into the rules that are exported? They look like this:

def rules():
    return [
        *collect_rules(),
        *RuffFixRequest.rules(),
        *RuffLintRequest.rules(),
        *RuffFormatRequest.rules(),
        *pex.rules(),
    ]

but it doesn’t seem like there’s a way to selectively disable rules or targets based on the subsystem options?

ah, I was looking into if we could integrate this check into the skip option, but that would require to know which goal it was being evaluated for.. while researching this, I noticed there are already the

only

options, such as

pants fix --only=ruff

would run only the ruff fixer... so what I think we'd like to have is the opposite, so we have skip on the tool level, but not skip on the goal level which could look like

pants fix --skip=ruff

to run all fixers, except ruff.

Ok yeah, that makes sense. The main purpose behind the original question was to make the opt-in smooth for existing users who also use Black. But if that’s not really achievable, then maybe the callout would suffice.

yea I think so

I skimmed this. Is there still an open question?

Nope! PR has been merged, but with the caveat that the only way to opt out of Ruff formatting is to use the

--only

option with

pants fmt

Yeah. Admittedly the current design has tool-granularity, and wasn't made with a single tool being both a code changer and a code complainer

I think we get far by simply adding

skip

as complementary to

only

for the goals…

I'm trying to use

pants fix ::

and it still formats code with ruff. I've set

[fmt].only

and it is not respected by

pants fix ::

Yes, all formatters are also fixers. You can use

pants --skip-ruff fix ::

if you want to run all fixers except for ruff

but I want ruff to run fixers but not change the formatting

Here's the code for the process: https://github.com/pantsbuild/pants/blob/525cda380cb08f18b164be3005333857ef2cfc8c/src/python/pants/backend/python/lint/ruff/rules.py#L67

I'm running pants version 2.20.0dev3, you've reference the commit from stable branch 2.18

Ah I missed the context of this particular thread

Scrolling through here to find the new stuff, but I’m guessing this is the crux issue? https://pantsbuild.slack.com/archives/C0D7TNJHL/p1703710511414419?thread_ts=1699632752.483199&cid=C0D7TNJHL

yeah, I'm trying to figure out how to run ruff fixer and not formatter

The code for the fixer should basically do what you want: https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/python/lint/ruff/rules.py#L78-L82 It runs

ruff check --fix .

instead of

ruff check . --fix

Lemme try this on my own repo and see if I can trace it down

I think I found what's going on.

[fmt].only

actually works, it only runs black, but for some reason black now ignores

# fmt: on

comments in our repo. Not sure why though

Yeah so what do the logs like these above the checks look like?

16:13:42.75 [INFO] Completed: Format with shfmt - shfmt made no changes.
16:13:42.76 [INFO] Completed: Fix with Autoflake - autoflake made no changes.
16:13:42.76 [INFO] Completed: Fix with Autoflake - autoflake made no changes.
16:13:42.81 [INFO] Completed: Fix with ruff - ruff --fix made no changes.
16:13:42.81 [INFO] Completed: Fix with ruff - ruff --fix made no changes.
16:13:42.82 [INFO] Completed: Format with ruff - ruff made no changes.
16:13:42.82 [INFO] Completed: Format with ruff - ruff made no changes.

here is the output with skip_formatters

$ pants fix ::

✓ ruff --fix made no changes.

Hmm, but since

ruff --fix

also ran that second time, I am guessing the command was

pants fix ::

?

yes

Got it, yeah. That definitely extends a bit deeper than just Ruff and something we could try to fix - feel free to file an issue for it and I can look deeper at how to implement this underneath. But for now, does

skip_formatters

sufficiently work around the issue?

yes,

pants fix fmt ::

works with

skip_formatters

You can always create a command-line alias for it too if that helps reduce the typing. I have this in my `.zshrc`:

function pants_check_all() {
    pants fix lint check test ::
}
alias pca=pants_check_all

Regarding aliases theres also

cli.alias

option for pants. ;)

I spend more time thinking about the problem, maybe this is a better solution: https://github.com/pantsbuild/pants/pull/20358