Add formatter support for call and class definition Arguments (#6274)

## Summary

This PR leverages the `Arguments` AST node introduced in #6259 in the
formatter, which ensures that we correctly handle trailing comments in
calls, like:

```python
f(
  1,
  # comment
)

pass
```

(Previously, this was treated as a leading comment on `pass`.)

This also allows us to unify the argument handling across calls and
class definitions.

## Test Plan

A bunch of new fixture tests, plus improved Black compatibility.
This commit is contained in:
Charlie Marsh 2023-08-02 11:54:22 -04:00 committed by GitHub
parent b095b7204b
commit 4c53bfe896
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 640 additions and 252 deletions

View file

@ -0,0 +1,152 @@
use ruff_formatter::write;
use ruff_python_ast::node::{AnyNodeRef, AstNode};
use ruff_python_ast::{Arguments, Expr, ExprCall, Ranged};
use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer};
use ruff_text_size::{TextRange, TextSize};
use crate::builders::empty_parenthesized_with_dangling_comments;
use crate::comments::trailing_comments;
use crate::expression::expr_generator_exp::GeneratorExpParentheses;
use crate::expression::parentheses::{
parenthesized, NeedsParentheses, OptionalParentheses, Parentheses,
};
use crate::prelude::*;
use crate::FormatNodeRule;
#[derive(Default)]
pub struct FormatArguments;
impl FormatNodeRule<Arguments> for FormatArguments {
fn fmt_fields(&self, item: &Arguments, f: &mut PyFormatter) -> FormatResult<()> {
// We have a case with `f()` without any argument, which is a special case because we can
// have a comment with no node attachment inside:
// ```python
// f(
// # This call has a dangling comment.
// )
// ```
if item.args.is_empty() && item.keywords.is_empty() {
let comments = f.context().comments().clone();
return write!(
f,
[empty_parenthesized_with_dangling_comments(
text("("),
comments.dangling_comments(item),
text(")"),
)]
);
}
// If the arguments are non-empty, then a dangling comment indicates a comment on the
// same line as the opening parenthesis, e.g.:
// ```python
// f( # This call has a dangling comment.
// a,
// b,
// c,
// )
let comments = f.context().comments().clone();
let dangling_comments = comments.dangling_comments(item.as_any_node_ref());
write!(f, [trailing_comments(dangling_comments)])?;
let all_arguments = format_with(|f: &mut PyFormatter| {
let source = f.context().source();
let mut joiner = f.join_comma_separated(item.end());
match item.args.as_slice() {
[arg] if item.keywords.is_empty() => {
match arg {
Expr::GeneratorExp(generator_exp) => joiner.entry(
generator_exp,
&generator_exp
.format()
.with_options(GeneratorExpParentheses::StripIfOnlyFunctionArg),
),
other => {
let parentheses =
if is_single_argument_parenthesized(arg, item.end(), source) {
Parentheses::Always
} else {
Parentheses::Never
};
joiner.entry(other, &other.format().with_options(parentheses))
}
};
}
args => {
joiner
.entries(
// We have the parentheses from the call so the item never need any
args.iter()
.map(|arg| (arg, arg.format().with_options(Parentheses::Preserve))),
)
.nodes(item.keywords.iter());
}
}
joiner.finish()
});
write!(
f,
[
// The outer group is for things like
// ```python
// get_collection(
// hey_this_is_a_very_long_call,
// it_has_funny_attributes_asdf_asdf,
// too_long_for_the_line,
// really=True,
// )
// ```
// The inner group is for things like:
// ```python
// get_collection(
// hey_this_is_a_very_long_call, it_has_funny_attributes_asdf_asdf, really=True
// )
// ```
// TODO(konstin): Doesn't work see wrongly formatted test
parenthesized("(", &group(&all_arguments), ")")
]
)
}
fn fmt_dangling_comments(&self, _node: &Arguments, _f: &mut PyFormatter) -> FormatResult<()> {
// Handled in `fmt_fields`
Ok(())
}
}
impl NeedsParentheses for ExprCall {
fn needs_parentheses(
&self,
_parent: AnyNodeRef,
context: &PyFormatContext,
) -> OptionalParentheses {
self.func.needs_parentheses(self.into(), context)
}
}
fn is_single_argument_parenthesized(argument: &Expr, call_end: TextSize, source: &str) -> bool {
let mut has_seen_r_paren = false;
for token in
SimpleTokenizer::new(source, TextRange::new(argument.end(), call_end)).skip_trivia()
{
match token.kind() {
SimpleTokenKind::RParen => {
if has_seen_r_paren {
return true;
}
has_seen_r_paren = true;
}
// Skip over any trailing comma
SimpleTokenKind::Comma => continue,
_ => {
// Passed the arguments
break;
}
}
}
false
}

View file

@ -1,4 +1,5 @@
pub(crate) mod alias;
pub(crate) mod arguments;
pub(crate) mod comprehension;
pub(crate) mod decorator;
pub(crate) mod elif_else_clause;