Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
Fixes #10402 

## Summary

For SIM103, detect and simplify the following case:

[playground
link](https://play.ruff.rs/d98570aa-b180-495b-8600-5c4c3fd02526)
```python
def main():
    if foo > 5:
        return True
    return False
```

## Test Plan

Unit tested only.
This commit is contained in:
Ottavio Hartman
2024-03-17 20:15:28 -04:00
committed by GitHub
parent 229a50a2c8
commit 526abebbae
6 changed files with 280 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM103.py vendored
View File

@@ -84,3 +84,22 @@ def f():
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
crates/ruff_linter/src/checkers/ast/analyze/statement.rs
View File

@@ -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_);

1
crates/ruff_linter/src/rules/flake8_simplify/mod.rs
View File

@@ -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!(

77
crates/ruff_linter/src/rules/flake8_simplify/rules/needless_bool.rs
View File

@@ -1,5 +1,6 @@
use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::traversal;
use ruff_python_ast::{self as ast, Arguments, ElifElseClause, Expr, ExprContext, Stmt};
use ruff_python_semantic::analyze::typing::{is_sys_version_block, is_type_checking_block};
use ruff_text_size::{Ranged, TextRange};
@@ -16,7 +17,7 @@ use crate::fix::snippet::SourceCodeSnippet;
///
/// ## Example
/// ```python
/// if foo:
/// if x > 0:
/// return True
/// else:
/// return False
@@ -24,11 +25,20 @@ use crate::fix::snippet::SourceCodeSnippet;
///
/// Use instead:
/// ```python
/// return bool(foo)
/// return x > 0
/// ```
///
/// In [preview], this rule will also flag implicit `else` cases, as in:
/// ```python
/// if x > 0:
/// return True
/// return False
/// ```
///
/// ## References
/// - [Python documentation: Truth Value Testing](https://docs.python.org/3/library/stdtypes.html#truth-value-testing)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
#[violation]
pub struct NeedlessBool {
condition: SourceCodeSnippet,
@@ -62,23 +72,41 @@ 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: &Stmt) {
let Stmt::If(stmt_if) = stmt else { return };
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 (if_test, if_body, else_body, range) = match elif_else_clauses.as_slice() {
// if-else case
// if-else case:
// ```python
// if x > 0:
// return True
// else:
// return False
// ```
[ElifElseClause {
body: else_body,
test: None,
..
}] => (if_test.as_ref(), if_body, else_body, stmt_if.range()),
}] => (
if_test.as_ref(),
if_body,
else_body.as_slice(),
stmt_if.range(),
),
// elif-else case
// ```python
// if x > 0:
// return True
// elif x < 0:
// return False
// ```
[.., ElifElseClause {
body: elif_body,
test: Some(elif_test),
@@ -90,12 +118,47 @@ pub(crate) fn needless_bool(checker: &mut Checker, stmt_if: &ast::StmtIf) {
}] => (
elif_test,
elif_body,
else_body,
else_body.as_slice(),
TextRange::new(elif_range.start(), else_range.end()),
),
// if-implicit-else case:
// ```python
// if x > 0:
// return True
// return False
// ```
[] if checker.settings.preview.is_enabled() => {
// Fetching the next sibling is expensive, so do some validation early.
if is_one_line_return_bool(if_body).is_none() {
return;
}
// Fetch the next sibling statement.
let Some(next_stmt) = checker
.semantic()
.current_statement_parent()
.and_then(|parent| traversal::suite(stmt, parent))
.and_then(|suite| traversal::next_sibling(stmt, suite))
else {
return;
};
// If the next sibling is not a return statement, abort.
if !next_stmt.is_return_stmt() {
return;
}
(
if_test.as_ref(),
if_body,
std::slice::from_ref(next_stmt),
TextRange::new(stmt_if.start(), next_stmt.end()),
)
}
_ => return,
};
// Both branches must be one-liners that return a boolean.
let (Some(if_return), Some(else_return)) = (
is_one_line_return_bool(if_body),
is_one_line_return_bool(else_body),

2
crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__SIM103_SIM103.py.snap
View File

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

189
crates/ruff_linter/src/rules/flake8_simplify/snapshots/ruff_linter__rules__flake8_simplify__tests__preview__SIM103_SIM103.py.snap Normal file
View File

@@ -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