Skip to content

Commit

Permalink
Formatter: Add EmptyWithDanglingComments helper (#5951)
Browse files Browse the repository at this point in the history
**Summary** Add a `EmptyWithDanglingComments` format helper that formats
comments inside empty parentheses, brackets or curly braces. Previously,
this was implemented separately, and partially incorrectly, for each use
case.

Empty `()`, `[]` and `{}` are special because there can be dangling
comments, and they can be in
two positions:
```python
x = [  # end-of-line
    # own line
]
```
These comments are dangling because they can't be assigned to any
element inside as they would
in all other cases.

**Test Plan** Added a regression test.

145 (from previously 149) instances of unstable formatting remaining.

```
$ cargo run --bin ruff_dev --release -- format-dev --stability-check --error-file formatter-ecosystem-errors.txt --multi-project target/checkouts > formatter-ecosystem-progress.txt
$ rg "Unstable formatting" target/formatter-ecosystem-errors.txt | wc -l
145
```
  • Loading branch information
konstin authored Jul 23, 2023
1 parent f886b58 commit 46f8961
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,6 @@
# comment
3: True,
}

x={ # dangling end of line comment
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,6 @@
# Deleted
) # Completed
# Done

del ( # dangling end of line comment
)
52 changes: 52 additions & 0 deletions crates/ruff_python_formatter/src/builders.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::Ranged;

use crate::comments::{dangling_comments, SourceComment};
use ruff_formatter::{format_args, write, Argument, Arguments};
use ruff_python_trivia::{
lines_after, skip_trailing_trivia, SimpleToken, SimpleTokenKind, SimpleTokenizer,
Expand Down Expand Up @@ -323,6 +324,57 @@ impl<'fmt, 'ast, 'buf> JoinCommaSeparatedBuilder<'fmt, 'ast, 'buf> {
}
}

/// Format comments inside empty parentheses, brackets or curly braces.
///
/// Empty `()`, `[]` and `{}` are special because there can be dangling comments, and they can be in
/// two positions:
/// ```python
/// x = [ # end-of-line
/// # own line
/// ]
/// ```
/// These comments are dangling because they can't be assigned to any element inside as they would
/// in all other cases.
pub(crate) fn empty_parenthesized_with_dangling_comments(
opening: StaticText,
comments: &[SourceComment],
closing: StaticText,
) -> EmptyWithDanglingComments {
EmptyWithDanglingComments {
opening,
comments,
closing,
}
}

pub(crate) struct EmptyWithDanglingComments<'a> {
opening: StaticText,
comments: &'a [SourceComment],
closing: StaticText,
}

impl<'ast> Format<PyFormatContext<'ast>> for EmptyWithDanglingComments<'_> {
fn fmt(&self, f: &mut Formatter<PyFormatContext>) -> FormatResult<()> {
let end_of_line_split = self
.comments
.partition_point(|comment| comment.line_position().is_end_of_line());
debug_assert!(self.comments[end_of_line_split..]
.iter()
.all(|comment| comment.line_position().is_own_line()));
write!(
f,
[group(&format_args![
self.opening,
// end-of-line comments
dangling_comments(&self.comments[..end_of_line_split]),
// own line comments, which need to be indented
soft_block_indent(&dangling_comments(&self.comments[end_of_line_split..])),
self.closing
])]
)
}
}

#[cfg(test)]
mod tests {
use rustpython_parser::ast::ModModule;
Expand Down
11 changes: 6 additions & 5 deletions crates/ruff_python_formatter/src/expression/expr_call.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use ruff_text_size::{TextRange, TextSize};
use rustpython_parser::ast::{Expr, ExprCall, Ranged};

use crate::builders::empty_parenthesized_with_dangling_comments;
use ruff_formatter::write;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};

