From 001e5adec5d0334bec533903b0286dea9ddbf58d Mon Sep 17 00:00:00 2001 From: InSync Date: Sun, 19 Jan 2025 00:06:46 +0700 Subject: [PATCH] [`flake8-simplify`] Avoid double negations (`SIM103`) (#15562) ## Summary Related to [this comment](https://github.com/astral-sh/ruff/issues/6184#issuecomment-2578673788) at #6184. --------- Co-authored-by: Dylan <53534755+dylwil3@users.noreply.github.com> --- .../test/fixtures/flake8_simplify/SIM103.py | 36 ++ .../src/rules/flake8_simplify/mod.rs | 1 + .../flake8_simplify/rules/needless_bool.rs | 51 ++- ...ke8_simplify__tests__SIM103_SIM103.py.snap | 132 ++++++ ...ify__tests__preview__SIM103_SIM103.py.snap | 382 ++++++++++++++++++ 5 files changed, 592 insertions(+), 10 deletions(-) create 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/resources/test/fixtures/flake8_simplify/SIM103.py b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py index f42b9dc262..5654197d08 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py @@ -123,3 +123,39 @@ def f(): if 10 < a: return False return True + + +def f(): + if 10 in a: + return False + return True + + +def f(): + if 10 not in a: + return False + return True + + +def f(): + if a is 10: + return False + return True + + +def f(): + if a is not 10: + return False + return True + + +def f(): + if a == 10: + return False + return True + + +def f(): + if a != 10: + return False + return True diff --git a/crates/ruff_linter/src/rules/flake8_simplify/mod.rs b/crates/ruff_linter/src/rules/flake8_simplify/mod.rs index 6fb706df17..50274bc9aa 100644 --- a/crates/ruff_linter/src/rules/flake8_simplify/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_simplify/mod.rs @@ -58,6 +58,7 @@ 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 5ab3994de1..320fdb21a4 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 @@ -42,6 +42,10 @@ 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)] @@ -206,19 +210,46 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { } else { // If the return values are inverted, wrap the condition in a `not`. if inverted { - if let Expr::UnaryOp(ast::ExprUnaryOp { - op: ast::UnaryOp::Not, - operand, - .. - }) = if_test - { - Some((**operand).clone()) - } else { - Some(Expr::UnaryOp(ast::ExprUnaryOp { + match if_test { + Expr::UnaryOp(ast::ExprUnaryOp { + op: ast::UnaryOp::Not, + operand, + .. + }) => Some((**operand).clone()), + + Expr::Compare(ast::ExprCompare { + ops, + 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] + ) => + { + let ([op], [right]) = (ops.as_ref(), comparators.as_ref()) else { + unreachable!("Single comparison with multiple comparators"); + }; + + Some(Expr::Compare(ast::ExprCompare { + ops: Box::new([op.negate()]), + left: left.clone(), + comparators: Box::new([right.clone()]), + range: TextRange::default(), + })) + } + + _ => Some(Expr::UnaryOp(ast::ExprUnaryOp { op: ast::UnaryOp::Not, operand: Box::new(if_test.clone()), range: TextRange::default(), - })) + })), } } else if if_test.is_compare_expr() { // If the condition is a comparison, we can replace it with the condition, since we 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 bb3c4d87a6..471be94489 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 @@ -248,3 +248,135 @@ SIM103.py:123:5: SIM103 [*] Return the condition `not 10 < a` directly 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 `not 10 in a` directly + | +128 | def f(): +129 | / if 10 in a: +130 | | return False +131 | | return True + | |_______________^ SIM103 + | + = help: Replace with `return not 10 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 not 10 in a +132 130 | +133 131 | +134 132 | def f(): + +SIM103.py:135:5: SIM103 [*] Return the condition `not 10 not in a` directly + | +134 | def f(): +135 | / if 10 not in a: +136 | | return False +137 | | return True + | |_______________^ SIM103 + | + = help: Replace with `return not 10 not 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 not 10 not in a +138 136 | +139 137 | +140 138 | def f(): + +SIM103.py:141:5: SIM103 [*] Return the condition `not a is 10` directly + | +140 | def f(): +141 | / if a is 10: +142 | | return False +143 | | return True + | |_______________^ SIM103 + | + = help: Replace with `return not a is 10` + +ℹ Unsafe fix +138 138 | +139 139 | +140 140 | def f(): +141 |- if a is 10: +142 |- return False +143 |- return True + 141 |+ return not a is 10 +144 142 | +145 143 | +146 144 | def f(): + +SIM103.py:147:5: SIM103 [*] Return the condition `not a is not 10` directly + | +146 | def f(): +147 | / if a is not 10: +148 | | return False +149 | | return True + | |_______________^ SIM103 + | + = help: Replace with `return not a is not 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 not a is not 10 +150 148 | +151 149 | +152 150 | def f(): + +SIM103.py:153:5: SIM103 [*] Return the condition `not a == 10` directly + | +152 | def f(): +153 | / if a == 10: +154 | | return False +155 | | return True + | |_______________^ SIM103 + | + = help: Replace with `return not a == 10` + +ℹ Unsafe fix +150 150 | +151 151 | +152 152 | def f(): +153 |- if a == 10: +154 |- return False +155 |- return True + 153 |+ return not a == 10 +156 154 | +157 155 | +158 156 | def f(): + +SIM103.py:159:5: SIM103 [*] Return the condition `not a != 10` directly + | +158 | def f(): +159 | / if a != 10: +160 | | return False +161 | | return True + | |_______________^ SIM103 + | + = help: Replace with `return not a != 10` + +ℹ Unsafe fix +156 156 | +157 157 | +158 158 | def f(): +159 |- if a != 10: +160 |- return False +161 |- return True + 159 |+ return not 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 new file mode 100644 index 0000000000..58286e133c --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap @@ -0,0 +1,382 @@ +--- +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