From c19cd586700ed82ba802efefbede334814141bfb Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Thu, 13 Mar 2025 08:45:50 +0100 Subject: [PATCH] [`flake8-simplify`] Avoid double negation in fixes (`SIM103`) (#16684) ## Summary This PR stabilizes the fixes improvements made in https://github.com/astral-sh/ruff/pull/15562 (released with ruff 0.9.3 in mid January). There's no open issue or PR related to the changed fix behavior. This is not a breaking change. The fix was only gated behind preview to get some more test coverage before releasing. --- .../src/rules/flake8_simplify/mod.rs | 1 - .../flake8_simplify/rules/needless_bool.rs | 25 +- ...ke8_simplify__tests__SIM103_SIM103.py.snap | 36 +- ...ify__tests__preview__SIM103_SIM103.py.snap | 382 ------------------ 4 files changed, 28 insertions(+), 416 deletions(-) delete mode 100644 crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap diff --git a/crates/ruff_linter/src/rules/flake8_simplify/mod.rs b/crates/ruff_linter/src/rules/flake8_simplify/mod.rs index 50274bc9aa..6fb706df17 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::NeedlessBool, Path::new("SIM103.py"))] #[test_case(Rule::IfElseBlockInsteadOfIfExp, Path::new("SIM108.py"))] fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!( diff --git a/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs b/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs index 091a4bcc09..9b73053450 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs @@ -14,7 +14,7 @@ use crate::fix::snippet::SourceCodeSnippet; /// /// ## Why is this bad? /// `if` statements that return `True` for a truthy condition and `False` for -/// a falsey condition can be replaced with boolean casts. +/// a falsy condition can be replaced with boolean casts. /// /// ## Example /// Given: @@ -42,10 +42,6 @@ use crate::fix::snippet::SourceCodeSnippet; /// return x > 0 /// ``` /// -/// ## Preview -/// In preview, double negations such as `not a != b`, `not a not in b`, `not a is not b` -/// will be simplified to `a == b`, `a in b` and `a is b`, respectively. -/// /// ## References /// - [Python documentation: Truth Value Testing](https://docs.python.org/3/library/stdtypes.html#truth-value-testing) #[derive(ViolationMetadata)] @@ -222,16 +218,15 @@ pub(crate) fn needless_bool(checker: &Checker, stmt: &Stmt) { left, comparators, .. - }) if checker.settings.preview.is_enabled() - && matches!( - ops.as_ref(), - [ast::CmpOp::Eq - | ast::CmpOp::NotEq - | ast::CmpOp::In - | ast::CmpOp::NotIn - | ast::CmpOp::Is - | ast::CmpOp::IsNot] - ) => + }) if matches!( + ops.as_ref(), + [ast::CmpOp::Eq + | ast::CmpOp::NotEq + | ast::CmpOp::In + | ast::CmpOp::NotIn + | ast::CmpOp::Is + | ast::CmpOp::IsNot] + ) => { let ([op], [right]) = (ops.as_ref(), comparators.as_ref()) else { unreachable!("Single comparison with multiple comparators"); diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap index 471be94489..58286e133c 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap @@ -252,7 +252,7 @@ SIM103.py:123:5: SIM103 [*] Return the condition `not 10 < a` directly 127 125 | 128 126 | def f(): -SIM103.py:129:5: SIM103 [*] Return the condition `not 10 in a` directly +SIM103.py:129:5: SIM103 [*] Return the condition `10 not in a` directly | 128 | def f(): 129 | / if 10 in a: @@ -260,7 +260,7 @@ SIM103.py:129:5: SIM103 [*] Return the condition `not 10 in a` directly 131 | | return True | |_______________^ SIM103 | - = help: Replace with `return not 10 in a` + = help: Replace with `return 10 not in a` ℹ Unsafe fix 126 126 | @@ -269,12 +269,12 @@ SIM103.py:129:5: SIM103 [*] Return the condition `not 10 in a` directly 129 |- if 10 in a: 130 |- return False 131 |- return True - 129 |+ return not 10 in a + 129 |+ return 10 not in a 132 130 | 133 131 | 134 132 | def f(): -SIM103.py:135:5: SIM103 [*] Return the condition `not 10 not in a` directly +SIM103.py:135:5: SIM103 [*] Return the condition `10 in a` directly | 134 | def f(): 135 | / if 10 not in a: @@ -282,7 +282,7 @@ SIM103.py:135:5: SIM103 [*] Return the condition `not 10 not in a` directly 137 | | return True | |_______________^ SIM103 | - = help: Replace with `return not 10 not in a` + = help: Replace with `return 10 in a` ℹ Unsafe fix 132 132 | @@ -291,12 +291,12 @@ SIM103.py:135:5: SIM103 [*] Return the condition `not 10 not in a` directly 135 |- if 10 not in a: 136 |- return False 137 |- return True - 135 |+ return not 10 not in a + 135 |+ return 10 in a 138 136 | 139 137 | 140 138 | def f(): -SIM103.py:141:5: SIM103 [*] Return the condition `not a is 10` directly +SIM103.py:141:5: SIM103 [*] Return the condition `a is not 10` directly | 140 | def f(): 141 | / if a is 10: @@ -304,7 +304,7 @@ SIM103.py:141:5: SIM103 [*] Return the condition `not a is 10` directly 143 | | return True | |_______________^ SIM103 | - = help: Replace with `return not a is 10` + = help: Replace with `return a is not 10` ℹ Unsafe fix 138 138 | @@ -313,12 +313,12 @@ SIM103.py:141:5: SIM103 [*] Return the condition `not a is 10` directly 141 |- if a is 10: 142 |- return False 143 |- return True - 141 |+ return not a is 10 + 141 |+ return a is not 10 144 142 | 145 143 | 146 144 | def f(): -SIM103.py:147:5: SIM103 [*] Return the condition `not a is not 10` directly +SIM103.py:147:5: SIM103 [*] Return the condition `a is 10` directly | 146 | def f(): 147 | / if a is not 10: @@ -326,7 +326,7 @@ SIM103.py:147:5: SIM103 [*] Return the condition `not a is not 10` directly 149 | | return True | |_______________^ SIM103 | - = help: Replace with `return not a is not 10` + = help: Replace with `return a is 10` ℹ Unsafe fix 144 144 | @@ -335,12 +335,12 @@ SIM103.py:147:5: SIM103 [*] Return the condition `not a is not 10` directly 147 |- if a is not 10: 148 |- return False 149 |- return True - 147 |+ return not a is not 10 + 147 |+ return a is 10 150 148 | 151 149 | 152 150 | def f(): -SIM103.py:153:5: SIM103 [*] Return the condition `not a == 10` directly +SIM103.py:153:5: SIM103 [*] Return the condition `a != 10` directly | 152 | def f(): 153 | / if a == 10: @@ -348,7 +348,7 @@ SIM103.py:153:5: SIM103 [*] Return the condition `not a == 10` directly 155 | | return True | |_______________^ SIM103 | - = help: Replace with `return not a == 10` + = help: Replace with `return a != 10` ℹ Unsafe fix 150 150 | @@ -357,12 +357,12 @@ SIM103.py:153:5: SIM103 [*] Return the condition `not a == 10` directly 153 |- if a == 10: 154 |- return False 155 |- return True - 153 |+ return not a == 10 + 153 |+ return a != 10 156 154 | 157 155 | 158 156 | def f(): -SIM103.py:159:5: SIM103 [*] Return the condition `not a != 10` directly +SIM103.py:159:5: SIM103 [*] Return the condition `a == 10` directly | 158 | def f(): 159 | / if a != 10: @@ -370,7 +370,7 @@ SIM103.py:159:5: SIM103 [*] Return the condition `not a != 10` directly 161 | | return True | |_______________^ SIM103 | - = help: Replace with `return not a != 10` + = help: Replace with `return a == 10` ℹ Unsafe fix 156 156 | @@ -379,4 +379,4 @@ SIM103.py:159:5: SIM103 [*] Return the condition `not a != 10` directly 159 |- if a != 10: 160 |- return False 161 |- return True - 159 |+ return not a != 10 + 159 |+ return a == 10 diff --git a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap deleted file mode 100644 index 58286e133c..0000000000 --- a/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap +++ /dev/null @@ -1,382 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs ---- -SIM103.py:3:5: SIM103 [*] Return the condition `bool(a)` directly - | -1 | def f(): -2 | # SIM103 -3 | / if a: -4 | | return True -5 | | else: -6 | | return False - | |____________________^ SIM103 - | - = help: Replace with `return bool(a)` - -ℹ Unsafe fix -1 1 | def f(): -2 2 | # SIM103 -3 |- if a: -4 |- return True -5 |- else: -6 |- return False - 3 |+ return bool(a) -7 4 | -8 5 | -9 6 | def f(): - -SIM103.py:11:5: SIM103 [*] Return the condition `a == b` directly - | - 9 | def f(): -10 | # SIM103 -11 | / if a == b: -12 | | return True -13 | | else: -14 | | return False - | |____________________^ SIM103 - | - = help: Replace with `return a == b` - -ℹ Unsafe fix -8 8 | -9 9 | def f(): -10 10 | # SIM103 -11 |- if a == b: -12 |- return True -13 |- else: -14 |- return False - 11 |+ return a == b -15 12 | -16 13 | -17 14 | def f(): - -SIM103.py:21:5: SIM103 [*] Return the condition `bool(b)` directly - | -19 | if a: -20 | return 1 -21 | / elif b: -22 | | return True -23 | | else: -24 | | return False - | |____________________^ SIM103 - | - = help: Replace with `return bool(b)` - -ℹ Unsafe fix -18 18 | # SIM103 -19 19 | if a: -20 20 | return 1 -21 |- elif b: -22 |- return True -23 |- else: -24 |- return False - 21 |+ return bool(b) -25 22 | -26 23 | -27 24 | def f(): - -SIM103.py:32:9: SIM103 [*] Return the condition `bool(b)` directly - | -30 | return 1 -31 | else: -32 | / if b: -33 | | return True -34 | | else: -35 | | return False - | |________________________^ SIM103 - | - = help: Replace with `return bool(b)` - -ℹ Unsafe fix -29 29 | if a: -30 30 | return 1 -31 31 | else: -32 |- if b: -33 |- return True -34 |- else: -35 |- return False - 32 |+ return bool(b) -36 33 | -37 34 | -38 35 | def f(): - -SIM103.py:57:5: SIM103 [*] Return the condition `not a` directly - | -55 | def f(): -56 | # SIM103 -57 | / if a: -58 | | return False -59 | | else: -60 | | return True - | |___________________^ SIM103 - | - = help: Replace with `return not a` - -ℹ Unsafe fix -54 54 | -55 55 | def f(): -56 56 | # SIM103 -57 |- if a: -58 |- return False -59 |- else: -60 |- return True - 57 |+ return not a -61 58 | -62 59 | -63 60 | def f(): - -SIM103.py:83:5: SIM103 Return the condition directly - | -81 | def bool(): -82 | return False -83 | / if a: -84 | | return True -85 | | else: -86 | | return False - | |____________________^ SIM103 - | - = help: Inline condition - -SIM103.py:91:5: SIM103 [*] Return the condition `not (keys is not None and notice.key not in keys)` directly - | -89 | def f(): -90 | # SIM103 -91 | / if keys is not None and notice.key not in keys: -92 | | return False -93 | | else: -94 | | return True - | |___________________^ SIM103 - | - = help: Replace with `return not (keys is not None and notice.key not in keys)` - -ℹ Unsafe fix -88 88 | -89 89 | def f(): -90 90 | # SIM103 -91 |- if keys is not None and notice.key not in keys: -92 |- return False -93 |- else: -94 |- return True - 91 |+ return not (keys is not None and notice.key not in keys) -95 92 | -96 93 | -97 94 | ### - -SIM103.py:104:5: SIM103 [*] Return the condition `bool(a)` directly - | -102 | def f(): -103 | # SIM103 -104 | / if a: -105 | | return True -106 | | return False - | |________________^ SIM103 - | - = help: Replace with `return bool(a)` - -ℹ Unsafe fix -101 101 | -102 102 | def f(): -103 103 | # SIM103 -104 |- if a: -105 |- return True -106 |- return False - 104 |+ return bool(a) -107 105 | -108 106 | -109 107 | def f(): - -SIM103.py:111:5: SIM103 [*] Return the condition `not a` directly - | -109 | def f(): -110 | # SIM103 -111 | / if a: -112 | | return False -113 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return not a` - -ℹ Unsafe fix -108 108 | -109 109 | def f(): -110 110 | # SIM103 -111 |- if a: -112 |- return False -113 |- return True - 111 |+ return not a -114 112 | -115 113 | -116 114 | def f(): - -SIM103.py:117:5: SIM103 [*] Return the condition `10 < a` directly - | -116 | def f(): -117 | / if not 10 < a: -118 | | return False -119 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return 10 < a` - -ℹ Unsafe fix -114 114 | -115 115 | -116 116 | def f(): -117 |- if not 10 < a: -118 |- return False -119 |- return True - 117 |+ return 10 < a -120 118 | -121 119 | -122 120 | def f(): - -SIM103.py:123:5: SIM103 [*] Return the condition `not 10 < a` directly - | -122 | def f(): -123 | / if 10 < a: -124 | | return False -125 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return not 10 < a` - -ℹ Unsafe fix -120 120 | -121 121 | -122 122 | def f(): -123 |- if 10 < a: -124 |- return False -125 |- return True - 123 |+ return not 10 < a -126 124 | -127 125 | -128 126 | def f(): - -SIM103.py:129:5: SIM103 [*] Return the condition `10 not in a` directly - | -128 | def f(): -129 | / if 10 in a: -130 | | return False -131 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return 10 not in a` - -ℹ Unsafe fix -126 126 | -127 127 | -128 128 | def f(): -129 |- if 10 in a: -130 |- return False -131 |- return True - 129 |+ return 10 not in a -132 130 | -133 131 | -134 132 | def f(): - -SIM103.py:135:5: SIM103 [*] Return the condition `10 in a` directly - | -134 | def f(): -135 | / if 10 not in a: -136 | | return False -137 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return 10 in a` - -ℹ Unsafe fix -132 132 | -133 133 | -134 134 | def f(): -135 |- if 10 not in a: -136 |- return False -137 |- return True - 135 |+ return 10 in a -138 136 | -139 137 | -140 138 | def f(): - -SIM103.py:141:5: SIM103 [*] Return the condition `a is not 10` directly - | -140 | def f(): -141 | / if a is 10: -142 | | return False -143 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return a is not 10` - -ℹ Unsafe fix -138 138 | -139 139 | -140 140 | def f(): -141 |- if a is 10: -142 |- return False -143 |- return True - 141 |+ return a is not 10 -144 142 | -145 143 | -146 144 | def f(): - -SIM103.py:147:5: SIM103 [*] Return the condition `a is 10` directly - | -146 | def f(): -147 | / if a is not 10: -148 | | return False -149 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return a is 10` - -ℹ Unsafe fix -144 144 | -145 145 | -146 146 | def f(): -147 |- if a is not 10: -148 |- return False -149 |- return True - 147 |+ return a is 10 -150 148 | -151 149 | -152 150 | def f(): - -SIM103.py:153:5: SIM103 [*] Return the condition `a != 10` directly - | -152 | def f(): -153 | / if a == 10: -154 | | return False -155 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return a != 10` - -ℹ Unsafe fix -150 150 | -151 151 | -152 152 | def f(): -153 |- if a == 10: -154 |- return False -155 |- return True - 153 |+ return a != 10 -156 154 | -157 155 | -158 156 | def f(): - -SIM103.py:159:5: SIM103 [*] Return the condition `a == 10` directly - | -158 | def f(): -159 | / if a != 10: -160 | | return False -161 | | return True - | |_______________^ SIM103 - | - = help: Replace with `return a == 10` - -ℹ Unsafe fix -156 156 | -157 157 | -158 158 | def f(): -159 |- if a != 10: -160 |- return False -161 |- return True - 159 |+ return a == 10