From 3eed71393de6aa67ea4e8f143aa38faf04b06777 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 6 Aug 2023 22:15:59 -0400 Subject: [PATCH] Fixup comment handling on inner parenthesis in function definition --- .../test/fixtures/ruff/statement/function.py | 76 +++++++++ .../src/comments/placement.rs | 1 + .../src/other/parameters.rs | 83 ++++++++-- .../format@statement__function.py.snap | 149 ++++++++++++++++++ 4 files changed, 294 insertions(+), 15 deletions(-) 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 95573170b12b7..42e643032ca2f 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 @@ -295,3 +295,79 @@ def f(*args, b, **kwds, ): pass def f(*, b, **kwds, ): pass def f(a, *args, b, **kwds, ): pass def f(a, *, b, **kwds, ): pass + +# Handle comments on open parenthesis. +def f( + # first + # second +): + ... + + +def f( # first + # second +): # third + ... + + +def f( # first +): # second + ... + + +def f( + a, + /, + # first + b + # second +): + ... + + +def f( # first + *, + # second + b + # third +): + ... + + +def f( # first + # second + *, + # third + b + # fourth +): + ... + + +def f( # first + a, + # second +): # third + ... + + +def f( # first + a +): # second + ... + + +def f( # first + a + # second +): # third + ... + + +def f( # first + a, + / # second + , + # third +): + ... diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index 728ea3e7ca2f1..79c56ca277afd 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -32,6 +32,7 @@ pub(super) fn place_comment<'a>( comment.or_else(|comment| match comment.enclosing_node() { AnyNodeRef::Parameters(arguments) => { handle_parameters_separator_comment(comment, arguments, locator) + .or_else(|comment| handle_bracketed_end_of_line_comment(comment, locator)) } AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) => { handle_bracketed_end_of_line_comment(comment, locator) diff --git a/crates/ruff_python_formatter/src/other/parameters.rs b/crates/ruff_python_formatter/src/other/parameters.rs index 950f6dcaf9345..b9daee7721f73 100644 --- a/crates/ruff_python_formatter/src/other/parameters.rs +++ b/crates/ruff_python_formatter/src/other/parameters.rs @@ -1,18 +1,17 @@ use std::usize; -use ruff_python_ast::{Parameters, Ranged}; -use ruff_text_size::{TextRange, TextSize}; - use ruff_formatter::{format_args, write, FormatRuleWithOptions}; use ruff_python_ast::node::{AnyNodeRef, AstNode}; +use ruff_python_ast::{Parameters, Ranged}; use ruff_python_trivia::{SimpleToken, SimpleTokenKind, SimpleTokenizer}; +use ruff_text_size::{TextRange, TextSize}; +use crate::builders::empty_parenthesized_with_dangling_comments; use crate::comments::{ - dangling_comments, leading_comments, leading_node_comments, trailing_comments, - CommentLinePosition, SourceComment, + leading_comments, leading_node_comments, trailing_comments, CommentLinePosition, SourceComment, }; use crate::context::{NodeLevel, WithNodeLevel}; -use crate::expression::parentheses::parenthesized; +use crate::expression::parentheses::parenthesized_with_dangling_comments; use crate::prelude::*; use crate::FormatNodeRule; @@ -61,9 +60,47 @@ impl FormatNodeRule for FormatParameters { kwarg, } = item; + let (slash, star) = find_argument_separators(f.context().source(), item); + let comments = f.context().comments().clone(); let dangling = comments.dangling_comments(item); - let (slash, star) = find_argument_separators(f.context().source(), item); + + // First dangling comment: trailing the opening parenthesis, e.g.: + // ```python + // def f( # comment + // x, + // y, + // z, + // ): ... + // TODO(charlie): We already identified this comment as such in placement.rs. Consider + // labeling it as such. See: https://github.com/astral-sh/ruff/issues/5247. + let parenthesis_comments_end = usize::from(dangling.first().is_some_and(|comment| { + if comment.line_position().is_end_of_line() { + // Ensure that there are no tokens between the open bracket and the comment. + let mut lexer = SimpleTokenizer::new( + f.context().source(), + TextRange::new(item.start(), comment.start()), + ) + .skip_trivia() + .skip_while(|t| { + matches!( + t.kind(), + SimpleTokenKind::LParen + | SimpleTokenKind::LBrace + | SimpleTokenKind::LBracket + ) + }); + if lexer.next().is_none() { + return true; + } + } + false + })); + + // Separate into (dangling comments on the open parenthesis) and (dangling comments on the + // argument separators, e.g., `*` or `/`). + let (parenthesis_dangling, parameters_dangling) = + dangling.split_at(parenthesis_comments_end); let format_inner = format_with(|f: &mut PyFormatter| { let separator = format_with(|f| write!(f, [text(","), soft_line_break_or_space()])); @@ -76,10 +113,18 @@ impl FormatNodeRule for FormatParameters { last_node = Some(parameter_with_default.into()); } + // Second dangling comment: trailing the slash, e.g.: + // ```python + // def f( + // x, + // /, # comment + // y, + // z, + // ): ... let slash_comments_end = if posonlyargs.is_empty() { 0 } else { - let slash_comments_end = dangling.partition_point(|comment| { + let slash_comments_end = parameters_dangling.partition_point(|comment| { let assignment = assign_argument_separator_comment_placement( slash.as_ref(), star.as_ref(), @@ -95,7 +140,7 @@ impl FormatNodeRule for FormatParameters { }); joiner.entry(&CommentsAroundText { text: "/", - comments: &dangling[..slash_comments_end], + comments: ¶meters_dangling[..slash_comments_end], }); slash_comments_end }; @@ -135,7 +180,7 @@ impl FormatNodeRule for FormatParameters { // ``` joiner.entry(&CommentsAroundText { text: "*", - comments: &dangling[slash_comments_end..], + comments: ¶meters_dangling[slash_comments_end..], }); } @@ -202,14 +247,22 @@ impl FormatNodeRule for FormatParameters { // No parameters, format any dangling comments between `()` write!( f, - [ + [empty_parenthesized_with_dangling_comments( text("("), - block_indent(&dangling_comments(dangling)), - text(")") - ] + dangling, + text(")"), + )] ) } else { - write!(f, [parenthesized("(", &group(&format_inner), ")")]) + write!( + f, + [parenthesized_with_dangling_comments( + "(", + parenthesis_dangling, + &group(&format_inner), + ")" + )] + ) } } 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 9b0021e5ad012..8a7676fce96f6 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 @@ -301,6 +301,82 @@ def f(*args, b, **kwds, ): pass def f(*, b, **kwds, ): pass def f(a, *args, b, **kwds, ): pass def f(a, *, b, **kwds, ): pass + +# Handle comments on open parenthesis. +def f( + # first + # second +): + ... + + +def f( # first + # second +): # third + ... + + +def f( # first +): # second + ... + + +def f( + a, + /, + # first + b + # second +): + ... + + +def f( # first + *, + # second + b + # third +): + ... + + +def f( # first + # second + *, + # third + b + # fourth +): + ... + + +def f( # first + a, + # second +): # third + ... + + +def f( # first + a +): # second + ... + + +def f( # first + a + # second +): # third + ... + + +def f( # first + a, + / # second + , + # third +): + ... ``` ## Output @@ -753,6 +829,79 @@ def f( **kwds, ): pass + + +# Handle comments on open parenthesis. +def f( + # first + # second +): + ... + + +def f( # first + # second +): # third + ... + + +def f(): # first # second + ... + + +def f( + a, + /, + # first + b, + # second +): + ... + + +def f( # first + *, + # second + b, + # third +): + ... + + +def f( # first + # second + *, + # third + b, + # fourth +): + ... + + +def f( # first + a, + # second +): # third + ... + + +def f(a): # first # second + ... + + +def f( # first + a, + # second +): # third + ... + + +def f( # first + a, + # third + /, # second +): + ... ```