diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py index 3553c57926..3aacd5926d 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/function.py +++ b/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 diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 41813d443b..badc8667ff 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/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, - SimpleToken, SimpleTokenKind, SimpleTokenizer, + find_only_token_in_range, first_non_trivia_token, indentation_at_offset, BackwardsTokenizer, + 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) => { - // 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::Parameter(parameter) => handle_parameter_comment(comment, parameter, locator), 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. /// diff --git a/crates/ruff_python_formatter/src/other/parameter.rs b/crates/ruff_python_formatter/src/other/parameter.rs index fb0e84fbcd..8a634928fc 100644 --- a/crates/ruff_python_formatter/src/other/parameter.rs +++ b/crates/ruff_python_formatter/src/other/parameter.rs @@ -1,7 +1,6 @@ -use ruff_formatter::write; -use ruff_python_ast::Parameter; - +use crate::expression::parentheses::is_expression_parenthesized; use crate::prelude::*; +use ruff_python_ast::Parameter; #[derive(Default)] pub struct FormatParameter; @@ -16,8 +15,22 @@ impl FormatNodeRule for FormatParameter { name.format().fmt(f)?; - if let Some(annotation) = annotation { - write!(f, [token(":"), space(), annotation.format()])?; + if let Some(annotation) = annotation.as_deref() { + 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(()) diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap index 3bdabdf35a..480f37a879 100644 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__function_trailing_comma.py.snap +++ b/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 -- 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 + arg: ( # 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), ++ int | range | memoryview + # comment with non-return annotation + ), ): pass -def foo(arg: int): # only before +def foo( -+ # only before -+ arg: (int), ++ arg: ( # only before ++ int ++ ), +): pass - def foo( -- arg: ( -- int -- # only after -- ), -+ # only after -+ arg: (int), - ): - pass - ``` ## Ruff Output @@ -337,31 +313,36 @@ def foo() -> ( def foo( - # comment with non-return annotation - # comment with non-return annotation - arg: (int), + arg: ( # comment with non-return annotation + int + # comment with non-return annotation + ), ): pass def foo( - # comment with non-return annotation - # comment with non-return annotation - arg: (int | range | memoryview), + arg: ( # comment with non-return annotation + int | range | memoryview + # comment with non-return annotation + ), ): pass def foo( - # only before - arg: (int), + arg: ( # only before + int + ), ): pass def foo( - # only after - arg: (int), + arg: ( + int + # only after + ), ): pass diff --git a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap index 2e353d6aa2..64eaa01b0b 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@statement__function.py.snap +++ b/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 ``` diff --git a/crates/ruff_python_trivia/src/tokenizer.rs b/crates/ruff_python_trivia/src/tokenizer.rs index 13181a7489..b480214acb 100644 --- a/crates/ruff_python_trivia/src/tokenizer.rs +++ b/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 -/// of the character, the second item the non-trivia character. -/// -/// Returns `None` if the range is empty or only contains trivia (whitespace or comments). +/// Returns `Some` if the source code after `offset` contains any non-trivia character./// +/// Returns `None` if the text after `offset` is empty or only contains trivia (whitespace or comments). pub fn first_non_trivia_token(offset: TextSize, code: &str) -> Option { SimpleTokenizer::starts_at(offset, code) .skip_trivia()