diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py index a0c3656550..6c248291ef 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM102.py @@ -156,3 +156,12 @@ if False: if True: if a: pass + + +# SIM102 +def f(): + if a: + pass + elif b: + if c: + d diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs index 45ebbfb40b..518f0139bd 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_if.rs @@ -312,7 +312,8 @@ fn find_last_nested_if(body: &[Stmt]) -> Option<(&Expr, &Stmt)> { }) } -fn nested_if_body(stmt_if: &StmtIf) -> Option<(&[Stmt], TextRange)> { +/// Returns the body, the range of the `if` or `elif` and whether the range is for an `if` or `elif` +fn nested_if_body(stmt_if: &StmtIf) -> Option<(&[Stmt], TextRange, bool)> { let StmtIf { test, body, @@ -322,15 +323,15 @@ fn nested_if_body(stmt_if: &StmtIf) -> Option<(&[Stmt], TextRange)> { // It must be the last condition, otherwise there could be another `elif` or `else` that only // depends on the outer of the two conditions - let (test, body, range) = if let Some(clause) = elif_else_clauses.last() { + let (test, body, range, is_elif) = if let Some(clause) = elif_else_clauses.last() { if let Some(test) = &clause.test { - (test, &clause.body, clause.range()) + (test, &clause.body, clause.range(), true) } else { // The last condition is an `else` (different rule) return None; } } else { - (test.as_ref(), body, stmt_if.range()) + (test.as_ref(), body, stmt_if.range(), false) }; // The nested if must be the only child, otherwise there is at least one more statement that @@ -355,12 +356,12 @@ fn nested_if_body(stmt_if: &StmtIf) -> Option<(&[Stmt], TextRange)> { return None; } - Some((body, range)) + Some((body, range, is_elif)) } /// SIM102 pub(crate) fn nested_if_statements(checker: &mut Checker, stmt_if: &StmtIf, parent: Option<&Stmt>) { - let Some((body, range)) = nested_if_body(stmt_if) else { + let Some((body, range, is_elif)) = nested_if_body(stmt_if) else { return; }; @@ -376,7 +377,7 @@ pub(crate) fn nested_if_statements(checker: &mut Checker, stmt_if: &StmtIf, pare // Check if the parent is already emitting a larger diagnostic including this if statement if let Some(Stmt::If(stmt_if)) = parent { - if let Some((body, _range)) = nested_if_body(stmt_if) { + if let Some((body, _range, _is_elif)) = nested_if_body(stmt_if) { // In addition to repeating the `nested_if_body` and `find_last_nested_if` check, we // also need to be the first child in the parent if matches!(&body[0], Stmt::If(inner) if inner == stmt_if) @@ -400,7 +401,12 @@ pub(crate) fn nested_if_statements(checker: &mut Checker, stmt_if: &StmtIf, pare .comment_ranges() .intersects(TextRange::new(range.start(), nested_if.start())) { - match fix_if::fix_nested_if_statements(checker.locator(), checker.stylist(), range) { + match fix_if::fix_nested_if_statements( + checker.locator(), + checker.stylist(), + range, + is_elif, + ) { Ok(edit) => { if edit .content() diff --git a/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs b/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs index 3b21d0539d..ad575808f4 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs @@ -34,6 +34,7 @@ pub(crate) fn fix_nested_if_statements( locator: &Locator, stylist: &Stylist, range: TextRange, + is_elif: bool, ) -> Result { // Infer the indentation of the outer block. let Some(outer_indent) = whitespace::indentation(locator, &range) else { @@ -45,7 +46,6 @@ pub(crate) fn fix_nested_if_statements( // If this is an `elif`, we have to remove the `elif` keyword for now. (We'll // restore the `el` later on.) - let is_elif = contents.starts_with("elif"); let module_text = if is_elif { Cow::Owned(contents.replacen("elif", "if", 1)) } else { diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap index fe9a9834cf..ea461e39cc 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM102_SIM102.py.snap @@ -332,4 +332,26 @@ SIM102.py:132:5: SIM102 [*] Use a single `if` statement instead of nested `if` s 136 135 | print("bar") 137 136 | +SIM102.py:165:5: SIM102 [*] Use a single `if` statement instead of nested `if` statements + | +163 | if a: +164 | pass +165 | elif b: + | _____^ +166 | | if c: + | |_____________^ SIM102 +167 | d + | + = help: Combine `if` statements using `and` + +ℹ Suggested fix +162 162 | def f(): +163 163 | if a: +164 164 | pass +165 |- elif b: +166 |- if c: +167 |- d + 165 |+ elif b and c: + 166 |+ d +