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 85f00aec21..e1b868888d 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py @@ -52,32 +52,32 @@ def f(): return False +def f(): + # SIM103 + if a: + return False + else: + return True + + +def f(): + # OK + if a: + return False + else: + return False + + +def f(): + # OK + if a: + return True + else: + return True + + def f(): # SIM103 (but not fixable) - if a: - return False - else: - return True - - -def f(): - # OK - if a: - return False - else: - return False - - -def f(): - # OK - if a: - return True - else: - return True - - -def f(): - # OK def bool(): return False if a: @@ -86,6 +86,14 @@ def f(): return False +def f(): + # SIM103 + if keys is not None and notice.key not in keys: + return False + else: + return True + + ### # Positive cases (preview) ### 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 93f533de3f..0ab4856483 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 @@ -41,8 +41,8 @@ use crate::fix::snippet::SourceCodeSnippet; /// [preview]: https://docs.astral.sh/ruff/preview/ #[violation] pub struct NeedlessBool { - condition: SourceCodeSnippet, - replacement: Option, + condition: Option, + negate: bool, } impl Violation for NeedlessBool { @@ -50,21 +50,22 @@ impl Violation for NeedlessBool { #[derive_message_formats] fn message(&self) -> String { - let NeedlessBool { condition, .. } = self; - if let Some(condition) = condition.full_display() { + let NeedlessBool { condition, negate } = self; + + if let Some(condition) = condition.as_ref().and_then(SourceCodeSnippet::full_display) { format!("Return the condition `{condition}` directly") + } else if *negate { + format!("Return the negated condition directly") } else { format!("Return the condition directly") } } fn fix_title(&self) -> Option { - let NeedlessBool { replacement, .. } = self; - if let Some(replacement) = replacement - .as_ref() - .and_then(SourceCodeSnippet::full_display) - { - Some(format!("Replace with `{replacement}`")) + let NeedlessBool { condition, .. } = self; + + if let Some(condition) = condition.as_ref().and_then(SourceCodeSnippet::full_display) { + Some(format!("Replace with `return {condition}`")) } else { Some(format!("Inline condition")) } @@ -191,29 +192,21 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { return; } - let condition = checker.locator().slice(if_test); - let replacement = if checker.indexer().has_comments(&range, checker.locator()) { + // Generate the replacement condition. + let condition = if checker.indexer().has_comments(&range, checker.locator()) { None } else { // If the return values are inverted, wrap the condition in a `not`. if inverted { - let node = ast::StmtReturn { - value: Some(Box::new(Expr::UnaryOp(ast::ExprUnaryOp { - op: ast::UnaryOp::Not, - operand: Box::new(if_test.clone()), - range: TextRange::default(), - }))), + Some(Expr::UnaryOp(ast::ExprUnaryOp { + op: ast::UnaryOp::Not, + operand: Box::new(if_test.clone()), range: TextRange::default(), - }; - Some(checker.generator().stmt(&node.into())) + })) } else if if_test.is_compare_expr() { // If the condition is a comparison, we can replace it with the condition, since we // know it's a boolean. - let node = ast::StmtReturn { - value: Some(Box::new(if_test.clone())), - range: TextRange::default(), - }; - Some(checker.generator().stmt(&node.into())) + Some(if_test.clone()) } else if checker.semantic().is_builtin("bool") { // Otherwise, we need to wrap the condition in a call to `bool`. let func_node = ast::ExprName { @@ -221,7 +214,7 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { ctx: ExprContext::Load, range: TextRange::default(), }; - let value_node = ast::ExprCall { + let call_node = ast::ExprCall { func: Box::new(func_node.into()), arguments: Arguments { args: Box::from([if_test.clone()]), @@ -230,20 +223,32 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt: &Stmt) { }, range: TextRange::default(), }; - let return_node = ast::StmtReturn { - value: Some(Box::new(value_node.into())), - range: TextRange::default(), - }; - Some(checker.generator().stmt(&return_node.into())) + Some(Expr::Call(call_node)) } else { None } }; + // Generate the replacement `return` statement. + let replacement = condition.as_ref().map(|expr| { + Stmt::Return(ast::StmtReturn { + value: Some(Box::new(expr.clone())), + range: TextRange::default(), + }) + }); + + // Generate source code. + let replacement = replacement + .as_ref() + .map(|stmt| checker.generator().stmt(stmt)); + let condition = condition + .as_ref() + .map(|expr| checker.generator().expr(expr)); + let mut diagnostic = Diagnostic::new( NeedlessBool { - condition: SourceCodeSnippet::from_str(condition), - replacement: replacement.clone().map(SourceCodeSnippet::new), + condition: condition.map(SourceCodeSnippet::new), + negate: inverted, }, range, ); 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 2ed80d20b0..b656f42371 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 @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs --- -SIM103.py:3:5: SIM103 [*] Return the condition `a` directly +SIM103.py:3:5: SIM103 [*] Return the condition `bool(a)` directly | 1 | def f(): 2 | # SIM103 @@ -52,7 +52,7 @@ SIM103.py:11:5: SIM103 [*] Return the condition `a == b` directly 16 13 | 17 14 | def f(): -SIM103.py:21:5: SIM103 [*] Return the condition `b` directly +SIM103.py:21:5: SIM103 [*] Return the condition `bool(b)` directly | 19 | if a: 20 | return 1 @@ -78,7 +78,7 @@ SIM103.py:21:5: SIM103 [*] Return the condition `b` directly 26 23 | 27 24 | def f(): -SIM103.py:32:9: SIM103 [*] Return the condition `b` directly +SIM103.py:32:9: SIM103 [*] Return the condition `bool(b)` directly | 30 | return 1 31 | else: @@ -104,10 +104,10 @@ SIM103.py:32:9: SIM103 [*] Return the condition `b` directly 37 34 | 38 35 | def f(): -SIM103.py:57:5: SIM103 [*] Return the condition `a` directly +SIM103.py:57:5: SIM103 [*] Return the condition `not a` directly | 55 | def f(): -56 | # SIM103 (but not fixable) +56 | # SIM103 57 | if a: | _____^ 58 | | return False @@ -120,7 +120,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly ℹ Unsafe fix 54 54 | 55 55 | def f(): -56 56 | # SIM103 (but not fixable) +56 56 | # SIM103 57 |- if a: 58 |- return False 59 |- else: @@ -130,7 +130,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly 62 59 | 63 60 | def f(): -SIM103.py:83:5: SIM103 Return the condition `a` directly +SIM103.py:83:5: SIM103 Return the condition directly | 81 | def bool(): 82 | return False @@ -142,3 +142,29 @@ SIM103.py:83:5: SIM103 Return the condition `a` directly | |____________________^ 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 | ### 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 index 868129a6d1..a71b6622c2 100644 --- 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 @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs --- -SIM103.py:3:5: SIM103 [*] Return the condition `a` directly +SIM103.py:3:5: SIM103 [*] Return the condition `bool(a)` directly | 1 | def f(): 2 | # SIM103 @@ -52,7 +52,7 @@ SIM103.py:11:5: SIM103 [*] Return the condition `a == b` directly 16 13 | 17 14 | def f(): -SIM103.py:21:5: SIM103 [*] Return the condition `b` directly +SIM103.py:21:5: SIM103 [*] Return the condition `bool(b)` directly | 19 | if a: 20 | return 1 @@ -78,7 +78,7 @@ SIM103.py:21:5: SIM103 [*] Return the condition `b` directly 26 23 | 27 24 | def f(): -SIM103.py:32:9: SIM103 [*] Return the condition `b` directly +SIM103.py:32:9: SIM103 [*] Return the condition `bool(b)` directly | 30 | return 1 31 | else: @@ -104,10 +104,10 @@ SIM103.py:32:9: SIM103 [*] Return the condition `b` directly 37 34 | 38 35 | def f(): -SIM103.py:57:5: SIM103 [*] Return the condition `a` directly +SIM103.py:57:5: SIM103 [*] Return the condition `not a` directly | 55 | def f(): -56 | # SIM103 (but not fixable) +56 | # SIM103 57 | if a: | _____^ 58 | | return False @@ -120,7 +120,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly ℹ Unsafe fix 54 54 | 55 55 | def f(): -56 56 | # SIM103 (but not fixable) +56 56 | # SIM103 57 |- if a: 58 |- return False 59 |- else: @@ -130,7 +130,7 @@ SIM103.py:57:5: SIM103 [*] Return the condition `a` directly 62 59 | 63 60 | def f(): -SIM103.py:83:5: SIM103 Return the condition `a` directly +SIM103.py:83:5: SIM103 Return the condition directly | 81 | def bool(): 82 | return False @@ -143,47 +143,73 @@ SIM103.py:83:5: SIM103 Return the condition `a` directly | = help: Inline condition -SIM103.py:96:5: SIM103 [*] Return the condition `a` directly +SIM103.py:91:5: SIM103 [*] Return the condition `not (keys is not None and notice.key not in keys)` directly | -94 | def f(): -95 | # SIM103 -96 | if a: +89 | def f(): +90 | # SIM103 +91 | if keys is not None and notice.key not in keys: | _____^ -97 | | return True -98 | | return False - | |________________^ SIM103 +92 | | return False +93 | | else: +94 | | return True + | |___________________^ SIM103 | - = help: Replace with `return bool(a)` + = help: Replace with `return not (keys is not None and notice.key not in keys)` ℹ Unsafe fix -93 93 | -94 94 | def f(): -95 95 | # SIM103 -96 |- if a: -97 |- return True -98 |- return False - 96 |+ return bool(a) -99 97 | -100 98 | -101 99 | def f(): +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:103:5: SIM103 [*] Return the condition `a` directly +SIM103.py:104:5: SIM103 [*] Return the condition `bool(a)` directly | -101 | def f(): -102 | # SIM103 -103 | if a: +102 | def f(): +103 | # SIM103 +104 | if a: | _____^ -104 | | return False -105 | | return True +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 -100 100 | -101 101 | def f(): -102 102 | # SIM103 -103 |- if a: -104 |- return False -105 |- return True - 103 |+ return not a +108 108 | +109 109 | def f(): +110 110 | # SIM103 +111 |- if a: +112 |- return False +113 |- return True + 111 |+ return not a