Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Support inverted comparisons (PLR1730) #10920

Merged
merged 2 commits into from
Apr 13, 2024
Merged

[pylint] Support inverted comparisons (PLR1730) #10920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1db1489
[`pylint`] - add more aggressive logic (`PLR1730`)
diceroll123 Apr 13, 2024
3706006
Another test
charliermarsh Apr 13, 2024
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
25 changes: 25 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/pylint/if_stmt_min_max.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,28 @@ def __le__(self, b):
value.
attr
) = 3

class Foo:
_min = 0
_max = 0

def foo(self, value) -> None:
if value < self._min:
self._min = value
if value > self._max:
self._max = value

if self._min < value:
self._min = value
if self._max > value:
self._max = value

if value <= self._min:
self._min = value
if value >= self._max:
self._max = value

if self._min <= value:
self._min = value
if self._max >= value:
self._max = value
47 changes: 33 additions & 14 deletions crates/ruff_linter/src/rules/pylint/rules/if_stmt_min_max.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,27 +107,46 @@ pub(crate) fn if_stmt_min_max(checker: &mut Checker, stmt_if: &ast::StmtIf) {
return;
};

// Determine whether to use `min()` or `max()`, and whether to flip the
// order of the arguments, which is relevant for breaking ties.
let (min_max, flip_args) = match op {
CmpOp::Gt => (MinMax::Min, true),
CmpOp::GtE => (MinMax::Min, false),
CmpOp::Lt => (MinMax::Max, true),
CmpOp::LtE => (MinMax::Max, false),
_ => return,
};

let [right] = &**comparators else {
return;
};

let left_cmp = ComparableExpr::from(left);
let body_target_cmp = ComparableExpr::from(body_target);
let right_statement_cmp = ComparableExpr::from(right);
let right_cmp = ComparableExpr::from(right);
let body_value_cmp = ComparableExpr::from(body_value);
if left_cmp != body_target_cmp || right_statement_cmp != body_value_cmp {
return;
}

let left_is_target = left_cmp == body_target_cmp;
let right_is_target = right_cmp == body_target_cmp;
let left_is_value = left_cmp == body_value_cmp;
let right_is_value = right_cmp == body_value_cmp;

// Determine whether to use `min()` or `max()`, and whether to flip the
// order of the arguments, which is relevant for breaking ties.
// Also ensure that we understand the operation we're trying to do,
// by checking both sides of the comparison and assignment.
let (min_max, flip_args) = match (
left_is_target,
right_is_target,
left_is_value,
right_is_value,
) {
(true, false, false, true) => match op {
CmpOp::Lt => (MinMax::Max, true),
CmpOp::LtE => (MinMax::Max, false),
CmpOp::Gt => (MinMax::Min, true),
CmpOp::GtE => (MinMax::Min, false),
_ => return,
},
(false, true, true, false) => match op {
CmpOp::Lt => (MinMax::Min, true),
CmpOp::LtE => (MinMax::Min, false),
CmpOp::Gt => (MinMax::Max, true),
CmpOp::GtE => (MinMax::Max, false),
_ => return,
},
_ => return,
};

let (arg1, arg2) = if flip_args {
(left.as_ref(), right)
Expand Down
190 changes: 190 additions & 0 deletions ...rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1730_if_stmt_min_max.py.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ if_stmt_min_max.py:132:1: PLR1730 [*] Replace `if` statement with `min` call
135 | | attr
136 | | ) = 3
| |_________^ PLR1730
137 |
138 | class Foo:
|
= help: Replace with `min` call

Expand All @@ -294,3 +296,191 @@ if_stmt_min_max.py:132:1: PLR1730 [*] Replace `if` statement with `min` call
135 134 | attr
136 |- ) = 3
135 |+ ) = min(value.attr, 3)
137 136 |
138 137 | class Foo:
139 138 | _min = 0

if_stmt_min_max.py:143:9: PLR1730 [*] Replace `if` statement with `self._min = min(value, self._min)`
|
142 | def foo(self, value) -> None:
143 | if value < self._min:
| _________^
144 | | self._min = value
| |_____________________________^ PLR1730
145 | if value > self._max:
146 | self._max = value
|
= help: Replace with `self._min = min(value, self._min)`

ℹ Safe fix
140 140 | _max = 0
141 141 |
142 142 | def foo(self, value) -> None:
143 |- if value < self._min:
144 |- self._min = value
143 |+ self._min = min(value, self._min)
145 144 | if value > self._max:
146 145 | self._max = value
147 146 |

