From 453078971c2d8afac6b7d2181530a3eb764587b5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 7 Aug 2023 09:43:57 -0400 Subject: [PATCH] Use `parenthesized_with_dangling_comments` in arguments formatter (#6376) ## Summary Fixes an instability whereby this: ```python def get_recent_deployments(threshold_days: int) -> Set[str]: # Returns a list of deployments not older than threshold days # including `/root/zulip` directory if it exists. recent = set() threshold_date = datetime.datetime.now() - datetime.timedelta( # noqa: DTZ005 days=threshold_days ) ``` Was being formatted as: ```python def get_recent_deployments(threshold_days: int) -> Set[str]: # Returns a list of deployments not older than threshold days # including `/root/zulip` directory if it exists. recent = set() threshold_date = ( datetime.datetime.now() - datetime.timedelta(days=threshold_days) # noqa: DTZ005 ) ``` Which was in turn being formatted as: ```python def get_recent_deployments(threshold_days: int) -> Set[str]: # Returns a list of deployments not older than threshold days # including `/root/zulip` directory if it exists. recent = set() threshold_date = ( datetime.datetime.now() - datetime.timedelta(days=threshold_days) # noqa: DTZ005 ) ``` The second-to-third formattings still differs from Black because we aren't taking the line suffix into account when splitting (https://github.com/astral-sh/ruff/issues/6377), but the first formatting is correct and should be unchanged (i.e., the first-to-second formattings is incorrect, and fixed here). ## Test Plan `cargo run --bin ruff_dev -- format-dev --stability-check ../zulip` --- .../test/fixtures/ruff/expression/call.py | 4 +++ .../src/other/arguments.rs | 33 ++++++++++--------- .../snapshots/format@expression__call.py.snap | 8 +++++ 3 files changed, 30 insertions(+), 15 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py index a158a2ec4dfdc1..82166ba564ca20 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/call.py @@ -111,3 +111,7 @@ def f(*args, **kwargs): 1 # abc ) + +threshold_date = datetime.datetime.now() - datetime.timedelta( # comment + days=threshold_days_threshold_days +) diff --git a/crates/ruff_python_formatter/src/other/arguments.rs b/crates/ruff_python_formatter/src/other/arguments.rs index e99c31807c8ed1..05dfa5e3475b4e 100644 --- a/crates/ruff_python_formatter/src/other/arguments.rs +++ b/crates/ruff_python_formatter/src/other/arguments.rs @@ -5,9 +5,8 @@ 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, Parentheses}; +use crate::expression::parentheses::{parenthesized_with_dangling_comments, Parentheses}; use crate::prelude::*; use crate::FormatNodeRule; @@ -35,18 +34,6 @@ impl FormatNodeRule for FormatArguments { ); } - // 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()); @@ -84,6 +71,17 @@ impl FormatNodeRule for FormatArguments { joiner.finish() }); + // 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, [ @@ -103,7 +101,12 @@ impl FormatNodeRule for FormatArguments { // ) // ``` // TODO(konstin): Doesn't work see wrongly formatted test - parenthesized("(", &group(&all_arguments), ")") + parenthesized_with_dangling_comments( + "(", + dangling_comments, + &group(&all_arguments), + ")" + ) ] ) } diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap index 8120fdd4d8bb97..45bcc8a76ab866 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__call.py.snap @@ -117,6 +117,10 @@ f ( 1 # abc ) + +threshold_date = datetime.datetime.now() - datetime.timedelta( # comment + days=threshold_days_threshold_days +) ``` ## Output @@ -230,6 +234,10 @@ f( 1 # abc ) + +threshold_date = datetime.datetime.now() - datetime.timedelta( # comment + days=threshold_days_threshold_days +) ```