Skip to content

Commit

Permalink
Fix unstable formatting of trailing end-of-line comments of parenthes…
Browse files Browse the repository at this point in the history
…ized attribute values (#16187)
  • Loading branch information
MichaReiser authored Feb 18, 2025
1 parent 82eae51 commit 31180a8
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,20 @@
f(111111111111111111111111111111111111111111111111111111111111111111111111111111111)
+ 1
).bit_length()


# Regression test for https://github.com/astral-sh/ruff/issues/16151
result = (
(await query_the_thing(mypy_doesnt_understand)) # type: ignore[x]
.foo()
.bar()
)

(
(
a # trailing end-of-line
# trailing own-line
) # trailing closing parentheses
# dangling before dot
.b # trailing end-of-line
)
6 changes: 2 additions & 4 deletions crates/ruff_python_formatter/src/comments/placement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,10 +1391,8 @@ fn handle_attribute_comment<'a>(
.take_while(|token| token.kind == SimpleTokenKind::RParen)
.last()
{
return if comment.start() < right_paren.start() {
CommentPlacement::trailing(attribute.value.as_ref(), comment)
} else {
CommentPlacement::dangling(comment.enclosing_node(), comment)
if comment.start() < right_paren.start() {
return CommentPlacement::trailing(attribute.value.as_ref(), comment);
};
}

Expand Down
48 changes: 40 additions & 8 deletions crates/ruff_python_formatter/src/expression/expr_attribute.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use ruff_formatter::{write, FormatRuleWithOptions};
use ruff_python_ast::AnyNodeRef;
use ruff_python_ast::{Expr, ExprAttribute, ExprNumberLiteral, Number};
use ruff_python_trivia::{find_only_token_in_range, SimpleTokenKind};
use ruff_python_trivia::{find_only_token_in_range, SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextRange};

use crate::comments::dangling_comments;
Expand Down Expand Up @@ -50,21 +50,16 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
if parenthesize_value {
// Don't propagate the call chain layout.
value.format().with_options(Parentheses::Always).fmt(f)?;

// Format the dot on its own line.
soft_line_break().fmt(f)?;
} else {
match value.as_ref() {
Expr::Attribute(expr) => {
expr.format().with_options(call_chain_layout).fmt(f)?;
}
Expr::Call(expr) => {
expr.format().with_options(call_chain_layout).fmt(f)?;
soft_line_break().fmt(f)?;
}
Expr::Subscript(expr) => {
expr.format().with_options(call_chain_layout).fmt(f)?;
soft_line_break().fmt(f)?;
}
_ => {
value.format().with_options(Parentheses::Never).fmt(f)?;
Expand All @@ -77,19 +72,56 @@ impl FormatNodeRule<ExprAttribute> for FormatExprAttribute {
value.format().with_options(Parentheses::Never).fmt(f)?;
}

let comments = f.context().comments().clone();

// Always add a line break if the value is parenthesized and there's an
// end of line comment on the same line as the closing parenthesis.
// ```python
// (
// (
// a
// ) # `end_of_line_comment`
// .
// b
// )
// ```
let has_trailing_end_of_line_comment =
SimpleTokenizer::starts_at(value.end(), f.context().source())
.skip_trivia()
.take_while(|token| token.kind == SimpleTokenKind::RParen)
.last()
.is_some_and(|right_paren| {
let trailing_value_comments = comments.trailing(&**value);
trailing_value_comments.iter().any(|comment| {
comment.line_position().is_end_of_line()
&& comment.start() > right_paren.end()
})
});

if has_trailing_end_of_line_comment {
hard_line_break().fmt(f)?;
}
// Allow the `.` on its own line if this is a fluent call chain
// and the value either requires parenthesizing or is a call or subscript expression
// (it's a fluent chain but not the first element).
else if call_chain_layout == CallChainLayout::Fluent {
if parenthesize_value || value.is_call_expr() || value.is_subscript_expr() {
soft_line_break().fmt(f)?;
}
}

// Identify dangling comments before and after the dot:
// ```python
// (
// (
// a
// ) # `before_dot`
// )
// # `before_dot`
// . # `after_dot`
// # `after_dot`
// b
// )
// ```
let comments = f.context().comments().clone();
let dangling = comments.dangling(item);
let (before_dot, after_dot) = if dangling.is_empty() {
(dangling, dangling)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/attribute.py
snapshot_kind: text
---
## Input
```python
Expand Down Expand Up @@ -159,6 +158,23 @@ result = (
f(111111111111111111111111111111111111111111111111111111111111111111111111111111111)
+ 1
).bit_length()
# Regression test for https://github.com/astral-sh/ruff/issues/16151
result = (
(await query_the_thing(mypy_doesnt_understand)) # type: ignore[x]
.foo()
.bar()
)
(
(
a # trailing end-of-line
# trailing own-line
) # trailing closing parentheses
# dangling before dot
.b # trailing end-of-line
)
```

## Output
Expand Down Expand Up @@ -295,4 +311,21 @@ result = (
f(111111111111111111111111111111111111111111111111111111111111111111111111111111111)
+ 1
).bit_length()
# Regression test for https://github.com/astral-sh/ruff/issues/16151
result = (
(await query_the_thing(mypy_doesnt_understand)) # type: ignore[x]
.foo()
.bar()
)
(
(
a # trailing end-of-line
# trailing own-line
) # trailing closing parentheses
# dangling before dot
.b # trailing end-of-line
)
```

0 comments on commit 31180a8

Please sign in to comment.