Rename applicability levels to Safe, Unsafe, and Display (#7843)

After working with the previous change in
#7821 I found the names a bit
unclear and their relationship with the user-facing API muddied. Since
the applicability is exposed to the user directly in our JSON output, I
think it's important that these names align with our configuration
options. I've replaced `Manual` or `Never` with `Display` which captures
our intent for these fixes (only for display). Here, we create room for
future levels, such as `HasPlaceholders`, which wouldn't fit into the
`Always`/`Sometimes`/`Never` levels.

Unlike #7819, this retains the
flat enum structure which is easier to work with.

crates/ruff_cli/tests/format.rs

@@ -175,7 +175,7 @@ import os
  },
  "filename": "-",
  "fix": {
- "applicability": "always",
+ "applicability": "safe",
  "edits": [
  {
  "content": "",

crates/ruff_cli/tests/snapshots/integration_test__stdin_json.snap

@@ -24,7 +24,7 @@ exit_code: 1
  },
  "filename": "/path/to/F401.py",
  "fix": {
- "applicability": "always",
+ "applicability": "safe",
  "edits": [
  {
  "content": "",

crates/ruff_diagnostics/src/fix.rs

@@ -10,17 +10,17 @@ use crate::edit::Edit;
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(rename_all = "lowercase"))]
 pub enum Applicability {
- /// The fix is unsafe and should only be manually applied by the user.
+ /// The fix is unsafe and should only be displayed for manual application by the user.
  /// The fix is likely to be incorrect or the resulting code may have invalid syntax.
- Never,
+ Display,
 
  /// The fix is unsafe and should only be applied with user opt-in.
  /// The fix may be what the user intended, but it is uncertain; the resulting code will have valid syntax.
- Sometimes,
+ Unsafe,
 
  /// The fix is safe and can always be applied.
  /// The fix is definitely what the user intended, or it maintains the exact meaning of the code.
- Always,
+ Safe,
 }
 
 /// Indicates the level of isolation required to apply a fix.
@@ -47,62 +47,62 @@ pub struct Fix {
 }
 
 impl Fix {
- /// Create a new [`Fix`] that can [always](Applicability::Always) be applied from an [`Edit`] element.
- pub fn always_applies(edit: Edit) -> Self {
+ /// Create a new [`Fix`] that is [safe](Applicability::Safe) to apply from an [`Edit`] element.
+ pub fn safe_edit(edit: Edit) -> Self {
  Self {
  edits: vec![edit],
- applicability: Applicability::Always,
+ applicability: Applicability::Safe,
  isolation_level: IsolationLevel::default(),
  }
  }
 
- /// Create a new [`Fix`] that can [always](Applicability::Always) be applied from multiple [`Edit`] elements.
- pub fn always_applies_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
+ /// Create a new [`Fix`] that is [safe](Applicability::Safe) to apply from multiple [`Edit`] elements.
+ pub fn safe_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
  let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect();
  edits.sort_by_key(|edit| (edit.start(), edit.end()));
  Self {
  edits,
- applicability: Applicability::Always,
+ applicability: Applicability::Safe,
  isolation_level: IsolationLevel::default(),
  }
  }
 
- /// Create a new [`Fix`] that can [sometimes](Applicability::Sometimes) be applied from an [`Edit`] element.
- pub fn sometimes_applies(edit: Edit) -> Self {
+ /// Create a new [`Fix`] that is [unsafe](Applicability::Unsafe) to apply from an [`Edit`] element.
+ pub fn unsafe_edit(edit: Edit) -> Self {
  Self {
  edits: vec![edit],
- applicability: Applicability::Sometimes,
+ applicability: Applicability::Unsafe,
  isolation_level: IsolationLevel::default(),
  }
  }
 
- /// Create a new [`Fix`] that can [sometimes](Applicability::Sometimes) be applied from multiple [`Edit`] elements.
- pub fn sometimes_applies_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
+ /// Create a new [`Fix`] that is [unsafe](Applicability::Unsafe) to apply from multiple [`Edit`] elements.
+ pub fn unsafe_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
  let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect();
  edits.sort_by_key(|edit| (edit.start(), edit.end()));
  Self {
  edits,
- applicability: Applicability::Sometimes,
+ applicability: Applicability::Unsafe,
  isolation_level: IsolationLevel::default(),
  }
  }
 
- /// Create a new [`Fix`] that should [never](Applicability::Never) be applied from an [`Edit`] element .
- pub fn never_applies(edit: Edit) -> Self {
+ /// Create a new [`Fix`] that should only [display](Applicability::Display) and not apply from an [`Edit`] element .
+ pub fn display_edit(edit: Edit) -> Self {
  Self {
  edits: vec![edit],
- applicability: Applicability::Never,
+ applicability: Applicability::Display,
  isolation_level: IsolationLevel::default(),
  }
  }
 
- /// Create a new [`Fix`] that should [never](Applicability::Never) be applied from multiple [`Edit`] elements.
- pub fn never_applies_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
+ /// Create a new [`Fix`] that should only [display](Applicability::Display) and not apply from multiple [`Edit`] elements.
+ pub fn display_edits(edit: Edit, rest: impl IntoIterator<Item = Edit>) -> Self {
  let mut edits: Vec<Edit> = std::iter::once(edit).chain(rest).collect();
  edits.sort_by_key(|edit| (edit.start(), edit.end()));
  Self {
  edits,
- applicability: Applicability::Never,
+ applicability: Applicability::Display,
  isolation_level: IsolationLevel::default(),
  }
  }

crates/ruff_linter/src/checkers/ast/analyze/bindings.rs

@@ -38,7 +38,7 @@ pub(crate) fn bindings(checker: &mut Checker) {
  binding,
  checker.locator,
  )
- .map(Fix::always_applies)
+ .map(Fix::safe_edit)
  });
  }
  checker.diagnostics.push(diagnostic);

crates/ruff_linter/src/checkers/noqa.rs

@@ -110,10 +110,8 @@ pub(crate) fn check_noqa(
  let mut diagnostic =
  Diagnostic::new(UnusedNOQA { codes: None }, directive.range());
  if settings.rules.should_fix(diagnostic.kind.rule()) {
- diagnostic.set_fix(Fix::sometimes_applies(delete_noqa(
- directive.range(),
- locator,
- )));
+ diagnostic
+ .set_fix(Fix::unsafe_edit(delete_noqa(directive.range(), locator)));
  }
  diagnostics.push(diagnostic);
  }
@@ -177,17 +175,15 @@ pub(crate) fn check_noqa(
  );
  if settings.rules.should_fix(diagnostic.kind.rule()) {
  if valid_codes.is_empty() {
- diagnostic.set_fix(Fix::sometimes_applies(delete_noqa(
+ diagnostic.set_fix(Fix::unsafe_edit(delete_noqa(
  directive.range(),
  locator,
  )));
  } else {
- diagnostic.set_fix(Fix::sometimes_applies(
- Edit::range_replacement(
- format!("# noqa: {}", valid_codes.join(", ")),
- directive.range(),
- ),
- ));
+ diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+ format!("# noqa: {}", valid_codes.join(", ")),
+ directive.range(),
+ )));
  }
  }
  diagnostics.push(diagnostic);

crates/ruff_linter/src/fix/mod.rs

@@ -163,7 +163,7 @@ mod tests {
  // The choice of rule here is arbitrary.
  kind: MissingNewlineAtEndOfFile.into(),
  range: edit.range(),
- fix: Some(Fix::always_applies(edit)),
+ fix: Some(Fix::safe_edit(edit)),
  parent: None,
  })
  .collect()

crates/ruff_linter/src/message/diff.rs

@@ -53,9 +53,9 @@ impl Display for Diff<'_> {
 
  let message = match self.fix.applicability() {
  // TODO(zanieb): Adjust this messaging once it's user-facing
- Applicability::Always => "Fix",
- Applicability::Sometimes => "Suggested fix",
- Applicability::Never => "Possible fix",
+ Applicability::Safe => "Fix",
+ Applicability::Unsafe => "Suggested fix",
+ Applicability::Display => "Possible fix",
  };
  writeln!(f, "ℹ {}", message.blue())?;
 

crates/ruff_linter/src/message/mod.rs

@@ -178,9 +178,10 @@ def fibonacci(n):
  },
  TextRange::new(TextSize::from(7), TextSize::from(9)),
  )
