[flake8-simplify] Stabilize further simplification to binary expressions in autofix for if-else-block-instead-of-if-exp (SIM108) (#18506)

This commit is contained in:
Dylan 2025-06-06 16:46:43 -05:00 committed by Brent Westbrook
parent 02b5376a3c
commit 5cf2c40d13
5 changed files with 22 additions and 413 deletions

View file

@ -73,11 +73,6 @@ pub(crate) const fn is_only_add_return_none_at_end_enabled(settings: &LinterSett
settings.preview.is_enabled()
}
// https://github.com/astral-sh/ruff/pull/12796
pub(crate) const fn is_simplify_ternary_to_binary_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
}
// https://github.com/astral-sh/ruff/pull/16719
pub(crate) const fn is_fix_manual_dict_comprehension_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()

View file

@ -58,7 +58,6 @@ mod tests {
Ok(())
}
#[test_case(Rule::IfElseBlockInsteadOfIfExp, Path::new("SIM108.py"))]
#[test_case(Rule::MultipleWithStatements, Path::new("SIM117.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(

View file

@ -7,13 +7,14 @@ use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
use crate::fix::edits::fits;
use crate::preview::is_simplify_ternary_to_binary_enabled;
use crate::{Edit, Fix, FixAvailability, Violation};
/// ## What it does
/// Check for `if`-`else`-blocks that can be replaced with a ternary operator.
/// Moreover, in [preview], check if these ternary expressions can be
/// further simplified to binary expressions.
/// Check for `if`-`else`-blocks that can be replaced with a ternary
/// or binary operator.
///
/// The lint is suppressed if the suggested replacement would exceed
/// the maximum line length configured in [pycodestyle.max-line-length].
///
/// ## Why is this bad?
/// `if`-`else`-blocks that assign a value to a variable in both branches can
@ -33,7 +34,7 @@ use crate::{Edit, Fix, FixAvailability, Violation};
/// bar = x if foo else y
/// ```
///
/// Or, in [preview]:
/// Or:
///
/// ```python
/// if cond:
@ -57,8 +58,8 @@ use crate::{Edit, Fix, FixAvailability, Violation};
/// ## References
/// - [Python documentation: Conditional expressions](https://docs.python.org/3/reference/expressions.html#conditional-expressions)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
/// [code coverage]: https://github.com/nedbat/coveragepy/issues/509
/// [pycodestyle.max-line-length]: https://docs.astral.sh/ruff/settings/#lint_pycodestyle_max-line-length
#[derive(ViolationMetadata)]
pub(crate) struct IfElseBlockInsteadOfIfExp {
/// The ternary or binary expression to replace the `if`-`else`-block.
@ -184,16 +185,12 @@ pub(crate) fn if_else_block_instead_of_if_exp(checker: &Checker, stmt_if: &ast::
//
// The match statement below implements the following
// logic:
// - If `test == body_value` and preview enabled, replace with `target_var = test or else_value`
// - If `test == not body_value` and preview enabled, replace with `target_var = body_value and else_value`
// - If `not test == body_value` and preview enabled, replace with `target_var = body_value and else_value`
// - If `test == body_value`, replace with `target_var = test or else_value`
// - If `test == not body_value`, replace with `target_var = body_value and else_value`
// - If `not test == body_value`, replace with `target_var = body_value and else_value`
// - Otherwise, replace with `target_var = body_value if test else else_value`
let (contents, assignment_kind) = match (
is_simplify_ternary_to_binary_enabled(checker.settings),
test,
body_value,
) {
(true, test_node, body_node)
let (contents, assignment_kind) = match (test, body_value) {
(test_node, body_node)
if ComparableExpr::from(test_node) == ComparableExpr::from(body_node)
&& !contains_effect(test_node, |id| checker.semantic().has_builtin_binding(id)) =>
{
@ -201,7 +198,7 @@ pub(crate) fn if_else_block_instead_of_if_exp(checker: &Checker, stmt_if: &ast::
let binary = assignment_binary_or(target_var, body_value, else_value);
(checker.generator().stmt(&binary), AssignmentKind::Binary)
}
(true, test_node, body_node)
(test_node, body_node)
if (test_node.as_unary_op_expr().is_some_and(|op_expr| {
op_expr.op.is_not()
&& ComparableExpr::from(&op_expr.operand) == ComparableExpr::from(body_node)

View file

@ -118,7 +118,7 @@ SIM108.py:117:1: SIM108 Use ternary operator `x = 3 if True else 5` instead of `
|
= help: Replace `if`-`else`-block with `x = 3 if True else 5`
SIM108.py:141:1: SIM108 [*] Use ternary operator `z = cond if cond else other_cond` instead of `if`-`else`-block
SIM108.py:141:1: SIM108 [*] Use binary operator `z = cond or other_cond` instead of `if`-`else`-block
|
139 | # SIM108 - should suggest
140 | # z = cond or other_cond
@ -130,7 +130,7 @@ SIM108.py:141:1: SIM108 [*] Use ternary operator `z = cond if cond else other_co
145 |
146 | # SIM108 - should suggest
|
= help: Replace `if`-`else`-block with `z = cond if cond else other_cond`
= help: Replace `if`-`else`-block with `z = cond or other_cond`
Unsafe fix
138 138 |
@ -140,12 +140,12 @@ SIM108.py:141:1: SIM108 [*] Use ternary operator `z = cond if cond else other_co
142 |- z = cond
143 |-else:
144 |- z = other_cond
141 |+z = cond if cond else other_cond
141 |+z = cond or other_cond
145 142 |
146 143 | # SIM108 - should suggest
147 144 | # z = cond and other_cond
SIM108.py:148:1: SIM108 [*] Use ternary operator `z = cond if not cond else other_cond` instead of `if`-`else`-block
SIM108.py:148:1: SIM108 [*] Use binary operator `z = cond and other_cond` instead of `if`-`else`-block
|
146 | # SIM108 - should suggest
147 | # z = cond and other_cond
@ -157,7 +157,7 @@ SIM108.py:148:1: SIM108 [*] Use ternary operator `z = cond if not cond else othe
152 |
153 | # SIM108 - should suggest
|
= help: Replace `if`-`else`-block with `z = cond if not cond else other_cond`
= help: Replace `if`-`else`-block with `z = cond and other_cond`
Unsafe fix
145 145 |
@ -167,12 +167,12 @@ SIM108.py:148:1: SIM108 [*] Use ternary operator `z = cond if not cond else othe
149 |- z = cond
150 |-else:
151 |- z = other_cond
148 |+z = cond if not cond else other_cond
148 |+z = cond and other_cond
152 149 |
153 150 | # SIM108 - should suggest
154 151 | # z = not cond and other_cond
SIM108.py:155:1: SIM108 [*] Use ternary operator `z = not cond if cond else other_cond` instead of `if`-`else`-block
SIM108.py:155:1: SIM108 [*] Use binary operator `z = not cond and other_cond` instead of `if`-`else`-block
|
153 | # SIM108 - should suggest
154 | # z = not cond and other_cond
@ -184,7 +184,7 @@ SIM108.py:155:1: SIM108 [*] Use ternary operator `z = not cond if cond else othe
159 |
160 | # SIM108 does not suggest
|
= help: Replace `if`-`else`-block with `z = not cond if cond else other_cond`
= help: Replace `if`-`else`-block with `z = not cond and other_cond`
Unsafe fix
152 152 |
@ -194,7 +194,7 @@ SIM108.py:155:1: SIM108 [*] Use ternary operator `z = not cond if cond else othe
156 |- z = not cond
157 |-else:
158 |- z = other_cond
155 |+z = not cond if cond else other_cond
155 |+z = not cond and other_cond
159 156 |
160 157 | # SIM108 does not suggest
161 158 | # a binary option in these cases,

View file

@ -1,382 +0,0 @@
---
source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
---
SIM108.py:2:1: SIM108 [*] Use ternary operator `b = c if a else d` instead of `if`-`else`-block
|
1 | # SIM108
2 | / if a:
3 | | b = c
4 | | else:
5 | | b = d
| |_________^ SIM108
6 |
7 | # OK
|
= help: Replace `if`-`else`-block with `b = c if a else d`
Unsafe fix
1 1 | # SIM108
2 |-if a:
3 |- b = c
4 |-else:
5 |- b = d
2 |+b = c if a else d
6 3 |
7 4 | # OK
8 5 | b = c if a else d
SIM108.py:30:5: SIM108 [*] Use ternary operator `b = 1 if a else 2` instead of `if`-`else`-block
|
28 | pass
29 | else:
30 | / if a:
31 | | b = 1
32 | | else:
33 | | b = 2
| |_____________^ SIM108
|
= help: Replace `if`-`else`-block with `b = 1 if a else 2`
Unsafe fix
27 27 | if True:
28 28 | pass
29 29 | else:
30 |- if a:
31 |- b = 1
32 |- else:
33 |- b = 2
30 |+ b = 1 if a else 2
34 31 |
35 32 |
36 33 | import sys
SIM108.py:58:1: SIM108 Use ternary operator `abc = x if x > 0 else -x` instead of `if`-`else`-block
|
57 | # SIM108 (without fix due to comments)
58 | / if x > 0:
59 | | # test test
60 | | abc = x
61 | | else:
62 | | # test test test
63 | | abc = -x
| |____________^ SIM108
|
= help: Replace `if`-`else`-block with `abc = x if x > 0 else -x`
SIM108.py:82:1: SIM108 [*] Use ternary operator `b = "cccccccccccccccccccccccccccccccccß" if a else "ddddddddddddddddddddddddddddddddd💣"` instead of `if`-`else`-block
|
81 | # SIM108
82 | / if a:
83 | | b = "cccccccccccccccccccccccccccccccccß"
84 | | else:
85 | | b = "ddddddddddddddddddddddddddddddddd💣"
| |_____________________________________________^ SIM108
|
= help: Replace `if`-`else`-block with `b = "cccccccccccccccccccccccccccccccccß" if a else "ddddddddddddddddddddddddddddddddd💣"`
Unsafe fix
79 79 |
80 80 |
81 81 | # SIM108
82 |-if a:
83 |- b = "cccccccccccccccccccccccccccccccccß"
84 |-else:
85 |- b = "ddddddddddddddddddddddddddddddddd💣"
82 |+b = "cccccccccccccccccccccccccccccccccß" if a else "ddddddddddddddddddddddddddddddddd💣"
86 83 |
87 84 |
88 85 | # OK (too long)
SIM108.py:105:1: SIM108 Use ternary operator `exitcode = 0 if True else 1` instead of `if`-`else`-block
|
104 | # SIM108 (without fix due to trailing comment)
105 | / if True:
106 | | exitcode = 0
107 | | else:
108 | | exitcode = 1 # Trailing comment
| |________________^ SIM108
|
= help: Replace `if`-`else`-block with `exitcode = 0 if True else 1`
SIM108.py:112:1: SIM108 Use ternary operator `x = 3 if True else 5` instead of `if`-`else`-block
|
111 | # SIM108
112 | / if True: x = 3 # Foo
113 | | else: x = 5
| |___________^ SIM108
|
= help: Replace `if`-`else`-block with `x = 3 if True else 5`
SIM108.py:117:1: SIM108 Use ternary operator `x = 3 if True else 5` instead of `if`-`else`-block
|
116 | # SIM108
117 | / if True: # Foo
118 | | x = 3
119 | | else:
120 | | x = 5
| |_________^ SIM108
|
= help: Replace `if`-`else`-block with `x = 3 if True else 5`
SIM108.py:141:1: SIM108 [*] Use binary operator `z = cond or other_cond` instead of `if`-`else`-block
|
139 | # SIM108 - should suggest
140 | # z = cond or other_cond
141 | / if cond:
142 | | z = cond
143 | | else:
144 | | z = other_cond
| |__________________^ SIM108
145 |
146 | # SIM108 - should suggest
|
= help: Replace `if`-`else`-block with `z = cond or other_cond`
Unsafe fix
138 138 |
139 139 | # SIM108 - should suggest
140 140 | # z = cond or other_cond
141 |-if cond:
142 |- z = cond
143 |-else:
144 |- z = other_cond
141 |+z = cond or other_cond
145 142 |
146 143 | # SIM108 - should suggest
147 144 | # z = cond and other_cond
SIM108.py:148:1: SIM108 [*] Use binary operator `z = cond and other_cond` instead of `if`-`else`-block
|
146 | # SIM108 - should suggest
147 | # z = cond and other_cond
148 | / if not cond:
149 | | z = cond
150 | | else:
151 | | z = other_cond
| |__________________^ SIM108
152 |
153 | # SIM108 - should suggest
|
= help: Replace `if`-`else`-block with `z = cond and other_cond`
Unsafe fix
145 145 |
146 146 | # SIM108 - should suggest
147 147 | # z = cond and other_cond
148 |-if not cond:
149 |- z = cond
150 |-else:
151 |- z = other_cond
148 |+z = cond and other_cond
152 149 |
153 150 | # SIM108 - should suggest
154 151 | # z = not cond and other_cond
SIM108.py:155:1: SIM108 [*] Use binary operator `z = not cond and other_cond` instead of `if`-`else`-block
|
153 | # SIM108 - should suggest
154 | # z = not cond and other_cond
155 | / if cond:
156 | | z = not cond
157 | | else:
158 | | z = other_cond
| |__________________^ SIM108
159 |
160 | # SIM108 does not suggest
|
= help: Replace `if`-`else`-block with `z = not cond and other_cond`
Unsafe fix
152 152 |
153 153 | # SIM108 - should suggest
154 154 | # z = not cond and other_cond
155 |-if cond:
156 |- z = not cond
157 |-else:
158 |- z = other_cond
155 |+z = not cond and other_cond
159 156 |
160 157 | # SIM108 does not suggest
161 158 | # a binary option in these cases,
SIM108.py:167:1: SIM108 [*] Use ternary operator `z = 1 if True else other` instead of `if`-`else`-block
|
165 | # (Of course, these specific expressions
166 | # should be simplified for other reasons...)
167 | / if True:
168 | | z = 1
169 | | else:
170 | | z = other
| |_____________^ SIM108
171 |
172 | if False:
|
= help: Replace `if`-`else`-block with `z = 1 if True else other`
Unsafe fix
164 164 | # so, e.g. `True == 1`.
165 165 | # (Of course, these specific expressions
166 166 | # should be simplified for other reasons...)
167 |-if True:
168 |- z = 1
169 |-else:
170 |- z = other
167 |+z = 1 if True else other
171 168 |
172 169 | if False:
173 170 | z = 1
SIM108.py:172:1: SIM108 [*] Use ternary operator `z = 1 if False else other` instead of `if`-`else`-block
|
170 | z = other
171 |
172 | / if False:
173 | | z = 1
174 | | else:
175 | | z = other
| |_____________^ SIM108
176 |
177 | if 1:
|
= help: Replace `if`-`else`-block with `z = 1 if False else other`
Unsafe fix
169 169 | else:
170 170 | z = other
171 171 |
172 |-if False:
173 |- z = 1
174 |-else:
175 |- z = other
172 |+z = 1 if False else other
176 173 |
177 174 | if 1:
178 175 | z = True
SIM108.py:177:1: SIM108 [*] Use ternary operator `z = True if 1 else other` instead of `if`-`else`-block
|
175 | z = other
176 |
177 | / if 1:
178 | | z = True
179 | | else:
180 | | z = other
| |_____________^ SIM108
181 |
182 | # SIM108 does not suggest a binary option in this
|
= help: Replace `if`-`else`-block with `z = True if 1 else other`
Unsafe fix
174 174 | else:
175 175 | z = other
176 176 |
177 |-if 1:
178 |- z = True
179 |-else:
180 |- z = other
177 |+z = True if 1 else other
181 178 |
182 179 | # SIM108 does not suggest a binary option in this
183 180 | # case, since we'd be reducing the number of calls
SIM108.py:185:1: SIM108 [*] Use ternary operator `z = foo() if foo() else other` instead of `if`-`else`-block
|
183 | # case, since we'd be reducing the number of calls
184 | # from Two to one.
185 | / if foo():
186 | | z = foo()
187 | | else:
188 | | z = other
| |_____________^ SIM108
189 |
190 | # SIM108 does not suggest a binary option in this
|
= help: Replace `if`-`else`-block with `z = foo() if foo() else other`
Unsafe fix
182 182 | # SIM108 does not suggest a binary option in this
183 183 | # case, since we'd be reducing the number of calls
184 184 | # from Two to one.
185 |-if foo():
186 |- z = foo()
187 |-else:
188 |- z = other
185 |+z = foo() if foo() else other
189 186 |
190 187 | # SIM108 does not suggest a binary option in this
191 188 | # case, since we'd be reducing the number of calls
SIM108.py:193:1: SIM108 [*] Use ternary operator `z = not foo() if foo() else other` instead of `if`-`else`-block
|
191 | # case, since we'd be reducing the number of calls
192 | # from Two to one.
193 | / if foo():
194 | | z = not foo()
195 | | else:
196 | | z = other
| |_____________^ SIM108
|
= help: Replace `if`-`else`-block with `z = not foo() if foo() else other`
Unsafe fix
190 190 | # SIM108 does not suggest a binary option in this
191 191 | # case, since we'd be reducing the number of calls
192 192 | # from Two to one.
193 |-if foo():
194 |- z = not foo()
195 |-else:
196 |- z = other
193 |+z = not foo() if foo() else other
197 194 |
198 195 |
199 196 | # These two cases double as tests for f-string quote preservation. The first
SIM108.py:202:1: SIM108 [*] Use ternary operator `var = "str" if cond else f"{first}-{second}"` instead of `if`-`else`-block
|
200 | # f-string should preserve its double quotes, and the second should preserve
201 | # single quotes
202 | / if cond:
203 | | var = "str"
204 | | else:
205 | | var = f"{first}-{second}"
| |_____________________________^ SIM108
206 |
207 | if cond:
|
= help: Replace `if`-`else`-block with `var = "str" if cond else f"{first}-{second}"`
Unsafe fix
199 199 | # These two cases double as tests for f-string quote preservation. The first
200 200 | # f-string should preserve its double quotes, and the second should preserve
201 201 | # single quotes
202 |-if cond:
203 |- var = "str"
204 |-else:
205 |- var = f"{first}-{second}"
202 |+var = "str" if cond else f"{first}-{second}"
206 203 |
207 204 | if cond:
208 205 | var = "str"
SIM108.py:207:1: SIM108 [*] Use ternary operator `var = "str" if cond else f'{first}-{second}'` instead of `if`-`else`-block
|
205 | var = f"{first}-{second}"
206 |
207 | / if cond:
208 | | var = "str"
209 | | else:
210 | | var = f'{first}-{second}'
| |_____________________________^ SIM108
|
= help: Replace `if`-`else`-block with `var = "str" if cond else f'{first}-{second}'`
Unsafe fix
204 204 | else:
205 205 | var = f"{first}-{second}"
206 206 |
207 |-if cond:
208 |- var = "str"
209 |-else:
210 |- var = f'{first}-{second}'
207 |+var = "str" if cond else f'{first}-{second}'