Skip to content

Commit 37cdd61

Browse files
ntBrentBre
authored
Fix lambda body formatting for multiline calls and subscripts (#23866)
## Summary This PR fixes #23851 by expanding the special case for fluent call chains that previously used `parenthesize_if_expands` to include any calls or subscripts with `OptionalParentheses::Multiline`. The minimal change is in the third commit (just swapping out the fluent chain check for `matches!(needs_parentheses, OptionalParentheses::Multiline)`), but then I reordered the conditional branches in a way that seemed to make more sense. ## Test Plan New tests based on the issue
1 parent a25a4df commit 37cdd61

3 files changed

Lines changed: 147 additions & 33 deletions

File tree

‎crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/lambda.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,3 +813,29 @@ def test_string_temporal_compare_between(con, op, left, right): ...
813813
lambda # dangling header comment
814814
: (x := 1)
815815
)
816+
817+
818+
# Regression tests for https://github.com/astral-sh/ruff/issues/23851
819+
foo = lambda x: 'hello this is a really long string that then get concatenated ' 'with another string that prints {x!r}'
820+
821+
foo = lambda x: 'hello this is a really long string that then get concatenated ' 'with another string that prints {x!r}'.format(x=x)
822+
823+
foo = lambda x: 'hello this is a really long string that then get concatenated ' 'with another string that prints {x!r}'[x]
824+
825+
foo = lambda x: ('hello this is a really long string that then get concatenated ' and 'with another string that prints {x!r}').foo()
826+
827+
foo = lambda: (result := some_really_long_callable_expression)(extra_argument_one, extra_argument_two)
828+
829+
foo = lambda: (result := some_really_long_subscriptable_expression)[extra_long_index_expression]
830+
831+
foo = lambda: (await some_coroutine)(extra_argument_one, extra_argument_two, extra_argument_three)
832+
833+
foo = lambda: call()(extra_argument_one, extra_argument_two, extra_argument_threeeeeeeee)
834+
835+
foo = lambda: call()(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
836+
837+
foo = lambda: call(argument_one,)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
838+
839+
foo = lambda: call(argument_one, argument_two, argument_threeeeeeeeeeeeeeeeeeeeeeeeeeee)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
840+
841+
foo = lambda: callllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll(argument_one, argument_two, argument_threeeeeeeeeeeeeeeeeeeeeeeeeeee)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)

‎crates/ruff_python_formatter/src/expression/expr_lambda.rs‎

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ use ruff_text_size::Ranged;
44

55
use crate::builders::parenthesize_if_expands;
66
use crate::comments::{SourceComment, dangling_comments, leading_comments, trailing_comments};
7+
use crate::expression::has_own_parentheses;
78
use crate::expression::parentheses::{
89
NeedsParentheses, OptionalParentheses, Parentheses, is_expression_parenthesized,
910
};
10-
use crate::expression::{CallChainLayout, has_own_parentheses};
1111
use crate::other::parameters::ParametersParentheses;
1212
use crate::prelude::*;
1313

@@ -259,6 +259,7 @@ struct FormatBody<'a> {
259259
}
260260

