Fix placement of inline parameter comments (#13379)

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

@@ -458,3 +458,108 @@ def foo(x: S) -> S: ...
 
 @decorator # comment
 def foo(x: S) -> S: ...
+
+
+# Regression tests for https://github.com/astral-sh/ruff/issues/13369
+def foo(
+    arg: (  # comment with non-return annotation
+        int
+        # comment with non-return annotation
+    ),
+):
+    pass
+
+
+def foo(
+    arg: (  # comment with non-return annotation
+        int
+        | range
+        | memoryview
+        # comment with non-return annotation
+    ),
+):
+    pass
+
+def foo(arg: (
+        int
+        # only after
+)):
+    pass
+
+# Asserts that "incorrectly" placed comments don't *move* by fixing https://github.com/astral-sh/ruff/issues/13369
+def foo(
+    # comment with non-return annotation
+    # comment with non-return annotation
+    arg: (int),
+):
+    pass
+
+
+# Comments between *args and **kwargs
+def args_no_type_annotation(*
+    # comment
+    args): pass
+
+def args_type_annotation(*
+    # comment
+    args: int): pass
+
+def args_trailing_end_of_line_comment(* # comment
+    args): pass
+
+def args_blank_line_comment(*
+
+    # comment
+
+    args): pass
+
+def args_with_leading_parameter_comment(
+    # What comes next are arguments
+    *
+    # with an inline comment
+    args): pass
+
+
+def kargs_no_type_annotation(**
+    # comment
+    kwargs): pass
+
+def kwargs_type_annotation(**
+    # comment
+    kwargs: int): pass
+
+
+def args_many_comments(
+    # before
+    *
+    # between * and name
+    args # trailing args
+    # after name
+    ): pass
+
+
+def args_many_comments_with_type_annotation(
+    # before
+    *
+    # between * and name
+    args # trailing args
+    # before colon
+    :  # after colon
+    # before type
+    int # trailing type
+    # after type
+    ): pass
+
+
+
+def args_with_type_annotations_no_after_colon_comment(
+    # before
+    *
+    # between * and name
+    args # trailing args
+    # before colon
+    :
+    # before type
+    int # trailing type
+    # after type
+    ): pass

crates/ruff_python_formatter/src/comments/placement.rs

@@ -2,10 +2,12 @@ use std::cmp::Ordering;
 
 use ast::helpers::comment_indentation_after;
 use ruff_python_ast::whitespace::indentation;
-use ruff_python_ast::{self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameters};
+use ruff_python_ast::{
+    self as ast, AnyNodeRef, Comprehension, Expr, ModModule, Parameter, Parameters,
+};
 use ruff_python_trivia::{
-    find_only_token_in_range, indentation_at_offset, BackwardsTokenizer, CommentRanges,
+    find_only_token_in_range, first_non_trivia_token, indentation_at_offset, BackwardsTokenizer,
-    SimpleToken, SimpleTokenKind, SimpleTokenizer,
+    CommentRanges, SimpleToken, SimpleTokenKind, SimpleTokenizer,
 };
 use ruff_source_file::Locator;
 use ruff_text_size::{Ranged, TextLen, TextRange};
@@ -202,14 +204,7 @@ fn handle_enclosed_comment<'a>(
                 }
             })
         }
-        AnyNodeRef::Parameter(parameter) => {
+        AnyNodeRef::Parameter(parameter) => handle_parameter_comment(comment, parameter, locator),
-            // E.g. a comment between the `*` or `**` and the parameter name.
-            if comment.preceding_node().is_none() || comment.following_node().is_none() {
-                CommentPlacement::leading(parameter, comment)
-            } else {
-                CommentPlacement::Default(comment)
-            }
-        }
         AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) | AnyNodeRef::PatternArguments(_) => {
             handle_bracketed_end_of_line_comment(comment, locator)
         }
@@ -760,6 +755,41 @@ fn handle_parameters_separator_comment<'a>(
     CommentPlacement::Default(comment)
 }
 
