From 40407dcce52eaf760cfe38590b8d5273d4b8b98e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 14 Aug 2023 09:52:19 -0400 Subject: [PATCH] Avoid marking inner-parenthesized comments as dangling bracket comments (#6517) ## Summary The bracketed-end-of-line comment rule is meant to assign comments like this as "immediately following the bracket": ```python f( # comment 1 ) ``` However, the logic was such that we treated this equivalently: ```python f( ( # comment 1 ) ) ``` This PR modifies the placement logic to ensure that we only skip the opening bracket, and not any nested brackets. The above is now formatted as: ```python f( ( # comment 1 ) ) ``` (But will be corrected once we handle parenthesized comments properly.) ## Test Plan `cargo test` Confirmed no change in similarity score. --- .../test/fixtures/ruff/expression/call.py | 6 ++++++ .../src/comments/placement.rs | 20 ++++++++++++------- .../snapshots/format@expression__call.py.snap | 13 ++++++++++++ 3 files changed, 32 insertions(+), 7 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 82166ba564..03502701ec 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 @@ -115,3 +115,9 @@ f ( threshold_date = datetime.datetime.now() - datetime.timedelta( # comment days=threshold_days_threshold_days ) + +f( + ( # comment + 1 + ) +) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index d55ec6c91b..d8d8c03578 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -1150,14 +1150,20 @@ fn handle_bracketed_end_of_line_comment<'a>( locator.contents(), TextRange::new(comment.enclosing_node().start(), comment.start()), ) - .skip_trivia() - .skip_while(|t| { - matches!( - t.kind(), - SimpleTokenKind::LParen | SimpleTokenKind::LBrace | SimpleTokenKind::LBracket - ) - }); + .skip_trivia(); + // Skip the opening parenthesis. + let Some(paren) = lexer.next() else { + return CommentPlacement::Default(comment); + }; + debug_assert!(matches!( + paren.kind(), + SimpleTokenKind::LParen | SimpleTokenKind::LBrace | SimpleTokenKind::LBracket + )); + + // If there are no additional tokens between the open parenthesis and the comment, then + // it should be attached as a dangling comment on the brackets, rather than a leading + // comment on the first argument. if lexer.next().is_none() { return CommentPlacement::dangling(comment.enclosing_node(), comment); } 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 283a06b31a..b6dab5b5df 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 @@ -121,6 +121,12 @@ f ( threshold_date = datetime.datetime.now() - datetime.timedelta( # comment days=threshold_days_threshold_days ) + +f( + ( # comment + 1 + ) +) ``` ## Output @@ -241,6 +247,13 @@ f( threshold_date = datetime.datetime.now() - datetime.timedelta( # comment days=threshold_days_threshold_days ) + +f( + ( + # comment + 1 + ) +) ```