mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-30 23:27:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Comments outside expression parentheses (#7873)

Browse source 
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

Fixes https://github.com/astral-sh/ruff/issues/7448
Fixes https://github.com/astral-sh/ruff/issues/7892

I've removed automatic dangling comment formatting, we're doing manual
dangling comment formatting everywhere anyway (the
assert-all-comments-formatted ensures this) and dangling comments would
break the formatting there.

## Test Plan

New test file.

---------

Co-authored-by: Micha Reiser <micha@reiser.io>
This commit is contained in:
konsti 2023-10-19 11:24:11 +02:00 • committed by GitHub
parent 67b043482a
commit 8f9753f58e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 652 additions and 124 deletions
Download patch file Download diff file Expand all files Collapse all files

11
crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/unary.py vendored
View file

@ -161,3 +161,14 @@ if True:
+ "WARNING: Removing listed files. Do you really want to continue. yes/n)? "
):
pass
# https://github.com/astral-sh/ruff/issues/7448
x = (
# a
not # b
# c
( # d
# e
True
)
)

113
crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/expression_parentheses_comments.py vendored Normal file
View file

@ -0,0 +1,113 @@
list_with_parenthesized_elements1 = [
# comment leading outer
(
# comment leading inner
1 + 2 # comment trailing inner
) # comment trailing outer
]
list_with_parenthesized_elements2 = [
# leading outer
(1 + 2)
]
list_with_parenthesized_elements3 = [
# leading outer
(1 + 2) # trailing outer
]
list_with_parenthesized_elements4 = [
# leading outer
(1 + 2), # trailing outer
]
list_with_parenthesized_elements5 = [
(1), # trailing outer
(2), # trailing outer
]
nested_parentheses1 = (
(
(
1
) # i
) # j
) # k
nested_parentheses2 = [
(
(
(
1
) # i
# i2
) # j
# j2
) # k
# k2
]
nested_parentheses3 = (
( # a
( # b
1
) # i
) # j
) # k
nested_parentheses4 = [
# a
( # b
# c
( # d
# e
( #f
1
) # i
# i2
) # j
# j2
) # k
# k2
]
x = (
# unary comment
not
# in-between comment
(
# leading inner
"a"
),
not # in-between comment
(
# leading inner
"b"
),
not
( # in-between comment
# leading inner
"c"
),
# 1
not # 2
( # 3
# 4
"d"
)
)
if (
# unary comment
not
# in-between comment
(
# leading inner
1
)
):
pass
# Make sure we keep a inside the parentheses
# https://github.com/astral-sh/ruff/issues/7892
x = (
# a
( # b
1
)
)

4
crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/with.py vendored
View file

@ -86,10 +86,6 @@ with (
)
): pass
with (a # trailing same line comment
# trailing own line comment
) as b: pass
with (
a # trailing same line comment
# trailing own line comment

40
crates/ruff_python_formatter/src/comments/placement.rs
View file

@ -1878,8 +1878,7 @@ fn handle_lambda_comment<'a>(
CommentPlacement::Default(comment)
}
/// Attach trailing end-of-line comments on the operator as dangling comments on the enclosing
/// node.
/// Move comment between a unary op and its operand before the unary op by marking them as trailing.
///
/// For example, given:
/// ```python
@ -1896,26 +1895,27 @@ fn handle_unary_op_comment<'a>(
unary_op: &'a ast::ExprUnaryOp,
locator: &Locator,
) -> CommentPlacement<'a> {
if comment.line_position().is_own_line() {
return CommentPlacement::Default(comment);
}
if comment.start() > unary_op.operand.start() {
return CommentPlacement::Default(comment);
}
let tokenizer = SimpleTokenizer::new(
let mut tokenizer = SimpleTokenizer::new(
locator.contents(),
TextRange::new(comment.start(), unary_op.operand.start()),
);
if tokenizer
.skip_trivia()
.any(|token| token.kind == SimpleTokenKind::LParen)
{
return CommentPlacement::Default(comment);
TextRange::new(unary_op.start(), unary_op.operand.start()),
)
.skip_trivia();
let op_token = tokenizer.next();
debug_assert!(op_token.is_some_and(|token| matches!(
token.kind,
SimpleTokenKind::Tilde
| SimpleTokenKind::Not
| SimpleTokenKind::Plus
| SimpleTokenKind::Minus
)));
let up_to = tokenizer
.find(|token| token.kind == SimpleTokenKind::LParen)
.map_or(unary_op.operand.start(), |lparen| lparen.start());
if comment.end() < up_to {
CommentPlacement::leading(unary_op, comment)
} else {
CommentPlacement::Default(comment)
}
CommentPlacement::dangling(comment.enclosing_node(), comment)
}
/// Attach an end-of-line comment immediately following an open bracket as a dangling comment on

