Format call expressions (without call chaining) (#5341)

## Summary

This formats call expressions with magic trailing comma and parentheses
behaviour but without call chaining

## Test Plan

Lots of new test fixtures, including some that don't work yet

Author: konstin

Cargo.lock

@@ -50,14 +50,14 @@ toml = { version = "0.7.2" }
 # v0.0.1
 libcst = { git = "https://github.com/charliermarsh/LibCST", rev = "80e4c1399f95e5beb532fdd1e209ad2dbb470438" }
 
-# Please tag the RustPython version everytime you update its revision here.
+# Please tag the RustPython version everytime you update its revision here and in fuzz/Cargo.toml
 # Tagging the version ensures that older ruff versions continue to build from source even when we rebase our RustPython fork.
-# Current tag: v0.0.6
-ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8078663b6c914c1cb86993e427764f7c422fc12c" }
-rustpython-ast = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8078663b6c914c1cb86993e427764f7c422fc12c" , default-features = false, features = ["num-bigint"]}
-rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8078663b6c914c1cb86993e427764f7c422fc12c", default-features = false, features = ["num-bigint"] }
-rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8078663b6c914c1cb86993e427764f7c422fc12c", default-features = false }
-rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "8078663b6c914c1cb86993e427764f7c422fc12c" , default-features = false, features = ["full-lexer", "num-bigint"] }
+# Current tag: v0.0.7
+ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "c174bbf1f29527edd43d432326327f16f47ab9e0" }
+rustpython-ast = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "c174bbf1f29527edd43d432326327f16f47ab9e0" , default-features = false, features = ["num-bigint"]}
+rustpython-format = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "c174bbf1f29527edd43d432326327f16f47ab9e0", default-features = false, features = ["num-bigint"] }
+rustpython-literal = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "c174bbf1f29527edd43d432326327f16f47ab9e0", default-features = false }
+rustpython-parser = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "c174bbf1f29527edd43d432326327f16f47ab9e0" , default-features = false, features = ["full-lexer", "num-bigint"] }
 
 [profile.release]
 lto = "fat"

Cargo.toml

@@ -15,6 +15,7 @@ use ruff_python_ast::source_code::{Indexer, Locator, SourceFileBuilder, Stylist}
 
 use crate::autofix::{fix_file, FixResult};
 use crate::directives;
+#[cfg(not(fuzzing))]
 use crate::jupyter::Notebook;
 use crate::linter::{check_path, LinterResult};
 use crate::message::{Emitter, EmitterContext, Message, TextEmitter};

crates/ruff/src/test.rs

@@ -0,0 +1,83 @@
+from unittest.mock import MagicMock
+
+
+def f(*args, **kwargs):
+    pass
+
+this_is_a_very_long_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd = 1
+session = MagicMock()
+models = MagicMock()
+
+f()
+
+f(1)
+
+f(x=2)
+
+f(1, x=2)
+
+f(
+    this_is_a_very_long_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd
+)
+f(
+    this_is_a_very_long_keyword_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd=1
+)
+
+f(
+    1,
+    mixed_very_long_arguments=1,
+)
+
+f(
+    this_is_a_very_long_argument_asökdhflakjslaslhfdlaffahflahsöfdhasägporejfäalkdsjäfalisjäfdlkasjd,
+    these_arguments_have_values_that_need_to_break_because_they_are_too_long1=(100000 - 100000000000),
+    these_arguments_have_values_that_need_to_break_because_they_are_too_long2="akshfdlakjsdfad" + "asdfasdfa",
+    these_arguments_have_values_that_need_to_break_because_they_are_too_long3=session,
+)
+
+f(
+    # dangling comment
+)
+
+
+f(
+    only=1, short=1, arguments=1
+)
+
+f(
+    hey_this_is_a_long_call, it_has_funny_attributes_that_breaks_into_three_lines=1
+)
+
+f(
+    hey_this_is_a_very_long_call=1, it_has_funny_attributes_asdf_asdf=1, too_long_for_the_line=1, really=True
+)
+
+# TODO(konstin): Call chains/fluent interface (https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#call-chains)
+result = (
+    session.query(models.Customer.id)
+    .filter(
+        models.Customer.account_id == 10000,
+        models.Customer.email == "[email protected]",
+    )
+    .order_by(models.Customer.id.asc())
+    .all()
+)
+# TODO(konstin): Black has this special case for comment placement where everything stays in one line
+f(
+    "aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa", "aaaaaaaa"
+)
+
+f(
+    session,
+    b=1,
+    ** # oddly placed end-of-line comment
+    dict()
+)
+f(
+    session,
+    b=1,
+    **
+    # oddly placed own line comment
+    dict()
+)
+

crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/call.py

@@ -989,7 +989,7 @@ fn handle_dict_unpacking_comment<'a>(
     match comment.enclosing_node() {
         // TODO: can maybe also add AnyNodeRef::Arguments here, but tricky to test due to
         // https://github.com/astral-sh/ruff/issues/5176
-        AnyNodeRef::ExprDict(_) => {}
+        AnyNodeRef::ExprDict(_) | AnyNodeRef::Keyword(_) => {}
         _ => {
             return CommentPlacement::Default(comment);
         }
@@ -1015,12 +1015,22 @@ fn handle_dict_unpacking_comment<'a>(
     )
     .skip_trivia();
 
