From 865c89800ee7bc03ec43b87ce8c0f063a4f50a7d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 23 Sep 2023 22:08:44 -0400 Subject: [PATCH] Avoid searching for bracketed comments in unparenthesized generators (#7627) Similar to tuples, a generator _can_ be parenthesized or unparenthesized. Only search for bracketed comments if it contains its own parentheses. Closes https://github.com/astral-sh/ruff/issues/7623. --- .../fixtures/ruff/expression/generator_exp.py | 17 +++++++++ .../src/comments/placement.rs | 7 +++- .../src/expression/expr_generator_exp.rs | 11 +++--- .../src/expression/expr_tuple.rs | 2 +- .../format@expression__generator_exp.py.snap | 36 +++++++++++++++++++ 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/generator_exp.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/generator_exp.py index 1fa75f69868a0..d41c5335332cc 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/generator_exp.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/expression/generator_exp.py @@ -52,3 +52,20 @@ aaaaaaaaaaaaaaaaaaaaa = ( o for o in self.registry.values if o.__class__ is not ModelAdmin ) + +# Regression test for: https://github.com/astral-sh/ruff/issues/7623 +tuple( + 0 # comment + for x in y +) + +tuple( + (0 # comment + for x in y) +) + +tuple( + ( # comment + 0 for x in y + ) +) diff --git a/crates/ruff_python_formatter/src/comments/placement.rs b/crates/ruff_python_formatter/src/comments/placement.rs index cad5066678327..bd67a0771a704 100644 --- a/crates/ruff_python_formatter/src/comments/placement.rs +++ b/crates/ruff_python_formatter/src/comments/placement.rs @@ -11,6 +11,7 @@ use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use crate::comments::visitor::{CommentPlacement, DecoratedComment}; +use crate::expression::expr_generator_exp::is_generator_parenthesized; use crate::expression::expr_slice::{assign_comment_in_slice, ExprSliceCommentSection}; use crate::expression::expr_tuple::is_tuple_parenthesized; use crate::other::parameters::{ @@ -290,12 +291,16 @@ fn handle_enclosed_comment<'a>( AnyNodeRef::ExprFString(fstring) => CommentPlacement::dangling(fstring, comment), AnyNodeRef::ExprList(_) | AnyNodeRef::ExprSet(_) - | AnyNodeRef::ExprGeneratorExp(_) | AnyNodeRef::ExprListComp(_) | AnyNodeRef::ExprSetComp(_) => handle_bracketed_end_of_line_comment(comment, locator), AnyNodeRef::ExprTuple(tuple) if is_tuple_parenthesized(tuple, locator.contents()) => { handle_bracketed_end_of_line_comment(comment, locator) } + AnyNodeRef::ExprGeneratorExp(generator) + if is_generator_parenthesized(generator, locator.contents()) => + { + handle_bracketed_end_of_line_comment(comment, locator) + } _ => CommentPlacement::Default(comment), } } diff --git a/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs b/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs index db577e3d3cdc9..5ac878a1876ac 100644 --- a/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs +++ b/crates/ruff_python_formatter/src/expression/expr_generator_exp.rs @@ -102,8 +102,9 @@ impl NeedsParentheses for ExprGeneratorExp { } } -fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> bool { - // / Count the number of open parentheses between the start of the tuple and the first element. +/// Return `true` if a generator is parenthesized in the source code. +pub(crate) fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> bool { + // Count the number of open parentheses between the start of the generator and the first element. let open_parentheses_count = SimpleTokenizer::new( source, TextRange::new(generator.start(), generator.elt.start()), @@ -115,7 +116,7 @@ fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> boo return false; } - // Count the number of parentheses between the end of the first element and its trailing comma. + // Count the number of parentheses between the end of the generator and its trailing comma. let close_parentheses_count = SimpleTokenizer::new( source, TextRange::new( @@ -130,7 +131,7 @@ fn is_generator_parenthesized(generator: &ExprGeneratorExp, source: &str) -> boo .filter(|token| token.kind() == SimpleTokenKind::RParen) .count(); - // If the number of open parentheses is greater than the number of close parentheses, the generator - // is parenthesized. + // If the number of open parentheses is greater than the number of close parentheses, the + // generator is parenthesized. open_parentheses_count > close_parentheses_count } diff --git a/crates/ruff_python_formatter/src/expression/expr_tuple.rs b/crates/ruff_python_formatter/src/expression/expr_tuple.rs index cb3f848f2dbe1..abddb2ac35519 100644 --- a/crates/ruff_python_formatter/src/expression/expr_tuple.rs +++ b/crates/ruff_python_formatter/src/expression/expr_tuple.rs @@ -221,7 +221,7 @@ impl NeedsParentheses for ExprTuple { } } -/// Check if a tuple has already had parentheses in the input +/// Return `true` if a tuple is parenthesized in the source code. pub(crate) fn is_tuple_parenthesized(tuple: &ExprTuple, source: &str) -> bool { let Some(elt) = tuple.elts.first() else { return false; diff --git a/crates/ruff_python_formatter/tests/snapshots/format@expression__generator_exp.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@expression__generator_exp.py.snap index cc166a931b005..bce2611ab0621 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@expression__generator_exp.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@expression__generator_exp.py.snap @@ -58,6 +58,23 @@ a = ( aaaaaaaaaaaaaaaaaaaaa = ( o for o in self.registry.values if o.__class__ is not ModelAdmin ) + +# Regression test for: https://github.com/astral-sh/ruff/issues/7623 +tuple( + 0 # comment + for x in y +) + +tuple( + (0 # comment + for x in y) +) + +tuple( + ( # comment + 0 for x in y + ) +) ``` ## Output @@ -122,6 +139,25 @@ a = ( aaaaaaaaaaaaaaaaaaaaa = ( o for o in self.registry.values if o.__class__ is not ModelAdmin ) + +# Regression test for: https://github.com/astral-sh/ruff/issues/7623 +tuple( + 0 # comment + for x in y +) + +tuple( + ( + 0 # comment + for x in y + ) +) + +tuple( + ( # comment + 0 for x in y + ) +) ```