Format trailing operator comments as dangling (#7427)

## Summary

Given a trailing operator comment in a unary expression, like:

```python
if (
  not  # comment
  a):
    ...
```

We were attaching these to the operand (`a`), but formatting them in the
unary operator via special handling. Parents shouldn't format the
comments of their children, so this instead attaches them as dangling
comments on the unary expression. (No intended change in formatting.)
This commit is contained in:
Charlie Marsh 2023-09-15 20:34:09 -04:00 committed by GitHub
parent f4d50a2aec
commit cc9e84c144
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 73 additions and 37 deletions

View file

@ -146,3 +146,8 @@ if (
# comment
a):
...
if (
not # comment
a):
...

View file

@ -208,6 +208,7 @@ fn handle_enclosed_comment<'a>(
AnyNodeRef::PatternKeyword(pattern_keyword) => {
handle_pattern_keyword_comment(comment, pattern_keyword, locator)
}
AnyNodeRef::ExprUnaryOp(unary_op) => handle_unary_op_comment(comment, unary_op, locator),
AnyNodeRef::ExprNamedExpr(_) => handle_named_expr_comment(comment, locator),
AnyNodeRef::ExprDict(_) => handle_dict_unpacking_comment(comment, locator)
.or_else(|comment| handle_bracketed_end_of_line_comment(comment, locator)),
@ -1583,6 +1584,46 @@ fn handle_named_expr_comment<'a>(
}
}
/// Attach trailing end-of-line comments on the operator as dangling comments on the enclosing
/// node.
///
/// For example, given:
/// ```python
/// (
/// not # comment
/// True
/// )
/// ```
///
/// The `# comment` will be attached as a dangling comment on the enclosing node, to ensure that
/// it remains on the same line as the operator.
fn handle_unary_op_comment<'a>(
comment: DecoratedComment<'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(
locator.contents(),
TextRange::new(comment.start(), unary_op.operand.start()),
);
if tokenizer
.skip_trivia()
.any(|token| token.kind == SimpleTokenKind::LParen)
{
return 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
/// enclosing node.
///

View file

@ -1,11 +1,11 @@
use ruff_python_ast::node::AnyNodeRef;
use ruff_python_ast::ExprUnaryOp;
use ruff_python_ast::UnaryOp;
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{Ranged, TextLen, TextRange};
use crate::comments::trailing_comments;
use crate::expression::parentheses::{NeedsParentheses, OptionalParentheses};
use crate::comments::{trailing_comments, SourceComment};
use crate::expression::parentheses::{
is_expression_parenthesized, NeedsParentheses, OptionalParentheses,
};
use crate::prelude::*;
#[derive(Default)]
@ -29,21 +29,14 @@ impl FormatNodeRule<ExprUnaryOp> for FormatExprUnaryOp {
token(operator).fmt(f)?;
let comments = f.context().comments().clone();
let dangling = comments.dangling(item);
// Split off the comments that follow after the operator and format them as trailing comments.
// ```python
// (not # comment
// a)
// ```
let leading_operand_comments = comments.leading(operand.as_ref());
let trailing_operator_comments_end =
leading_operand_comments.partition_point(|p| p.line_position().is_end_of_line());
let (trailing_operator_comments, leading_operand_comments) =
leading_operand_comments.split_at(trailing_operator_comments_end);
if !trailing_operator_comments.is_empty() {
trailing_comments(trailing_operator_comments).fmt(f)?;
}
trailing_comments(dangling).fmt(f)?;
// Insert a line break if the operand has comments but itself is not parenthesized.
// ```python
@ -52,8 +45,8 @@ impl FormatNodeRule<ExprUnaryOp> for FormatExprUnaryOp {
// # comment
// a)
// ```
if !leading_operand_comments.is_empty()
&& !is_operand_parenthesized(item, f.context().source())
if comments.has_leading(operand.as_ref())
&& !is_expression_parenthesized(operand.as_ref().into(), f.context().source())
{
hard_line_break().fmt(f)?;
} else if op.is_not() {
@ -62,6 +55,14 @@ impl FormatNodeRule<ExprUnaryOp> for FormatExprUnaryOp {
operand.format().fmt(f)
}
fn fmt_dangling_comments(
&self,
_dangling_comments: &[SourceComment],
_f: &mut PyFormatter,
) -> FormatResult<()> {
Ok(())
}
}
impl NeedsParentheses for ExprUnaryOp {
@ -71,31 +72,10 @@ impl NeedsParentheses for ExprUnaryOp {
context: &PyFormatContext,
) -> OptionalParentheses {
// We preserve the parentheses of the operand. It should not be necessary to break this expression.
if is_operand_parenthesized(self, context.source()) {
if is_expression_parenthesized(self.operand.as_ref().into(), context.source()) {
OptionalParentheses::Never
} else {
OptionalParentheses::Multiline
}
}
}
fn is_operand_parenthesized(unary: &ExprUnaryOp, source: &str) -> bool {
let operator_len = match unary.op {
UnaryOp::Invert => '~'.text_len(),
UnaryOp::Not => "not".text_len(),
UnaryOp::UAdd => '+'.text_len(),
UnaryOp::USub => '-'.text_len(),
};
let trivia_range = TextRange::new(unary.start() + operator_len, unary.operand.start());
if let Some(token) = SimpleTokenizer::new(source, trivia_range)
.skip_trivia()
.next()
{
debug_assert_eq!(token.kind(), SimpleTokenKind::LParen);
true
} else {
false
}
}

View file

@ -152,6 +152,11 @@ if (
# comment
a):
...
if (
not # comment
a):
...
```
## Output
@ -317,6 +322,11 @@ if (
a
):
...
if (
not a # comment
):
...
```