diff --git a/README.md b/README.md index 62de049aaf..b23b6d4c64 100644 --- a/README.md +++ b/README.md @@ -971,9 +971,10 @@ For more, see [flake8-simplify](https://pypi.org/project/flake8-simplify/0.19.3/ | ---- | ---- | ------- | --- | | SIM101 | DuplicateIsinstanceCall | Multiple `isinstance` calls for `...`, merge into a single call | 🛠 | | SIM102 | NestedIfStatements | Use a single `if` statement instead of nested `if` statements | | +| SIM103 | ReturnBoolConditionDirectly | Return the condition `...` directly | 🛠 | | SIM105 | UseContextlibSuppress | Use `contextlib.suppress(...)` instead of try-except-pass | | | SIM107 | ReturnInTryExceptFinally | Don't use `return` in `try`/`except` and `finally` | | -| SIM108 | UseTernaryOperator | Use ternary operator `..` instead of if-else-block | 🛠 | +| SIM108 | UseTernaryOperator | Use ternary operator `...` instead of if-else-block | 🛠 | | SIM109 | CompareWithTuple | Use `value in (..., ...)` instead of `value == ... or value == ...` | 🛠 | | SIM110 | ConvertLoopToAny | Use `return any(x for x in y)` instead of `for` loop | 🛠 | | SIM111 | ConvertLoopToAll | Use `return all(x for x in y)` instead of `for` loop | 🛠 | diff --git a/resources/test/fixtures/flake8_simplify/SIM103.py b/resources/test/fixtures/flake8_simplify/SIM103.py new file mode 100644 index 0000000000..7fa8d66c1e --- /dev/null +++ b/resources/test/fixtures/flake8_simplify/SIM103.py @@ -0,0 +1,20 @@ +def f(): + if a: # SIM103 + return True + else: + return False + + +def f(): + if a: # OK + foo() + return True + else: + return False + + +def f(): + if a: # OK + return "foo" + else: + return False diff --git a/ruff.schema.json b/ruff.schema.json index fa816fc9b0..0781ea7b7c 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -941,6 +941,7 @@ "SIM10", "SIM101", "SIM102", + "SIM103", "SIM105", "SIM107", "SIM108", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 2d07234f91..1361856d73 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1184,6 +1184,9 @@ where if self.settings.enabled.contains(&CheckCode::SIM102) { flake8_simplify::plugins::nested_if_statements(self, stmt); } + if self.settings.enabled.contains(&CheckCode::SIM103) { + flake8_simplify::plugins::return_bool_condition_directly(self, stmt); + } if self.settings.enabled.contains(&CheckCode::SIM108) { flake8_simplify::plugins::use_ternary_operator( self, diff --git a/src/flake8_simplify/mod.rs b/src/flake8_simplify/mod.rs index 67b41f3b96..ea5f7e60d7 100644 --- a/src/flake8_simplify/mod.rs +++ b/src/flake8_simplify/mod.rs @@ -14,8 +14,10 @@ mod tests { #[test_case(CheckCode::SIM101, Path::new("SIM101.py"); "SIM101")] #[test_case(CheckCode::SIM102, Path::new("SIM102.py"); "SIM102")] + #[test_case(CheckCode::SIM103, Path::new("SIM103.py"); "SIM103")] #[test_case(CheckCode::SIM105, Path::new("SIM105.py"); "SIM105")] #[test_case(CheckCode::SIM107, Path::new("SIM107.py"); "SIM107")] + #[test_case(CheckCode::SIM108, Path::new("SIM108.py"); "SIM108")] #[test_case(CheckCode::SIM109, Path::new("SIM109.py"); "SIM109")] #[test_case(CheckCode::SIM110, Path::new("SIM110.py"); "SIM110")] #[test_case(CheckCode::SIM111, Path::new("SIM111.py"); "SIM111")] @@ -29,7 +31,6 @@ mod tests { #[test_case(CheckCode::SIM222, Path::new("SIM222.py"); "SIM222")] #[test_case(CheckCode::SIM223, Path::new("SIM223.py"); "SIM223")] #[test_case(CheckCode::SIM300, Path::new("SIM300.py"); "SIM300")] - #[test_case(CheckCode::SIM108, Path::new("SIM108.py"); "SIM108")] fn checks(check_code: CheckCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", check_code.as_ref(), path.to_string_lossy()); let checks = test_path( diff --git a/src/flake8_simplify/plugins/ast_if.rs b/src/flake8_simplify/plugins/ast_if.rs index 4992c20ec7..dfaf5944e4 100644 --- a/src/flake8_simplify/plugins/ast_if.rs +++ b/src/flake8_simplify/plugins/ast_if.rs @@ -1,6 +1,6 @@ use rustpython_ast::{Constant, Expr, ExprKind, Stmt, StmtKind}; -use crate::ast::helpers::{create_expr, create_stmt, unparse_stmt}; +use crate::ast::helpers::{create_expr, create_stmt, unparse_expr, unparse_stmt}; use crate::ast::types::Range; use crate::autofix::Fix; use crate::checkers::ast::Checker; @@ -65,6 +65,45 @@ pub fn nested_if_statements(checker: &mut Checker, stmt: &Stmt) { )); } +fn is_one_line_return_bool(stmts: &[Stmt]) -> bool { + if stmts.len() != 1 { + return false; + } + let StmtKind::Return { value } = &stmts[0].node else { + return false; + }; + let Some(ExprKind::Constant { value, .. }) = value.as_ref().map(|value| &value.node) else { + return false; + }; + matches!(value, Constant::Bool(_)) +} + +/// SIM103 +pub fn return_bool_condition_directly(checker: &mut Checker, stmt: &Stmt) { + let StmtKind::If { test, body, orelse } = &stmt.node else { + return; + }; + if !(is_one_line_return_bool(body) && is_one_line_return_bool(orelse)) { + return; + } + let condition = unparse_expr(test, checker.style); + let mut check = Check::new( + CheckKind::ReturnBoolConditionDirectly(condition), + Range::from_located(stmt), + ); + if checker.patch(&CheckCode::SIM103) { + let return_stmt = create_stmt(StmtKind::Return { + value: Some(test.clone()), + }); + check.amend(Fix::replacement( + unparse_stmt(&return_stmt, checker.style), + stmt.location, + stmt.end_location.unwrap(), + )); + } + checker.checks.push(check); +} + fn ternary(target_var: &Expr, body_value: &Expr, test: &Expr, orelse_value: &Expr) -> Stmt { create_stmt(StmtKind::Assign { targets: vec![target_var.clone()], diff --git a/src/flake8_simplify/plugins/mod.rs b/src/flake8_simplify/plugins/mod.rs index b0c7743112..cf81bd256f 100644 --- a/src/flake8_simplify/plugins/mod.rs +++ b/src/flake8_simplify/plugins/mod.rs @@ -2,7 +2,7 @@ pub use ast_bool_op::{ a_and_not_a, a_or_not_a, and_false, compare_with_tuple, duplicate_isinstance_call, or_true, }; pub use ast_for::convert_loop_to_any_all; -pub use ast_if::{nested_if_statements, use_ternary_operator}; +pub use ast_if::{nested_if_statements, return_bool_condition_directly, use_ternary_operator}; pub use ast_with::multiple_with_statements; pub use key_in_dict::{key_in_dict_compare, key_in_dict_for}; pub use return_in_try_except_finally::return_in_try_except_finally; diff --git a/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM103_SIM103.py.snap b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM103_SIM103.py.snap new file mode 100644 index 0000000000..b6c04bc2b6 --- /dev/null +++ b/src/flake8_simplify/snapshots/ruff__flake8_simplify__tests__SIM103_SIM103.py.snap @@ -0,0 +1,22 @@ +--- +source: src/flake8_simplify/mod.rs +expression: checks +--- +- kind: + ReturnBoolConditionDirectly: a + location: + row: 2 + column: 4 + end_location: + row: 5 + column: 20 + fix: + content: return a + location: + row: 2 + column: 4 + end_location: + row: 5 + column: 20 + parent: ~ + diff --git a/src/registry.rs b/src/registry.rs index 32b38b7460..13480fad38 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -220,6 +220,7 @@ pub enum CheckCode { // flake8-simplify SIM101, SIM102, + SIM103, SIM105, SIM107, SIM108, @@ -967,6 +968,7 @@ pub enum CheckKind { SysVersionCmpStr10, SysVersionSlice1Referenced, // flake8-simplify + ReturnBoolConditionDirectly(String), UseTernaryOperator(String), CompareWithTuple(String, Vec, String), DuplicateIsinstanceCall(String), @@ -1418,9 +1420,10 @@ impl CheckCode { // flake8-simplify CheckCode::SIM101 => CheckKind::DuplicateIsinstanceCall("...".to_string()), CheckCode::SIM102 => CheckKind::NestedIfStatements, + CheckCode::SIM103 => CheckKind::ReturnBoolConditionDirectly("...".to_string()), CheckCode::SIM105 => CheckKind::UseContextlibSuppress("...".to_string()), CheckCode::SIM107 => CheckKind::ReturnInTryExceptFinally, - CheckCode::SIM108 => CheckKind::UseTernaryOperator("..".to_string()), + CheckCode::SIM108 => CheckKind::UseTernaryOperator("...".to_string()), CheckCode::SIM109 => CheckKind::CompareWithTuple( "value".to_string(), vec!["...".to_string(), "...".to_string()], @@ -1978,6 +1981,7 @@ impl CheckCode { CheckCode::S501 => CheckCategory::Flake8Bandit, CheckCode::S506 => CheckCategory::Flake8Bandit, // flake8-simplify + CheckCode::SIM103 => CheckCategory::Flake8Simplify, CheckCode::SIM101 => CheckCategory::Flake8Simplify, CheckCode::SIM102 => CheckCategory::Flake8Simplify, CheckCode::SIM105 => CheckCategory::Flake8Simplify, @@ -2256,6 +2260,7 @@ impl CheckKind { CheckKind::NegateNotEqualOp(..) => &CheckCode::SIM202, CheckKind::NestedIfStatements => &CheckCode::SIM102, CheckKind::OrTrue => &CheckCode::SIM222, + CheckKind::ReturnBoolConditionDirectly(..) => &CheckCode::SIM103, CheckKind::ReturnInTryExceptFinally => &CheckCode::SIM107, CheckKind::UseContextlibSuppress(..) => &CheckCode::SIM105, CheckKind::UseTernaryOperator(..) => &CheckCode::SIM108, @@ -3018,6 +3023,9 @@ impl CheckKind { "`sys.version[:1]` referenced (python10), use `sys.version_info`".to_string() } // flake8-simplify + CheckKind::ReturnBoolConditionDirectly(cond) => { + format!("Return the condition `{cond}` directly") + } CheckKind::CompareWithTuple(value, values, or_op) => { let values = values.join(", "); format!("Use `{value} in ({values})` instead of `{or_op}`") @@ -3774,6 +3782,7 @@ impl CheckKind { | CheckKind::RemoveSixCompat | CheckKind::ReplaceStdoutStderr | CheckKind::ReplaceUniversalNewlines + | CheckKind::ReturnBoolConditionDirectly(..) | CheckKind::RewriteCElementTree | CheckKind::RewriteListComprehension | CheckKind::RewriteMockImport(..) @@ -3998,6 +4007,9 @@ impl CheckKind { Some(format!("Replace with `except {name}`")) } CheckKind::RemoveSixCompat => Some("Remove `six` usage".to_string()), + CheckKind::ReturnBoolConditionDirectly(cond) => { + Some(format!("Replace with `return {cond}`")) + } CheckKind::SectionNameEndsInColon(name) => Some(format!("Add colon to \"{name}\"")), CheckKind::SectionNotOverIndented(name) => { Some(format!("Remove over-indentation from \"{name}\""))