use crate::comments::dangling_comments;
use crate::expression::expr_generator_exp::GeneratorExpParentheses;
use crate::expression::parentheses::{
parenthesized, NeedsParentheses, OptionalParentheses, Parentheses,
Expand Down Expand Up @@ -34,14 +34,15 @@ impl FormatNodeRule<ExprCall> for FormatExprCall {
// ```
if args.is_empty() && keywords.is_empty() {
let comments = f.context().comments().clone();
let comments = comments.dangling_comments(item);
return write!(
f,
[
func.format(),
text("("),
dangling_comments(comments),
text(")")
empty_parenthesized_with_dangling_comments(
text("("),
comments.dangling_comments(item),
text(")"),
)
]
);
}
Expand Down
18 changes: 9 additions & 9 deletions crates/ruff_python_formatter/src/expression/expr_dict.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::comments::{dangling_node_comments, leading_comments};
use crate::builders::empty_parenthesized_with_dangling_comments;
use crate::comments::leading_comments;
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::FormatNodeRule;
Expand Down Expand Up @@ -64,14 +65,13 @@ impl FormatNodeRule<ExprDict> for FormatExprDict {
debug_assert_eq!(keys.len(), values.len());

if values.is_empty() {
return write!(
f,
[
&text("{"),
block_indent(&dangling_node_comments(item)),
&text("}"),
]
);
let comments = f.context().comments().clone();
return empty_parenthesized_with_dangling_comments(
text("{"),
comments.dangling_comments(item),
text("}"),
)
.fmt(f);
}

let format_pairs = format_with(|f| {
Expand Down
28 changes: 3 additions & 25 deletions crates/ruff_python_formatter/src/expression/expr_list.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::comments::{dangling_comments, CommentLinePosition};
use crate::builders::empty_parenthesized_with_dangling_comments;
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::FormatNodeRule;
use ruff_formatter::{format_args, write};
use ruff_python_ast::node::AnyNodeRef;
use rustpython_parser::ast::{ExprList, Ranged};

Expand All @@ -20,30 +19,9 @@ impl FormatNodeRule<ExprList> for FormatExprList {
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);

// The empty list is special because there can be dangling comments, and they can be in two
// positions:
// ```python
// a3 = [ # end-of-line
// # own line
// ]
// ```
// In all other cases comments get assigned to a list element
if elts.is_empty() {
let end_of_line_split = dangling.partition_point(|comment| {
comment.line_position() == CommentLinePosition::EndOfLine
});
debug_assert!(dangling[end_of_line_split..]
.iter()
.all(|comment| comment.line_position() == CommentLinePosition::OwnLine));
return write!(
f,
[group(&format_args![
text("["),
dangling_comments(&dangling[..end_of_line_split]),
soft_block_indent(&dangling_comments(&dangling[end_of_line_split..])),
text("]")
])]
);
return empty_parenthesized_with_dangling_comments(text("["), dangling, text("]"))
.fmt(f);
}

debug_assert!(
Expand Down
23 changes: 6 additions & 17 deletions crates/ruff_python_formatter/src/expression/expr_tuple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ use rustpython_parser::ast::{Expr, Ranged};
use ruff_formatter::{format_args, write, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;

use crate::builders::parenthesize_if_expands;
use crate::comments::{dangling_comments, CommentLinePosition};
use crate::builders::{empty_parenthesized_with_dangling_comments, parenthesize_if_expands};
use crate::expression::parentheses::{
parenthesized, NeedsParentheses, OptionalParentheses, Parentheses,
};
Expand Down Expand Up @@ -79,22 +78,12 @@ impl FormatNodeRule<ExprTuple> for FormatExprTuple {
match elts.as_slice() {
[] => {
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);
let end_of_line_split = dangling.partition_point(|comment| {
comment.line_position() == CommentLinePosition::EndOfLine
});
debug_assert!(dangling[end_of_line_split..]
.iter()
.all(|comment| comment.line_position() == CommentLinePosition::OwnLine));
write!(
f,
[group(&format_args![
text("("),
dangling_comments(&dangling[..end_of_line_split]),
soft_block_indent(&dangling_comments(&dangling[end_of_line_split..])),
text(")")
])]
return empty_parenthesized_with_dangling_comments(
text("("),
comments.dangling_comments(item),
text(")"),
)
.fmt(f);
}
[single] => match self.parentheses {
TupleParentheses::Subscript
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ a = {
# comment
3: True,
}
x={ # dangling end of line comment
}
```

## Output
Expand Down Expand Up @@ -126,6 +129,9 @@ a = {
# comment
3: True,
}
x = { # dangling end of line comment
}
```


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ del (
# Deleted
) # Completed
# Done
del ( # dangling end of line comment
)
```

## Output
Expand Down Expand Up @@ -211,6 +214,9 @@ del (
# Deleted
) # Completed
# Done
del ( # dangling end of line comment
)
```


Expand Down

0 comments on commit 46f8961

Please sign in to comment.