Generalize comment-after-bracket handling to lists, sets, etc. (#6320)

## Summary

We already support preserving the end-of-line comment in calls and type
parameters, as in:

```python
foo(  # comment
    bar,
)
```

This PR adds the same behavior for lists, sets, comprehensions, etc.,
such that we preserve:

```python
[  # comment
    1,
    2,
    3,
]
```

And related cases.
This commit is contained in:
Charlie Marsh 2023-08-03 21:28:05 -04:00 committed by GitHub
parent d3aa8b4ee0
commit 1d8759d5df
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 265 additions and 99 deletions

View file

@ -478,7 +478,7 @@ impl<Context> std::fmt::Debug for LineSuffix<'_, Context> {
}
/// Inserts a boundary for line suffixes that forces the printer to print all pending line suffixes.
/// Helpful if a line sufix shouldn't pass a certain point.
/// Helpful if a line suffix shouldn't pass a certain point.
///
/// ## Examples
///

View file

@ -86,3 +86,10 @@ selected_choices = {
k: str(v)
for vvvvvvvvvvvvvvvvvvvvvvv in value if str(v) not in self.choices.field.empty_values
}
{
k: v
for ( # foo
x, aaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaay) in z
}

View file

@ -25,3 +25,9 @@ len(
# trailing
)
)
len(
# leading
a for b in c
# trailing
)

View file

@ -32,3 +32,15 @@ c1 = [ # trailing open bracket
2, # trailing item
# leading close bracket
] # trailing close bracket
[ # end-of-line comment
]
[ # end-of-line comment
# own-line comment
]
[ # end-of-line comment
1
]

View file