219
crates/ruff_python_formatter/src/expression/mod.rs
View file

@ -1,18 +1,18 @@
use std::cmp::Ordering;
use itertools::Itertools;
use std::slice;
use ruff_formatter::{
write, FormatOwnedWithRule, FormatRefWithRule, FormatRule, FormatRuleWithOptions,
};
use ruff_python_ast as ast;
use ruff_python_ast::parenthesize::parentheses_iterator;
use ruff_python_ast::visitor::preorder::{walk_expr, PreorderVisitor};
use ruff_python_ast::AnyNodeRef;
use ruff_python_ast::{Constant, Expr, ExpressionRef, Operator};
use ruff_python_ast::{AnyNodeRef, Constant, Expr, ExpressionRef, Operator};
use ruff_python_trivia::CommentRanges;
use ruff_text_size::Ranged;
use crate::builders::parenthesize_if_expands;
use crate::comments::leading_comments;
use crate::comments::{leading_comments, trailing_comments, LeadingDanglingTrailingComments};
use crate::context::{NodeLevel, WithNodeLevel};
use crate::expression::parentheses::{
is_expression_parenthesized, optional_parentheses, parenthesized, NeedsParentheses,
@ -102,7 +102,6 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
Expr::Slice(expr) => expr.format().fmt(f),
Expr::IpyEscapeCommand(expr) => expr.format().fmt(f),
});
let parenthesize = match parentheses {
Parentheses::Preserve => is_expression_parenthesized(
expression.into(),
@ -113,32 +112,13 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
// Fluent style means we already have parentheses
Parentheses::Never => false,
};
if parenthesize {
// Any comments on the open parenthesis of a `node`.
//
// For example, `# comment` in:
// ```python
// ( # comment
// foo.bar
// )
// ```
let comments = f.context().comments().clone();
let leading = comments.leading(expression);
if let Some((index, open_parenthesis_comment)) = leading
.iter()
.find_position(|comment| comment.line_position().is_end_of_line())
{
write!(
f,
[
leading_comments(&leading[..index]),
parenthesized("(", &format_expr, ")")
.with_dangling_comments(std::slice::from_ref(open_parenthesis_comment))
]
)
} else {
let comment = f.context().comments().clone();
let node_comments = comment.leading_dangling_trailing(expression);
if !node_comments.has_leading() && !node_comments.has_trailing() {
parenthesized("(", &format_expr, ")").fmt(f)
} else {
format_with_parentheses_comments(expression, &node_comments, f)
}
} else {
let level = match f.context().node_level() {
@ -155,6 +135,185 @@ impl FormatRule<Expr, PyFormatContext<'_>> for FormatExpr {
}
}
/// The comments below are trailing on the addition, but it's also outside the
/// parentheses
/// ```python
/// x = [
/// # comment leading
/// (1 + 2) # comment trailing
/// ]
/// ```
/// as opposed to
/// ```python
/// x = [(
/// # comment leading
/// 1 + 2 # comment trailing
/// )]
/// ```
/// , where the comments are inside the parentheses. That is also affects list
/// formatting, where we want to avoid moving the comments after the comma inside
/// the parentheses:
/// ```python
/// data = [
/// (
/// b"\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00"
/// b"\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"
/// ), # Point (0 0)
/// ]
/// ```
/// We could mark those comments as trailing in list but it's easier to handle
/// them here too.
///
/// So given
/// ```python
/// x = [
/// # comment leading outer
/// (
/// # comment leading inner
/// 1 + 2 # comment trailing inner
/// ) # comment trailing outer
/// ]
/// ```
/// we want to keep the inner an outer comments outside the parentheses and the inner ones inside.
/// This is independent of whether they are own line or end-of-line comments, though end-of-line
/// comments can become own line comments when we discard nested parentheses.
///
/// Style decision: When there are multiple nested parentheses around an expression, we consider the
/// outermost parentheses the relevant ones and discard the others.
fn format_with_parentheses_comments(
expression: &Expr,
node_comments: &LeadingDanglingTrailingComments,
f: &mut PyFormatter,
) -> FormatResult<()> {
// First part: Split the comments
// TODO(konstin): We don't have the parent, which is a problem:
// ```python
// f(
// # a
// (a)
// )
// ```
// gets formatted as
// ```python
// f(
// (
// # a
// a
// )
// )
// ```
let range_with_parens = parentheses_iterator(
expression.into(),
None,
f.context().comments().ranges(),
f.context().source(),
)
.last();
let (leading_split, trailing_split) = if let Some(range_with_parens) = range_with_parens {
let leading_split = node_comments
.leading
.partition_point(|comment| comment.start() < range_with_parens.start());
let trailing_split = node_comments
.trailing
.partition_point(|comment| comment.start() < range_with_parens.end());
(leading_split, trailing_split)
} else {
(0, node_comments.trailing.len())
};
let (leading_outer, leading_inner) = node_comments.leading.split_at(leading_split);
let (trailing_inner, trailing_outer) = node_comments.trailing.split_at(trailing_split);
// Preserve an opening parentheses comment
// ```python
// a = ( # opening parentheses comment
// # leading inner
// 1
// )
// ```
let (parentheses_comment, leading_inner) = match leading_inner.split_first() {
Some((first, rest)) if first.line_position().is_end_of_line() => {
(slice::from_ref(first), rest)
}
_ => (Default::default(), node_comments.leading),
};
// Second Part: Format
// The code order is a bit strange here, we format:
// * outer leading comment
// * opening parenthesis
// * opening parenthesis comment
// * inner leading comments
// * the expression itself
// * inner trailing comments
// * the closing parenthesis
// * outer trailing comments
let fmt_fields = format_with(|f| match expression {
Expr::BoolOp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::NamedExpr(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::BinOp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::UnaryOp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Lambda(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::IfExp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Dict(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Set(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::ListComp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::SetComp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::DictComp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::GeneratorExp(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Await(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Yield(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::YieldFrom(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Compare(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Call(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::FormattedValue(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::FString(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Constant(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Attribute(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Subscript(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Starred(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Name(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::List(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Tuple(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::Slice(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
Expr::IpyEscapeCommand(expr) => FormatNodeRule::fmt_fields(expr.format().rule(), expr, f),
});
leading_comments(leading_outer).fmt(f)?;
// Custom FormatNodeRule::fmt variant that only formats the inner comments
let format_node_rule_fmt = format_with(|f| {
// No need to handle suppression comments, those are statement only
leading_comments(leading_inner).fmt(f)?;
let is_source_map_enabled = f.options().source_map_generation().is_enabled();
if is_source_map_enabled {
source_position(expression.start()).fmt(f)?;
}
fmt_fields.fmt(f)?;
if is_source_map_enabled {
source_position(expression.end()).fmt(f)?;
}
trailing_comments(trailing_inner).fmt(f)
});
// The actual parenthesized formatting
parenthesized("(", &format_node_rule_fmt, ")")
.with_dangling_comments(parentheses_comment)
.fmt(f)?;
trailing_comments(trailing_outer).fmt(f)?;
Ok(())
}
/// Wraps an expression in an optional parentheses except if its [`NeedsParentheses::needs_parentheses`] implementation
/// indicates that it is okay to omit the parentheses. For example, parentheses can always be omitted for lists,
/// because they already bring their own parentheses.

1
crates/ruff_python_formatter/src/lib.rs
View file

@ -60,7 +60,6 @@ where
}
self.fmt_fields(node, f)?;
self.fmt_dangling_comments(node_comments.dangling, f)?;
if is_source_map_enabled {
source_position(node.end()).fmt(f)?;

15
crates/ruff_python_formatter/tests/snapshots/black_compatibility@miscellaneous__long_strings_flag_disabled.py.snap
View file

@ -320,17 +320,6 @@ long_unmergable_string_with_pragma = (
"formatting"
)
@@ -221,8 +217,8 @@
func_with_bad_comma(
(
"This is a really long string argument to a function that has a trailing comma"
- " which should NOT be there."
- ), # comment after comma
+ " which should NOT be there." # comment after comma
+ ),
)
func_with_bad_parens_that_wont_fit_in_one_line(
```
## Ruff Output
@ -555,8 +544,8 @@ func_with_bad_comma(
func_with_bad_comma(
(
"This is a really long string argument to a function that has a trailing comma"
" which should NOT be there." # comment after comma
),
" which should NOT be there."
), # comment after comma
)
func_with_bad_parens_that_wont_fit_in_one_line(

5
crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap
View file

@ -458,8 +458,9 @@ func(
)
func(
# outer comment
( # inner comment
(
# outer comment
# inner comment
[]
)
)

56
crates/ruff_python_formatter/tests/snapshots/format@expression__unary.py.snap
View file

@ -167,6 +167,17 @@ if True:
+ "WARNING: Removing listed files. Do you really want to continue. yes/n)? "
):
pass
# https://github.com/astral-sh/ruff/issues/7448
x = (
# a
not # b
# c
( # d
# e
True
)
)
```
## Output
@ -217,35 +228,31 @@ if +(
pass
if (
not
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
if (
~
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
if (
-
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
if (
+
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
@ -254,8 +261,8 @@ if (
if (
# unary comment
# operand comment
not (
# operand comment
# comment
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
@ -286,28 +293,31 @@ if not (
## Trailing operator comments
if (
not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment
if ( # comment
not aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
if (
~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment
# comment
~aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
if (
-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment
# comment
-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
if (
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa # comment
# comment
+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+ bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
):
pass
@ -327,13 +337,14 @@ if (
pass
if (
not
# comment
a
not a
):
pass
if not a: # comment
if ( # comment
not a
):
pass
# Regression test for: https://github.com/astral-sh/ruff/issues/7423
@ -345,6 +356,17 @@ if True:
+ "WARNING: Removing listed files. Do you really want to continue. yes/n)? "
):
pass
# https://github.com/astral-sh/ruff/issues/7448
x = (
# a
# b
# c
not ( # d
# e
True
)
)
```

225
crates/ruff_python_formatter/tests/snapshots/format@parentheses__expression_parentheses_comments.py.snap Normal file
View file

@ -0,0 +1,225 @@
---
source: crates/ruff_python_formatter/tests/fixtures.rs
input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/parentheses/expression_parentheses_comments.py
---
## Input
```py
list_with_parenthesized_elements1 = [
# comment leading outer
(
# comment leading inner
1 + 2 # comment trailing inner
) # comment trailing outer
]
list_with_parenthesized_elements2 = [
# leading outer
(1 + 2)
]
list_with_parenthesized_elements3 = [
# leading outer
(1 + 2) # trailing outer
]
list_with_parenthesized_elements4 = [
# leading outer
(1 + 2), # trailing outer
]
list_with_parenthesized_elements5 = [
(1), # trailing outer
(2), # trailing outer
]
nested_parentheses1 = (
(
(
1
) # i
) # j
) # k
nested_parentheses2 = [
(
(
(
1
) # i
# i2
) # j
# j2
) # k
# k2
]
nested_parentheses3 = (
( # a
( # b
1
) # i
) # j
) # k
nested_parentheses4 = [
# a
( # b
# c
( # d
# e
( #f
1
) # i
# i2
) # j
# j2
) # k
# k2
]
x = (
# unary comment
not
# in-between comment
(
# leading inner
"a"
),
not # in-between comment
(
# leading inner
"b"
),
not
( # in-between comment
# leading inner
"c"
),
# 1
not # 2
( # 3
# 4
"d"
)
)
if (
# unary comment
not
# in-between comment
(
# leading inner
1
)
):
pass
# Make sure we keep a inside the parentheses
# https://github.com/astral-sh/ruff/issues/7892
x = (
# a
( # b
1
)
)
```
## Output
```py
list_with_parenthesized_elements1 = [
# comment leading outer
(
# comment leading inner
1 + 2 # comment trailing inner
) # comment trailing outer
]
list_with_parenthesized_elements2 = [
# leading outer
(1 + 2)
]
list_with_parenthesized_elements3 = [
# leading outer
(1 + 2) # trailing outer
]
list_with_parenthesized_elements4 = [
# leading outer
(1 + 2), # trailing outer
]
list_with_parenthesized_elements5 = [
(1), # trailing outer
(2), # trailing outer
]
nested_parentheses1 = (
1 # i # j
) # k
nested_parentheses2 = [
(
1 # i
# i2
# j
# j2
) # k
# k2
]
nested_parentheses3 = ( # a
# b
1 # i # j
) # k
nested_parentheses4 = [
# a
( # b
# c
# d
# e
# f
1 # i
# i2
# j
# j2
) # k
# k2
]
x = (
# unary comment
# in-between comment
not (
# leading inner
"a"
),
# in-between comment
not (
# leading inner
"b"
),
not ( # in-between comment
# leading inner
"c"
),
# 1
# 2
not ( # 3
# 4
"d"
),
)
if (
# unary comment
# in-between comment
not (
# leading inner
1
)
):
pass
# Make sure we keep a inside the parentheses
# https://github.com/astral-sh/ruff/issues/7892
x = (
# a
# b
1
)
```

10
crates/ruff_python_formatter/tests/snapshots/format@statement__with.py.snap
View file

@ -92,10 +92,6 @@ with (
)
): pass
with (a # trailing same line comment
# trailing own line comment
) as b: pass
with (
a # trailing same line comment
# trailing own line comment
@ -420,12 +416,6 @@ with (
) as b:
pass
with (
a # trailing same line comment
# trailing own line comment
) as b:
pass
with (
(
a