diff --git a/resources/test/fixtures/flake8_simplify/SIM102.py b/resources/test/fixtures/flake8_simplify/SIM102.py index 79c5e9a359..18680bc9a2 100644 --- a/resources/test/fixtures/flake8_simplify/SIM102.py +++ b/resources/test/fixtures/flake8_simplify/SIM102.py @@ -92,3 +92,40 @@ if node.module: "multiprocessing." ): print("Bad module!") + + +# OK +if a: + if b: + print("foo") +else: + print("bar") + +# OK +if a: + if b: + if c: + print("foo") + else: + print("bar") +else: + print("bar") + +# OK +if a: + # SIM 102 + if b: + if c: + print("foo") +else: + print("bar") + + +# OK +if a: + if b: + if c: + print("foo") + print("baz") +else: + print("bar") diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index c31e677d51..12b4ebfdf8 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1386,6 +1386,7 @@ where stmt, test, body, + orelse, self.current_stmt_parent().map(Into::into), ); } diff --git a/src/rules/flake8_simplify/rules/ast_if.rs b/src/rules/flake8_simplify/rules/ast_if.rs index dc21410a52..34eb8c79e7 100644 --- a/src/rules/flake8_simplify/rules/ast_if.rs +++ b/src/rules/flake8_simplify/rules/ast_if.rs @@ -50,7 +50,7 @@ fn is_main_check(expr: &Expr) -> bool { /// ``` fn find_last_nested_if(body: &[Stmt]) -> Option<(&Expr, &Stmt)> { let [Stmt { node: StmtKind::If { test, body: inner_body, orelse }, ..}] = body else { return None }; - if !(orelse.is_empty() && body.len() == 1) { + if !orelse.is_empty() { return None; } find_last_nested_if(inner_body).or_else(|| { @@ -67,12 +67,10 @@ pub fn nested_if_statements( stmt: &Stmt, test: &Expr, body: &[Stmt], + orelse: &[Stmt], parent: Option<&Stmt>, ) { - if is_main_check(test) { - return; - } - + // If the parent could contain a nested if-statement, abort. if let Some(parent) = parent { if let StmtKind::If { body, orelse, .. } = &parent.node { if orelse.is_empty() && body.len() == 1 { @@ -81,6 +79,16 @@ pub fn nested_if_statements( } } + // If this if-statement has an else clause, or more than one child, abort. + if !(orelse.is_empty() && body.len() == 1) { + return; + } + + if is_main_check(test) { + return; + } + + // Find the deepest nested if-statement, to inform the range. let Some((test, first_stmt)) = find_last_nested_if(body) else { return; }; diff --git a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap index bb64580020..a47c3410fe 100644 --- a/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap +++ b/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap @@ -148,4 +148,21 @@ expression: diagnostics row: 95 column: 0 parent: ~ +- kind: + NestedIfStatements: ~ + location: + row: 117 + column: 4 + end_location: + row: 118 + column: 13 + fix: + content: " if b and c:\n print(\"foo\")\n" + location: + row: 117 + column: 0 + end_location: + row: 120 + column: 0 + parent: ~