mirror of https://github.com/astral-sh/ruff
Avoid nested-if violations when outer-if has else clause (#2095)
It looks like we need `do`-`while`-like semantics here with an additional outer check. Closes #2094.
This commit is contained in:
parent
4fb0c6e3ad
commit
8d46d3bfa6
|
|
@ -92,3 +92,40 @@ if node.module:
|
||||||
"multiprocessing."
|
"multiprocessing."
|
||||||
):
|
):
|
||||||
print("Bad module!")
|
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")
|
||||||
|
|
|
||||||
|
|
@ -1386,6 +1386,7 @@ where
|
||||||
stmt,
|
stmt,
|
||||||
test,
|
test,
|
||||||
body,
|
body,
|
||||||
|
orelse,
|
||||||
self.current_stmt_parent().map(Into::into),
|
self.current_stmt_parent().map(Into::into),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -50,7 +50,7 @@ fn is_main_check(expr: &Expr) -> bool {
|
||||||
/// ```
|
/// ```
|
||||||
fn find_last_nested_if(body: &[Stmt]) -> Option<(&Expr, &Stmt)> {
|
fn find_last_nested_if(body: &[Stmt]) -> Option<(&Expr, &Stmt)> {
|
||||||
let [Stmt { node: StmtKind::If { test, body: inner_body, orelse }, ..}] = body else { return None };
|
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;
|
return None;
|
||||||
}
|
}
|
||||||
find_last_nested_if(inner_body).or_else(|| {
|
find_last_nested_if(inner_body).or_else(|| {
|
||||||
|
|
@ -67,12 +67,10 @@ pub fn nested_if_statements(
|
||||||
stmt: &Stmt,
|
stmt: &Stmt,
|
||||||
test: &Expr,
|
test: &Expr,
|
||||||
body: &[Stmt],
|
body: &[Stmt],
|
||||||
|
orelse: &[Stmt],
|
||||||
parent: Option<&Stmt>,
|
parent: Option<&Stmt>,
|
||||||
) {
|
) {
|
||||||
if is_main_check(test) {
|
// If the parent could contain a nested if-statement, abort.
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
if let Some(parent) = parent {
|
if let Some(parent) = parent {
|
||||||
if let StmtKind::If { body, orelse, .. } = &parent.node {
|
if let StmtKind::If { body, orelse, .. } = &parent.node {
|
||||||
if orelse.is_empty() && body.len() == 1 {
|
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 {
|
let Some((test, first_stmt)) = find_last_nested_if(body) else {
|
||||||
return;
|
return;
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -148,4 +148,21 @@ expression: diagnostics
|
||||||
row: 95
|
row: 95
|
||||||
column: 0
|
column: 0
|
||||||
parent: ~
|
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: ~
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue