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 8 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 @@ -34,6 +34,17 @@ def f():
else:
return False

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

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

def f():
# OK
Expand Down Expand Up @@ -84,3 +95,4 @@ def bool():
return True
else:
return False

3 changes: 0 additions & 3 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Expand Up @@ -1078,9 +1078,6 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::IfWithSameArms) {
flake8_simplify::rules::if_with_same_arms(checker, if_);
}
if checker.enabled(Rule::NeedlessBool) {
flake8_simplify::rules::needless_bool(checker, if_);
}
if checker.enabled(Rule::IfElseBlockInsteadOfDictLookup) {
flake8_simplify::rules::if_else_block_instead_of_dict_lookup(checker, if_);
}
Expand Down
5 changes: 4 additions & 1 deletion crates/ruff_linter/src/checkers/ast/analyze/suite.rs
Expand Up @@ -2,11 +2,14 @@ use ruff_python_ast::Stmt;

use crate::checkers::ast::Checker;
use crate::codes::Rule;
use crate::rules::flake8_pie;
use crate::rules::{flake8_pie, flake8_simplify};

/// Run lint rules over a suite of [`Stmt`] syntax nodes.
pub(crate) fn suite(suite: &[Stmt], checker: &mut Checker) {
if checker.enabled(Rule::UnnecessaryPlaceholder) {
flake8_pie::rules::unnecessary_placeholder(checker, suite);
}
if checker.enabled(Rule::NeedlessBool) {
flake8_simplify::rules::needless_bool(checker, suite);
}
}
Expand Up @@ -62,15 +62,43 @@ 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, body: &[Stmt]) {
let mut iter = body.iter().peekable();

while let Some(stmt) = iter.next() {
match stmt {
ast::Stmt::If(stmt_if) => {
let stmt_return = match iter.peek() {
Some(ast::Stmt::Return(stmt_return)) => Some(stmt_return),
_ => None,
};

check_needless_bool(checker, stmt_if, stmt_return);
}
_ => continue,
}
}
}

pub(crate) fn check_needless_bool(
checker: &mut Checker,
stmt_if: &ast::StmtIf,
next_stmt: Option<&ast::StmtReturn>,
) {
let ast::StmtIf {
test: if_test,
body: if_body,
elif_else_clauses,
range: _,
} = stmt_if;

// Extract an `if` or `elif` (that returns) followed by an else (that returns the same value)
let return_stmt = next_stmt.map(|stmt| {
vec![ast::Stmt::Return(ast::StmtReturn {
value: stmt.value.clone(),
range: stmt.range,
})]
});

let (if_test, if_body, else_body, range) = match elif_else_clauses.as_slice() {
// if-else case
[ElifElseClause {
Expand All @@ -93,6 +121,16 @@ 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
[] => match next_stmt {
Some(ast::StmtReturn { range, .. }) => (
if_test.as_ref(),
if_body,
return_stmt.as_ref().unwrap(),
TextRange::new(stmt_if.range().start(), range.end()),
),
_ => return,
},
_ => return,
};

Expand Down
Expand Up @@ -88,6 +88,8 @@ SIM103.py:32:9: SIM103 [*] Return the condition `b` directly
34 | | else:
35 | | return False
| |________________________^ SIM103
36 |
37 | def f():
|
= help: Replace with `return bool(b)`

Expand All @@ -101,46 +103,107 @@ SIM103.py:32:9: SIM103 [*] Return the condition `b` directly
35 |- return False
32 |+ return bool(b)
36 33 |
37 34 |
38 35 | def f():
37 34 | def f():
38 35 | # SIM103

SIM103.py:57:5: SIM103 [*] Return the condition `a` directly
SIM103.py:39:5: SIM103 [*] Return the condition `a` directly
|
55 | def f():
56 | # SIM103 (but not fixable)
57 | if a:
37 | def f():
38 | # SIM103
39 | if a:
| _____^
58 | | return False
59 | | else:
60 | | return True
40 | | return True
41 | | return False
| |________________^ SIM103
42 |
43 | def f():
|
= help: Replace with `return bool(a)`

ℹ Unsafe fix
36 36 |
37 37 | def f():
38 38 | # SIM103
39 |- if a:
40 |- return True
41 |- return False
39 |+ return bool(a)
42 40 |
43 41 | def f():
44 42 | # SIM103

SIM103.py:45:5: SIM103 [*] Return the condition `a` directly
|
43 | def f():
44 | # SIM103
45 | if a:
| _____^
46 | | return False
47 | | return True
| |_______________^ SIM103
48 |
49 | def f():
|
= help: Replace with `return not a`

ℹ Unsafe fix
42 42 |
43 43 | def f():
44 44 | # SIM103
45 |- if a:
46 |- return False
47 |- return True
45 |+ return not a
48 46 |
49 47 | def f():
50 48 | # OK

SIM103.py:68:5: SIM103 [*] Return the condition `a` directly
|
66 | def f():
67 | # SIM103 (but not fixable)
68 | if a:
| _____^
69 | | return False
70 | | else:
71 | | 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():
65 65 |
66 66 | def f():
67 67 | # SIM103 (but not fixable)
68 |- if a:
69 |- return False
70 |- else:
71 |- return True
68 |+ return not a
72 69 |
73 70 |
74 71 | def f():

SIM103.py:83:5: SIM103 Return the condition `a` directly
SIM103.py:94:5: SIM103 [*] Return the condition `a` directly
|
81 | def bool():
82 | return False
83 | if a:
92 | def bool():
93 | return False
94 | if a:
| _____^
84 | | return True
85 | | else:
86 | | return False
95 | | return True
96 | | else:
97 | | return False
| |____________________^ SIM103
|
= help: Inline condition

= help: Replace with `return bool(a)`

ottaviohartman marked this conversation as resolved.
Show resolved Hide resolved
ℹ Unsafe fix
91 91 | # OK
92 92 | def bool():
93 93 | return False
94 |- if a:
95 |- return True
96 |- else:
97 |- return False
94 |+ return bool(a)
98 95 |