Fix attribute chain own line comments (#5340)

## Motation

Previously,
```python
x = (
    a1
    .a2
    # a
    .  # b
    # c
    a3
)
```
got formatted as
```python
x = a1.a2
# a
.  # b
# c
a3
```
which is invalid syntax. This fixes that.

## Summary

This implements a basic form of attribute chaining
(<https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#call-chains>)
by checking if any inner attribute access contains an own line comment,
and if this is the case, adds parentheses around the outermost attribute
access while disabling parentheses for all inner attribute expressions.
We want to replace this with an implementation that uses recursion or a
stack while formatting instead of in `needs_parentheses` and also
includes calls rather sooner than later, but i'm fixing this now because
i'm uncomfortable with having known invalid syntax generation in the
formatter.

## Test Plan

I added new fixtures.
This commit is contained in:
konstin 2023-06-26 11:13:07 +02:00 committed by GitHub
parent 8879927b9a
commit a52cd47c7f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 258 additions and 18 deletions

View file

@ -1077,7 +1077,19 @@ fn handle_attribute_comment<'a>(
.expect("Expected the `.` character after the value");
if TextRange::new(dot.end(), attribute.attr.start()).contains(comment.slice().start()) {
CommentPlacement::dangling(attribute.into(), comment)
if comment.line_position().is_end_of_line() {
// Attach to node with b
// ```python
// x322 = (
// a
// . # end-of-line dot comment 2
// b
// )
// ```
CommentPlacement::trailing(comment.enclosing_node(), comment)
} else {
CommentPlacement::dangling(attribute.into(), comment)
}
} else {
CommentPlacement::Default(comment)
}

View file

@ -27,25 +27,28 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
})
);
if needs_parentheses {
value.format().with_options(Parenthesize::Always).fmt(f)?;
} else {
value.format().fmt(f)?;
}
let comments = f.context().comments().clone();
if comments.has_trailing_own_line_comments(value.as_ref()) {
hard_line_break().fmt(f)?;
}
let dangling_comments = comments.dangling_comments(item);
let leading_attribute_comments_start =
dangling_comments.partition_point(|comment| comment.line_position().is_end_of_line());
let (trailing_dot_comments, leading_attribute_comments) =
dangling_comments.split_at(leading_attribute_comments_start);
if needs_parentheses {
value.format().with_options(Parenthesize::Always).fmt(f)?;
} else if let Expr::Attribute(expr_attribute) = value.as_ref() {
// We're in a attribute chain (`a.b.c`). The outermost node adds parentheses if
// required, the inner ones don't need them so we skip the `Expr` formatting that
// normally adds the parentheses.
expr_attribute.format().fmt(f)?;
} else {
value.format().fmt(f)?;
}
if comments.has_trailing_own_line_comments(value.as_ref()) {
hard_line_break().fmt(f)?;
}
write!(
f,
[
@ -68,6 +71,28 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
}
}
/// Checks if there are any own line comments in an attribute chain (a.b.c). This method is
/// recursive up to the innermost expression that the attribute chain starts behind.
fn has_breaking_comments_attribute_chain(
expr_attribute: &ExprAttribute,
comments: &Comments,
) -> bool {
if comments
.dangling_comments(expr_attribute)
.iter()
.any(|comment| comment.line_position().is_own_line())
|| comments.has_trailing_own_line_comments(expr_attribute)
{
return true;
}
if let Expr::Attribute(inner) = expr_attribute.value.as_ref() {
return has_breaking_comments_attribute_chain(inner, comments);
}
return comments.has_trailing_own_line_comments(expr_attribute.value.as_ref());
}
impl NeedsParentheses for ExprAttribute {
fn needs_parentheses(
&self,
@ -75,6 +100,10 @@ impl NeedsParentheses for ExprAttribute {
source: &str,
comments: &Comments,
) -> Parentheses {
if has_breaking_comments_attribute_chain(self, comments) {
return Parentheses::Always;
}
match default_expression_needs_parentheses(self.into(), parenthesize, source, comments) {
Parentheses::Optional => Parentheses::Never,
parentheses => parentheses,