From 5e2bb8ca075e6967acd4c6790485aac0f7562d19 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 6 Nov 2023 06:45:10 -0800 Subject: [PATCH] Add a `Fix` constructor that takes `Applicability` as an argument (#8514) ## Summary If you want to create an edit with dynamic applicability, you have to branch and repeat the edit entirely between the two branches. If you further need the edit itself to be dynamic (e.g., perhaps you have a single edit in one case, vs. multiple in another), you suddenly have four branches. This PR just adds an alternate constructor that takes applicability as an argument, as an escape hatch. --- crates/ruff_diagnostics/src/fix.rs | 24 ++++++++++++++++ .../rules/unnecessary_call_around_sorted.rs | 17 +++++------ .../unnecessary_paren_on_raise_exception.rs | 28 +++++++++++-------- 3 files changed, 50 insertions(+), 19 deletions(-) diff --git a/crates/ruff_diagnostics/src/fix.rs b/crates/ruff_diagnostics/src/fix.rs index befb5c1c5650d1..d44ef411ce039c 100644 --- a/crates/ruff_diagnostics/src/fix.rs +++ b/crates/ruff_diagnostics/src/fix.rs @@ -107,6 +107,30 @@ impl Fix { } } + /// Create a new [`Fix`] with the specified [`Applicability`] to apply an [`Edit`] element. + pub fn applicable_edit(edit: Edit, applicability: Applicability) -> Self { + Self { + edits: vec![edit], + applicability, + isolation_level: IsolationLevel::default(), + } + } + + /// Create a new [`Fix`] with the specified [`Applicability`] to apply multiple [`Edit`] elements. + pub fn applicable_edits( + edit: Edit, + rest: impl IntoIterator, + applicability: Applicability, + ) -> Self { + let mut edits: Vec = std::iter::once(edit).chain(rest).collect(); + edits.sort_by_key(|edit| (edit.start(), edit.end())); + Self { + edits, + applicability, + isolation_level: IsolationLevel::default(), + } + } + /// Return the [`TextSize`] of the first [`Edit`] in the [`Fix`]. pub fn min_start(&self) -> Option { self.edits.first().map(Edit::start) diff --git a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs index 7117d099916870..a9191e1495e34a 100644 --- a/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs +++ b/crates/ruff_linter/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix}; +use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr}; use ruff_text_size::Ranged; @@ -83,13 +83,14 @@ pub(crate) fn unnecessary_call_around_sorted( expr.range(), ); diagnostic.try_set_fix(|| { - let edit = - fixes::fix_unnecessary_call_around_sorted(expr, checker.locator(), checker.stylist())?; - if outer.id == "reversed" { - Ok(Fix::unsafe_edit(edit)) - } else { - Ok(Fix::safe_edit(edit)) - } + Ok(Fix::applicable_edit( + fixes::fix_unnecessary_call_around_sorted(expr, checker.locator(), checker.stylist())?, + if outer.id == "reversed" { + Applicability::Unsafe + } else { + Applicability::Safe + }, + )) }); checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_raise/rules/unnecessary_paren_on_raise_exception.rs b/crates/ruff_linter/src/rules/flake8_raise/rules/unnecessary_paren_on_raise_exception.rs index 2fde8e5a8f3203..4a0bffc74bc7dd 100644 --- a/crates/ruff_linter/src/rules/flake8_raise/rules/unnecessary_paren_on_raise_exception.rs +++ b/crates/ruff_linter/src/rules/flake8_raise/rules/unnecessary_paren_on_raise_exception.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_diagnostics::{AlwaysFixableViolation, Applicability, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr}; use ruff_python_semantic::BindingKind; @@ -103,17 +103,23 @@ pub(crate) fn unnecessary_paren_on_raise_exception(checker: &mut Checker, expr: .next() .is_some_and(char::is_alphanumeric) { - diagnostic.set_fix(if exception_type.is_some() { - Fix::safe_edit(Edit::range_replacement(" ".to_string(), arguments.range())) - } else { - Fix::unsafe_edit(Edit::range_replacement(" ".to_string(), arguments.range())) - }); + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement(" ".to_string(), arguments.range()), + if exception_type.is_some() { + Applicability::Safe + } else { + Applicability::Unsafe + }, + )); } else { - diagnostic.set_fix(if exception_type.is_some() { - Fix::safe_edit(Edit::range_deletion(arguments.range())) - } else { - Fix::unsafe_edit(Edit::range_deletion(arguments.range())) - }); + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_deletion(arguments.range()), + if exception_type.is_some() { + Applicability::Safe + } else { + Applicability::Unsafe + }, + )); } checker.diagnostics.push(diagnostic);