+    // 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;
+
     // we start from the preceding node but we skip its token
     for token in tokens.by_ref() {
         // Skip closing parentheses that are not part of the node range
         if token.kind == TokenKind::RParen {
             continue;
         }
+        // The Keyword case
+        if token.kind == TokenKind::Star {
+            count += 1;
+            break;
+        }
+        // The dict case
         debug_assert!(
             matches!(
                 token,
@@ -1034,9 +1044,6 @@ fn handle_dict_unpacking_comment<'a>(
         break;
     }
 
-    // if the remaining tokens from the previous node is exactly `**`,
-    // re-assign the comment to the one that follows the stars
-    let mut count = 0;
     for token in tokens {
         if token.kind != TokenKind::Star {
             return CommentPlacement::Default(comment);
@@ -1050,19 +1057,19 @@ fn handle_dict_unpacking_comment<'a>(
     CommentPlacement::Default(comment)
 }
 
-// Own line comments coming after the node are always dangling comments
-// ```python
-// (
-//      a
-//      # trailing a comment
-//      . # dangling comment
-//      # or this
-//      b
-// )
-// ```
+/// Own line comments coming after the node are always dangling comments
+/// ```python
+/// (
+///      a
+///      # trailing a comment
+///      . # dangling comment
+///      # or this
+///      b
+/// )
+/// ```
 fn handle_attribute_comment<'a>(
     comment: DecoratedComment<'a>,
-    locator: &Locator,
+    _locator: &Locator,
 ) -> CommentPlacement<'a> {
     let Some(attribute) = comment.enclosing_node().expr_attribute() else {
         return CommentPlacement::Default(comment);
@@ -1073,14 +1080,13 @@ fn handle_attribute_comment<'a>(
         return CommentPlacement::Default(comment);
     }
 
-    let between_value_and_attr = TextRange::new(attribute.value.end(), attribute.attr.start());
-
-    let dot = SimpleTokenizer::new(locator.contents(), between_value_and_attr)
-        .skip_trivia()
-        .next()
-        .expect("Expected the `.` character after the value");
-
-    if TextRange::new(dot.end(), attribute.attr.start()).contains(comment.slice().start()) {
+    if TextRange::new(attribute.value.end(), attribute.attr.start())
+        .contains(comment.slice().start())
+    {
+        // ```text
+        // value   .   attr
+        //      ^^^^^^^ the range of dangling comments
+        // ```
         if comment.line_position().is_end_of_line() {
             // Attach to node with b
             // ```python

crates/ruff_python_formatter/src/comments/placement.rs

@@ -1,8 +1,10 @@
-use crate::comments::Comments;
+use crate::builders::PyFormatterExtensions;
+use crate::comments::{dangling_comments, Comments};
 use crate::expression::parentheses::{
     default_expression_needs_parentheses, NeedsParentheses, Parentheses, Parenthesize,
 };
-use crate::{not_yet_implemented_custom_text, FormatNodeRule, PyFormatter};
+use crate::{AsFormat, FormatNodeRule, PyFormatter};
+use ruff_formatter::prelude::{format_with, group, soft_block_indent, text};
 use ruff_formatter::{write, Buffer, FormatResult};
 use rustpython_parser::ast::ExprCall;
 
@@ -11,19 +13,75 @@ pub struct FormatExprCall;
 
 impl FormatNodeRule<ExprCall> for FormatExprCall {
     fn fmt_fields(&self, item: &ExprCall, f: &mut PyFormatter) -> FormatResult<()> {
-        if item.args.is_empty() && item.keywords.is_empty() {
-            write!(
-                f,
-                [not_yet_implemented_custom_text("NOT_IMPLEMENTED_call()")]
-            )
-        } else {
-            write!(
+        let ExprCall {
+            range: _,
+            func,
+            args,
+            keywords,
+        } = item;
+
+        // 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 function has a dangling comment
+        // )
+        // ```
+        if args.is_empty() && keywords.is_empty() {
+            let comments = f.context().comments().clone();
+            let comments = comments.dangling_comments(item);
+            return write!(
                 f,
-                [not_yet_implemented_custom_text(
-                    "NOT_IMPLEMENTED_call(NOT_IMPLEMENTED_arg)"
-                )]
-            )
+                [
+                    func.format(),
+                    text("("),
+                    dangling_comments(comments),
+                    text(")")
+                ]
+            );
         }
+
+        let all_args = format_with(|f| {
+            f.join_comma_separated()
+                .entries(
+                    // We have the parentheses from the call so the arguments never need any
+                    args.iter()
+                        .map(|arg| (arg, arg.format().with_options(Parenthesize::Never))),
+                )
+                .nodes(keywords.iter())
+                .finish()
+        });
+
+        write!(
+            f,
+            [
+                func.format(),
+                text("("),
+                // 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
+                &group(&soft_block_indent(&group(&all_args))),
+                text(")")
+            ]
+        )
+    }
+
+    fn fmt_dangling_comments(&self, _node: &ExprCall, _f: &mut PyFormatter) -> FormatResult<()> {
+        // Handled in `fmt_fields`
+        Ok(())
     }
 }
 

crates/ruff_python_formatter/src/expression/expr_call.rs

@@ -13,10 +13,11 @@ impl FormatNodeRule<Keyword> for FormatKeyword {
             arg,
             value,
         } = item;
-        if let Some(argument) = arg {
-            write!(f, [argument.format(), text("=")])?;
+        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()])
         }
-
-        value.format().fmt(f)
     }
 }