From 5cf2c40d1363a5ee556cd5982033a61c387dd47c Mon Sep 17 00:00:00 2001 From: Dylan Date: Fri, 6 Jun 2025 16:46:43 -0500 Subject: [PATCH] [`flake8-simplify`] Stabilize further simplification to binary expressions in autofix for `if-else-block-instead-of-if-exp` (`SIM108`) (#18506) --- crates/ruff_linter/src/preview.rs | 5 - .../src/rules/flake8_simplify/mod.rs | 1 - .../rules/if_else_block_instead_of_if_exp.rs | 29 +- ...ke8_simplify__tests__SIM108_SIM108.py.snap | 18 +- ...ify__tests__preview__SIM108_SIM108.py.snap | 382 ------------------ 5 files changed, 22 insertions(+), 413 deletions(-) delete mode 100644 crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index e88d683be6..dcf496e1b6 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -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() diff --git a/crates/ruff_linter/src/rules/flake8_simplify/mod.rs b/crates/ruff_linter/src/rules/flake8_simplify/mod.rs index 1041df4615..2e0bd735dc 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/mod.rs @@ -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!( diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_if_exp.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_if_exp.rs index 44c89d37c3..1166a272e3 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_if_exp.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/if_else_block_instead_of_if_exp.rs @@ -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) diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap index a18c5fdac5..b4d70317ad 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM108_SIM108.py.snap @@ -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, diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap deleted file mode 100644 index b4d70317ad..0000000000 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM108_SIM108.py.snap +++ /dev/null @@ -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}'