+/// Associate comments that come before the `:` starting the type annotation or before the
+/// parameter's name for unannotated parameters as leading parameter-comments.
+///
+/// The parameter's name isn't a node to which comments can be associated.
+/// That's why we pull out all comments that come before the expression name or the type annotation
+/// and make them leading parameter comments. For example:
+/// * `* # comment\nargs`
+/// * `arg # comment\n : int`
+///
+/// Associate comments with the type annotation when possible.
+fn handle_parameter_comment<'a>(
+    comment: DecoratedComment<'a>,
+    parameter: &'a Parameter,
+    locator: &Locator,
+) -> CommentPlacement<'a> {
+    if parameter.annotation.as_deref().is_some() {
+        let colon = first_non_trivia_token(parameter.name.end(), locator.contents()).expect(
+            "A annotated parameter should have a colon following its name when it is valid syntax.",
+        );
+
+        assert_eq!(colon.kind(), SimpleTokenKind::Colon);
+
+        if comment.start() < colon.start() {
+            // The comment is before the colon, pull it out and make it a leading comment of the parameter.
+            CommentPlacement::leading(parameter, comment)
+        } else {
+            CommentPlacement::Default(comment)
+        }
+    } else if comment.start() < parameter.name.start() {
+        CommentPlacement::leading(parameter, comment)
+    } else {
+        CommentPlacement::Default(comment)
+    }
+}
+
 /// Handles comments between the left side and the operator of a binary expression (trailing comments of the left),
 /// and trailing end-of-line comments that are on the same line as the operator.
 ///

crates/ruff_python_formatter/src/other/parameter.rs

@@ -1,7 +1,6 @@
-use ruff_formatter::write;
+use crate::expression::parentheses::is_expression_parenthesized;
-use ruff_python_ast::Parameter;
-
 use crate::prelude::*;
+use ruff_python_ast::Parameter;
 
 #[derive(Default)]
 pub struct FormatParameter;