@ -2,10 +2,7 @@ use std::cmp::Ordering;
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::whitespace::indentation;
use ruff_python_ast::{
self as ast, Comprehension, Expr, ExprAttribute, ExprBinOp, ExprIfExp, ExprSlice, ExprStarred,
MatchCase, Parameters, Ranged,
};
use ruff_python_ast::{self as ast, Comprehension, Expr, MatchCase, Parameters, Ranged};
use ruff_python_trivia::{
indentation_at_offset, PythonWhitespace, SimpleToken, SimpleTokenKind, SimpleTokenizer,
};
@ -50,9 +47,9 @@ pub(super) fn place_comment<'a>(
locator,
)
}
AnyNodeRef::ExprDict(_) | AnyNodeRef::Keyword(_) => {
handle_dict_unpacking_comment(comment, locator)
}
AnyNodeRef::Keyword(_) => handle_dict_unpacking_comment(comment, locator),
AnyNodeRef::ExprDict(_) => handle_dict_unpacking_comment(comment, locator)
.then_with(|comment| handle_bracketed_end_of_line_comment(comment, locator)),
AnyNodeRef::ExprIfExp(expr_if) => handle_expr_if_comment(comment, expr_if, locator),
AnyNodeRef::ExprSlice(expr_slice) => handle_slice_comments(comment, expr_slice, locator),
AnyNodeRef::ExprStarred(starred) => {
@ -77,6 +74,13 @@ pub(super) fn place_comment<'a>(
handle_leading_class_with_decorators_comment(comment, class_def)
}
AnyNodeRef::StmtImportFrom(import_from) => handle_import_from_comment(comment, import_from),
AnyNodeRef::ExprList(_)
| AnyNodeRef::ExprSet(_)
| AnyNodeRef::ExprGeneratorExp(_)
| AnyNodeRef::ExprListComp(_)
| AnyNodeRef::ExprSetComp(_)
| AnyNodeRef::ExprDictComp(_)
| AnyNodeRef::ExprTuple(_) => handle_bracketed_end_of_line_comment(comment, locator),
_ => CommentPlacement::Default(comment),
})
}
@ -633,7 +637,7 @@ fn handle_parameters_separator_comment<'a>(
/// ```
fn handle_trailing_binary_expression_left_or_operator_comment<'a>(
comment: DecoratedComment<'a>,
binary_expression: &'a ExprBinOp,
binary_expression: &'a ast::ExprBinOp,
locator: &Locator,
) -> CommentPlacement<'a> {
// Only if there's a preceding node (in which case, the preceding node is `left`).
@ -797,10 +801,10 @@ fn handle_module_level_own_line_comment_before_class_or_function_comment<'a>(
/// ```
fn handle_slice_comments<'a>(
comment: DecoratedComment<'a>,
expr_slice: &'a ExprSlice,
expr_slice: &'a ast::ExprSlice,
locator: &Locator,
) -> CommentPlacement<'a> {
let ExprSlice {
let ast::ExprSlice {
range: _,
lower,
upper,
@ -907,7 +911,7 @@ fn handle_leading_class_with_decorators_comment<'a>(
}
/// Handles comments between `**` and the variable name in dict unpacking
/// It attaches these to the appropriate value node
/// It attaches these to the appropriate value node.
///
/// ```python
/// {
@ -945,7 +949,7 @@ fn handle_dict_unpacking_comment<'a>(
// if the remaining tokens from the previous node are exactly `**`,
// re-assign the comment to the one that follows the stars
let mut count = 0;
let mut count = 0u32;
// we start from the preceding node but we skip its token
if let Some(token) = tokens.next() {
@ -992,7 +996,7 @@ fn handle_dict_unpacking_comment<'a>(
/// ```
fn handle_attribute_comment<'a>(
comment: DecoratedComment<'a>,
attribute: &'a ExprAttribute,
attribute: &'a ast::ExprAttribute,
) -> CommentPlacement<'a> {
debug_assert!(
comment.preceding_node().is_some(),
@ -1039,10 +1043,10 @@ fn handle_attribute_comment<'a>(
/// happens if the comments are in a weird position but it also doesn't hurt handling it.
fn handle_expr_if_comment<'a>(
comment: DecoratedComment<'a>,
expr_if: &'a ExprIfExp,
expr_if: &'a ast::ExprIfExp,
locator: &Locator,
) -> CommentPlacement<'a> {
let ExprIfExp {
let ast::ExprIfExp {
range: _,
test,
body,
@ -1096,7 +1100,7 @@ fn handle_expr_if_comment<'a>(
/// ```
fn handle_trailing_expression_starred_star_end_of_line_comment<'a>(
comment: DecoratedComment<'a>,
starred: &'a ExprStarred,
starred: &'a ast::ExprStarred,
) -> CommentPlacement<'a> {
if comment.line_position().is_own_line() {
return CommentPlacement::Default(comment);

View file

@ -1,14 +1,17 @@
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;
use ruff_formatter::{format_args, write};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::Ranged;
use ruff_python_ast::{Expr, ExprDict};
use ruff_text_size::TextRange;
use crate::builders::empty_parenthesized_with_dangling_comments;
use crate::comments::leading_comments;
use crate::expression::parentheses::{
parenthesized_with_dangling_comments, NeedsParentheses, OptionalParentheses,
};
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatExprDict;
@ -64,13 +67,11 @@ impl FormatNodeRule<ExprDict> for FormatExprDict {
debug_assert_eq!(keys.len(), values.len());
if values.is_empty() {
let comments = f.context().comments().clone();
return empty_parenthesized_with_dangling_comments(
text("{"),
comments.dangling_comments(item),
text("}"),
)
let dangling = comments.dangling_comments(item);
if values.is_empty() {
return empty_parenthesized_with_dangling_comments(text("{"), dangling, text("}"))
.fmt(f);
}
@ -85,7 +86,7 @@ impl FormatNodeRule<ExprDict> for FormatExprDict {
joiner.finish()
});
parenthesized("{", &format_pairs, "}").fmt(f)
parenthesized_with_dangling_comments("{", dangling, &format_pairs, "}").fmt(f)
}
fn fmt_dangling_comments(&self, _node: &ExprDict, _f: &mut PyFormatter) -> FormatResult<()> {

View file

@ -1,7 +1,3 @@
use crate::context::PyFormatContext;
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::AsFormat;
use crate::{FormatNodeRule, FormattedIterExt, PyFormatter};
use ruff_formatter::prelude::{
format_args, format_with, group, soft_line_break_or_space, space, text,
};
@ -9,6 +5,13 @@ use ruff_formatter::{write, Buffer, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprDictComp;
use crate::context::PyFormatContext;
use crate::expression::parentheses::{
parenthesized_with_dangling_comments, NeedsParentheses, OptionalParentheses,
};
use crate::AsFormat;
use crate::{FormatNodeRule, FormattedIterExt, PyFormatter};
#[derive(Default)]
pub struct FormatExprDictComp;
@ -27,10 +30,14 @@ impl FormatNodeRule<ExprDictComp> for FormatExprDictComp {
.finish()
});
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);
write!(
f,
[parenthesized(
[parenthesized_with_dangling_comments(
"{",
dangling,
&group(&format_args!(
group(&key.format()),
text(":"),

View file

@ -1,12 +1,14 @@
use ruff_formatter::{format_args, write, Buffer, FormatResult, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprGeneratorExp;
use crate::comments::leading_comments;
use crate::context::PyFormatContext;
use crate::expression::parentheses::parenthesized;
use crate::expression::parentheses::parenthesized_with_dangling_comments;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::AsFormat;
use crate::{FormatNodeRule, PyFormatter};
use ruff_formatter::{format_args, write, Buffer, FormatResult, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprGeneratorExp;
#[derive(Eq, PartialEq, Debug, Default)]
pub enum GeneratorExpParentheses {
@ -48,10 +50,14 @@ impl FormatNodeRule<ExprGeneratorExp> for FormatExprGeneratorExp {
.finish()
});
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);
if self.parentheses == GeneratorExpParentheses::StripIfOnlyFunctionArg {
write!(
f,
[
leading_comments(dangling),
group(&elt.format()),
soft_line_break_or_space(),
group(&joined),
@ -60,8 +66,9 @@ impl FormatNodeRule<ExprGeneratorExp> for FormatExprGeneratorExp {
} else {
write!(
f,
[parenthesized(
[parenthesized_with_dangling_comments(
"(",
dangling,
&format_args!(
group(&elt.format()),
soft_line_break_or_space(),
@ -72,6 +79,15 @@ impl FormatNodeRule<ExprGeneratorExp> for FormatExprGeneratorExp {
)
}
}
fn fmt_dangling_comments(
&self,
_node: &ExprGeneratorExp,
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
impl NeedsParentheses for ExprGeneratorExp {

View file

@ -1,10 +1,14 @@
use crate::builders::empty_parenthesized_with_dangling_comments;
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::FormatNodeRule;
use ruff_formatter::prelude::{format_with, text};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::{ExprList, Ranged};
use crate::builders::empty_parenthesized_with_dangling_comments;
use crate::expression::parentheses::{
parenthesized_with_dangling_comments, NeedsParentheses, OptionalParentheses,
};
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatExprList;
@ -24,18 +28,13 @@ impl FormatNodeRule<ExprList> for FormatExprList {
.fmt(f);
}
debug_assert!(
dangling.is_empty(),
"A non-empty expression list has dangling comments"
);
let items = format_with(|f| {
f.join_comma_separated(item.end())
.nodes(elts.iter())
.finish()
});
parenthesized("[", &items, "]").fmt(f)
parenthesized_with_dangling_comments("[", dangling, &items, "]").fmt(f)
}
fn fmt_dangling_comments(&self, _node: &ExprList, _f: &mut PyFormatter) -> FormatResult<()> {

View file

@ -1,11 +1,14 @@
use ruff_formatter::{format_args, write, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprListComp;
use crate::context::PyFormatContext;
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::expression::parentheses::{
parenthesized_with_dangling_comments, NeedsParentheses, OptionalParentheses,
};
use crate::prelude::*;
use crate::AsFormat;
use crate::{FormatNodeRule, PyFormatter};
use ruff_formatter::{format_args, write, Buffer, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprListComp;
#[derive(Default)]
pub struct FormatExprListComp;
@ -24,10 +27,14 @@ impl FormatNodeRule<ExprListComp> for FormatExprListComp {
.finish()
});
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);
write!(
f,
[parenthesized(
[parenthesized_with_dangling_comments(
"[",
dangling,
&group(&format_args![
group(&elt.format()),
soft_line_break_or_space(),
@ -37,6 +44,15 @@ impl FormatNodeRule<ExprListComp> for FormatExprListComp {
)]
)
}
fn fmt_dangling_comments(
&self,
_node: &ExprListComp,
_f: &mut PyFormatter,
) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
impl NeedsParentheses for ExprListComp {

View file

@ -1,8 +1,9 @@
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::{ExprSet, Ranged};
use ruff_python_ast::node::AnyNodeRef;
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::expression::parentheses::{
parenthesized_with_dangling_comments, NeedsParentheses, OptionalParentheses,
};
use crate::prelude::*;
use crate::FormatNodeRule;
@ -21,7 +22,15 @@ impl FormatNodeRule<ExprSet> for FormatExprSet {
.finish()
});
parenthesized("{", &joined, "}").fmt(f)
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);
parenthesized_with_dangling_comments("{", dangling, &joined, "}").fmt(f)
}
fn fmt_dangling_comments(&self, _node: &ExprSet, _f: &mut PyFormatter) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}

View file

@ -1,12 +1,15 @@
use crate::context::PyFormatContext;
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::prelude::*;
use crate::AsFormat;
use crate::{FormatNodeRule, PyFormatter};
use ruff_formatter::{format_args, write, Buffer, FormatResult};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprSetComp;
use crate::context::PyFormatContext;
use crate::expression::parentheses::{
parenthesized_with_dangling_comments, NeedsParentheses, OptionalParentheses,
};
use crate::prelude::*;
use crate::AsFormat;
use crate::{FormatNodeRule, PyFormatter};
#[derive(Default)]
pub struct FormatExprSetComp;
@ -24,10 +27,14 @@ impl FormatNodeRule<ExprSetComp> for FormatExprSetComp {
.finish()
});
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);
write!(
f,
[parenthesized(
[parenthesized_with_dangling_comments(
"{",
dangling,
&group(&format_args!(
group(&elt.format()),
soft_line_break_or_space(),
@ -37,6 +44,11 @@ impl FormatNodeRule<ExprSetComp> for FormatExprSetComp {
)]
)
}
fn fmt_dangling_comments(&self, _node: &ExprSetComp, _f: &mut PyFormatter) -> FormatResult<()> {
// Handled as part of `fmt_fields`
Ok(())
}
}
impl NeedsParentheses for ExprSetComp {

View file

@ -6,7 +6,9 @@ use ruff_formatter::{format_args, write, FormatRuleWithOptions};
use ruff_python_ast::node::AnyNodeRef;
use crate::builders::{empty_parenthesized_with_dangling_comments, parenthesize_if_expands};
use crate::expression::parentheses::{parenthesized, NeedsParentheses, OptionalParentheses};
use crate::expression::parentheses::{
parenthesized_with_dangling_comments, NeedsParentheses, OptionalParentheses,
};
use crate::prelude::*;
#[derive(Eq, PartialEq, Debug, Default)]
@ -31,7 +33,6 @@ pub enum TupleParentheses {
/// Handle the special cases where we don't include parentheses at all.
///
///
/// Black never formats tuple targets of for loops with parentheses if inside a comprehension.
/// For example, tuple targets will always be formatted on the same line, except when an element supports
/// line-breaking in an un-parenthesized context.
@ -104,6 +105,9 @@ impl FormatNodeRule<ExprTuple> for FormatExprTuple {
ctx: _,
} = item;
let comments = f.context().comments().clone();
let dangling = comments.dangling_comments(item);
// Handle the edge cases of an empty tuple and a tuple with one element
//
// there can be dangling comments, and they can be in two
@ -116,12 +120,7 @@ impl FormatNodeRule<ExprTuple> for FormatExprTuple {
// In all other cases comments get assigned to a list element
match elts.as_slice() {
[] => {
let comments = f.context().comments().clone();
return empty_parenthesized_with_dangling_comments(
text("("),
comments.dangling_comments(item),
text(")"),
)
return empty_parenthesized_with_dangling_comments(text("("), dangling, text(")"))
.fmt(f);
}
[single] => match self.parentheses {
@ -133,7 +132,13 @@ impl FormatNodeRule<ExprTuple> for FormatExprTuple {
_ =>
// A single element tuple always needs parentheses and a trailing comma, except when inside of a subscript
{
parenthesized("(", &format_args![single.format(), text(",")], ")").fmt(f)
parenthesized_with_dangling_comments(
"(",
dangling,
&format_args![single.format(), text(",")],
")",
)
.fmt(f)
}
},
// If the tuple has parentheses, we generally want to keep them. The exception are for
@ -142,9 +147,11 @@ impl FormatNodeRule<ExprTuple> for FormatExprTuple {
// Unlike other expression parentheses, tuple parentheses are part of the range of the
// tuple itself.
_ if is_parenthesized(*range, elts, f.context().source())
&& self.parentheses != TupleParentheses::NeverPreserve =>
&& !(self.parentheses == TupleParentheses::NeverPreserve
&& dangling.is_empty()) =>
{
parenthesized("(", &ExprSequence::new(item), ")").fmt(f)
parenthesized_with_dangling_comments("(", dangling, &ExprSequence::new(item), ")")
.fmt(f)
}
_ => match self.parentheses {
TupleParentheses::Never => {

View file

@ -1,10 +1,10 @@
use ruff_python_ast::Ranged;
use ruff_formatter::prelude::tag::Condition;
use ruff_formatter::{format_args, write, Argument, Arguments};
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::Ranged;
use ruff_python_trivia::{first_non_trivia_token, SimpleToken, SimpleTokenKind, SimpleTokenizer};
use crate::comments::{dangling_comments, SourceComment};
use crate::context::{NodeLevel, WithNodeLevel};
use crate::prelude::*;
@ -110,6 +110,26 @@ where
{
FormatParenthesized {
left,
comments: &[],
content: Argument::new(content),
right,
}
}
/// Formats `content` enclosed by the `left` and `right` parentheses, along with any dangling
/// comments that on the parentheses themselves.
pub(crate) fn parenthesized_with_dangling_comments<'content, 'ast, Content>(
left: &'static str,
comments: &'content [SourceComment],
content: &'content Content,
right: &'static str,
) -> FormatParenthesized<'content, 'ast>
where
Content: Format<PyFormatContext<'ast>>,
{
FormatParenthesized {
left,
comments,
content: Argument::new(content),
right,
}
@ -117,6 +137,7 @@ where
pub(crate) struct FormatParenthesized<'content, 'ast> {
left: &'static str,
comments: &'content [SourceComment],
content: Argument<'content, PyFormatContext<'ast>>,
right: &'static str,
}
@ -124,12 +145,22 @@ pub(crate) struct FormatParenthesized<'content, 'ast> {
impl<'ast> Format<PyFormatContext<'ast>> for FormatParenthesized<'_, 'ast> {
fn fmt(&self, f: &mut Formatter<PyFormatContext<'ast>>) -> FormatResult<()> {
let inner = format_with(|f| {
if self.comments.is_empty() {
group(&format_args![
text(self.left),
&soft_block_indent(&Arguments::from(&self.content)),
text(self.right)
])
.fmt(f)
} else {
group(&format_args![
text(self.left),
&line_suffix(&dangling_comments(self.comments)),
&group(&soft_block_indent(&Arguments::from(&self.content))),
text(self.right)
])
.fmt(f)
}
});
let current_level = f.context().node_level();

View file

@ -70,8 +70,7 @@ x={ # dangling end of line comment
## Output
```py
# before
{
# open
{ # open
key: value # key # colon # value
} # close
# after

View file

@ -92,6 +92,13 @@ selected_choices = {
k: str(v)
for vvvvvvvvvvvvvvvvvvvvvvv in value if str(v) not in self.choices.field.empty_values
}
{
k: v
for ( # foo
x, aaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaay) in z
}
```
## Output
@ -201,11 +208,9 @@ selected_choices = {
}
# Leading
{
# Leading
{ # Leading
k: v # Trailing
for a, a, a, a, a, a, a, a, a, a, (
# Trailing
for a, a, a, a, a, a, a, a, a, a, ( # Trailing
a,
a,
a,
@ -241,6 +246,11 @@ selected_choices = {
for vvvvvvvvvvvvvvvvvvvvvvv in value
if str(v) not in self.choices.field.empty_values
}
{
k: v
for (x, aaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaayaaaay) in z # foo
}
```

View file

@ -31,6 +31,12 @@ len(
# trailing
)
)
len(
# leading
a for b in c
# trailing
)
```
## Output
@ -56,6 +62,13 @@ f((1) for _ in (a))
# black keeps these atm, but intends to remove them in the future:
# https://github.com/psf/black/issues/2943
len(
# leading
a
for b in c
# trailing
)
len(
# leading
a

View file

@ -38,6 +38,18 @@ c1 = [ # trailing open bracket
2, # trailing item
# leading close bracket
] # trailing close bracket
[ # end-of-line comment
]
[ # end-of-line comment
# own-line comment
]
[ # end-of-line comment
1
]
```
## Output
@ -66,14 +78,22 @@ b3 = [
]
# Comment placement in non-empty lists
c1 = [
# trailing open bracket
c1 = [ # trailing open bracket
# leading item
1,
# between
2, # trailing item
# leading close bracket
] # trailing close bracket
[] # end-of-line comment
[ # end-of-line comment
# own-line comment
]
[1] # end-of-line comment
```

View file

@ -248,14 +248,12 @@ f4 = ( # end-of-line
) # trailing
# Comments in other tuples
g1 = (
# a
g1 = ( # a
# b
1, # c
# d
) # e
g2 = (
# a
g2 = ( # a
# b
1, # c
# d

View file

@ -204,8 +204,7 @@ raise hello( # sould I stay here
# just a comment here
) # trailing comment
raise (
# sould I stay here
raise ( # sould I stay here
test,
# just a comment here
) # trailing comment