if_stmt_min_max.py:145:9: PLR1730 [*] Replace `if` statement with `self._max = max(value, self._max)`
|
143 | if value < self._min:
144 | self._min = value
145 | if value > self._max:
| _________^
146 | | self._max = value
| |_____________________________^ PLR1730
147 |
148 | if self._min < value:
|
= help: Replace with `self._max = max(value, self._max)`

ℹ Safe fix
142 142 | def foo(self, value) -> None:
143 143 | if value < self._min:
144 144 | self._min = value
145 |- if value > self._max:
146 |- self._max = value
145 |+ self._max = max(value, self._max)
147 146 |
148 147 | if self._min < value:
149 148 | self._min = value

if_stmt_min_max.py:148:9: PLR1730 [*] Replace `if` statement with `self._min = max(self._min, value)`
|
146 | self._max = value
147 |
148 | if self._min < value:
| _________^
149 | | self._min = value
| |_____________________________^ PLR1730
150 | if self._max > value:
151 | self._max = value
|
= help: Replace with `self._min = max(self._min, value)`

ℹ Safe fix
145 145 | if value > self._max:
146 146 | self._max = value
147 147 |
148 |- if self._min < value:
149 |- self._min = value
148 |+ self._min = max(self._min, value)
150 149 | if self._max > value:
151 150 | self._max = value
152 151 |

if_stmt_min_max.py:150:9: PLR1730 [*] Replace `if` statement with `self._max = min(self._max, value)`
|
148 | if self._min < value:
149 | self._min = value
150 | if self._max > value:
| _________^
151 | | self._max = value
| |_____________________________^ PLR1730
152 |
153 | if value <= self._min:
|
= help: Replace with `self._max = min(self._max, value)`

ℹ Safe fix
147 147 |
148 148 | if self._min < value:
149 149 | self._min = value
150 |- if self._max > value:
151 |- self._max = value
150 |+ self._max = min(self._max, value)
152 151 |
153 152 | if value <= self._min:
154 153 | self._min = value

if_stmt_min_max.py:153:9: PLR1730 [*] Replace `if` statement with `self._min = min(self._min, value)`
|
151 | self._max = value
152 |
153 | if value <= self._min:
| _________^
154 | | self._min = value
| |_____________________________^ PLR1730
155 | if value >= self._max:
156 | self._max = value
|
= help: Replace with `self._min = min(self._min, value)`

ℹ Safe fix
150 150 | if self._max > value:
151 151 | self._max = value
152 152 |
153 |- if value <= self._min:
154 |- self._min = value
153 |+ self._min = min(self._min, value)
155 154 | if value >= self._max:
156 155 | self._max = value
157 156 |

if_stmt_min_max.py:155:9: PLR1730 [*] Replace `if` statement with `self._max = max(self._max, value)`
|
153 | if value <= self._min:
154 | self._min = value
155 | if value >= self._max:
| _________^
156 | | self._max = value
| |_____________________________^ PLR1730
157 |
158 | if self._min <= value:
|
= help: Replace with `self._max = max(self._max, value)`

ℹ Safe fix
152 152 |
153 153 | if value <= self._min:
154 154 | self._min = value
155 |- if value >= self._max:
156 |- self._max = value
155 |+ self._max = max(self._max, value)
157 156 |
158 157 | if self._min <= value:
159 158 | self._min = value

if_stmt_min_max.py:158:9: PLR1730 [*] Replace `if` statement with `self._min = max(value, self._min)`
|
156 | self._max = value
157 |
158 | if self._min <= value:
| _________^
159 | | self._min = value
| |_____________________________^ PLR1730
160 | if self._max >= value:
161 | self._max = value
|
= help: Replace with `self._min = max(value, self._min)`

ℹ Safe fix
155 155 | if value >= self._max:
156 156 | self._max = value
157 157 |
158 |- if self._min <= value:
159 |- self._min = value
158 |+ self._min = max(value, self._min)
160 159 | if self._max >= value:
161 160 | self._max = value

if_stmt_min_max.py:160:9: PLR1730 [*] Replace `if` statement with `self._max = min(value, self._max)`
|
158 | if self._min <= value:
159 | self._min = value
160 | if self._max >= value:
| _________^
161 | | self._max = value
| |_____________________________^ PLR1730
|
= help: Replace with `self._max = min(value, self._max)`

ℹ Safe fix
157 157 |
158 158 | if self._min <= value:
159 159 | self._min = value
160 |- if self._max >= value:
161 |- self._max = value
160 |+ self._max = min(value, self._max)
Loading