@@ -16,8 +15,22 @@ impl FormatNodeRule<Parameter> for FormatParameter {
 
         name.format().fmt(f)?;
 
-        if let Some(annotation) = annotation {
+        if let Some(annotation) = annotation.as_deref() {
-            write!(f, [token(":"), space(), annotation.format()])?;
+            token(":").fmt(f)?;
+
+            if f.context().comments().has_leading(annotation)
+                && !is_expression_parenthesized(
+                    annotation.into(),
+                    f.context().comments().ranges(),
+                    f.context().source(),
+                )
+            {
+                hard_line_break().fmt(f)?;
+            } else {
+                space().fmt(f)?;
+            }
+
+            annotation.format().fmt(f)?;
         }
 
         Ok(())

crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap

@@ -142,53 +142,29 @@ variable: (
  ): ...
  
  
-@@ -143,34 +141,31 @@
+@@ -153,16 +151,18 @@
- 
  
  def foo(
--    arg: (  # comment with non-return annotation
+     arg: (  # comment with non-return annotation
--        int
--        # comment with non-return annotation
--    ),
-+    # comment with non-return annotation
-+    # comment with non-return annotation
-+    arg: (int),
- ):
-     pass
- 
- 
- def foo(
--    arg: (  # comment with non-return annotation
 -        int
 -        | range
 -        | memoryview
--        # comment with non-return annotation
++        int | range | memoryview
--    ),
+         # comment with non-return annotation
-+    # comment with non-return annotation
+     ),
-+    # comment with non-return annotation
-+    arg: (int | range | memoryview),
  ):
      pass
  
  
 -def foo(arg: int):  # only before
 +def foo(
-+    # only before
++    arg: (  # only before
-+    arg: (int),
++        int
++    ),
 +):
      pass
  
  
- def foo(
--    arg: (
--        int
--        # only after
--    ),
-+    # only after
-+    arg: (int),
- ):
-     pass
- 
 ```
 
 ## Ruff Output
@@ -337,31 +313,36 @@ def foo() -> (
 
 
 def foo(
+    arg: (  # comment with non-return annotation
+        int
         # comment with non-return annotation
-    # comment with non-return annotation
+    ),
-    arg: (int),
 ):
     pass
 
 
 def foo(
+    arg: (  # comment with non-return annotation
+        int | range | memoryview
         # comment with non-return annotation
-    # comment with non-return annotation
+    ),
-    arg: (int | range | memoryview),
 ):
     pass
 
 
 def foo(
-    # only before
+    arg: (  # only before
-    arg: (int),
+        int
+    ),
 ):
     pass
 
 
 def foo(
+    arg: (
+        int
         # only after
-    arg: (int),
+    ),
 ):
     pass
 

crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap

@@ -464,6 +464,111 @@ def foo(x: S) -> S: ...
 
 @decorator # comment
 def foo(x: S) -> S: ...
+
+
+# Regression tests for https://github.com/astral-sh/ruff/issues/13369
+def foo(
+    arg: (  # comment with non-return annotation
+        int
+        # comment with non-return annotation
+    ),
+):
+    pass
+
+
+def foo(
+    arg: (  # comment with non-return annotation
+        int
+        | range
+        | memoryview
+        # comment with non-return annotation
+    ),
+):
+    pass
+
+def foo(arg: (
+        int
+        # only after
+)):
+    pass
+
+# Asserts that "incorrectly" placed comments don't *move* by fixing https://github.com/astral-sh/ruff/issues/13369
+def foo(
+    # comment with non-return annotation
+    # comment with non-return annotation
+    arg: (int),
+):
+    pass
+
+
+# Comments between *args and **kwargs
+def args_no_type_annotation(*
+    # comment
+    args): pass
+
+def args_type_annotation(*
+    # comment
+    args: int): pass
+
+def args_trailing_end_of_line_comment(* # comment
+    args): pass
+
+def args_blank_line_comment(*
+
+    # comment
+
+    args): pass
+
+def args_with_leading_parameter_comment(
+    # What comes next are arguments
+    *
+    # with an inline comment
+    args): pass
+
+
+def kargs_no_type_annotation(**
+    # comment
+    kwargs): pass
+
+def kwargs_type_annotation(**
+    # comment
+    kwargs: int): pass
+
+
+def args_many_comments(
+    # before
+    *
+    # between * and name
+    args # trailing args
+    # after name
+    ): pass
+
+
+def args_many_comments_with_type_annotation(
+    # before
+    *
+    # between * and name
+    args # trailing args
+    # before colon
+    :  # after colon
+    # before type
+    int # trailing type
+    # after type
+    ): pass
+
+
+
+def args_with_type_annotations_no_after_colon_comment(
+    # before
+    *
+    # between * and name
+    args # trailing args
+    # before colon
+    :
+    # before type
+    int # trailing type
+    # after type
+    ): pass
 ```
 
 ## Output
@@ -1089,6 +1194,130 @@ def foo(x: S) -> S: ...
 
 @decorator  # comment
 def foo(x: S) -> S: ...
+
+
+# Regression tests for https://github.com/astral-sh/ruff/issues/13369
+def foo(
+    arg: (  # comment with non-return annotation
+        int
+        # comment with non-return annotation
+    ),
+):
+    pass
+
+
+def foo(
+    arg: (  # comment with non-return annotation
+        int | range | memoryview
+        # comment with non-return annotation
+    ),
+):
+    pass
+
+
+def foo(
+    arg: (
+        int
+        # only after
+    ),
+):
+    pass
+
+
+# Asserts that "incorrectly" placed comments don't *move* by fixing https://github.com/astral-sh/ruff/issues/13369
+def foo(
+    # comment with non-return annotation
+    # comment with non-return annotation
+    arg: (int),
+):
+    pass
+
+
+# Comments between *args and **kwargs
+def args_no_type_annotation(
+    # comment
+    *args,
+):
+    pass
+
+
+def args_type_annotation(
+    # comment
+    *args: int,
+):
+    pass
+
+
+def args_trailing_end_of_line_comment(
+    # comment
+    *args,
+):
+    pass
+
+
+def args_blank_line_comment(
+    # comment
+    *args,
+):
+    pass
+
+
+def args_with_leading_parameter_comment(
+    # What comes next are arguments
+    # with an inline comment
+    *args,
+):
+    pass
+
+
+def kargs_no_type_annotation(
+    # comment
+    **kwargs,
+):
+    pass
+
+
+def kwargs_type_annotation(
+    # comment
+    **kwargs: int,
+):
+    pass
+
+
+def args_many_comments(
+    # before
+    # between * and name
+    *args,  # trailing args
+    # after name
+):
+    pass
+
+
+def args_many_comments_with_type_annotation(
+    # before
+    # between * and name
+    # trailing args
+    # before colon
+    *args:
+    # after colon
+    # before type
+    int,  # trailing type
+    # after type
+):
+    pass
+
+
+def args_with_type_annotations_no_after_colon_comment(
+    # before
+    # between * and name
+    # trailing args
+    # before colon
+    *args:
+    # before type
+    int,  # trailing type
+    # after type
+):
+    pass
 ```
 
 

crates/ruff_python_trivia/src/tokenizer.rs

@@ -4,14 +4,12 @@ use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
 
 use crate::{is_python_whitespace, Cursor};
 
-/// Searches for the first non-trivia character in `range`.
+/// Searches for the first non-trivia character after `offset`.
 ///
 /// The search skips over any whitespace and comments.
 ///
-/// Returns `Some` if the range contains any non-trivia character. The first item is the absolute offset
+/// Returns `Some` if the source code after `offset` contains any non-trivia character.///
-/// of the character, the second item the non-trivia character.
+/// Returns `None` if the text after `offset` is empty or only contains trivia (whitespace or comments).
-///
-/// Returns `None` if the range is empty or only contains trivia (whitespace or comments).
 pub fn first_non_trivia_token(offset: TextSize, code: &str) -> Option<SimpleToken> {
     SimpleTokenizer::starts_at(offset, code)
         .skip_trivia()