Avoiding grouping comprehension targets separately from conditions (#7429)

## Summary

Black seems to treat comprehension targets and conditions as if they're
in a single group -- so if the comprehension expands, all conditions are
rendered on their own line, etc.

Closes https://github.com/astral-sh/ruff/issues/7421.

## Test Plan

`cargo test`

No change in any of the similarity metrics.

Before:

| project | similarity index | total files | changed files |

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99982 | 2760 | 37 |
| transformers | 0.99957 | 2587 | 399 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99983 | 3496 | 18 |
| warehouse | 0.99923 | 648 | 18 |
| zulip | 0.99962 | 1437 | 22 |

After:

| project | similarity index | total files | changed files |

|--------------|------------------:|------------------:|------------------:|
| cpython | 0.76083 | 1789 | 1632 |
| django | 0.99982 | 2760 | 37 |
| transformers | 0.99957 | 2587 | 399 |
| twine | 1.00000 | 33 | 0 |
| typeshed | 0.99983 | 3496 | 18 |
| warehouse | 0.99923 | 648 | 18 |
| zulip | 0.99962 | 1437 | 22 |
This commit is contained in:
Charlie Marsh 2023-09-16 13:19:34 -04:00 committed by GitHub
parent 22770fb4be
commit 7e2eba2592
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 89 additions and 13 deletions

View file

@ -73,3 +73,25 @@ a = [
aaaaaaaaaaaaaaaaaaaaa = [ aaaaaaaaaaaaaaaaaaaaa = [
o for o in self.registry.values if o.__class__ is not ModelAdmin o for o in self.registry.values if o.__class__ is not ModelAdmin
] ]
[
task
for head_name in head_nodes
if (task := self.get_task_by_name(head_name))
if not None
]
[
f(x, y,)
for head_name in head_nodes if head_name
]
[
f(x, y)
for head_name in f(x, y,) if head_name
]
[
x
for (x, y,) in z if head_name
]

View file

@ -56,16 +56,14 @@ impl FormatNodeRule<Comprehension> for FormatComprehension {
[ [
token("for"), token("for"),
trailing_comments(before_target_comments), trailing_comments(before_target_comments),
group(&format_args!( Spacer(target),
Spacer(target), ExprTupleWithoutParentheses(target),
ExprTupleWithoutParentheses(target), in_spacer,
in_spacer, leading_comments(before_in_comments),
leading_comments(before_in_comments), token("in"),
token("in"), trailing_comments(trailing_in_comments),
trailing_comments(trailing_in_comments), Spacer(iter),
Spacer(iter), iter.format(),
iter.format(),
)),
] ]
)?; )?;
if !ifs.is_empty() { if !ifs.is_empty() {
@ -79,18 +77,18 @@ impl FormatNodeRule<Comprehension> for FormatComprehension {
dangling_if_comments dangling_if_comments
.partition_point(|comment| comment.line_position().is_own_line()), .partition_point(|comment| comment.line_position().is_own_line()),
); );
joiner.entry(&group(&format_args!( joiner.entry(&format_args!(
leading_comments(own_line_if_comments), leading_comments(own_line_if_comments),
token("if"), token("if"),
trailing_comments(end_of_line_if_comments), trailing_comments(end_of_line_if_comments),
Spacer(if_case), Spacer(if_case),
if_case.format(), if_case.format(),
))); ));
} }
joiner.finish() joiner.finish()
}); });
write!(f, [soft_line_break_or_space(), group(&joined)])?; write!(f, [soft_line_break_or_space(), joined])?;
} }
Ok(()) Ok(())
} }

View file

@ -79,6 +79,28 @@ a = [
aaaaaaaaaaaaaaaaaaaaa = [ aaaaaaaaaaaaaaaaaaaaa = [
o for o in self.registry.values if o.__class__ is not ModelAdmin o for o in self.registry.values if o.__class__ is not ModelAdmin
] ]
[
task
for head_name in head_nodes
if (task := self.get_task_by_name(head_name))
if not None
]
[
f(x, y,)
for head_name in head_nodes if head_name
]
[
f(x, y)
for head_name in f(x, y,) if head_name
]
[
x
for (x, y,) in z if head_name
]
``` ```
## Output ## Output
@ -178,6 +200,40 @@ a = [
aaaaaaaaaaaaaaaaaaaaa = [ aaaaaaaaaaaaaaaaaaaaa = [
o for o in self.registry.values if o.__class__ is not ModelAdmin o for o in self.registry.values if o.__class__ is not ModelAdmin
] ]
[
task
for head_name in head_nodes
if (task := self.get_task_by_name(head_name))
if not None
]
[
f(
x,
y,
)
for head_name in head_nodes
if head_name
]
[
f(x, y)
for head_name in f(
x,
y,
)
if head_name
]
[
x
for (
x,
y,
) in z
if head_name
]
``` ```