Skip to content

Commit 5f7e034

Browse files
Parenthesize expression in RUF050 fix (astral-sh#24234)
## Summary Closes astral-sh#24220.
1 parent c8214d1 commit 5f7e034

3 files changed

Lines changed: 149 additions & 46 deletions

File tree

crates/ruff_linter/resources/test/fixtures/ruff/RUF050.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,20 @@ def nested():
5858
if x and foo():
5959
pass
6060

61+
# Multiline expression that needs outer parentheses
62+
if (
63+
id(0)
64+
+ 0
65+
):
66+
pass
67+
68+
# Multiline call stays a single expression statement
69+
if foo(
70+
1,
71+
2,
72+
):
73+
pass
74+
6175
# Walrus operator with call
6276
if (x := foo()):
6377
pass

crates/ruff_linter/src/rules/ruff/rules/unnecessary_if.rs

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use ruff_macros::{ViolationMetadata, derive_message_formats};
44
use ruff_python_ast::helpers::{
55
any_over_expr, comment_indentation_after, contains_effect, is_stub_body,
66
};
7-
use ruff_python_ast::token::TokenKind;
7+
use ruff_python_ast::token::{TokenKind, Tokens, parenthesized_range};
88
use ruff_python_ast::whitespace::indentation;
99
use ruff_python_ast::{Expr, StmtIf};
1010
use ruff_python_semantic::analyze::typing;
@@ -109,13 +109,7 @@ pub(crate) fn unnecessary_if(checker: &Checker, stmt: &StmtIf) {
109109

110110
if has_side_effects {
111111
// Replace `if cond: pass` with `cond` as an expression statement.
112-
// Walrus operators need parentheses to be valid as statements.
113-
let condition_text = checker.locator().slice(test.range());
114-
let replacement = if test.is_named_expr() {
115-
format!("({condition_text})")
116-
} else {
117-
condition_text.to_string()
118-
};
112+
let replacement = condition_as_expression_statement(test, stmt, checker);
119113
let edit = Edit::range_replacement(replacement, stmt.range());
120114
diagnostic.set_fix(Fix::safe_edit(edit));
121115
} else {
@@ -129,6 +123,45 @@ pub(crate) fn unnecessary_if(checker: &Checker, stmt: &StmtIf) {
129123
}
130124
}
131125

126+
/// Return the `if` condition in a form that remains a single valid expression statement.
127+
fn condition_as_expression_statement(test: &Expr, stmt: &StmtIf, checker: &Checker) -> String {
128+
let has_top_level_line_break = has_top_level_line_break(test.range(), checker.tokens());
129+
130+
if has_top_level_line_break
131+
&& let Some(range) = parenthesized_range(test.into(), stmt.into(), checker.tokens())
132+
{
133+
return checker.locator().slice(range).to_string();
134+
}
135+
136+
let condition_text = checker.locator().slice(test.range());
137+
if test.is_named_expr() || has_top_level_line_break {
138+
format!("({condition_text})")
139+
} else {
140+
condition_text.to_string()
141+
}
142+
}
143+
144+
/// Returns `true` if an expression contains a line break at the top level.
145+
///
146+
/// Such expressions need parentheses to remain a single expression statement when extracted from
147+
/// an `if` condition.
148+
fn has_top_level_line_break(range: TextRange, tokens: &Tokens) -> bool {
149+
let mut nesting = 0u32;
150+
151+
for token in tokens.in_range(range) {
152+
match token.kind() {
153+
TokenKind::Lpar | TokenKind::Lsqb | TokenKind::Lbrace => nesting += 1,
154+
TokenKind::Rpar | TokenKind::Rsqb | TokenKind::Rbrace => {
155+
nesting = nesting.saturating_sub(1);
156+
}
157+
TokenKind::Newline | TokenKind::NonLogicalNewline if nesting == 0 => return true,
158+
_ => {}
159+
}
160+
}
161+
162+
false
163+
}
164+
132165
/// Returns `true` if the `if` statement contains a comment
133166
fn if_contains_comments(stmt: &StmtIf, checker: &Checker) -> bool {
134167
let source = checker.source();

crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF050_RUF050.py.snap

Lines changed: 94 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ RUF050 [*] Empty `if` statement
254254
59 | | pass
255255
| |________^
256256
60 |
257-
61 | # Walrus operator with call
257+
61 | # Multiline expression that needs outer parentheses
258258
|
259259
help: Remove the `if` statement
260260
55 | pass
@@ -264,67 +264,123 @@ help: Remove the `if` statement
264264
- pass
265265
58 + x and foo()
266266
59 |
267-
60 | # Walrus operator with call
268-
61 | if (x := foo()):
267+
60 | # Multiline expression that needs outer parentheses
268+
61 | if (
269269

270270
RUF050 [*] Empty `if` statement
271271
--> RUF050.py:62:1
272272
|
273-
61 | # Walrus operator with call
274-
62 | / if (x := foo()):
275-
63 | | pass
273+
61 | # Multiline expression that needs outer parentheses
274+
62 | / if (
275+
63 | | id(0)
276+
64 | | + 0
277+
65 | | ):
278+
66 | | pass
276279
| |________^
277-
64 |
278-
65 | # Walrus operator without call
280+
67 |
281+
68 | # Multiline call stays a single expression statement
279282
|
280283
help: Remove the `if` statement
281284
59 | pass
282285
60 |
283-
61 | # Walrus operator with call
286+
61 | # Multiline expression that needs outer parentheses
287+
- if (
288+
62 + (
289+
63 | id(0)
290+
64 | + 0
291+
- ):
292+
- pass
293+
65 + )
294+
66 |
295+
67 | # Multiline call stays a single expression statement
296+
68 | if foo(
297+
298+
RUF050 [*] Empty `if` statement
299+
--> RUF050.py:69:1
300+
|
301+
68 | # Multiline call stays a single expression statement
302+
69 | / if foo(
303+
70 | | 1,
304+
71 | | 2,
305+
72 | | ):
306+
73 | | pass
307+
| |________^
308+
74 |
309+
75 | # Walrus operator with call
310+
|
311+
help: Remove the `if` statement
312+
66 | pass
313+
67 |
314+
68 | # Multiline call stays a single expression statement
315+
- if foo(
316+
69 + foo(
317+
70 | 1,
318+
71 | 2,
319+
- ):
320+
- pass
321+
72 + )
322+
73 |
323+
74 | # Walrus operator with call
324+
75 | if (x := foo()):
325+
326+
RUF050 [*] Empty `if` statement
327+
--> RUF050.py:76:1
328+
|
329+
75 | # Walrus operator with call
330+
76 | / if (x := foo()):
331+
77 | | pass
332+
| |________^
333+
78 |
334+
79 | # Walrus operator without call
335+
|
336+
help: Remove the `if` statement
337+
73 | pass
338+
74 |
339+
75 | # Walrus operator with call
284340
- if (x := foo()):
285341
- pass
286-
62 + (x := foo())
287-
63 |
288-
64 | # Walrus operator without call
289-
65 | if (x := y):
342+
76 + (x := foo())
343+
77 |
344+
78 | # Walrus operator without call
345+
79 | if (x := y):
290346

291347
RUF050 [*] Empty `if` statement
292-
--> RUF050.py:66:1
348+
--> RUF050.py:80:1
293349
|
294-
65 | # Walrus operator without call
295-
66 | / if (x := y):
296-
67 | | pass
350+
79 | # Walrus operator without call
351+
80 | / if (x := y):
352+
81 | | pass
297353
| |________^
298-
68 |
299-
69 | # Only statement in a suite
354+
82 |
355+
83 | # Only statement in a suite
300356
|
301357
help: Remove the `if` statement
302-
63 | pass
303-
64 |
304-
65 | # Walrus operator without call
358+
77 | pass
359+
78 |
360+
79 | # Walrus operator without call
305361
- if (x := y):
306362
- pass
307-
66 + (x := y)
308-
67 |
309-
68 | # Only statement in a suite
310-
69 | class Foo:
363+
80 + (x := y)
364+
81 |
365+
82 | # Only statement in a suite
366+
83 | class Foo:
311367

312368
RUF050 [*] Empty `if` statement
313-
--> RUF050.py:71:5
369+
--> RUF050.py:85:5
314370
|
315-
69 | # Only statement in a suite
316-
70 | class Foo:
317-
71 | / if foo():
318-
72 | | pass
371+
83 | # Only statement in a suite
372+
84 | class Foo:
373+
85 | / if foo():
374+
86 | | pass
319375
| |____________^
320376
|
321377
help: Remove the `if` statement
322-
68 |
323-
69 | # Only statement in a suite
324-
70 | class Foo:
378+
82 |
379+
83 | # Only statement in a suite
380+
84 | class Foo:
325381
- if foo():
326382
- pass
327-
71 + foo()
328-
72 |
329-
73 |
330-
74 | ### No errors
383+
85 + foo()
384+
86 |
385+
87 |
386+
88 | ### No errors

0 commit comments

Comments
 (0)