Skip to content

Commit

Permalink
Avoid omitting parentheses for trailing attributes on call expressions (
Browse files Browse the repository at this point in the history
astral-sh#6322)

## Summary

This PR modifies our `can_omit_optional_parentheses` rules to ensure
that if we see a call followed by an attribute, we treat that as an
attribute access rather than a splittable call expression.

This in turn ensures that we wrap like:

```python
ct_match = aaaaaaaaaaact_id == self.get_content_type(
    obj=rel_obj, using=instance._state.db
)
```

For calls, but:

```python
ct_match = (
    aaaaaaaaaaact_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)
```

For calls with trailing attribute accesses.

Closes astral-sh#6065.

## Test Plan

Similarity index before:

- `zulip`: 0.99436
- `django`: 0.99779
- `warehouse`: 0.99504
- `transformers`: 0.99403
- `cpython`: 0.75912
- `typeshed`: 0.72293

And after:

- `zulip`: 0.99436
- `django`: 0.99780
- `warehouse`: 0.99504
- `transformers`: 0.99404
- `cpython`: 0.75913
- `typeshed`: 0.72293
  • Loading branch information
charliermarsh authored and durumu committed Aug 12, 2023
1 parent 4148d34 commit c5898cc
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,55 @@
>= c
)
]

def f():
return (
unicodedata.normalize("NFKC", s1).casefold()
== unicodedata.normalize("NFKC", s2).casefold()
)

# Call expressions with trailing attributes.

ct_match = (
aaaaaaaaaaact_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)

ct_match = (
{aaaaaaaaaaaaaaaa} == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)

ct_match = (
(aaaaaaaaaaaaaaaa) == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)

ct_match = aaaaaaaaaaact_id == self.get_content_type(
obj=rel_obj, using=instance._state.db
)

# Call expressions with trailing subscripts.

ct_match = (
aaaaaaaaaaact_id == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
)

ct_match = (
{aaaaaaaaaaaaaaaa} == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
)

ct_match = (
(aaaaaaaaaaaaaaaa) == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
)

# Subscripts expressions with trailing attributes.

ct_match = (
aaaaaaaaaaact_id == self.get_content_type[obj, rel_obj, using, instance._state.db].id
)

ct_match = (
{aaaaaaaaaaaaaaaa} == self.get_content_type[obj, rel_obj, using, instance._state.db].id
)

ct_match = (
(aaaaaaaaaaaaaaaa) == self.get_content_type[obj, rel_obj, using, instance._state.db].id
)
6 changes: 4 additions & 2 deletions crates/ruff_python_formatter/src/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,9 @@ impl Format<PyFormatContext<'_>> for MaybeParenthesizeExpression<'_> {
Parenthesize::Optional | Parenthesize::IfBreaks => needs_parentheses,
};

let can_omit_optional_parentheses = can_omit_optional_parentheses(expression, f.context());
match needs_parentheses {
OptionalParentheses::Multiline if *parenthesize != Parenthesize::IfRequired => {
if can_omit_optional_parentheses {
if can_omit_optional_parentheses(expression, f.context()) {
optional_parentheses(&expression.format().with_options(Parentheses::Never))
.fmt(f)
} else {
Expand Down Expand Up @@ -407,9 +406,12 @@ impl<'input> CanOmitOptionalParenthesesVisitor<'input> {
attr: _,
ctx: _,
}) => {
self.visit_expr(value);
if has_parentheses(value, self.source) {
self.update_max_priority(OperatorPriority::Attribute);
}
self.last = Some(expr);
return;
}

Expr::NamedExpr(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,58 @@ return 1 == 2 and (
>= c
)
]
def f():
return (
unicodedata.normalize("NFKC", s1).casefold()
== unicodedata.normalize("NFKC", s2).casefold()
)
# Call expressions with trailing attributes.
ct_match = (
aaaaaaaaaaact_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)
ct_match = (
{aaaaaaaaaaaaaaaa} == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)
ct_match = (
(aaaaaaaaaaaaaaaa) == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)
ct_match = aaaaaaaaaaact_id == self.get_content_type(
obj=rel_obj, using=instance._state.db
)
# Call expressions with trailing subscripts.
ct_match = (
aaaaaaaaaaact_id == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
)
ct_match = (
{aaaaaaaaaaaaaaaa} == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
)
ct_match = (
(aaaaaaaaaaaaaaaa) == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
)
# Subscripts expressions with trailing attributes.
ct_match = (
aaaaaaaaaaact_id == self.get_content_type[obj, rel_obj, using, instance._state.db].id
)
ct_match = (
{aaaaaaaaaaaaaaaa} == self.get_content_type[obj, rel_obj, using, instance._state.db].id
)
ct_match = (
(aaaaaaaaaaaaaaaa) == self.get_content_type[obj, rel_obj, using, instance._state.db].id
)
```
## Output
Expand Down Expand Up @@ -171,6 +223,60 @@ return 1 == 2 and (
>= c
)
]
def f():
return unicodedata.normalize("NFKC", s1).casefold() == unicodedata.normalize(
"NFKC", s2
).casefold()
# Call expressions with trailing attributes.
ct_match = (
aaaaaaaaaaact_id == self.get_content_type(obj=rel_obj, using=instance._state.db).id
)
ct_match = {aaaaaaaaaaaaaaaa} == self.get_content_type(
obj=rel_obj, using=instance._state.db
).id
ct_match = (aaaaaaaaaaaaaaaa) == self.get_content_type(
obj=rel_obj, using=instance._state.db
).id
ct_match = aaaaaaaaaaact_id == self.get_content_type(
obj=rel_obj, using=instance._state.db
)
# Call expressions with trailing subscripts.
ct_match = (
aaaaaaaaaaact_id == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
)
ct_match = {
aaaaaaaaaaaaaaaa
} == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
ct_match = (
aaaaaaaaaaaaaaaa
) == self.get_content_type(obj=rel_obj, using=instance._state.db)[id]
# Subscripts expressions with trailing attributes.
ct_match = (
aaaaaaaaaaact_id
== self.get_content_type[obj, rel_obj, using, instance._state.db].id
)
ct_match = {
aaaaaaaaaaaaaaaa
} == self.get_content_type[obj, rel_obj, using, instance._state.db].id
ct_match = (
aaaaaaaaaaaaaaaa
) == self.get_content_type[obj, rel_obj, using, instance._state.db].id
```
Expand Down

0 comments on commit c5898cc

Please sign in to comment.