Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-simplify] Detect implicit else cases in needless-bool (SIM103) #10414

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -84,3 +84,22 @@ def bool():
return True
else:
return False


###
# Positive cases (preview)
###


def f():
# SIM103
if a:
return True
return False


def f():
# SIM103
if a:
return False
return True
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Expand Up @@ -1079,7 +1079,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
flake8_simplify::rules::if_with_same_arms(checker, if_);
}
if checker.enabled(Rule::NeedlessBool) {
flake8_simplify::rules::needless_bool(checker, if_);
flake8_simplify::rules::needless_bool(checker, stmt);
}
if checker.enabled(Rule::IfElseBlockInsteadOfDictLookup) {
flake8_simplify::rules::if_else_block_instead_of_dict_lookup(checker, if_);
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_simplify/mod.rs
Expand Up @@ -56,6 +56,7 @@ mod tests {
Ok(())
}

#[test_case(Rule::NeedlessBool, Path::new("SIM103.py"))]
#[test_case(Rule::YodaConditions, Path::new("SIM300.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
Expand Down
@@ -1,3 +1,4 @@
use ast::traversal;
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Arguments, ElifElseClause, Expr, ExprContext, Stmt};
Expand Down Expand Up @@ -62,14 +63,17 @@ impl Violation for NeedlessBool {
}

/// SIM103
pub(crate) fn needless_bool(checker: &mut Checker, stmt_if: &ast::StmtIf) {
pub(crate) fn needless_bool(checker: &mut Checker, stmt: &ast::Stmt) {
let ast::Stmt::If(stmt_if) = stmt else { return };
let ast::StmtIf {
test: if_test,
body: if_body,
elif_else_clauses,
range: _,
} = stmt_if;

let stmt_return_body;

// Extract an `if` or `elif` (that returns) followed by an else (that returns the same value)
let (if_test, if_body, else_body, range) = match elif_else_clauses.as_slice() {
// if-else case
Expand All @@ -93,6 +97,36 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt_if: &ast::StmtIf) {
else_body,
TextRange::new(elif_range.start(), else_range.end()),
),
// if-implicit-else case
[] if checker.settings.preview.is_enabled() => {
let next_stmt = {
// Get the next statement after the if statement and check if it's a return statement.
checker
.semantic()
.current_statement_parent()
// Get the parent of the if statement.
.and_then(|parent| traversal::suite(stmt, parent))
// Get the next sibling of the if statement.
.and_then(|suite| traversal::next_sibling(stmt, suite))
.and_then(|stmt| match stmt {
ast::Stmt::Return(stmt_return) => Some(stmt_return),
_ => None,
})
};

let Some(stmt_return) = next_stmt else { return };
stmt_return_body = vec![ast::Stmt::Return(ast::StmtReturn {
value: stmt_return.value.clone(),
range: stmt_return.range,
})];

(
if_test.as_ref(),
if_body,
&stmt_return_body,
TextRange::new(stmt_if.range().start(), stmt_return.range.end()),
)
}
_ => return,
};

Expand Down
Expand Up @@ -142,5 +142,3 @@ SIM103.py:83:5: SIM103 Return the condition `a` directly
| |____________________^ SIM103
|
= help: Inline condition


@@ -0,0 +1,189 @@
---
source: crates/ruff_linter/src/rules/flake8_simplify/mod.rs
---
SIM103.py:3:5: SIM103 [*] Return the condition `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 `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 `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 `a` directly
|
55 | def f():
56 | # SIM103 (but not fixable)
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 (but not fixable)
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 `a` directly
|
81 | def bool():
82 | return False
83 | if a:
| _____^
84 | | return True
85 | | else:
86 | | return False
| |____________________^ SIM103
|
= help: Inline condition

SIM103.py:96:5: SIM103 [*] Return the condition `a` directly
|
94 | def f():
95 | # SIM103
96 | if a:
| _____^
97 | | return True
98 | | return False
| |________________^ SIM103
|
= help: Replace with `return bool(a)`

ℹ 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():

SIM103.py:103:5: SIM103 [*] Return the condition `a` directly
|
101 | def f():
102 | # SIM103
103 | if a:
| _____^
104 | | return False
105 | | 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