Handle keyword comments between = and value (#6883)

## Summary

This PR adds comment handling for comments between the `=` and the
`value` for keywords, as in the following cases:

```python
func(
    x  # dangling
    =  # dangling
    # dangling
    1,
    **  # dangling
    y
)
```

(Comments after the `**` were already handled in some cases, but I've
unified the handling with the `=` handling.)

Note that, previously, comments between the `**` and its value were
rendered as trailing comments on the value (so they'd appear after `y`).
This struck me as odd since it effectively re-ordered the comment with
respect to its closest AST node (the value). I've made them leading
comments, though I don't know that that's a significant improvement. I
could also imagine us leaving them where they are.
This commit is contained in:
Charlie Marsh 2023-08-30 09:52:51 -04:00 committed by GitHub
parent a3f4d7745a
commit e2b2b1759f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 211 additions and 17 deletions

View file

@ -191,7 +191,10 @@ fn handle_enclosed_comment<'a>(
locator,
)
}
AnyNodeRef::Keyword(_) => handle_dict_unpacking_comment(comment, locator),
AnyNodeRef::Keyword(keyword) => handle_keyword_comment(comment, keyword, locator),
AnyNodeRef::PatternKeyword(pattern_keyword) => {
handle_pattern_keyword_comment(comment, pattern_keyword, 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)),
@ -940,6 +943,74 @@ fn handle_leading_class_with_decorators_comment<'a>(
CommentPlacement::Default(comment)
}
/// Handles comments between a keyword's identifier and value:
/// ```python
/// func(
/// x # dangling
/// = # dangling
/// # dangling
/// 1,
/// ** # dangling
/// y
/// )
/// ```
fn handle_keyword_comment<'a>(
comment: DecoratedComment<'a>,
keyword: &'a ast::Keyword,
locator: &Locator,
) -> CommentPlacement<'a> {
let start = keyword.arg.as_ref().map_or(keyword.start(), Ranged::end);
// If the comment is parenthesized, it should be attached to the value:
// ```python
// func(
// x=( # comment
// 1
// )
// )
// ```
let mut tokenizer =
SimpleTokenizer::new(locator.contents(), TextRange::new(start, comment.start()));
if tokenizer.any(|token| token.kind == SimpleTokenKind::LParen) {
return CommentPlacement::Default(comment);
}
CommentPlacement::leading(comment.enclosing_node(), comment)
}
/// Handles comments between a pattern keyword's identifier and value:
/// ```python
/// case Point2D(
/// x # dangling
/// = # dangling
/// # dangling
/// 1
/// )
/// ```
fn handle_pattern_keyword_comment<'a>(
comment: DecoratedComment<'a>,
pattern_keyword: &'a ast::PatternKeyword,
locator: &Locator,
) -> CommentPlacement<'a> {
// If the comment is parenthesized, it should be attached to the value:
// ```python
// case Point2D(
// x=( # comment
// 1
// )
// )
// ```
let mut tokenizer = SimpleTokenizer::new(
locator.contents(),
TextRange::new(pattern_keyword.attr.end(), comment.start()),
);
if tokenizer.any(|token| token.kind == SimpleTokenKind::LParen) {
return CommentPlacement::Default(comment);
}
CommentPlacement::leading(comment.enclosing_node(), comment)
}
/// Handles comments between `**` and the variable name in dict unpacking
/// It attaches these to the appropriate value node.
///
@ -954,10 +1025,7 @@ fn handle_dict_unpacking_comment<'a>(
comment: DecoratedComment<'a>,
locator: &Locator,
) -> CommentPlacement<'a> {
debug_assert!(matches!(
comment.enclosing_node(),
AnyNodeRef::ExprDict(_) | AnyNodeRef::Keyword(_)
));
debug_assert!(matches!(comment.enclosing_node(), AnyNodeRef::ExprDict(_)));
// no node after our comment so we can't be between `**` and the name (node)
let Some(following) = comment.following_node() else {
@ -980,7 +1048,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
if tokens.any(|token| token.kind == SimpleTokenKind::DoubleStar) {
CommentPlacement::trailing(following, comment)
CommentPlacement::leading(following, comment)
} else {
CommentPlacement::Default(comment)
}

View file

@ -40,6 +40,8 @@ impl Format<PyFormatContext<'_>> for KeyValuePair<'_> {
])]
)
} else {
// TODO(charlie): Make these dangling comments on the `ExprDict`, and identify them
// dynamically, so as to avoid the parent rendering its child's comments.
let comments = f.context().comments().clone();
let leading_value_comments = comments.leading(self.value);
write!(

View file

@ -13,10 +13,10 @@ impl FormatNodeRule<Keyword> for FormatKeyword {
arg,
value,
} = item;
// Comments after the `=` or `**` are reassigned as leading comments on the value.
if let Some(arg) = arg {
write!(f, [arg.format(), text("="), value.format()])
} else {
// Comments after the stars are reassigned as trailing value comments
write!(f, [text("**"), value.format()])
}
}