Spoke with @MichaReiser offline. We're leaning towards keeping this as an unsolved problem for now. P0 release won't include a way for users to pass --fix and manually include Applicability::Suggested fixes.

@MichaReiser if that sounds acceptable, I can go ahead and start on the implementation.

To get the Applicability over to Checker, I think it makes sense to put the base_applicability on Settings.

I wouldn't add the base_applicability to Settings because Settings only contains fields that are present in the ruff.toml. But what I understand is that you're now recommending adding it to FixMode instead. Adding it to FixMode does make sense to me, but:

• I don't think we need to store the applicability for Generate because we can show all fixes in this mode.
• For Apply (and maybe Diff); I recommend a boolean flag instead: apply_suggested because that's the only information we need

Since, at this point, we have access to the base_applicability and all diagnostics, it should be relatively simple to filter either during or after Message construction (the latter could be done in Emitter.emit(), since we have access to the underlying fix).

What's the use case for filtering messages? My understanding is that we always show all fixes. --fix-unsafe only controls whether we apply suggested fixes (not whether we show them)

We also need to be careful about caching or include --fix-unsafe in the cache key (I would prefer not to do so)

@MichaReiser The message filtering would occur with --fix-only and --diff:

1. --fix-unsafe: I think its good to have the option. We can align on the final naming later.
   Cool. We could also call it --fix-all or something to denote that it fixes all Automatic and Suggested rules. But yeah, semantic changes should occur at the end.
   
   2. --fix-only: Should --fix-unsafe enable fixing the suggested fixes too?
   Yes. If you think of Applicability like a hierarchy, we should default to the highest level (i.e. Automatic - we don't want to do anything with Suggested if the user hasn't specified it) but always use the lowest level that the user specifies. If the user specifies --fix-unsafe, then --fix-only and --diff should include Suggested fixes.
   
   3. --diff: Should --fix-unsafe enable the suggested fixes? I think it should be aligned with --fix-only, considering that --diff implies --fix-only.
   See above

So the way I envision it is if they pass --fix-only without --fix-unsafe, it'd fix Automatic fixes (same with --diff). That means that we'd need to filter out Suggested fixes from that output.

That means that we'd need to filter out Suggested fixes from that output.

I think we mean the same thing but use different terminology.

What I mean by output is the diagnostics that we render to the users. Or the message when applying --fix: Fixed 5, 1 remaining.

I expect suggested fixes to be reported in the CLI output regardless of the --fix-unsafe option.

But we should only apply the automatic fixes for --fix and only print the diffs for the automatic fixes for --diff.

Yep, agreed - sorry for the confusion!

@evanrittenhouse these are just some scratches and unfinished thoughts.

The change looks good overall. Nice job! The things that came up in the discussion are:

• We agreed on the semantics
• --fix-unsafe feels strange when using in conjunction with --diff. Potentially remove the --fix prefix (downside, unclear that this is a fix specific option)
• Using fixable to promote a rule feels strange because it breaks with the symmetry of fixable and unfixable. Also, requiring to add all automatic fixes when promoting a suggested rule feels very cumbersome. That's why we're leaning towards removing that feature and wait to see if users need a way for promoting rules.

That said, this change breaks the workflow of users that use fix = true in their configuration because they can't opt in to the same set of rules as they used to before (some of them may now be unsafe). We discussed a few options but didn't come to a conclusion:

• Add unsafe = true to the setting: This would mirror the CLI API. Micha's concern is that we shouldn't encourage to run all rules by default (because it's unsafe after all).
• Add an option to manually promote suggested rules other than using fixable (e.g. fix.safe = [] or fix.extend_safe=[])
• ...

We'll need to solve this before shipping but it doesn't prevent us from working on a prototype implementation.

Cool, I'll get to work on a prototype then.

IMHO, I think the easiest solution would be to add unsafe = false by default and add something like fix.safe = [] to the config. Users could then specify individual rules via the latter option if they want to, and we protect the base case with unsafe defaulting to false. That's just me though :) I'm sure additional options will come up as we iterate through the PR.

I realised that there's another reason why we shouldn't use fixable to promote suggested fixes. The reason is that the fixability doesn't depend on the rule. Ruff has rules where the fix can either be automatic, suggested, or manual, depending on the specific violation. This makes it ambiguous what adding a rule to fixable means. Do only the automatic fixes become fixable or should the suggested fixes be applied to?

That's why I think a fix.safe = [] option (we could potentially allow fix.safe = "YOLO" (for @charliermarsh :P) is preferred. This still has the downside that it's an all-or-nothing opt-in for a specific rule. Meaning, they can't specify that the fix is safe only in circumstance x.

One concern that I have with safe is that:

The discussed semantic of safe is that it adds a suggested to the automatic rules set. However, the name gives the impression that only the listed rules are safe, replacing all other safe fixes.

Maybe we need a pair of option, similar to fixable and unfixable:

• fix.unsafe: Initialized with all suggested rules by default. Lists the fixes that are not applied without using --unsafe. Similar to fixable that gets initialized with all rules

• fix.safe: Empty by default. Promotes suggested fixes to be applied without the --unsafe. Similar to --unfixable that is empty by default

• You can enable all suggested rules by setting fix.unsafe to an empty list.

• You can demote an automatic rule by adding it to fix.unsafe. We may need a extend_unsafe so that you can do this without having to list all suggested rules. But we may want to leave this for later.

• You can promote a suggested rule by adding it to fix.safe.

My main struggle with this is that it is somewhat unintuitive. My expectation would be that fix.safe gets initialized with all *automaticrules andfix.unsafeis initialized with all *suggested* rules. The problem with that is that either settingsafeorunsafe` overrides all other rules, which is annoying to use.

fix.unsafe and fix.safe would only work if they are initialized as empty lists, because initializing safe with all rules would promote all rules to safe, and initializing unsafe with all rules would demote all fixes. We could initialize them as empty lists, but we would then need to come up with better names: promoted, demoted, promote_suggested = [], demote_automatic = []?

An alternative format, that has no precedence in our configuration, is to use a dictionary instead:

fix.overrides = {
	"RUF100": "automatic",
	"RUF101: "suggested",
}

@MichaReiser IMHO a dictionary is cleaner. Seems to be the easiest way to express what we want to express. What do you think?

Yeah sounds good! I think that's pretty aligned with what I was thinking originally :D

I'll work on getting the implementation to match - I may do the promotion/demotion stuff in a follow-up PR, if that works?

Wonderful! Yeah, I think promotion and demotion is definitely not needed to start.