Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
30 changes: 25 additions & 5 deletions crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
Expand All @@ -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 |
Expand All @@ -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 |
Expand Down Expand Up @@ -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"])`

Expand All @@ -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
Loading