- .with_fix(Fix::sometimes_applies(Edit::range_deletion(
- TextRange::new(TextSize::from(0), TextSize::from(10)),
- )));
+ .with_fix(Fix::unsafe_edit(Edit::range_deletion(TextRange::new(
+ TextSize::from(0),
+ TextSize::from(10),
+ ))));
 
  let fib_source = SourceFileBuilder::new("fib.py", fib).finish();
 
@@ -192,7 +193,7 @@ def fibonacci(n):
  },
  TextRange::new(TextSize::from(94), TextSize::from(95)),
  )
- .with_fix(Fix::sometimes_applies(Edit::deletion(
+ .with_fix(Fix::unsafe_edit(Edit::deletion(
  TextSize::from(94),
  TextSize::from(99),
  )));

crates/ruff_linter/src/message/snapshots/ruff_linter__message__json__tests__output.snap

@@ -11,7 +11,7 @@ expression: content
  },
  "filename": "fib.py",
  "fix": {
- "applicability": "sometimes",
+ "applicability": "unsafe",
  "edits": [
  {
  "content": "",
@@ -43,7 +43,7 @@ expression: content
  },
  "filename": "fib.py",
  "fix": {
- "applicability": "sometimes",
+ "applicability": "unsafe",
  "edits": [
  {
  "content": "",

crates/ruff_linter/src/message/snapshots/ruff_linter__message__json_lines__tests__output.snap

@@ -2,7 +2,7 @@
 source: crates/ruff_linter/src/message/json_lines.rs
 expression: content
 ---
-{"code":"F401","end_location":{"column":10,"row":1},"filename":"fib.py","fix":{"applicability":"sometimes","edits":[{"content":"","end_location":{"column":1,"row":2},"location":{"column":1,"row":1}}],"message":"Remove unused import: `os`"},"location":{"column":8,"row":1},"message":"`os` imported but unused","noqa_row":1,"url":"https://docs.astral.sh/ruff/rules/unused-import"}
-{"code":"F841","end_location":{"column":6,"row":6},"filename":"fib.py","fix":{"applicability":"sometimes","edits":[{"content":"","end_location":{"column":10,"row":6},"location":{"column":5,"row":6}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":6},"message":"Local variable `x` is assigned to but never used","noqa_row":6,"url":"https://docs.astral.sh/ruff/rules/unused-variable"}
+{"code":"F401","end_location":{"column":10,"row":1},"filename":"fib.py","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":1,"row":2},"location":{"column":1,"row":1}}],"message":"Remove unused import: `os`"},"location":{"column":8,"row":1},"message":"`os` imported but unused","noqa_row":1,"url":"https://docs.astral.sh/ruff/rules/unused-import"}
+{"code":"F841","end_location":{"column":6,"row":6},"filename":"fib.py","fix":{"applicability":"unsafe","edits":[{"content":"","end_location":{"column":10,"row":6},"location":{"column":5,"row":6}}],"message":"Remove assignment to unused variable `x`"},"location":{"column":5,"row":6},"message":"Local variable `x` is assigned to but never used","noqa_row":6,"url":"https://docs.astral.sh/ruff/rules/unused-variable"}
 {"code":"F821","end_location":{"column":5,"row":1},"filename":"undef.py","fix":null,"location":{"column":4,"row":1},"message":"Undefined name `a`","noqa_row":1,"url":"https://docs.astral.sh/ruff/rules/undefined-name"}
 

crates/ruff_linter/src/rules/eradicate/rules/commented_out_code.rs

@@ -68,7 +68,7 @@ pub(crate) fn commented_out_code(
  let mut diagnostic = Diagnostic::new(CommentedOutCode, *range);
 
  if settings.rules.should_fix(Rule::CommentedOutCode) {
- diagnostic.set_fix(Fix::never_applies(Edit::range_deletion(
+ diagnostic.set_fix(Fix::display_edit(Edit::range_deletion(
  locator.full_lines_range(*range),
  )));
  }

crates/ruff_linter/src/rules/flake8_annotations/rules/definition.rs

@@ -703,7 +703,7 @@ pub(crate) fn definition(
  function.identifier(),
  );
  if checker.patch(diagnostic.kind.rule()) {
- diagnostic.set_fix(Fix::sometimes_applies(Edit::insertion(
+ diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
  " -> None".to_string(),
  function.parameters.range().end(),
  )));
@@ -721,7 +721,7 @@ pub(crate) fn definition(
  );
  if checker.patch(diagnostic.kind.rule()) {
  if let Some(return_type) = simple_magic_return_type(name) {
- diagnostic.set_fix(Fix::sometimes_applies(Edit::insertion(
+ diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
  format!(" -> {return_type}"),
  function.parameters.range().end(),
  )));

crates/ruff_linter/src/rules/flake8_bugbear/rules/assert_false.rs

@@ -76,7 +76,7 @@ pub(crate) fn assert_false(checker: &mut Checker, stmt: &Stmt, test: &Expr, msg:
 
  let mut diagnostic = Diagnostic::new(AssertFalse, test.range());
  if checker.patch(diagnostic.kind.rule()) {
- diagnostic.set_fix(Fix::sometimes_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
  checker.generator().stmt(&assertion_error(msg)),
  stmt.range(),
  )));

crates/ruff_linter/src/rules/flake8_bugbear/rules/duplicate_exceptions.rs

@@ -147,7 +147,7 @@ fn duplicate_handler_exceptions<'a>(
  expr.range(),
  );
  if checker.patch(diagnostic.kind.rule()) {
- diagnostic.set_fix(Fix::always_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
  // Single exceptions don't require parentheses, but since we're _removing_
  // parentheses, insert whitespace as needed.
  if let [elt] = unique_elts.as_slice() {

crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs

@@ -86,7 +86,7 @@ pub(crate) fn getattr_with_constant(
 
  let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range());
  if checker.patch(diagnostic.kind.rule()) {
- diagnostic.set_fix(Fix::sometimes_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
  pad(
  if matches!(
  obj,

crates/ruff_linter/src/rules/flake8_bugbear/rules/mutable_argument_default.rs

@@ -200,8 +200,5 @@ fn move_initialization(
  }
 
  let initialization_edit = Edit::insertion(content, pos);
- Some(Fix::never_applies_edits(
- default_edit,
- [initialization_edit],
- ))
+ Some(Fix::display_edits(default_edit, [initialization_edit]))
 }

crates/ruff_linter/src/rules/flake8_bugbear/rules/redundant_tuple_in_exception_handler.rs

@@ -85,7 +85,7 @@ pub(crate) fn redundant_tuple_in_exception_handler(
  // ```
  // Otherwise, the output will be invalid syntax, since we're removing a set of
  // parentheses.
- diagnostic.set_fix(Fix::always_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
  pad(
  checker.generator().expr(elt),
  type_.range(),

crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs

@@ -109,7 +109,7 @@ pub(crate) fn setattr_with_constant(
  if expr == child.as_ref() {
  let mut diagnostic = Diagnostic::new(SetAttrWithConstant, expr.range());
  if checker.patch(diagnostic.kind.rule()) {
- diagnostic.set_fix(Fix::sometimes_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
  assignment(obj, name, value, checker.generator()),
  expr.range(),
  )));

crates/ruff_linter/src/rules/flake8_bugbear/rules/unreliable_callable_check.rs

@@ -83,7 +83,7 @@ pub(crate) fn unreliable_callable_check(
  if checker.patch(diagnostic.kind.rule()) {
  if id == "hasattr" {
  if checker.semantic().is_builtin("callable") {
- diagnostic.set_fix(Fix::always_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
  format!("callable({})", checker.locator().slice(obj)),
  expr.range(),
  )));

crates/ruff_linter/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs

@@ -168,7 +168,7 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, stmt_for: &ast
  .filter(|binding| binding.start() >= expr.start())
  .all(|binding| !binding.is_used())
  {
- diagnostic.set_fix(Fix::sometimes_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
  rename,
  expr.range(),
  )));

crates/ruff_linter/src/rules/flake8_commas/rules/trailing_commas.rs

@@ -325,9 +325,7 @@ pub(crate) fn trailing_commas(
  let comma = prev.spanned.unwrap();
  let mut diagnostic = Diagnostic::new(ProhibitedTrailingComma, comma.1);
  if settings.rules.should_fix(Rule::ProhibitedTrailingComma) {
- diagnostic.set_fix(Fix::always_applies(Edit::range_deletion(
- diagnostic.range(),
- )));
+ diagnostic.set_fix(Fix::safe_edit(Edit::range_deletion(diagnostic.range())));
  }
  diagnostics.push(diagnostic);
  }
@@ -367,7 +365,7 @@ pub(crate) fn trailing_commas(
  // removing any brackets in the same linter pass - doing both at the same time could
  // lead to a syntax error.
  let contents = locator.slice(missing_comma.1);
- diagnostic.set_fix(Fix::always_applies(Edit::range_replacement(
+ diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
  format!("{contents},"),
  missing_comma.1,
  )));

crates/ruff_linter/src/rules/flake8_comprehensions/fixes.rs

@@ -1293,7 +1293,7 @@ pub(crate) fn fix_unnecessary_comprehension_any_all(
  _ => whitespace_after_arg,
  };
 
- Ok(Fix::sometimes_applies(Edit::range_replacement(
+ Ok(Fix::unsafe_edit(Edit::range_replacement(
  tree.codegen_stylist(stylist),
  expr.range(),
  )))

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs

@@ -90,9 +90,9 @@ pub(crate) fn unnecessary_call_around_sorted(
  checker.stylist(),
  )?;
  if outer.id == "reversed" {
- Ok(Fix::sometimes_applies(edit))
+ Ok(Fix::unsafe_edit(edit))
  } else {
- Ok(Fix::always_applies(edit))
+ Ok(Fix::safe_edit(edit))
  }
  });
  }

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_collection_call.rs

@@ -88,7 +88,7 @@ pub(crate) fn unnecessary_collection_call(
  );
  if checker.patch(diagnostic.kind.rule()) {
  diagnostic.try_set_fix(|| {
- fixes::fix_unnecessary_collection_call(expr, checker).map(Fix::sometimes_applies)
+ fixes::fix_unnecessary_collection_call(expr, checker).map(Fix::unsafe_edit)
  });
  }
  checker.diagnostics.push(diagnostic);

crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs

@@ -67,7 +67,7 @@ fn add_diagnostic(checker: &mut Checker, expr: &Expr) {
  if checker.patch(diagnostic.kind.rule()) {
  diagnostic.try_set_fix(|| {
  fixes::fix_unnecessary_comprehension(expr, checker.locator(), checker.stylist())
- .map(Fix::sometimes_applies)
+ .map(Fix::unsafe_edit)
  });
  }
  checker.diagnostics.push(diagnostic);