Fix lambda body formatting for multiline calls and subscripts#23866
Fix lambda body formatting for multiline calls and subscripts#23866
Conversation
rather than special-casing fluent chains
|
MichaReiser
left a comment
There was a problem hiding this comment.
Thanks for looking into this. The change makes sense to me and relying on needs_parantheses is the right approach. The only thing that makes me a little hesitant is that I have a hard time judging the ecosystem impact.
What could be helpful is to try to come up with a few more tests (Claude might be helpful here). What other cases were affected by the same bug before your fix? Which cases should remain unchanged?
Can you think of cases where the new IR might result in different formatting than before because we no longer use best fitting? Specifically, are there cases where we'll now parenthesize the entire expression where we only parenthesized the call expression before?
|
Yep, makes sense! I was a bit disappointed to see an empty ecosystem report again. Claude unfortunately hasn't been too helpful yet, but I'll keep thinking about it. I think we're looking for cases that are still |
|
I pushed a couple of somewhat tricky cases for other expression types inside of calls, but these are handled fine on main. After looking through the branches in |
|
Sounds good. Thanks for taking the time to assess the impact of this change. An example like this (specifically testing the different break points) might be good to verify: foo = lambda: call()(extra_argument_one, extra_argument_two, extra_argument_three) |
Summary
This PR fixes #23851 by expanding the special case for fluent call chains that previously used
parenthesize_if_expandsto include any calls or subscripts withOptionalParentheses::Multiline. The minimal change is in the third commit (just swapping out the fluent chain check formatches!(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