Skip to content

Commit 62343a1

Browse files
dylwil3dylwil3
authored
Respect fmt: skip for compound statements on single line (#20633)
Closes #11216 Essentially the approach is to implement `Format` for a new struct `FormatClause` which is just a clause header _and_ its body. We then have the information we need to see whether there is a skip suppression comment on the last child in the body and it all fits on one line.
1 parent 8dad289 commit 62343a1

File tree

14 files changed

+986
-319
lines changed

14 files changed

+986
-319
lines changed

‎crates/ruff_python_formatter/resources/test/fixtures/ruff/fmt_skip/compound_one_liners.py‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
# Test cases for fmt: skip on compound statements that fit on one line
2+
3+
# Basic single-line compound statements
4+
def simple_func(): return "hello" # fmt: skip
5+
if True: print("condition met") # fmt: skip
6+
for i in range(5): print(i) # fmt: skip
7+
while x < 10: x += 1 # fmt: skip
8+
9+
# With expressions that would normally trigger formatting
10+
def long_params(a, b, c, d, e, f, g): return a + b + c + d + e + f + g # fmt: skip
11+
if some_very_long_condition_that_might_wrap: do_something_else_that_is_long() # fmt: skip
12+
13+
# Nested compound statements (outer should be preserved)
14+
if True:
15+
for i in range(10): print(i) # fmt: skip
16+
17+
# Multiple statements in body (should not apply - multiline)
18+
if True:
19+
x = 1
20+
y = 2 # fmt: skip
21+
22+
# With decorators - decorated function on one line
23+
@overload
24+
def decorated_func(x: int) -> str: return str(x) # fmt: skip
25+
26+
@property
27+
def prop_method(self): return self._value # fmt: skip
28+
29+
# Class definitions on one line
30+
class SimpleClass: pass # fmt: skip
31+
class GenericClass(Generic[T]): pass # fmt: skip
32+
33+
# Try/except blocks
34+
try: risky_operation() # fmt: skip
35+
except ValueError: handle_error() # fmt: skip
36+
except: handle_any_error() # fmt: skip
37+
else: success_case() # fmt: skip
38+
finally: cleanup() # fmt: skip
39+
40+
# Match statements (Python 3.10+)
41+
match value:
42+
case 1: print("one") # fmt: skip
43+
case _: print("other") # fmt: skip
44+
45+
# With statements
46+
with open("file.txt") as f: content = f.read() # fmt: skip
47+
with context_manager() as cm: result = cm.process() # fmt: skip
48+
49+
# Async variants
50+
async def async_func(): return await some_call() # fmt: skip
51+
async for item in async_iterator(): await process(item) # fmt: skip
52+
async with async_context() as ctx: await ctx.work() # fmt: skip
53+
54+
# Complex expressions that would normally format
55+
def complex_expr(): return [x for x in range(100) if x % 2 == 0 and x > 50] # fmt: skip
56+
if condition_a and condition_b or (condition_c and not condition_d): execute_complex_logic() # fmt: skip
57+
58+
# Edge case: comment positioning
59+
def func_with_comment(): # some comment
60+
return "value" # fmt: skip
61+
62+
# Edge case: multiple fmt: skip (only last one should matter)
63+
def multiple_skip(): return "test" # fmt: skip # fmt: skip
64+
65+
# Should NOT be affected (already multiline)
66+
def multiline_func():
67+
return "this should format normally"
68+
69+
if long_condition_that_spans \
70+
and continues_on_next_line:
71+
print("multiline condition")
72+
73+
# Mix of skipped and non-skipped
74+
for i in range(10): print(f"item {i}") # fmt: skip
75+
for j in range(5):
76+
print(f"formatted item {j}")
77+
78+
# With trailing comma that would normally be removed
79+
def trailing_comma_func(a, b, c,): return a + b + c # fmt: skip
80+
81+
# Dictionary/list comprehensions
82+
def dict_comp(): return {k: v for k, v in items.items() if v is not None} # fmt: skip
83+
def list_comp(): return [x * 2 for x in numbers if x > threshold_value] # fmt: skip
84+
85+
# Lambda in one-liner
86+
def with_lambda(): return lambda x, y, z: x + y + z if all([x, y, z]) else None # fmt: skip
87+
88+
# String formatting that would normally be reformatted
89+
def format_string(): return f"Hello {name}, you have {count} items in your cart totaling ${total:.2f}" # fmt: skip
90+
91+
# loop else clauses
92+
for i in range(2): print(i) # fmt: skip
93+
else: print("this") # fmt: skip
94+
95+
96+
while foo(): print(i) # fmt: skip
97+
else: print("this") # fmt: skip
98+
99+
# again but only the first skip
100+
for i in range(2): print(i) # fmt: skip
101+
else: print("this")
102+
103+
104+
while foo(): print(i) # fmt: skip
105+
else: print("this")
106+
107+
# again but only the second skip
108+
for i in range(2): print(i)
109+
else: print("this") # fmt: skip
110+
111+
112+
while foo(): print(i)
113+
else: print("this") # fmt: skip
114+
115+
# multiple statements in body
116+
if True: print("this"); print("that") # fmt: skip
117+
118+
# Examples with more comments
119+
120+
try: risky_operation() # fmt: skip
121+
# leading 1
122+
except ValueError: handle_error() # fmt: skip
123+
# leading 2
124+
except: handle_any_error() # fmt: skip
125+
# leading 3
126+
else: success_case() # fmt: skip
127+
# leading 4
128+
finally: cleanup() # fmt: skip
129+
# trailing
130+
131+
# multi-line before colon (should remain as is)
132+
if (
133+
long_condition
134+
): a + b # fmt: skip
135+
136+
# over-indented comment example
137+
# See https://github.com/astral-sh/ruff/pull/20633#issuecomment-3453288910
138+
# and https://github.com/astral-sh/ruff/pull/21185
139+
140+
for x in it: foo()
141+
# comment
142+
else: bar() # fmt: skip
143+
144+
145+
if this(
146+
'is a long',
147+
# commented
148+
'condition'
149+
): with_a_skip # fmt: skip

‎crates/ruff_python_formatter/src/other/except_handler_except_handler.rs‎

Lines changed: 58 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::expression::maybe_parenthesize_expression;
77
use crate::expression::parentheses::Parenthesize;
88
use crate::prelude::*;
99
use crate::preview::is_remove_parens_around_except_types_enabled;
10-
use crate::statement::clause::{ClauseHeader, clause_body, clause_header};
10+
use crate::statement::clause::{ClauseHeader, clause};
1111
use crate::statement::suite::SuiteKind;
1212

1313
#[derive(Copy, Clone, Default)]
@@ -55,77 +55,68 @@ impl FormatNodeRule<ExceptHandlerExceptHandler> for FormatExceptHandlerExceptHan
5555

5656
write!(
5757
f,
58-
[
59-
clause_header(
60-
ClauseHeader::ExceptHandler(item),
61-
dangling_comments,
62-
&format_with(|f: &mut PyFormatter| {
63-
write!(
64-
f,
65-
[
66-
token("except"),
67-
match except_handler_kind {
68-
ExceptHandlerKind::Regular => None,
69-
ExceptHandlerKind::Starred => Some(token("*")),
70-
}
71-
]
72-
)?;
58+
[clause(
59+
ClauseHeader::ExceptHandler(item),
60+
&format_with(|f: &mut PyFormatter| {
61+
write!(
62+
f,
63+
[
64+
token("except"),
65+
match except_handler_kind {
66+
ExceptHandlerKind::Regular => None,
67+
ExceptHandlerKind::Starred => Some(token("*")),
68+
}
69+
]
70+
)?;
7371

74-
match type_.as_deref() {
75-
// For tuples of exception types without an `as` name and on 3.14+, the
76-
// parentheses are optional.
77-
//
78-
// ```py
79-
// try:
80-
// ...
81-
// except BaseException, Exception: # Ok
82-
// ...
83-
// ```
84-
Some(Expr::Tuple(tuple))
85-
if f.options().target_version() >= PythonVersion::PY314
86-
&& is_remove_parens_around_except_types_enabled(
87-
f.context(),
72+
match type_.as_deref() {
73+
// For tuples of exception types without an `as` name and on 3.14+, the
74+
// parentheses are optional.
75+
//
76+
// ```py
77+
// try:
78+
// ...
79+
// except BaseException, Exception: # Ok
80+
// ...
81+
// ```
82+
Some(Expr::Tuple(tuple))
83+
if f.options().target_version() >= PythonVersion::PY314
84+
&& is_remove_parens_around_except_types_enabled(f.context())
85+
&& name.is_none() =>
86+
{
87+
write!(
88+
f,
89+
[
90+
space(),
91+
tuple.format().with_options(TupleParentheses::NeverPreserve)
92+
]
93+
)?;
94+
}
95+
Some(type_) => {
96+
write!(
97+
f,
98+
[
99+
space(),
100+
maybe_parenthesize_expression(
101+
type_,
102+
item,
103+
Parenthesize::IfBreaks
88104
)
89-
&& name.is_none() =>
90-
{
91-
write!(
92-
f,
93-
[
94-
space(),
95-
tuple
96-
.format()
97-
.with_options(TupleParentheses::NeverPreserve)
98-
]
99-
)?;
100-
}
101-
Some(type_) => {
102-
write!(
103-
f,
104-
[
105-
space(),
106-
maybe_parenthesize_expression(
107-
type_,
108-
item,
109-
Parenthesize::IfBreaks
110-
)
111-
]
112-
)?;
113-
if let Some(name) = name {
114-
write!(f, [space(), token("as"), space(), name.format()])?;
115-
}
105+
]
106+
)?;
107+
if let Some(name) = name {
108+
write!(f, [space(), token("as"), space(), name.format()])?;
116109
}
117-
_ => {}
118110
}
111+
_ => {}
112+
}
119113

120-
Ok(())
121-
}),
122-
),
123-
clause_body(
124-
body,
125-
SuiteKind::other(self.last_suite_in_statement),
126-
dangling_comments
127-
),
128-
]
114+
Ok(())
115+
}),
116+
dangling_comments,
117+
body,
118+
SuiteKind::other(self.last_suite_in_statement),
119+
)]
129120
)
130121
}
131122
}

‎crates/ruff_python_formatter/src/other/match_case.rs‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::expression::maybe_parenthesize_expression;
55
use crate::expression::parentheses::Parenthesize;
66
use crate::pattern::maybe_parenthesize_pattern;
77
use crate::prelude::*;
8-
use crate::statement::clause::{ClauseHeader, clause_body, clause_header};
8+
use crate::statement::clause::{ClauseHeader, clause};
99
use crate::statement::suite::SuiteKind;
1010

1111
#[derive(Default)]
@@ -46,23 +46,18 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
4646

4747
write!(
4848
f,
49-
[
50-
clause_header(
51-
ClauseHeader::MatchCase(item),
52-
dangling_item_comments,
53-
&format_args![
54-
token("case"),
55-
space(),
56-
maybe_parenthesize_pattern(pattern, item),
57-
format_guard
58-
],
59-
),
60-
clause_body(
61-
body,
62-
SuiteKind::other(self.last_suite_in_statement),
63-
dangling_item_comments
64-
),
65-
]
49+
[clause(
50+
ClauseHeader::MatchCase(item),
51+
&format_args![
52+
token("case"),
53+
space(),
54+
maybe_parenthesize_pattern(pattern, item),
55+
format_guard
56+
],
57+
dangling_item_comments,
58+
body,
59+
SuiteKind::other(self.last_suite_in_statement),
60+
)]
6661
)
6762
}
6863
}

0 commit comments

Comments
 (0)