Add Applicability to Fix

[MichaReiser]

Part of #4181 and depends on #4182

• Add a new Applicability enum in fix.rs similar to Rome's or Clippy's Applicability enums with three variants:
  • Safe (or Automatic, Always, MachineApplicable?)
  • MaybeIncorrect
  • Unspecified
• Add the applicability field to Fix
• Add the new safe and safe_edits constructor functions (identical to unspecified and unspecified_edits)
• Add the new maybe_incorrect and maybe_incorrect_edits constructor functions (identical to unspecified and unspecified_edits)
• Mark unspecified and unspecified_edits as deprecated (new lints should use either safe_* or maybe_incorrect_*). Add allow(deprecated)` at the call-sites.
• Change the Diff implementation to write Suggested Fix for MaybeIncorrect and Unspecified, and Safe fix for Applicability::Safe.

[zanieb]

FYI I've started working on this branched off of #4198

[MichaReiser]

This will allow us to change min_start to return a TextSize rather than Option<TextSize> because it is guaranteed that the Fix always contains at least one edit.

[zanieb]

I'm working on the names here and looking for something where they're coherent with each other. I'm curious if we're likely to add more levels in the future?

For example, here's an option centered on "certainty" that's consistent while leaving room for feature options

enum Applicability {
    /// The change is guaranteed to be correct and safe to apply automatically.
    Certain,

    /// The change is likely to be correct, but may require a manual review to ensure correctness.
    Likely,

    /// The change might not be correct, and requires manual review before applying.
    Uncertain,

    /// The change could introduce errors or break the code, and should be applied with caution.
    Risky,

    /// The applicability of the change is not specified or cannot be determined.
    Unspecified,
}

Or if we do not need more options in the future

enum Applicability {
    Safe,
    Unsafe,
    Unspecified
}

This option matches your safe/unsafe framing in the primary issue #4181 — I think this may be the clearest framing if there are not going to be additional options?

Or if we want to center on how it will be applied

enum Applicability {
    Automatic,
    Suggested,
    Unspecified
}

[MichaReiser]

I'm working on the names here and looking for something where they're coherent with each other. I'm curious if we're likely to add more levels in the future?

I don't think we can rule it out completely. For example, Clippy has a HasPlaceholders Applicability which could be useful.

Thank you for your thoughtful suggestions. They are way more consistent than my initial proposal :)

The challenge I'm facing right now evaluating the proposals is that we need to make a call on what a future Appicability level could be: Will it represent a more risky code fixe, or mark incomplete code fixes (that e.g. require manual edits from users)? Depending on that, an Applicability level around certainty` or how it is applied is more appropriate.

For example, here's an option centered on "certainty" that's consistent while leaving room for feature options

I'm trying to think about how the fix review workflows for Likely, Uncertain, and Risky would differ because it seems all of them require manual review. We could show some scary warnings around Uncertain and Risky fixes but I fear that it will be difficult for users to understand the differences (or why some fixes have scary warnings and others don't).

Or if we want to center on how it will be applied

That's why I'm leaning more towards this naming, but I fail to come up with a good name for a fix that we "propose" to a user but e.g. don't support applying automatically because we want users to manually apply, either because the code is incomplete or has a good chance to be incorrect. Would that be Manual?

What's your preference?

[zanieb]

I think the following makes sense

pub enum Applicability {
    /// The fix is definitely what the user intended, or maintains the exact meaning of the code.
    /// This fix should be automatically applied.
    Automatic,

    /// The fix may be what the user intended, but it is uncertain.
    /// The fix should result in valid code if it is applied.
    /// The fix can be applied with user opt-in.
    Suggested,

    /// The fix has a good chance of being incorrect or the code is incomplete.
    /// The fix may result in invalid code if it is applied.
    /// The fix should only be manually applied by the user.
    Manual,

    /// The applicability of the fix is unknown.
    #[default]
    Unspecified,
}

I agree that "certainty" doesn't really help with the typical user workflow.

[MichaReiser]

I think the following makes sense

I like it. Thanks for pushing for, and improving the naming!