diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py b/crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py index e316c3383ca9f..d5fa9d34c3e9f 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py @@ -237,3 +237,15 @@ def foo(self, value) -> None: # case 8: counter["a"] = max(counter["b"], counter["a"]) if counter["a"] > counter["b"]: counter["b"] = counter["a"] + +# https://github.com/astral-sh/ruff/issues/17311 + +# fix marked unsafe as delete comments +a, b = [], [] +if a >= b: + # very important comment + a = b + +# fix marked safe as preserve comments +if a >= b: + a = b # very important comment diff --git a/crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs b/crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs index 4e64179d1e373..4e3e923b62bcc 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs @@ -1,4 +1,4 @@ -use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; +use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::parenthesize::parenthesized_range; @@ -29,6 +29,19 @@ use crate::fix::snippet::SourceCodeSnippet; /// highest_score = max(highest_score, score) /// ``` /// +/// ## Fix safety +/// This fix is marked unsafe if it would delete any comments within the replacement range. +/// +/// An example to illustrate where comments are preserved and where they are not: +/// +/// ```py +/// a, b = 0, 10 +/// +/// if a >= b: # deleted comment +/// # deleted comment +/// a = b # preserved comment +/// ``` +/// /// ## References /// - [Python documentation: `max`](https://docs.python.org/3/library/functions.html#max) /// - [Python documentation: `min`](https://docs.python.org/3/library/functions.html#min) @@ -169,11 +182,18 @@ pub(crate) fn if_stmt_min_max(checker: &Checker, stmt_if: &ast::StmtIf) { stmt_if.range(), ); + let range_replacement = stmt_if.range(); + let applicability = if checker.comment_ranges().intersects(range_replacement) { + Applicability::Unsafe + } else { + Applicability::Safe + }; + if checker.semantic().has_builtin_binding(min_max.as_str()) { - diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( - replacement, - stmt_if.range(), - ))); + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement(replacement, range_replacement), + applicability, + )); } checker.report_diagnostic(diagnostic); diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap index 94b7385d0a67e..89aec8775ee6c 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap @@ -359,7 +359,7 @@ if_stmt_min_max.py:119:1: PLR1730 [*] Replace `if` statement with `A2 = max(A2, | = help: Replace with `A2 = max(A2, A1)` -ℹ Safe fix +ℹ Unsafe fix 116 116 | A1 = AA(0) 117 117 | A2 = AA(3) 118 118 | @@ -382,7 +382,7 @@ if_stmt_min_max.py:122:1: PLR1730 [*] Replace `if` statement with `A2 = max(A1, | = help: Replace with `A2 = max(A1, A2)` -ℹ Safe fix +ℹ Unsafe fix 119 119 | if A2 < A1: # [max-instead-of-if] 120 120 | A2 = A1 121 121 | @@ -405,7 +405,7 @@ if_stmt_min_max.py:125:1: PLR1730 [*] Replace `if` statement with `A2 = min(A2, | = help: Replace with `A2 = min(A2, A1)` -ℹ Safe fix +ℹ Unsafe fix 122 122 | if A2 <= A1: # [max-instead-of-if] 123 123 | A2 = A1 124 124 | @@ -428,7 +428,7 @@ if_stmt_min_max.py:128:1: PLR1730 [*] Replace `if` statement with `A2 = min(A1, | = help: Replace with `A2 = min(A1, A2)` -ℹ Safe fix +ℹ Unsafe fix 125 125 | if A2 > A1: # [min-instead-of-if] 126 126 | A2 = A1 127 127 | @@ -721,6 +721,8 @@ if_stmt_min_max.py:238:1: PLR1730 [*] Replace `if` statement with `counter["b"] 238 | / if counter["a"] > counter["b"]: 239 | | counter["b"] = counter["a"] | |_______________________________^ PLR1730 +240 | +241 | # https://github.com/astral-sh/ruff/issues/17311 | = help: Replace with `counter["b"] = max(counter["b"], counter["a"])` @@ -731,3 +733,48 @@ if_stmt_min_max.py:238:1: PLR1730 [*] Replace `if` statement with `counter["b"] 238 |-if counter["a"] > counter["b"]: 239 |- counter["b"] = counter["a"] 238 |+counter["b"] = max(counter["b"], counter["a"]) +240 239 | +241 240 | # https://github.com/astral-sh/ruff/issues/17311 +242 241 | + +if_stmt_min_max.py:245:1: PLR1730 [*] Replace `if` statement with `a = min(b, a)` + | +243 | # fix marked unsafe as delete comments +244 | a, b = [], [] +245 | / if a >= b: +246 | | # very important comment +247 | | a = b + | |_________^ PLR1730 +248 | +249 | # fix marked safe as preserve comments + | + = help: Replace with `a = min(b, a)` + +ℹ Unsafe fix +242 242 | +243 243 | # fix marked unsafe as delete comments +244 244 | a, b = [], [] +245 |-if a >= b: +246 |- # very important comment +247 |- a = b + 245 |+a = min(b, a) +248 246 | +249 247 | # fix marked safe as preserve comments +250 248 | if a >= b: + +if_stmt_min_max.py:250:1: PLR1730 [*] Replace `if` statement with `a = min(b, a)` + | +249 | # fix marked safe as preserve comments +250 | / if a >= b: +251 | | a = b # very important comment + | |_________^ PLR1730 + | + = help: Replace with `a = min(b, a)` + +ℹ Safe fix +247 247 | a = b +248 248 | +249 249 | # fix marked safe as preserve comments +250 |-if a >= b: +251 |- a = b # very important comment + 250 |+a = min(b, a) # very important comment