261261
impl Format<PyFormatContext<'_>> for FormatBody<'_> {
262+
#[expect(clippy::if_same_then_else)]
262263
fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> {
263264
let FormatBody {
264265
dangling_header_comments,
@@ -358,6 +359,24 @@ impl Format<PyFormatContext<'_>> for FormatBody<'_> {
358359
else if body_comments.has_leading() || body_comments.has_trailing_own_line() {
359360
body.format().with_options(Parentheses::Always).fmt(f)
360361
}
362+
// Include parentheses for cases that always require them, such as named expressions:
363+
//
364+
// ```py
365+
// lambda x: (y := x + 1)
366+
// ```
367+
else if matches!(needs_parentheses, OptionalParentheses::Always) {
368+
body.format().with_options(Parentheses::Always).fmt(f)
369+
}
370+
// Use `parenthesize_if_expands` for cases that require parentheses when broken over
371+
// multiple lines, including some calls and subscripts:
372+
//
373+
// ```py
374+
// lambda x: "implicitly" "concatenated {x}".format(x)
375+
// lambda x: "implicitly" "concatenated {x}"[x]
376+
// ```
377+
else if matches!(needs_parentheses, OptionalParentheses::Multiline) {
378+
parenthesize_if_expands(&body.format().with_options(Parentheses::Never)).fmt(f)
379+
}
361380
// Calls and subscripts require special formatting because they have their own
362381
// parentheses, but they can also have an arbitrary amount of text before the
363382
// opening parenthesis. We want to avoid cases where we keep a long callable on the
@@ -387,31 +406,20 @@ impl Format<PyFormatContext<'_>> for FormatBody<'_> {
387406
// )
388407
// ```
389408
else if matches!(body, Expr::Call(_) | Expr::Subscript(_)) {
390-
let unparenthesized = body.format().with_options(Parentheses::Never);
391-
if CallChainLayout::from_expression(
392-
body.into(),
393-
comments.ranges(),
394-
f.context().source(),
395-
)
396-
.is_fluent()
397-
{
398-
parenthesize_if_expands(&unparenthesized).fmt(f)
399-
} else {
400-
let unparenthesized = unparenthesized.memoized();
401-
if unparenthesized.inspect(f)?.will_break() {
402-
expand_parent().fmt(f)?;
403-
}
404-
405-
best_fitting![
406-
// body all flat
407-
unparenthesized,
408-
// body expanded
409-
group(&unparenthesized).should_expand(true),
410-
// parenthesized
411-
format_args![token("("), block_indent(&unparenthesized), token(")")]
412-
]
413-
.fmt(f)
409+
let unparenthesized = body.format().with_options(Parentheses::Never).memoized();
410+
if unparenthesized.inspect(f)?.will_break() {
411+
expand_parent().fmt(f)?;
414412
}
413+
414+
best_fitting![
415+
// body all flat
416+
unparenthesized,
417+
// body expanded
418+
group(&unparenthesized).should_expand(true),
419+
// parenthesized
420+
format_args![token("("), block_indent(&unparenthesized), token(")")]
421+
]
422+
.fmt(f)
415423
}
416424
// For other cases with their own parentheses, such as lists, sets, dicts, tuples,
417425
// etc., we can just format the body directly. Their own formatting results in the
@@ -433,14 +441,6 @@ impl Format<PyFormatContext<'_>> for FormatBody<'_> {
433441
else if has_own_parentheses(body, f.context()).is_some() {
434442
body.format().fmt(f)
435443
}
436-
// Include parentheses for cases that always require them, such as named expressions:
437-
//
438-
// ```py
439-
// lambda x: (y := x + 1)
440-
// ```
441-
else if matches!(needs_parentheses, OptionalParentheses::Always) {
442-
body.format().with_options(Parentheses::Always).fmt(f)
443-
}
444444
// Finally, for expressions without their own parentheses, use
445445
// `parenthesize_if_expands` to add parentheses around the body, only if it expands
446446
// across multiple lines. The `Parentheses::Never` here also removes unnecessary

‎crates/ruff_python_formatter/tests/snapshots/format@expression__lambda.py.snap‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,32 @@ lambda x: (
818818
lambda # dangling header comment
819819
: (x := 1)
820820
)
821+
822+
823+
# Regression tests for https://github.com/astral-sh/ruff/issues/23851
824+
foo = lambda x: 'hello this is a really long string that then get concatenated ' 'with another string that prints {x!r}'
825+
826+
foo = lambda x: 'hello this is a really long string that then get concatenated ' 'with another string that prints {x!r}'.format(x=x)
827+
828+
foo = lambda x: 'hello this is a really long string that then get concatenated ' 'with another string that prints {x!r}'[x]
829+
830+
foo = lambda x: ('hello this is a really long string that then get concatenated ' and 'with another string that prints {x!r}').foo()
831+
832+
foo = lambda: (result := some_really_long_callable_expression)(extra_argument_one, extra_argument_two)
833+
834+
foo = lambda: (result := some_really_long_subscriptable_expression)[extra_long_index_expression]
835+
836+
foo = lambda: (await some_coroutine)(extra_argument_one, extra_argument_two, extra_argument_three)
837+
838+
foo = lambda: call()(extra_argument_one, extra_argument_two, extra_argument_threeeeeeeee)
839+
840+
foo = lambda: call()(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
841+
842+
foo = lambda: call(argument_one,)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
843+
844+
foo = lambda: call(argument_one, argument_two, argument_threeeeeeeeeeeeeeeeeeeeeeeeeeee)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
845+
846+
foo = lambda: callllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll(argument_one, argument_two, argument_threeeeeeeeeeeeeeeeeeeeeeeeeeee)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
821847
```
822848

823849
## Output
@@ -1680,6 +1706,68 @@ lambda x: (x := 1)
16801706
x := 1
16811707
)
16821708
)
1709+
1710+
1711+
# Regression tests for https://github.com/astral-sh/ruff/issues/23851
1712+
foo = lambda x: (
1713+
"hello this is a really long string that then get concatenated "
1714+
"with another string that prints {x!r}"
1715+
)
1716+
1717+
foo = lambda x: (
1718+
"hello this is a really long string that then get concatenated "
1719+
"with another string that prints {x!r}".format(x=x)
1720+
)
1721+
1722+
foo = lambda x: (
1723+
"hello this is a really long string that then get concatenated "
1724+
"with another string that prints {x!r}"[x]
1725+
)
1726+
1727+
foo = lambda x: (
1728+
"hello this is a really long string that then get concatenated "
1729+
and "with another string that prints {x!r}"
1730+
).foo()
1731+
1732+
foo = lambda: (result := some_really_long_callable_expression)(
1733+
extra_argument_one, extra_argument_two
1734+
)
1735+
1736+
foo = lambda: (result := some_really_long_subscriptable_expression)[
1737+
extra_long_index_expression
1738+
]
1739+
1740+
foo = lambda: (await some_coroutine)(
1741+
extra_argument_one, extra_argument_two, extra_argument_three
1742+
)
1743+
1744+
foo = lambda: call()(
1745+
extra_argument_one, extra_argument_two, extra_argument_threeeeeeeee
1746+
)
1747+
1748+
foo = lambda: call()(
1749+
extra_argument_one,
1750+
extra_argument_twooooooooooooooooooo,
1751+
extra_argument_threeeeeeeee,
1752+
)
1753+
1754+
foo = lambda: call(
1755+
argument_one,
1756+
)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
1757+
1758+
foo = lambda: call(
1759+
argument_one, argument_two, argument_threeeeeeeeeeeeeeeeeeeeeeeeeeee
1760+
)(extra_argument_one, extra_argument_twooooooooooooooooooo, extra_argument_threeeeeeeee)
1761+
1762+
foo = lambda: (
1763+
callllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllllll(
1764+
argument_one, argument_two, argument_threeeeeeeeeeeeeeeeeeeeeeeeeeee
1765+
)(
1766+
extra_argument_one,
1767+
extra_argument_twooooooooooooooooooo,
1768+
extra_argument_threeeeeeeee,
1769+
)
1770+
)
16831771
```
16841772

16851773

0 commit comments

Comments
 (0)