From bffc95873dedcc7c3c913ee6162bf1c495d6829b Mon Sep 17 00:00:00 2001 From: chiri Date: Mon, 19 Jan 2026 18:17:24 +0300 Subject: [PATCH] [`refurb`] Make fix unsafe if it deletes comments (`FURB136`) (#22680) ## Summary ## Test Plan --- .../resources/test/fixtures/refurb/FURB136.py | 8 ++++++ .../src/rules/refurb/rules/if_expr_min_max.rs | 22 +++++++++++++--- ...es__refurb__tests__FURB136_FURB136.py.snap | 26 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB136.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB136.py index 44a8e8216d..55b219200d 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB136.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB136.py @@ -23,3 +23,11 @@ x if ( x > y ) else y # FURB136 + + +highest_score = ( + x + # text + if x > y + else y +) diff --git a/crates/ruff_linter/src/rules/refurb/rules/if_expr_min_max.rs b/crates/ruff_linter/src/rules/refurb/rules/if_expr_min_max.rs index a3bfcdd0dd..577efa94da 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/if_expr_min_max.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/if_expr_min_max.rs @@ -1,3 +1,4 @@ +use ruff_diagnostics::Applicability; use ruff_macros::{ViolationMetadata, derive_message_formats}; use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::{self as ast, CmpOp, Expr}; @@ -19,14 +20,21 @@ use crate::{Edit, Fix, FixAvailability, Violation}; /// /// ## Example /// ```python +/// score1, score2 = 4, 5 +/// /// highest_score = score1 if score1 > score2 else score2 /// ``` /// /// Use instead: /// ```python +/// score1, score2 = 4, 5 +/// /// highest_score = max(score2, score1) /// ``` /// +/// ## Fix safety +/// This rule's fix is marked as safe, unless the expression contains comments. +/// /// ## References /// - [Python documentation: `min`](https://docs.python.org/3.11/library/functions.html#min) /// - [Python documentation: `max`](https://docs.python.org/3.11/library/functions.html#max) @@ -141,10 +149,16 @@ pub(crate) fn if_expr_min_max(checker: &Checker, if_exp: &ast::ExprIf) { ); if checker.semantic().has_builtin_binding(min_max.as_str()) { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - replacement, - if_exp.range(), - ))); + let applicability = if checker.comment_ranges().intersects(if_exp.range()) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement(replacement, if_exp.range()), + applicability, + )); } } diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB136_FURB136.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB136_FURB136.py.snap index f5b0efd703..e30866e9f5 100644 --- a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB136_FURB136.py.snap +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB136_FURB136.py.snap @@ -181,3 +181,29 @@ help: Replace with `max(y, x)` - > y - ) else y # FURB136 22 + max(y, x) # FURB136 +23 | +24 | +25 | highest_score = ( + +FURB136 [*] Replace `if` expression with `max(y, x)` + --> FURB136.py:29:5 + | +28 | highest_score = ( +29 | / x +30 | | # text +31 | | if x > y +32 | | else y + | |__________^ +33 | ) + | +help: Replace with `max(y, x)` +26 | +27 | +28 | highest_score = ( + - x + - # text + - if x > y + - else y +29 + max(y, x) +30 | ) +note: This is an unsafe fix and may change runtime behavior